Nothing Special   »   [go: up one dir, main page]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing return value in initialize_lexer #196

Closed
Teemperor opened this issue Nov 22, 2016 · 6 comments
Closed

Missing return value in initialize_lexer #196

Teemperor opened this issue Nov 22, 2016 · 6 comments
Assignees
Labels
bug C++ parser Issues concerning parser generating
Milestone

Comments

@Teemperor
Copy link
Contributor

Generating a C++ parser from this grammar[1] produces code that crashes under clang++. The reason is that bnfc it doesn't write a return statement in the initialize_lexer function:

int initialize_lexer(FILE *inp) { yyrestart(inp); BEGIN YYINITIAL; }

BEGIN is #defined as #define BEGIN (yy_start) = 1 + 2 * and YYINITIAL as #define YYINITIAL 1.
So there is no return in this function which will result in SIGILL when compiled with clang++ (because it's undefined behavior in C++).

A simple fix for me was to add a simple "return 0;" at the end because the return value is never used in my parser. Looking at the code in bfnc that generates the lexer that doesn't seem like a reuseable fix though.

[1] http://www.cse.chalmers.se/edu/year/2016/course/DAT151_Programming_Language_Technology/laborations/lab2/CPP.cf

@gdetrez
Copy link
Contributor
gdetrez commented Nov 25, 2016

I didn't manage to reproduce this with master. Here is how I tried:

bnfc CPP.cf -o tmp --cpp -m
make -C tmp CC=clang++ CCFLAGS=''  -B 

But it seems that initialize_lexer is a bnfc thing (it's not required by flex or bison) in which case we should be able to just change the return type to void. Unless you see a problem that I missed with doing that?

@Teemperor
Copy link
Contributor Author

You need to run the parser. My output is:

teemperor@ftldrive ~/c/p/test> ls
CPP.cf
teemperor@ftldrive ~/c/p/test> bnfc -m -cpp CPP.cf 
65 rules accepted
writing new file ./Absyn.H
writing new file ./Absyn.C
writing new file ./CPP.l
writing new file ./CPP.y
writing new file ./Parser.H
writing new file ./Skeleton.H
writing new file ./Skeleton.C
writing new file ./Printer.H
writing new file ./Printer.C
writing new file ./Test.C
writing new file ./Makefile
teemperor@ftldrive ~/c/p/test> make CC=clang++
[lots of warnings]
CPP.l:76:68: warning: control reaches end of non-void function [-Wreturn-type]
int initialize_lexer(FILE *inp) { yyrestart(inp); BEGIN YYINITIAL; }
46 warnings generated.
clang++ -g -W -Wall -c Test.C
Linking TestCPP...
clang++ -g -W -Wall Absyn.o Lexer.o Parser.o Printer.o Test.o -o TestCPP
teemperor@ftldrive ~/c/p/test> ./TestCPP < /dev/null 
fish: “./TestCPP < /dev/null” terminated by signal SIGILL (Illegal instruction)

@gdetrez
Copy link
Contributor
gdetrez commented Nov 25, 2016

What version of bnfc are you using? It seem that using master, the problem has been fixed. This is what I have in CPP.l:

void initialize_lexer(FILE *inp) { yyrestart(inp); BEGIN YYINITIAL; } 

@Teemperor
Copy link
Contributor Author
Teemperor commented Nov 25, 2016

I got the bug using the latest release, but I can't reproduce the actual crash in master, so I guess that's fixed.

However, grep -R "int initialize_lexer" reveals that the wrong return type is still in CPP.y when using the master branch bnfc:

> grep -R initialize_lexer
CPP.l:void initialize_lexer(FILE *inp) { yyrestart(inp); BEGIN YYINITIAL; }
CPP.y:int initialize_lexer(FILE * inp);

It seems g++/clang++ can handle this and generate correct code, so it's not really a critical bug.

@gdetrez
Copy link
Contributor
gdetrez commented Nov 25, 2016

Thanks for looking at it. We should nonetheless make them consistent 👍

@andreasabel
Copy link
Member

Couldn't reproduce the problem, but fixed the return type of initialize_lexer to be void also in the parser.

@andreasabel andreasabel added this to the 2.8.4 milestone Jan 3, 2020
@andreasabel andreasabel self-assigned this Jan 3, 2020
@andreasabel andreasabel added the parser Issues concerning parser generating label Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug C++ parser Issues concerning parser generating
Projects
None yet
Development

No branches or pull requests

3 participants