-
Notifications
You must be signed in to change notification settings - Fork 854
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
Need to exit after first ERROR #29
Comments
Aside: Sorry I can't be of more help in actually fixing these, full-time employment imposes limits on what time I can devote to this. |
I find your bug reports really interesting and valuable. Please continue! |
@dneto0: I think I forgot to credit the venerable American Fuzzy Lop or afl-fuzz, which I have been using to help find these bugs. Though I do end up extending some of these crashes manually to find further bugs in the same paths by thinking with my brain cells; this particular bug is a good example where afl-fuzz if left entirely to its own devices, would confuse this bug with another similar bug(the eight The reason most of the finds have been in the preprocessor so far is because until the bugs are worked out in the preprocessor, it'll prioritize those paths for finding bugs. |
I was playing with this and got another crash from trying 150 #endif's in a row.
I bet that with enough #endif's and no other preprocessor directives, elsetracker underflows and stomps on other memory. The fact that you saw a problem with 7 #endifs is just by luck. It could have crashed or produced incorrect results earlier. |
Yeah, I'm aware of the deeper causes of some these bugs. As an aside, I'm looking at csmith to see if I could modify it to generate valid GLSL code which triggers bugs in the semantic analysis, since an unstructured binary fuzzer like afl-fuzz will probably not find as many semantic bugs as syntax validation bugs in the parser. |
Yes, thanks for the reports. I fixed a set of them recently, and will take another round again in a few days. A big class (most?) of them comes from continuing after an error is found, which requires leaving state valid. This is useful for testing and developing, but has no purpose on an end-user system. One alternative, for end users, is to just stop after the first error, unless in a developer/testing mode. This would matter for protecting against malicious software, where not all combinations of cascading errors need testing, just each first error. Comments? |
Speaking just for my use case I'd find it very useful to prioritise stability over additional error reporting. I'm embedding glslang for SPIR-V compilation within another program that takes in arbitrary user-provided shaders, so I need to be confident that errors in the input won't cause a crash. Obviously it's a "would be nice" kind of feature to report as many errors as safely possible, but that's purely for the user's information and I think most people are used to compilers bailing out at some point even if there are more errors to find. |
@johnkslang, @baldurk: I think a good balance to strike would be to set up an error accumulator which could be configured to stop on the first error, or not. We can continue to work out the kinks of the full-scan validation, and at the same time offer a mode which is theoretically more robust against remote exploitation(as well as general instability). The full-scan validation mode is very useful for something like a text editor or IDE, where you might want to display a number of outright errors in the program without necessarily repairing all of the others; though it's tough to tell whether subsequent code is truly incorrect if you know the code before it is incorrect. Thoughts? |
@baldurk: For your use case, my first instinct would be to isolate the validator/compiler in a completely separate process, and spawn it for each compilation task; this should also help prevent blocking, which would be a very big experience win if the program is interactive. |
@xorgy Yes, a switch to say which behavior is desired is what I have in mind. The caller of the library would have to request cascading errors; the default would be to stop after the first error. I would certainly use it in cascading mode, and fix bugs that uncovers, because it's more efficient to test multiple errors in the same compilation unit. |
If we are not inside an if macro, we cannot simply decrease elsetracker. Fixes KhronosGroup#29.
Reopening, as the merge request was just one example of this subject. |
…34, issue #35). Added -C option to request cascading errors. By default, will exit early, to avoid all error-recovery-based crashes. This works by simulating end-of-file in input on first error, so no need for exception handling, or stack unwinding, or any complex error checking/handling to get out of the stack.
I realized it's pretty easy to simulate end-of-file when an error is seen, causing the natural end of parsing without any exception handling, stack unwinding, extra tests everywhere to protect against previous error, etc. Commit a86836e does this. By default, crashes should no longer happen, because cascading errors from error recovery must be explicitly requested with -C (or the EShMsgCascadingErrors message flag through the API). This fixes several issues and generally makes glslang much more robust. |
…sion Update SPIR-V core grammar revision number
and a number of similar inputs exercise a segmentation fault bug in
glslang::TPpContext::tStringInput::scan
.The text was updated successfully, but these errors were encountered: