Fix build failure 'sizeof(clang::APValue::LV) <= DataSize': LV too big'

Ensure APValue::DataSize is bigger than APValue::LV. To do so,
this moves APValue::LV to APValue.h so that we can always allocate
DataSize to be bigger than LV. Previously, LV was hidden in the
source file and the type size of DataSize didn't take it into
account even though it has to be bigger than LV. This made the
implementation quite fragile to any change in the size of related
components.

rdar://137823177

Author: rapidsna

clang/include/clang/AST/APValue.h

@@ -13,6 +13,7 @@
 #ifndef LLVM_CLANG_AST_APVALUE_H
 #define LLVM_CLANG_AST_APVALUE_H
 
+#include "clang/AST/CharUnits.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/APFixedPoint.h"
 #include "llvm/ADT/APFloat.h"
@@ -29,7 +30,6 @@ template <typename T> class BasicReaderBase;
 
   class AddrLabelExpr;
   class ASTContext;
-  class CharUnits;
   class CXXRecordDecl;
   class Decl;
   class DiagnosticBuilder;
@@ -419,10 +419,41 @@ class APValue {
   };
   struct MemberPointerData;
 
+  struct LVBase {
+    APValue::LValueBase Base;
+    CharUnits Offset;
+    // BoundsSafety : While the base also holds a corresponding constant array type
+    // for forged pointer, we still keep track of forged size because the array
+    // size will be different from the actual forged size if it is not a multiple
+    // of element type size after a bitcast. The codegen doesn't round up/down
+    // the bounds to be a type-size multiple, we should keep it the same for
+    // constant emission. Once __builtin_forge_* has a type as an argument, we
+    // may consider round down the size with the element type size.
+    CharUnits ForgedSize;
+    // While 'Offset' is the offset within the LValue, 'ForgedOffset' is the
+    // offset of the base pointer of __builtin_unsafe_forge*. For example, in
+    // the following,
+    // '__bidi_indexable_unsafe_forge_bidi_indexable(base + N) + M'
+    // 'N' should be 'ForgedOffset' and 'M' should be 'Offset'. This way, the
+    // forged pointer itself becomes an LValue starting at base + 'ForgedOffset'.
+    CharUnits ForgedOffset;
+    unsigned PathLength;
+    bool IsNullPtr : 1;
+    bool IsOnePastTheEnd : 1;
+    bool IsForgeBidi : 1;
+    bool IsForgeSingle : 1;
+    bool IsForgeTerminatedBy : 1;
+  };
+
+  struct LVPlaceHolder {
+    LVBase Base;
+    LValuePathEntry Path[1];
+  };
+
   // We ensure elsewhere that Data is big enough for LV and MemberPointerData.
   typedef llvm::AlignedCharArrayUnion<void *, APSInt, APFloat, ComplexAPSInt,
                                       ComplexAPFloat, Vec, Arr, StructData,
-                                      UnionData, AddrLabelDiffData> DataType;
+                                      UnionData, AddrLabelDiffData, LVPlaceHolder> DataType;
   static const size_t DataSize = sizeof(DataType);
 
   DataType Data;

clang/lib/AST/APValue.cpp

@@ -195,34 +195,6 @@ QualType APValue::LValuePathSerializationHelper::getType() {
   return QualType::getFromOpaquePtr(Ty);
 }
 
-namespace {
-  struct LVBase {
-    APValue::LValueBase Base;
-    CharUnits Offset;
-    // BoundsSafety : While the base also holds a corresponding constant array type
-    // for forged pointer, we still keep track of forged size because the array
-    // size will be different from the actual forged size if it is not a multiple
-    // of element type size after a bitcast. The codegen doesn't round up/down
-    // the bounds to be a type-size multiple, we should keep it the same for
-    // constant emission. Once __builtin_forge_* has a type as an argument, we
-    // may consider round down the size with the element type size.
-    CharUnits ForgedSize;
-    // While 'Offset' is the offset within the LValue, 'ForgedOffset' is the
-    // offset of the base pointer of __builtin_unsafe_forge*. For example, in
-    // the following,
-    // '__bidi_indexable_unsafe_forge_bidi_indexable(base + N) + M'
-    // 'N' should be 'ForgedOffset' and 'M' should be 'Offset'. This way, the
-    // forged pointer itself becomes an LValue starting at base + 'ForgedOffset'.
-    CharUnits ForgedOffset;
-    unsigned PathLength;
-    bool IsNullPtr : 1;
-    bool IsOnePastTheEnd : 1;
-    bool IsForgeBidi : 1;
-    bool IsForgeSingle : 1;
-    bool IsForgeTerminatedBy : 1;
-  };
-}
-
 void *APValue::LValueBase::getOpaqueValue() const {
   return Ptr.getOpaqueValue();
 }
@@ -274,6 +246,7 @@ bool llvm::DenseMapInfo<clang::APValue::LValueBase>::isEqual(
 }
 
 struct APValue::LV : LVBase {
+  static_assert(DataSize >= sizeof(LVBase) && "LVBase too big");
   static const unsigned InlinePathSpace =
       (DataSize - sizeof(LVBase)) / sizeof(LValuePathEntry);