BNFC / bnfc

BNF Converter
http://bnfc.digitalgrammars.com/
583 stars 162 forks source link

[ fix #349 ] Make parser reentrant by using reentrant lexer #351

Closed andreasabel closed 3 years ago

andreasabel commented 3 years ago

[ fix #349 ] Make parser reentrant by using reentrant lexer

Further:

wangjia184 commented 3 years ago

Tested in linux/x64/gcc and macos/arm64/clang neither works

random errors when run my test cases

I tried cargo test -- --test-threads=1, this command should run test case one by one, the first one succeeds, then it fails.

Typical errors from MacOS/ARM64

test normalize::ppar_tests::ppar_should_accumulate_free_counts_from_both_branches ... ok
test normalize::ppar_tests::ppar_should_compile_both_branches_with_same_environment ... rholang_parser-83f0bbd31c1724da(62995,0x10503bd40) malloc: *** error for object 0x15b808200: pointer being freed was not allocated
rholang_parser-83f0bbd31c1724da(62995,0x10503bd40) malloc: *** set a breakpoint in malloc_error_break to debug

Typical errors from Linux/x64

test normalize::ppar_tests::ppar_should_accumulate_free_counts_from_both_branches ... ok
free(): invalid pointer
test normalize::ppar_tests::ppar_should_accumulate_free_counts_from_both_branches ... ok
test normalize::ppar_tests::ppar_should_compile_both_branches_with_same_environment ... free(): double free detected in tcache 2

From the errors, it seems double free issue, some pointer might be freed twice. But in fact, my app does not call free() since I changed my code to run the c parser in a short-alive child process (but when I run test cases, it is in current process)

andreasabel commented 3 years ago

I realized there is still a global variable in the lexer for lexing strings, it is literal_buffer. I have to make this local as well...

andreasabel commented 3 years ago

@wangjia184: I updated the PR to remove a global variable in the lexer I had overlooked previously. Could you please test again?

wangjia184 commented 3 years ago

@andreasabel still same error. In generated files, I see change like this one. image

Just make sure I updated my BNFC

andreasabel commented 3 years ago

@wangjia184 : Thanks, yes this was the last change I did.

I am a bit out of ideas now how to proceed, since I don't have some infrastructure for running the parser in parallel.

andreasabel commented 3 years ago

This project spilled over to the C++ backends. Since the work on the C backend broke the C++ backends, I took the forward route to (re)integrate the C++ lexer/parser generators into the C backend. This way, we also get reentrant parsers for C++.