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

Fix the infinite loop when an input file lacks EOF #228

Merged

Conversation

Qining
Copy link
Contributor

@Qining Qining commented Apr 11, 2016

The input scanner can be trapped in an infinite loop if the given input
file does not have EOF (and is not ended with a 'whitespace').

The problem is caused by unget(), which keeps rolling back the scanner
pointer without hitting an EOF at the end of the file. This makes getch()
function keep returning the last character of the file and never ends,
and the effect of advance() is always counteracted by unget().

@johnkslang
Copy link
Member

Doesn't this mean there was a get that returned EndOfInput, but that result was ignored? If so, this may be getting fixed in the wrong place.

@Qining
Copy link
Contributor Author

Qining commented Apr 12, 2016

Actually I'd say yes. In the switch/case statements of preprocessor/PpScanner.cpp, int TPpContext::tStringInput::scan(TPpToken* ppToken), the general pattern is like:
ch = getch();
loop if ch do not change state,
if ch can trigger state change, then ungetch().

The problem is the ungetch() call, which should check if (ch != EndOfFile) before calling it.

I was thinking whether I should add this if (ch !=...) to all the call sites of ungetch(). But in this initial commit, I plummeted the check to a lower layer. Maybe having the if statements in scan() is more legible?

Suggestion?

@Qining Qining force-pushed the fix-infinite-loop-due-to-eof-missing branch 2 times, most recently from 9bb993a to bff94a0 Compare April 12, 2016 15:32
@Qining
Copy link
Contributor Author

Qining commented Apr 15, 2016

Hi @johnkslang , should I add if (ch != EndOfFile) before all caller sites of ungetch() or keep this CL in this way?

The input scanner can be trapped in an infinite loop if the given input
file does not have EOF (and is not ended with a 'whitespace').

The problem is caused by unget(), which keeps rolling back the scanner
pointer without hitting an EOF at the end of the file. This makes getch()
function keep returning the last character of the file and never ends,
and the effect of advance() is always counteracted by unget().
@Qining
Copy link
Contributor Author

Qining commented Apr 27, 2016

Hi @johnkslang ,

Is this PR ready to go?

Thanks

@Qining Qining force-pushed the fix-infinite-loop-due-to-eof-missing branch from e4d0bc5 to 94a89d5 Compare April 27, 2016 14:22
@dneto0
Copy link
Contributor

dneto0 commented Apr 28, 2016

The proposed solution seems quite reasonable to me. But I think we need to update the descriptions of the get(), peek(), ungetch() to clarify their end-of-input edge case behaviours.

@johnkslang
Copy link
Member

I'm fine with it as long as it's truly a catchall for higher-level code that doesn't always, but eventually checks for end-of-input.

@Qining
Copy link
Contributor Author

Qining commented Apr 28, 2016

will work on the comments soon.

@johnkslang johnkslang merged commit 010e93f into KhronosGroup:master Apr 28, 2016
qingyuanzNV pushed a commit to qingyuanzNV/glslang that referenced this pull request Oct 18, 2022
…tizationModes_and_OverflowModes

Add missing capabilities to QuantizationModes and OverflowModes enumerants
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

Successfully merging this pull request may close these issues.

None yet

3 participants