diff options
author | Ethan Nicholas <ethannicholas@google.com> | 2018-03-01 15:05:17 -0500 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2018-03-01 20:42:04 +0000 |
commit | 68dd2c1fa051019354d0c441c78b3885d8e72a7a (patch) | |
tree | 9b518d216d71742c1948c574db089bdc627b060e /src/sksl | |
parent | a7f320507dcf765313e27001774042cf1882dfea (diff) |
Fixed SkSL use-after-free fuzzer bug and added defensive code to catch such problems in the future.
Bug: skia:7558
Change-Id: I5098c0ed08f2328828969e819db7785270b26656
Reviewed-on: https://skia-review.googlesource.com/111460
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Diffstat (limited to 'src/sksl')
-rw-r--r-- | src/sksl/SkSLIRGenerator.cpp | 13 | ||||
-rw-r--r-- | src/sksl/ir/SkSLSymbol.h | 2 | ||||
-rw-r--r-- | src/sksl/ir/SkSLVariable.h | 10 | ||||
-rw-r--r-- | src/sksl/ir/SkSLVariableReference.h | 3 |
4 files changed, 23 insertions, 5 deletions
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index 719dca9416..38c45f70ee 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -286,6 +286,9 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVa return nullptr; } value = this->coerce(std::move(value), *type); + if (!value) { + return nullptr; + } var->fWriteCount = 1; var->fInitialValue = value.get(); } @@ -761,7 +764,8 @@ void IRGenerator::convertFunction(const ASTFunction& f) { std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInterfaceBlock& intf) { std::shared_ptr<SymbolTable> old = fSymbolTable; - AutoSymbolTable table(this); + this->pushSymbolTable(); + std::shared_ptr<SymbolTable> symbols = fSymbolTable; std::vector<Type::Field> fields; bool haveRuntimeArray = false; bool foundRTAdjust = false; @@ -804,6 +808,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte } } } + this->popSymbolTable(); Type* type = new Type(intf.fOffset, intf.fTypeName, fields); old->takeOwnership(type); std::vector<std::unique_ptr<Expression>> sizes; @@ -826,11 +831,11 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte name += "[]"; } type = new Type(name, Type::kArray_Kind, *type, (int) count); - fSymbolTable->takeOwnership((Type*) type); + symbols->takeOwnership((Type*) type); sizes.push_back(std::move(converted)); } else { type = new Type(type->name() + "[]", Type::kArray_Kind, *type, -1); - fSymbolTable->takeOwnership((Type*) type); + symbols->takeOwnership((Type*) type); sizes.push_back(nullptr); } } @@ -858,7 +863,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte intf.fTypeName, intf.fInstanceName, std::move(sizes), - fSymbolTable)); + symbols)); } void IRGenerator::getConstantInt(const Expression& value, int64_t* out) { diff --git a/src/sksl/ir/SkSLSymbol.h b/src/sksl/ir/SkSLSymbol.h index 4ec8f156cc..43fb742d4f 100644 --- a/src/sksl/ir/SkSLSymbol.h +++ b/src/sksl/ir/SkSLSymbol.h @@ -29,6 +29,8 @@ struct Symbol : public IRNode { , fKind(kind) , fName(name) {} + virtual ~Symbol() {} + Kind fKind; StringFragment fName; diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h index f16bf4b515..b7cef10535 100644 --- a/src/sksl/ir/SkSLVariable.h +++ b/src/sksl/ir/SkSLVariable.h @@ -37,7 +37,15 @@ struct Variable : public Symbol { , fStorage(storage) , fInitialValue(initialValue) , fReadCount(0) - , fWriteCount(0) {} + , fWriteCount(initialValue ? 1 : 0) {} + + ~Variable() override { + // can't destroy a variable while there are remaining references to it + if (fInitialValue) { + --fWriteCount; + } + ASSERT(!fReadCount && !fWriteCount); + } virtual String description() const override { return fModifiers.description() + fType.fName + " " + fName; diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h index ad54d43515..e1f19ac742 100644 --- a/src/sksl/ir/SkSLVariableReference.h +++ b/src/sksl/ir/SkSLVariableReference.h @@ -45,6 +45,9 @@ struct VariableReference : public Expression { } ~VariableReference() override { + if (fRefKind != kRead_RefKind) { + fVariable.fWriteCount--; + } if (fRefKind != kWrite_RefKind) { fVariable.fReadCount--; } |