steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.82k stars 523 forks source link

vpi_free_object() causes segfault #428

Closed toddstrader closed 3 years ago

toddstrader commented 3 years ago

I was working on a Verilator VPI test and comparing against iverilog when I hit this:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000055a214e6cc91 in vpi_free_object ()
warning: ~/SandBox/homecvs/v4/verilator/src/.gdbinit: No such file or directory
(gdb) bt
#0  0x000055a214e6cc91 in vpi_free_object ()
#1  0x00007f68628c0c95 in TestVpiHandle::~TestVpiHandle() () from obj_iv/t_vpi_module/libvpi.so
#2  0x00007f68628c0a6a in mon_check () from obj_iv/t_vpi_module/libvpi.so
#3  0x00007f68628c0b4d in mon_check_vpi() () from obj_iv/t_vpi_module/libvpi.so
#4  0x000055a214e781fa in vpip_execute_vpi_call(vthread_s*, __vpiHandle*) ()
#5  0x000055a214e34a91 in of_VPI_CALL(vthread_s*, vvp_code_s*) ()
#6  0x000055a214e2cd92 in vthread_run(vthread_s*) ()
#7  0x000055a214e40a04 in schedule_simulate() ()
#8  0x000055a214dfc093 in main ()

If you checkout the PR below (or wait for it to land) you can run this to reproduce the problem:

test_regress/t/t_vpi_module.pl --iv

And if you run this, you'll skip the two calls to vpi_free_object() which cause the problem:

CFLAGS="-DIS_ICARUS" test_regress/t/t_vpi_module.pl --iv

And for clarity, here's what the Verilator test driver is running under the covers (in the test_regress directory):

$ vvp -n -m obj_iv/t_vpi_module/libvpi.so obj_iv/t_vpi_module/simiv
Segmentation fault (core dumped)

For reference: https://github.com/verilator/verilator/pull/2704

martinwhitaker commented 3 years ago

Reading the comments on the Verilator PR, it would seem this was a bug in your test, no?

toddstrader commented 3 years ago

I must confess that I'm still not seeing where the double free is coming from in the Verilator test. But Wilson does think it's an issue with the Verilator test code, so I imagine he's right. If I had more time I'd strip down the test code and figure out what I'm missing, but unfortunately that's not the rabbit hole I'm trying to go down right now.

Sorry for the noise.

martinwhitaker commented 3 years ago

From what I read, and the brief look I took at the code, I think the problem was that the TestVpiHandle destructor was calling vpi_free_object on the VPI handle it had stored. But if you use vpi_scan in a loop until it returns NULL, the handle you have is no longer valid (and gets automatically freed by the VPI runtime). You should only manually free the handle if you exit the iterate loop before you have scanned all the objects.