[clang] Add serialization support for the DynamicAllocLValue variant of APValue::LValueBase::Ptr

[nridge]

Summary

Fixes https://github.com/clangd/clangd/issues/1672

[nridge]

Add missing check for isDynamicAlloc

[nridge]

Requesting review.

The patch does not include an automated test because this is fixing a bug report with no test case (I spotted the issue by inspection based on the stack trace). The bug reporter did try the patch locally and confirm that it fixes the crash on their codebase.

[hokein]

I believe the fix is correct.

Though the fix has been verified by a real-world example, I think it would be nice to get a reproducible testcase. Looking at the stacktrace:

• the crash occurs during the pch deserialization
• and we miss handling the case where LValue base is a DynamicAllocLValue

so it looks like we can construct a DynamicAllocLValue case, and try to deserialize it (e.g. emitting a pch), and it should reproduce the crash.

[hokein]

I gave it a try, here is a testcase that triggers the crash

// clang -cc1 -emit-pch -o /tmp/t.pch /tmp/t.cpp
#ifndef HEADER
#define HEADER

struct A {  int *p; };
const A &w = A{ new int(10) };

#endif

[nridge]

Thank you for the suggested testcase!

[nridge]

Add testcase

[hokein]

Thanks, looks good to me.

clang/include/clang/AST/PropertiesBase.td

 }
 let Class = PropertyTypeCase<APValue, "LValue"> in {
   def : ReadHelper<[{
     auto lvalueBase = node.getLValueBase();
     const Expr *expr =
         lvalueBase ? lvalueBase.dyn_cast<const Expr *>() : nullptr;
     bool lvalueBaseIsExpr = (bool) expr;
     bool lvalueBaseIsTypeInfo = lvalueBase.is<TypeInfoLValue>();
+    bool lvalueBaseIsDynamicAlloc = lvalueBase.is<DynamicAllocLValue>();
     QualType elemTy;
     if (lvalueBase) {
       if (lvalueBaseIsTypeInfo) {
         elemTy = lvalueBase.getTypeInfoType();
+      } else if (lvalueBaseIsDynamicAlloc) {
+        elemTy = lvalueBase.getDynamicAllocType();
       } else if (lvalueBaseIsExpr) {
         elemTy = expr->getType();
       } else {
         elemTy = lvalueBase.get<const ValueDecl *>()->getType();
       }
     }
   }]>;
   def : Property<"hasLValuePath", Bool> {
     let Read = [{ node.hasLValuePath() }];
   }
   def : Property<"isLValueOnePastTheEnd", Bool> {
     let Read = [{ node.isLValueOnePastTheEnd() }];
   }
   def : Property<"isExpr", Bool> {
     let Read = [{ lvalueBaseIsExpr }];
   }
   def : Property<"isTypeInfo", Bool> {
     let Read = [{ lvalueBaseIsTypeInfo }];
   }
+  def : Property<"isDynamicAlloc", Bool> {
+    let Read = [{ lvalueBaseIsDynamicAlloc }];
+  }
   def : Property<"hasBase", Bool> {
     let Read = [{ static_cast<bool>(lvalueBase) }];
   }
   def : Property<"isNullPtr", Bool> {
     let Read = [{ node.isNullPointer() }];
   }
   def : Property<"typeInfo", QualType> {
     let Conditional = [{ hasBase && isTypeInfo }];
     let Read = [{
       QualType(node.getLValueBase().get<TypeInfoLValue>().getType(), 0)
     }];
   }
+  def : Property<"dynamicAlloc", UInt32> {
+    let Conditional = [{ hasBase && isDynamicAlloc }];
+    let Read = [{ node.getLValueBase().get<DynamicAllocLValue>().getIndex() }];
+  }
   def : Property<"type", QualType> {
-    let Conditional = [{ hasBase && isTypeInfo }];
-    let Read = [{ node.getLValueBase().getTypeInfoType() }];
+    let Conditional = [{ hasBase && (isTypeInfo || isDynamicAlloc) }];
+    let Read = [{ 
+      isTypeInfo
+        ? node.getLValueBase().getTypeInfoType()
+        : node.getLValueBase().getDynamicAllocType()
+    }];
   }
   def : Property<"callIndex", UInt32> {
     let Conditional = [{ hasBase && !isTypeInfo }];
     let Read = [{ node.getLValueBase().getCallIndex() }];
   }
   def : Property<"version", UInt32> {
     let Conditional = [{ hasBase && !isTypeInfo }];
     let Read = [{ node.getLValueBase().getVersion() }];
   }
   def : Property<"stmt", StmtRef> {
     let Conditional = [{ hasBase && !isTypeInfo && isExpr }];
     let Read = [{ const_cast<Expr *>(expr) }];
   }
   def : Property<"decl", DeclRef> {
-    let Conditional = [{ hasBase && !isTypeInfo && !isExpr }];
+    let Conditional = [{ hasBase && !isTypeInfo && !isDynamicAlloc && !isExpr }];
     let Read = [{ lvalueBase.get<const ValueDecl *>() }];
   }
   def : Property<"offsetQuantity", UInt32> {
     let Read = [{ node.getLValueOffset().getQuantity() }];
   }
   def : Property<"lvaluePath", LValuePathSerializationHelper> {
     let Conditional = [{ hasLValuePath }];
     let Read = [{
       APValue::LValuePathSerializationHelper(node.getLValuePath(), elemTy)
     }];
   }
   def : Creator<[{
     (void)ctx;
     APValue::LValueBase base;
     if (hasBase) {
       if (isTypeInfo) {
         base = APValue::LValueBase::getTypeInfo(
             TypeInfoLValue(typeInfo->getTypePtr()), *type);
+      } else if (isDynamicAlloc) {
+        base = APValue::LValueBase::getDynamicAlloc(
+            DynamicAllocLValue(*dynamicAlloc), *type);
       } else if (isExpr) {
         base = APValue::LValueBase(cast<Expr>(*stmt),
                                    *callIndex, *version);
       } else {
         base = APValue::LValueBase(cast<ValueDecl>(*decl),
                                    *callIndex, *version);
       }
     }

clang/test/AST/dynamic-alloc-lvalue.cpp

+// RUN: %clang_cc1 -emit-pch -o %t %s
+
+// Test that serialization/deserialization of a DynamicAllocLValue
+// variant of APValue does not crash.
+
+#ifndef HEADER
+#define HEADER
+
+struct A {  int *p; };
+const A &w = A{ new int(10) };
+
+#endif