Detach VarDeclarations and Variables from each other during deletion.

Variable and VarDeclarations cross-reference one another. They generally
get deleted around the same time, but this is not always the case. In
this CL we explicitly detach them from each other at destruction time
to avoid holding a stale pointer, removing the risk of accessing it
later. (Accessing null is still fatal, of course, but it's less
dangerous than using a recently-freed pointer, and easier to debug as
well.)

Modifying things in the symbol table requires a const_cast, but it's not
too risky to null out a pointer field on a conceptually-dead object.
A Variable without a VarDeclaration is inert.

Change-Id: Ie01244495a82a8007269522a561b2512c5f12384
Bug: oss-fuzz:32587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/390056
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

src/sksl/SkSLConstantFolder.cpp

@@ -218,7 +218,11 @@ const Expression* ConstantFolder::GetConstantValueForVariable(const Expression&
             break;
         }
         expr = var.initialValue();
-        SkASSERT(expr);
+        if (!expr) {
+            SkDEBUGFAILF("found a const variable without an initial value (%s)",
+                         var.description().c_str());
+            break;
+        }
         if (expr->isCompileTimeConstant()) {
             return expr;
         }

src/sksl/SkSLDehydrator.cpp

@@ -495,8 +495,7 @@ void Dehydrator::write(const ProgramElement& e) {
             this->write(en.typeName());
             AutoDehydratorSymbolTable symbols(this, en.symbols());
             for (const std::unique_ptr<const Symbol>& s : en.symbols()->fOwnedSymbols) {
-                SkASSERT(s->kind() == Symbol::Kind::kVariable);
-                Variable& v = (Variable&) *s;
+                const Variable& v = s->as<Variable>();
                 SkASSERT(v.initialValue());
                 const IntLiteral& i = v.initialValue()->as<IntLiteral>();
                 this->writeS32(i.value());

src/sksl/ir/SkSLVarDeclarations.h

@@ -38,6 +38,13 @@ public:
             , fArraySize(arraySize)
             , fValue(std::move(value)) {}
 
+    ~VarDeclaration() override {
+        // Unhook this VarDeclaration from its associated Variable, since we're being deleted.
+        if (fVar) {
+            fVar->detachDeadVarDeclaration();
+        }
+    }
+
     // Does proper error checking and type coercion; reports errors via ErrorReporter.
     static std::unique_ptr<Statement> Convert(const Context& context,
                                               Variable* var,
@@ -54,6 +61,8 @@ public:
     }
 
     const Variable& var() const {
+        // This should never be called after the Variable has been deleted.
+        SkASSERT(fVar);
         return *fVar;
     }
 

src/sksl/ir/SkSLVariable.cpp

@@ -11,6 +11,13 @@
 
 namespace SkSL {
 
+Variable::~Variable() {
+    // Unhook this Variable from its associated VarDeclaration, since we're being deleted.
+    if (fDeclaration) {
+        fDeclaration->setVar(nullptr);
+    }
+}
+
 const Expression* Variable::initialValue() const {
     return fDeclaration ? fDeclaration->value().get() : nullptr;
 }

src/sksl/ir/SkSLVariable.h

@@ -48,6 +48,8 @@ public:
     , fStorage(storage)
     , fBuiltin(builtin) {}
 
+    ~Variable() override;
+
     const Modifiers& modifiers() const {
         return *fModifiers;
     }
@@ -67,6 +69,12 @@ public:
         fDeclaration = declaration;
     }
 
+    void detachDeadVarDeclaration() const {
+        // The VarDeclaration is being deleted, so our reference to it has become stale.
+        // This variable is now dead, so it shouldn't matter that we are modifying its symbol.
+        const_cast<Variable*>(this)->fDeclaration = nullptr;
+    }
+
     String description() const override {
         return this->modifiers().description() + this->type().name() + " " + this->name();
     }