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

Stack overflow on EOF* #3359

Closed
KvanTTT opened this issue Nov 20, 2021 · 5 comments
Closed

Stack overflow on EOF* #3359

KvanTTT opened this issue Nov 20, 2021 · 5 comments

Comments

@KvanTTT
Copy link
Member

KvanTTT commented Nov 20, 2021

Consider the following grammar:

lexer grammar StackoverflowOnClosure;
ClosureEof: EOF*;

It fails with Stackoverflow on empty input:

at org.antlr.v4.runtime.misc.Array2DHashSet.getOrAdd(Array2DHashSet.java:59)
	at org.antlr.v4.runtime.atn.ATNConfigSet.add(ATNConfigSet.java:146)
	at org.antlr.v4.runtime.atn.ATNConfigSet.add(ATNConfigSet.java:122)
	at org.antlr.v4.runtime.atn.LexerATNSimulator.closure(LexerATNSimulator.java:446)
	at org.antlr.v4.runtime.atn.LexerATNSimulator.closure(LexerATNSimulator.java:455)
	at org.antlr.v4.runtime.atn.LexerATNSimulator.closure(LexerATNSimulator.java:455)

And it's freezing on any not empty input.

The samples is extracted from #1943 (comment)

KvanTTT added a commit to KvanTTT/antlr4 that referenced this issue Nov 20, 2021
@KvanTTT
Copy link
Member Author

KvanTTT commented Nov 20, 2021

It seems like the bug is similar to the bug with EPSILON_CLOSURE that I've already fixed. But here we have EOF_CLOSRE instead of EPSILON_CLOSURE. Also, closure on EOF is useless.

So, I fixed it in a similar way.

KvanTTT added a commit to KvanTTT/antlr4 that referenced this issue Nov 20, 2021
Remove EOFInClosure test since now it's not actual on runtime
@parrt
Copy link
Member

parrt commented Nov 21, 2021

Hmm..gotta be careful here. EOF is an actual condition/"char" that we sometimes wanna match. maybe not in * loop but certainly in a set

@KvanTTT
Copy link
Member Author

KvanTTT commented Nov 21, 2021

It works fine with set. There is no error for such case:

EofInSet: 'y' ('z' | EOF);

But the following code where EOF closure actually exists (a sample from runtime tests):

prog : stat EOF;
stat : 'x' ('y' | EOF)*?;

Should be rewritten in the following way:

prog : stat EOF;
stat : 'x' 'y'* EOF?;

Anyway, stack overflow on runtime is much worse than restricting EOF closures on grammar level.

@KvanTTT KvanTTT changed the title Stackoverflow on EOF* Stack overflow on EOF* Nov 21, 2021
KvanTTT added a commit to KvanTTT/antlr4 that referenced this issue Nov 25, 2021
Remove EOFInClosure test since now it's not actual on runtime
KvanTTT added a commit to KvanTTT/antlr4 that referenced this issue Dec 23, 2021
Remove EOFInClosure test since now it's not actual on runtime
KvanTTT added a commit to KvanTTT/antlr4 that referenced this issue Dec 24, 2021
Remove EOFInClosure test since now it's not actual on runtime
@parrt
Copy link
Member

parrt commented Dec 27, 2021

Should this be merged before or after your #3349 ?

@KvanTTT
Copy link
Member Author

KvanTTT commented Dec 27, 2021

The PR fixes this bug :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants