verilog: failed attempt to fix leak

[widlarizer]

Non-functional half-attempt at addressing #5180 (comment)

[garytwong]

Thank you for the debugging, and apologies for introducing the error! I can reproduce the issue and am contemplating a fix.

My inclination is that it might be cleaner to omit the delete from the TOK_IF handling around verilog_parser.y:2885, and instead defer cleanup until the end during optional_else handling at line 2906. It would require a bit more processing there to detect the case where an else if branch inside a unique if has orphaned the AST_BLOCK node created earlier. That's when the outstanding reference I had missed will be dropped by ast_stack.pop_back, and so ought to be the proper time to deallocate. But I don't have an implementation yet.

I expect to have more time to investigate in ~12 hours, unless you want to fix it first.

Thanks again!

[garytwong]

Please disregard my suggestion above... while that approach could probably have been made to work, it got complicated quickly. Your idea to clean up ast_stack directly is definitely better.

I rearranged your fix a little bit and now have a patch to run tests/verilog/unique_if_enc.ys and @KrystalDelusion's minimal test case from #5152 under SANITIZER=address with no use-after-free and no leaks. (See commit 50ac69d.) I will play with it a bit more to test there are no other regressions.

[garytwong]

OK, I've tested commit 50ac69d further and it seems to pass testing with SANITIZER=address for me. Should I submit a new PR on that revision? Or should I let you finish this one? Or something else?

[KrystalDelusion]

Should I submit a new PR on that revision?

That would be great, thanks :)

[widlarizer]

superseded by #5193