From fabed8bb79ce17af2988dd38e51a2fd59c48d6de Mon Sep 17 00:00:00 2001 From: John Stiles Date: Mon, 29 Mar 2021 09:38:59 -0400 Subject: [PATCH] Fix fuzzer-discovered error with variable declarations. As soon as a single VarDeclaration is successfully created, its Variable is added to the current symbol table. However, if a variable-declaration line declared several variables in a row, we would stop if ANY of the declarations contained an error and discard the entire statement, but would continue processing the rest of the program. This left us in a position where some Variables existed in the SymbolTable with valid, reachable names, but their corresponding VarDeclaration statement had been thrown away as erroneous. Since Variables point back to VarDeclarations for their initialValues, this gave us a stale pointer. Any future reference to that variable name which could trigger an access to its initialValue would read from this dead pointer. This CL fixes the conversion of VarDeclarations so that we no longer throw away any VarDeclarations associated with a successfully-parsed Variable. Change-Id: If8ec3c160933e48a0e1f36414234b3a849d8978c Bug: oss-fuzz:32587 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389636 Commit-Queue: John Stiles Commit-Queue: Brian Osman Auto-Submit: John Stiles Reviewed-by: Brian Osman --- gn/sksl_tests.gni | 1 + resources/sksl/errors/Ossfuzz32587.sksl | 2 ++ src/sksl/SkSLIRGenerator.cpp | 4 ++-- tests/sksl/errors/Ossfuzz32587.glsl | 5 +++++ 4 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 resources/sksl/errors/Ossfuzz32587.sksl create mode 100644 tests/sksl/errors/Ossfuzz32587.glsl diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni index 400df5a08c..cec6b1b5e1 100644 --- a/gn/sksl_tests.gni +++ b/gn/sksl_tests.gni @@ -142,6 +142,7 @@ sksl_error_tests = [ "/sksl/errors/Ossfuzz29849.sksl", "/sksl/errors/Ossfuzz31410.sksl", "/sksl/errors/Ossfuzz31469.sksl", + "/sksl/errors/Ossfuzz32587.sksl", "/sksl/errors/OverflowFloatLiteral.sksl", "/sksl/errors/OverflowIntLiteral.sksl", "/sksl/errors/OverflowParamArraySize.sksl", diff --git a/resources/sksl/errors/Ossfuzz32587.sksl b/resources/sksl/errors/Ossfuzz32587.sksl new file mode 100644 index 0000000000..b113c9d604 --- /dev/null +++ b/resources/sksl/errors/Ossfuzz32587.sksl @@ -0,0 +1,2 @@ +const float x=1, _=x1; +half x=x*8; diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index 40fe0b7213..aca0746c9d 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -457,13 +457,13 @@ StatementArray IRGenerator::convertVarDeclarations(const ASTNode& decls, arraySize = this->convertExpression(*iter++); } else { this->errorReporter().error(decls.fOffset, "array must have a size"); - return {}; + continue; } } if (iter != varDecl.end()) { value = this->convertExpression(*iter); if (!value) { - return {}; + continue; } } std::unique_ptr varDeclStmt = this->convertVarDeclaration(varDecl.fOffset, diff --git a/tests/sksl/errors/Ossfuzz32587.glsl b/tests/sksl/errors/Ossfuzz32587.glsl new file mode 100644 index 0000000000..4894152082 --- /dev/null +++ b/tests/sksl/errors/Ossfuzz32587.glsl @@ -0,0 +1,5 @@ +### Compilation failed: + +error: 1: unknown identifier 'x1' +error: 2: symbol 'x' was already defined +2 errors