Skip to content

Commit

Permalink
[import] Fix segfault on exit in import line trace test
Browse files Browse the repository at this point in the history
  • Loading branch information
ptpan committed Oct 26, 2023
1 parent 3b57776 commit f5dd086
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class VerilogVerilatorImportConfigs( BasePassConfigs ):

# We enforce the GNU makefile implicit rule that `LDLIBS` should only
# include library linker flags/names such as `-lfoo`.
"ld_libs" : "",
"ld_libs" : "-lpthread",

"c_flags" : "",
}
Expand Down Expand Up @@ -269,6 +269,7 @@ def create_vl_cmd( s ):
opt_level = "-O3"
loop_unroll = "--unroll-count 1000000"
stmt_unroll = "--unroll-stmts 1000000"
thread = "--threads 1"
trace = "--trace" if s.vl_trace else ""
coverage = "--coverage" if s.vl_coverage else ""
line_cov = "--coverage-line" if s.vl_line_coverage else ""
Expand All @@ -278,7 +279,7 @@ def create_vl_cmd( s ):
all_opts = [
top_module, mk_dir, include, en_assert, opt_level, loop_unroll,
# stmt_unroll, trace, warnings, flist, src, coverage,
stmt_unroll, trace, warnings, src, vlibs, coverage,
stmt_unroll, thread, trace, warnings, src, vlibs, coverage,
line_cov, toggle_cov,
]

Expand Down Expand Up @@ -325,6 +326,8 @@ def create_cc_cmd( s ):
if not s.is_default("c_flags"):
c_flags += f" {expand(s.c_flags)}"

c_flags += f" -std=c++11 -pthread -DVL_TIME_CONTEXT"

c_include_path = " ".join("-I"+p for p in s._get_all_includes() if p)
out_file = s.get_shared_lib_path()
c_src_files = " ".join(s._get_c_src_files())
Expand All @@ -334,7 +337,17 @@ def create_cc_cmd( s ):
s.vl_line_coverage or \
s.vl_toggle_coverage else ""

# PP: try not to introduce GNU unique symbols to the shared library.
# PP: eliminate GNU unique symbols in the compiled shared library.
# See https://stackoverflow.com/a/76664985
# ^ this says if a shared library has UNIQUE symbols in it a dlclose
# may become a no-op. The solution is to mark all symbols as `hidden`
# except for those we want to access in the C wrapper. Note that I've
# tried adding the g++ flag `-fvisibility=hidden` and
# __attribute__((visibility("default"))) to APIs, but the compiled
# shared library still contains UNIQUE symbols:
# % readelf -sW --dyn-syms libVRegTrace_noparam_v.so | grep '\bUNIQUE\b'
# The solution proposed in the above post is to use a linker version
# script which specifies visibility of all symbols.
linker_version_script_filename = "verilator_import_linker_version_script.lds"
dir_to_version_script = os.path.abspath(os.path.dirname(__file__))
ld_flags += f" -Wl,--version-script={os.path.join(dir_to_version_script, linker_version_script_filename)}"
Expand Down Expand Up @@ -460,7 +473,7 @@ def _compile_vl_srcs_from_vl_class_mk( s, all_lines, path, label ):
cxx_includes = " ".join(map(lambda x: "-I"+x, s._get_all_includes()))
for src in srcs:
file_path, obj_name = src
cxx_cmd = f"g++ {cxx_includes} -O1 -fPIC -c -o {obj_name}.o {file_path}"
cxx_cmd = f"g++ {cxx_includes} -std=c++11 -DVL_TIME_CONTEXT -O1 -fPIC -pthread -lpthread -c -o {obj_name}.o {file_path}"
if os.path.exists(f"{obj_name}.o"):
s.vprint(f"Skip compiling {obj_name}.o because it already exists.")
else:
Expand Down
22 changes: 20 additions & 2 deletions pymtl3/passes/backends/verilog/import_/test/ImportedObject_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,25 @@ def construct( s ):
# 0xFFFFFFFF unsigned
assert a.line_trace() == 'q = 4294967295'
a.sim_tick()
a.finalize()

# PP: This is a workaround to the $sformat issue I was seeing with Verilator
# 5.016. If I enable this finalize below, the C library will segfault after
# all pytest tests have finished. Digging deeper I found this could be
# reliably eliminated/reproduced by removing/adding the `thread_local`
# specifier to the temporary string used in VL_SFORMAT_X (verilated.h). Note
# that the program finishes just fine when this thread-local string is never
# used (VL_SFORMAT_X is never invoked). My theory is this could be a
# double-free situation: somehow the dynamically allocated memory that
# belongs to this thread-local string gets deleted twice: once when we unload
# the dynamic library (through a.finalize()) and once at the end of program
# execution.
#
# One stackoverflow post suggests this might be a compiler bug:
# https://stackoverflow.com/a/58366171
# I tried recompiling with the latest g++ on BRG server (scl-11) but the
# segfault persists.

# a.finalize()

def test_reg_infer_external_trace( do_test ):
# Test Verilog line trace
Expand Down Expand Up @@ -498,7 +516,7 @@ def construct( s ):
# 0xFFFFFFFF unsigned
assert a.line_trace() == 'q = 4294967295'
a.sim_tick()
a.finalize()
# a.finalize()

def test_incr_on_demand_vcd( do_test ):
class VIncr( Component, VerilogPlaceholder ):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@
void seq_eval( V{component_name}_t * m ) {{
V{vl_component_name} * model = (V{vl_component_name} *) m->model;
VerilatedContext * context_ptr = m->context_ptr;
// evaluate one time cycle
Expand Down Expand Up @@ -248,6 +249,7 @@
#endif
model->eval();
context_ptr->timeInc(1);
#if DUMP_VCD
if ( m->_vcd_en && (ON_DEMAND_VCD_ENABLE || !ON_DEMAND_DUMP_VCD) ) {{
Expand Down

0 comments on commit f5dd086

Please sign in to comment.