[clang] [clang][bytecode] Check for invalid RecordDecls in Pointer::toAPValue() (PR #216537)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

We can't pass forward declarations to ASTContext::getRecordLayout().

---
Full diff: https://github.com/llvm/llvm-project/pull/216537.diff


2 Files Affected:

- (modified) clang/lib/AST/ByteCode/Pointer.cpp (+21-8) 
- (modified) clang/test/AST/ByteCode/cxx23.cpp (+22) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 4f36d20b352cb..38314312c7e79 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -170,6 +170,11 @@ Pointer &Pointer::operator=(Pointer &&P) {
   return *this;
 }
 
+static bool validRecordDecl(const RecordDecl *D) {
+  D = D->getDefinition();
+  return D && !D->isInvalidDecl() && D->isCompleteDefinition();
+}
+
 APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
   llvm::SmallVector<APValue::LValuePathEntry, 5> Path;
 
@@ -222,11 +227,9 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
 
   CharUnits Offset = CharUnits::Zero();
 
-  auto getFieldOffset = [&](const FieldDecl *FD) -> CharUnits {
-    // This shouldn't happen, but if it does, don't crash inside
-    // getASTRecordLayout.
-    if (FD->getParent()->isInvalidDecl())
-      return CharUnits::Zero();
+  auto getFieldOffset = [&](const FieldDecl *FD) -> std::optional<CharUnits> {
+    if (!validRecordDecl(FD->getParent()))
+      return std::nullopt;
     const ASTRecordLayout &Layout = ASTCtx.getASTRecordLayout(FD->getParent());
     unsigned FieldIndex = FD->getFieldIndex();
     return ASTCtx.toCharUnitsFromBits(Layout.getFieldOffset(FieldIndex));
@@ -254,8 +257,12 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
         const auto *Dcl = Desc->asDecl();
         Path.push_back(APValue::LValuePathEntry({Dcl, /*IsVirtual=*/false}));
 
-        if (const auto *FD = dyn_cast_if_present<FieldDecl>(Dcl))
-          Offset += getFieldOffset(FD);
+        if (const auto *FD = dyn_cast_if_present<FieldDecl>(Dcl)) {
+          if (std::optional<CharUnits> FieldOffset = getFieldOffset(FD))
+            Offset += *FieldOffset;
+          else
+            return APValue();
+        }
 
         Ptr = Ptr.getBase();
       }
@@ -287,12 +294,18 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
         bool IsVirtual = false;
         if (const auto *FD = dyn_cast<FieldDecl>(BaseOrMember)) {
           Ptr = Ptr.getBase();
-          Offset += getFieldOffset(FD);
+          if (std::optional<CharUnits> FieldOffset = getFieldOffset(FD))
+            Offset += *FieldOffset;
+          else
+            return APValue();
         } else if (const auto *RD = dyn_cast<CXXRecordDecl>(BaseOrMember)) {
           IsVirtual = Ptr.isVirtualBaseClass();
           Ptr = Ptr.getBase();
           const Record *BaseRecord = Ptr.getRecord();
 
+          if (!validRecordDecl(BaseRecord->getDecl()))
+            return APValue();
+
           const ASTRecordLayout &Layout = ASTCtx.getASTRecordLayout(
               cast<CXXRecordDecl>(BaseRecord->getDecl()));
           if (IsVirtual)
diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp
index 5607ec9b59cb5..e0e72fafd4d03 100644
--- a/clang/test/AST/ByteCode/cxx23.cpp
+++ b/clang/test/AST/ByteCode/cxx23.cpp
@@ -667,4 +667,26 @@ namespace BrokenExplicitInstanceParam {
   static_assert( (&decltype(b)::operator())(1) == 1); // expected-error {{not an integral constant expression}}
 }
 
+namespace InvalidRecord {
+  struct S {
+    S();
+  };
+
+  template <typename T> void F(S &, T...);
+
+  struct SS {
+    template <typename T> SS(T &val) { __builtin_dump_struct(&val, F, s); }
+    S s;
+  };
+
+  template <typename T> S foo(const T &t) { return SS(t).s; }
+
+  struct A {
+    S s;
+  };
+
+  struct B : A; // all-error {{expected '{' after base class list}}
+
+  static_assert(foo(B{1, 2, 3}), "");
+}
 #endif

``````````

</details>


https://github.com/llvm/llvm-project/pull/216537