A-LPG / LPG2

The LALR parser generator (LPG) is a tool for developing scanners and parsers. Supports multi-language . Input is specified by BNF rules. LPG supports backtracking (to resolve ambiguity), automatic AST generation and grammar inheritance.
Eclipse Public License 2.0
5 stars 4 forks source link

Segfault on linux64 #1

Closed mingodad closed 2 years ago

mingodad commented 2 years ago

Trying to build this project on Ububtu 18.04 I needed to make several changes to be able to build it, also several chnges have been made to remove several compiler warnings with `-Wall -Wextra". When ti first build it was segfaulting here https://github.com/A-LPG/LPG2/blob/0a58ede665859d1d3c796d2edc8cf0c4e3e288d8/lpg2/src/parser.cpp#L96 I've added a test before doing the call and now it's not segfaulting but the output doesn't seem ok.

I'm attaching the output of git diff -u with my changes, just in case it can help.

lpg2-dad.diff.zip

mingodad commented 2 years ago

Several problems were reported by valgrind.

kuafuwang commented 2 years ago

Several problems were reported by valgrind.

Thanks. I will fix it soon.

kuafuwang commented 2 years ago

Trying to build this project on Ububtu 18.04 I needed to make several changes to be able to build it, also several chnges have been made to remove several compiler warnings with `-Wall -Wextra". When ti first build it was segfaulting here

https://github.com/A-LPG/LPG2/blob/0a58ede665859d1d3c796d2edc8cf0c4e3e288d8/lpg2/src/parser.cpp#L96

I've added a test before doing the call and now it's not segfaulting but the output doesn't seem ok. I'm attaching the output of git diff -u with my changes, just in case it can help.

lpg2-dad.diff.zip

I built it on wsl2 . It didn't have segfault. (Linux DESKTOP-3F0RF5B 5.10.60.1-microsoft-standard-WSL2 #1 SMP Wed Aug 25 23:20:18 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux) But when i add : -O3 option. It will sgfault. I didn't find out the problem. By the way .Can you give me the test which make you feel the output doesn't seem ok? Thank you.

mingodad commented 2 years ago

I just made a copy of https://github.com/A-LPG/LPG2/blob/main/lpg2/src/jikespg.g to a temp folder and executed lpg2 jikespg.g and then compared the generated files with the ones in src and jikespg_act.cpp and jikespg_init.cpp do not match.

mingodad commented 2 years ago

Also have you tried to execute valgrind lpg2 anygrammar.g ?

kuafuwang commented 2 years ago

I just made a copy of https://github.com/A-LPG/LPG2/blob/main/lpg2/src/jikespg.g to a temp folder and executed lpg2 jikespg.g and then compared the generated files with the ones in src and jikespg_act.cpp and jikespg_init.cpp do not match.

I used java.g to test yesterday,not jikespg.g. It's segfault when excecut lpg2 jikespg.g. I will check it when i off duty.

kuafuwang commented 2 years ago

Also have you tried to execute valgrind lpg2 anygrammar.g ?

Yes i do.
kuafu@DESKTOP-3F0RF5B:/mnt/e/LPG2/lpg2/bin$ valgrind --leak-check=full --show-leak-kinds=all ./lpg-linux_x86_64 jikespg.g You are right , several problems were reported by valgrind.

kuafuwang commented 2 years ago

I had made a mistake. I didn't assign an action on Rule: headers_action_segment_list ::= headers_action_segment_list action_segment I should write it like this headers_action_segment_list ::= headers_action_segment_list action_segment /.$Action $DefaultHeader { header_blocks.Next() = Token(2); } ./ Can you pull a PR about what you had change. Then I will merge it. And fix the problem on the version what you commit.

mingodad commented 2 years ago

Thank you for the fix ! With your fix it doesn't segfault, but still there is a small change in the grammar that it's not on the current lpg2/src/jikespg_act.cpp.

--- /home/mingo/dev/c/A_grammars/LPG2/lpg2/src/tmp/jikespg_act.cpp
+++ /home/mingo/dev/c/A_grammars/LPG2/lpg2/src/jikespg_act.cpp
@@ -504,7 +504,7 @@
 {
     int length = lex_stream -> NameStringLength(Token(1)) + 1;
     char *macro_name = new char[length + 1];
-    macro_name[0] = option -> escape;
+    macro_name[0] = option -> macro_prefix;
     strcpy(macro_name + 1, lex_stream -> NameString(Token(1)));

     MacroSymbol *macro_symbol = macro_table -> FindName(macro_name, length);
kuafuwang commented 2 years ago

Thank you for the fix ! With your fix it doesn't segfault, but still there is a small change in the grammar that it's not on the current lpg2/src/jikespg_act.cpp.

--- /home/mingo/dev/c/A_grammars/LPG2/lpg2/src/tmp/jikespg_act.cpp
+++ /home/mingo/dev/c/A_grammars/LPG2/lpg2/src/jikespg_act.cpp
@@ -504,7 +504,7 @@
 {
     int length = lex_stream -> NameStringLength(Token(1)) + 1;
     char *macro_name = new char[length + 1];
-    macro_name[0] = option -> escape;
+    macro_name[0] = option -> macro_prefix;
     strcpy(macro_name + 1, lex_stream -> NameString(Token(1)));

     MacroSymbol *macro_symbol = macro_table -> FindName(macro_name, length);

macro_name[0] = option -> macro_prefix; is ok.

mingodad commented 2 years ago

After accepting your invitation I was thinking that I had rights to push to this repository but I don't, here is the output of git diff -u that works for me and when executed under valgrind there is no report of invalid memory access. lpg2-dad.diff.zip

I did changes to the *Action.cpp where based on compiler warning and analyzing the code this changes seem to achieve the intended outcome:

@@ -1409,7 +1409,10 @@ void DartAction::GenerateAstType(ActionFileSymbol* ast_filename_symbol,
         b.Put(indentation); b.Put("    void setNextAst(IAst n){ nextAst = n; }\n");
         b.Put(indentation); b.Put("    void resetNextAst(){ nextAst = null; }\n");
     }
-    else b.Put(indentation); b.Put("    IAst? getNextAst(){ return null; }\n");
+    else
+    {
+   b.Put(indentation); b.Put("    IAst? getNextAst(){ return null; }\n");
+    }

     b.Put(indentation); b.Put("     late IToken leftIToken ;\n");
     b.Put(indentation); b.Put("     late IToken rightIToken ;\n");
kuafuwang commented 2 years ago

That's fine. If you didn't mind I commit the code which you changed, I will commit it later. Thank you for your time. Have a nice weekend.

kuafuwang commented 2 years ago

I had fixed this issue (thanks to you)

mingodad commented 2 years ago

It seems that you have missed the fixes for this https://github.com/A-LPG/LPG2/issues/1#issuecomment-979954557 see here https://github.com/A-LPG/LPG2/blob/3e03744190610785342d487943f289459f5ea848/lpg2/src/CppAction2.cpp#L1507 , without that fix (on several *Action.cpp it seems that we'll get invalid generated code by having two conflicting function definitions when option -> glr is true:

if (option -> glr)
    {
        b.Put(indentation); b.Put("    Ast* nextAst = nullptr;\n");
        b.Put(indentation); b.Put("    IAst* getNextAst() { return nextAst; }\n"); ///<<<< first definition
        b.Put(indentation); b.Put("    void setNextAst(IAst* n) { nextAst = n; }\n");
        b.Put(indentation); b.Put("    void resetNextAst() { nextAst = nullptr; }\n");
    }
    else b.Put(indentation); b.Put("    IAst* getNextAst() { return nullptr; }\n"); ///<<<< second conflicting definition
kuafuwang commented 2 years ago

It seems that you have missed the fixes for this #1 (comment) see here

https://github.com/A-LPG/LPG2/blob/3e03744190610785342d487943f289459f5ea848/lpg2/src/CppAction2.cpp#L1507

, without that fix (on several *Action.cpp it seems that we'll get invalid generated code by having two conflicting function definitions when option -> glr is true:

if (option -> glr)
    {
        b.Put(indentation); b.Put("    Ast* nextAst = nullptr;\n");
        b.Put(indentation); b.Put("    IAst* getNextAst() { return nextAst; }\n"); ///<<<< first definition
        b.Put(indentation); b.Put("    void setNextAst(IAst* n) { nextAst = n; }\n");
        b.Put(indentation); b.Put("    void resetNextAst() { nextAst = nullptr; }\n");
    }
    else b.Put(indentation); b.Put("    IAst* getNextAst() { return nullptr; }\n"); ///<<<< second conflicting definition

And it's a lesson to learn about that {} should come along with if or else. I will make a issue about this, and fixed it.

LPG didn't implement GLR. But when the glr is set. It will constructing a backtracking parser. if (glr) { lalr_level = 1; single_productions = false; backtrack = true; }

kuafuwang commented 2 years ago

After accepting your invitation I was thinking that I had rights to push to this repository but I don't At first ,I was wonder why you don't want to push , because I thought you should had the privilege to push. Now I know , I had misunderstand what your said. I had learn how to authorization the privilege. I had authorization all the member the admin privilege now. It's my bad. I'm sorry. I'm not very familiar with git. In my work place , we are commit code with SVN ,not git.