Don't rely on undefined behavior to store how a User object's allocation is laid out (#105714)

In `User::operator new` a single allocation is created to store the
`User` object itself, "intrusive" operands or a pointer for "hung off"
operands, and the descriptor. After allocation, details about the layout
(number of operands, how the operands are stored, if there is a
descriptor) are stored in the `User` object by settings its fields. The
`Value` and `User` constructors are then very careful not to initialize
these fields so that the values set during allocation can be
subsequently read. However, when the `User` object is returned from
`operator new` [its value is technically "indeterminate" and so reading
a field without first initializing it is undefined behavior (and will be
erroneous in
C++26)](https://en.cppreference.com/w/cpp/language/default_initialization#Indeterminate_and_erroneous_values).

We discovered this issue when trying to build LLVM using MSVC's [`/sdl`
flag](https://learn.microsoft.com/en-us/cpp/build/reference/sdl-enable-additional-security-checks?view=msvc-170)
which clears class fields after allocation (the docs say that this
feature shouldn't be turned on for custom allocators and should only
clear pointers, but that doesn't seem to match the implementation).
MSVC's behavior both with and without the `/sdl` flag is standards
conforming since a program is supposed to initialize storage before
reading from it, thus the compiler implementation changing any values
will never be observed in a well-formed program. The standard also
provides no provisions for making storage bytes not indeterminate by
setting them during allocation or `operator new`.

The fix for this is to create a set of types that encode the layout and
provide these to both `operator new` and the constructor:
* The `AllocMarker` types are used to select which `operator new` to
use.
* `AllocMarker` can then be implicitly converted to a `AllocInfo` which
tells the constructor how the type was laid out.

Author: dpaoliello

llvm/include/llvm/Analysis/MemorySSA.h

@@ -218,8 +218,8 @@ class MemoryAccess
   inline unsigned getID() const;
 
   MemoryAccess(LLVMContext &C, unsigned Vty, DeleteValueTy DeleteValue,
-               BasicBlock *BB, unsigned NumOperands)
-      : DerivedUser(Type::getVoidTy(C), Vty, nullptr, NumOperands, DeleteValue),
+               BasicBlock *BB, AllocInfo AllocInfo)
+      : DerivedUser(Type::getVoidTy(C), Vty, AllocInfo, DeleteValue),
         Block(BB) {}
 
   // Use deleteValue() to delete a generic MemoryAccess.
@@ -280,8 +280,8 @@ class MemoryUseOrDef : public MemoryAccess {
 
   MemoryUseOrDef(LLVMContext &C, MemoryAccess *DMA, unsigned Vty,
                  DeleteValueTy DeleteValue, Instruction *MI, BasicBlock *BB,
-                 unsigned NumOperands)
-      : MemoryAccess(C, Vty, DeleteValue, BB, NumOperands),
+                 AllocInfo AllocInfo)
+      : MemoryAccess(C, Vty, DeleteValue, BB, AllocInfo),
         MemoryInstruction(MI) {
     setDefiningAccess(DMA);
   }
@@ -307,15 +307,16 @@ class MemoryUseOrDef : public MemoryAccess {
 /// MemoryUse's is exactly the set of Instructions for which
 /// AliasAnalysis::getModRefInfo returns "Ref".
 class MemoryUse final : public MemoryUseOrDef {
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{1};
+
 public:
   DECLARE_TRANSPARENT_OPERAND_ACCESSORS(MemoryAccess);
 
   MemoryUse(LLVMContext &C, MemoryAccess *DMA, Instruction *MI, BasicBlock *BB)
-      : MemoryUseOrDef(C, DMA, MemoryUseVal, deleteMe, MI, BB,
-                       /*NumOperands=*/1) {}
+      : MemoryUseOrDef(C, DMA, MemoryUseVal, deleteMe, MI, BB, AllocMarker) {}
 
   // allocate space for exactly one operand
-  void *operator new(size_t S) { return User::operator new(S, 1); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
   void operator delete(void *Ptr) { User::operator delete(Ptr); }
 
   static bool classof(const Value *MA) {
@@ -367,19 +368,20 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(MemoryUse, MemoryAccess)
 /// associated with them. This use points to the nearest reaching
 /// MemoryDef/MemoryPhi.
 class MemoryDef final : public MemoryUseOrDef {
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{2};
+
 public:
   friend class MemorySSA;
 
   DECLARE_TRANSPARENT_OPERAND_ACCESSORS(MemoryAccess);
 
   MemoryDef(LLVMContext &C, MemoryAccess *DMA, Instruction *MI, BasicBlock *BB,
             unsigned Ver)
-      : MemoryUseOrDef(C, DMA, MemoryDefVal, deleteMe, MI, BB,
-                       /*NumOperands=*/2),
+      : MemoryUseOrDef(C, DMA, MemoryDefVal, deleteMe, MI, BB, AllocMarker),
         ID(Ver) {}
 
   // allocate space for exactly two operands
-  void *operator new(size_t S) { return User::operator new(S, 2); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
   void operator delete(void *Ptr) { User::operator delete(Ptr); }
 
   static bool classof(const Value *MA) {
@@ -474,8 +476,10 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(MemoryUseOrDef, MemoryAccess)
 /// Because MemoryUse's do not generate new definitions, they do not have this
 /// issue.
 class MemoryPhi final : public MemoryAccess {
+  constexpr static HungOffOperandsAllocMarker AllocMarker{};
+
   // allocate space for exactly zero operands
-  void *operator new(size_t S) { return User::operator new(S); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
 
 public:
   void operator delete(void *Ptr) { User::operator delete(Ptr); }
@@ -484,7 +488,7 @@ class MemoryPhi final : public MemoryAccess {
   DECLARE_TRANSPARENT_OPERAND_ACCESSORS(MemoryAccess);
 
   MemoryPhi(LLVMContext &C, BasicBlock *BB, unsigned Ver, unsigned NumPreds = 0)
-      : MemoryAccess(C, MemoryPhiVal, deleteMe, BB, 0), ID(Ver),
+      : MemoryAccess(C, MemoryPhiVal, deleteMe, BB, AllocMarker), ID(Ver),
         ReservedSpace(NumPreds) {
     allocHungoffUses(ReservedSpace);
   }

llvm/include/llvm/IR/Constant.h

@@ -41,8 +41,8 @@ class APInt;
 /// LLVM Constant Representation
 class Constant : public User {
 protected:
-  Constant(Type *ty, ValueTy vty, Use *Ops, unsigned NumOps)
-    : User(ty, vty, Ops, NumOps) {}
+  Constant(Type *ty, ValueTy vty, AllocInfo AllocInfo)
+      : User(ty, vty, AllocInfo) {}
 
   ~Constant() = default;
 

llvm/include/llvm/IR/Constants.h

@@ -51,16 +51,18 @@ template <class ConstantClass> struct ConstantAggrKeyType;
 /// Since they can be in use by unrelated modules (and are never based on
 /// GlobalValues), it never makes sense to RAUW them.
 class ConstantData : public Constant {
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{0};
+
   friend class Constant;
 
   Value *handleOperandChangeImpl(Value *From, Value *To) {
     llvm_unreachable("Constant data does not have operands!");
   }
 
 protected:
-  explicit ConstantData(Type *Ty, ValueTy VT) : Constant(Ty, VT, nullptr, 0) {}
+  explicit ConstantData(Type *Ty, ValueTy VT) : Constant(Ty, VT, AllocMarker) {}
 
-  void *operator new(size_t S) { return User::operator new(S, 0); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
 
 public:
   void operator delete(void *Ptr) { User::operator delete(Ptr); }
@@ -399,7 +401,8 @@ class ConstantAggregateZero final : public ConstantData {
 /// use operands.
 class ConstantAggregate : public Constant {
 protected:
-  ConstantAggregate(Type *T, ValueTy VT, ArrayRef<Constant *> V);
+  ConstantAggregate(Type *T, ValueTy VT, ArrayRef<Constant *> V,
+                    AllocInfo AllocInfo);
 
 public:
   /// Transparently provide more efficient getOperand methods.
@@ -425,7 +428,7 @@ class ConstantArray final : public ConstantAggregate {
   friend struct ConstantAggrKeyType<ConstantArray>;
   friend class Constant;
 
-  ConstantArray(ArrayType *T, ArrayRef<Constant *> Val);
+  ConstantArray(ArrayType *T, ArrayRef<Constant *> Val, AllocInfo AllocInfo);
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
@@ -457,7 +460,7 @@ class ConstantStruct final : public ConstantAggregate {
   friend struct ConstantAggrKeyType<ConstantStruct>;
   friend class Constant;
 
-  ConstantStruct(StructType *T, ArrayRef<Constant *> Val);
+  ConstantStruct(StructType *T, ArrayRef<Constant *> Val, AllocInfo AllocInfo);
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
@@ -509,7 +512,7 @@ class ConstantVector final : public ConstantAggregate {
   friend struct ConstantAggrKeyType<ConstantVector>;
   friend class Constant;
 
-  ConstantVector(VectorType *T, ArrayRef<Constant *> Val);
+  ConstantVector(VectorType *T, ArrayRef<Constant *> Val, AllocInfo AllocInfo);
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
@@ -890,9 +893,11 @@ class ConstantTargetNone final : public ConstantData {
 class BlockAddress final : public Constant {
   friend class Constant;
 
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{2};
+
   BlockAddress(Function *F, BasicBlock *BB);
 
-  void *operator new(size_t S) { return User::operator new(S, 2); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
@@ -936,9 +941,11 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(BlockAddress, Value)
 class DSOLocalEquivalent final : public Constant {
   friend class Constant;
 
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{1};
+
   DSOLocalEquivalent(GlobalValue *GV);
 
-  void *operator new(size_t S) { return User::operator new(S, 1); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
@@ -973,9 +980,11 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(DSOLocalEquivalent, Value)
 class NoCFIValue final : public Constant {
   friend class Constant;
 
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{1};
+
   NoCFIValue(GlobalValue *GV);
 
-  void *operator new(size_t S) { return User::operator new(S, 1); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
@@ -1013,10 +1022,12 @@ class ConstantPtrAuth final : public Constant {
   friend struct ConstantPtrAuthKeyType;
   friend class Constant;
 
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{4};
+
   ConstantPtrAuth(Constant *Ptr, ConstantInt *Key, ConstantInt *Disc,
                   Constant *AddrDisc);
 
-  void *operator new(size_t s) { return User::operator new(s, 4); }
+  void *operator new(size_t s) { return User::operator new(s, AllocMarker); }
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
@@ -1102,8 +1113,8 @@ class ConstantExpr : public Constant {
   Value *handleOperandChangeImpl(Value *From, Value *To);
 
 protected:
-  ConstantExpr(Type *ty, unsigned Opcode, Use *Ops, unsigned NumOps)
-      : Constant(ty, ConstantExprVal, Ops, NumOps) {
+  ConstantExpr(Type *ty, unsigned Opcode, AllocInfo AllocInfo)
+      : Constant(ty, ConstantExprVal, AllocInfo) {
     // Operation type (an Instruction opcode) is stored as the SubclassData.
     setValueSubclassData(Opcode);
   }

llvm/include/llvm/IR/DerivedUser.h

@@ -34,9 +34,9 @@ class DerivedUser : public User {
   DeleteValueTy DeleteValue;
 
 public:
-  DerivedUser(Type *Ty, unsigned VK, Use *U, unsigned NumOps,
+  DerivedUser(Type *Ty, unsigned VK, AllocInfo AllocInfo,
               DeleteValueTy DeleteValue)
-      : User(Ty, VK, U, NumOps), DeleteValue(DeleteValue) {}
+      : User(Ty, VK, AllocInfo), DeleteValue(DeleteValue) {}
 };
 
 } // end namespace llvm

llvm/include/llvm/IR/Function.h

@@ -72,6 +72,8 @@ class LLVM_ABI Function : public GlobalObject, public ilist_node<Function> {
   using const_arg_iterator = const Argument *;
 
 private:
+  constexpr static HungOffOperandsAllocMarker AllocMarker{};
+
   // Important things that make up a function!
   BasicBlockListType BasicBlocks;         ///< The basic blocks
 
@@ -171,13 +173,14 @@ class LLVM_ABI Function : public GlobalObject, public ilist_node<Function> {
   static Function *Create(FunctionType *Ty, LinkageTypes Linkage,
                           unsigned AddrSpace, const Twine &N = "",
                           Module *M = nullptr) {
-    return new Function(Ty, Linkage, AddrSpace, N, M);
+    return new (AllocMarker) Function(Ty, Linkage, AddrSpace, N, M);
   }
 
   // TODO: remove this once all users have been updated to pass an AddrSpace
   static Function *Create(FunctionType *Ty, LinkageTypes Linkage,
                           const Twine &N = "", Module *M = nullptr) {
-    return new Function(Ty, Linkage, static_cast<unsigned>(-1), N, M);
+    return new (AllocMarker)
+        Function(Ty, Linkage, static_cast<unsigned>(-1), N, M);
   }
 
   /// Creates a new function and attaches it to a module.

llvm/include/llvm/IR/GlobalAlias.h

@@ -28,6 +28,8 @@ template <typename ValueSubClass, typename... Args> class SymbolTableListTraits;
 class GlobalAlias : public GlobalValue, public ilist_node<GlobalAlias> {
   friend class SymbolTableListTraits<GlobalAlias>;
 
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{1};
+
   GlobalAlias(Type *Ty, unsigned AddressSpace, LinkageTypes Linkage,
               const Twine &Name, Constant *Aliasee, Module *Parent);
 
@@ -59,7 +61,7 @@ class GlobalAlias : public GlobalValue, public ilist_node<GlobalAlias> {
   static GlobalAlias *create(const Twine &Name, GlobalValue *Aliasee);
 
   // allocate space for exactly one operand
-  void *operator new(size_t S) { return User::operator new(S, 1); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
   void operator delete(void *Ptr) { User::operator delete(Ptr); }
 
   /// Provide fast operand accessors

llvm/include/llvm/IR/GlobalIFunc.h

@@ -34,6 +34,8 @@ template <typename ValueSubClass, typename... Args> class SymbolTableListTraits;
 class GlobalIFunc final : public GlobalObject, public ilist_node<GlobalIFunc> {
   friend class SymbolTableListTraits<GlobalIFunc>;
 
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{1};
+
   GlobalIFunc(Type *Ty, unsigned AddressSpace, LinkageTypes Linkage,
               const Twine &Name, Constant *Resolver, Module *Parent);
 
@@ -48,7 +50,7 @@ class GlobalIFunc final : public GlobalObject, public ilist_node<GlobalIFunc> {
                              Constant *Resolver, Module *Parent);
 
   // allocate space for exactly one operand
-  void *operator new(size_t S) { return User::operator new(S, 1); }
+  void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
   void operator delete(void *Ptr) { User::operator delete(Ptr); }
 
   /// Provide fast operand accessors

llvm/include/llvm/IR/GlobalObject.h

@@ -40,10 +40,9 @@ class GlobalObject : public GlobalValue {
   };
 
 protected:
-  GlobalObject(Type *Ty, ValueTy VTy, Use *Ops, unsigned NumOps,
-               LinkageTypes Linkage, const Twine &Name,
-               unsigned AddressSpace = 0)
-      : GlobalValue(Ty, VTy, Ops, NumOps, Linkage, Name, AddressSpace) {
+  GlobalObject(Type *Ty, ValueTy VTy, AllocInfo AllocInfo, LinkageTypes Linkage,
+               const Twine &Name, unsigned AddressSpace = 0)
+      : GlobalValue(Ty, VTy, AllocInfo, Linkage, Name, AddressSpace) {
     setGlobalValueSubClassData(0);
   }
   ~GlobalObject();

llvm/include/llvm/IR/GlobalValue.h

@@ -77,9 +77,9 @@ class GlobalValue : public Constant {
   };
 
 protected:
-  GlobalValue(Type *Ty, ValueTy VTy, Use *Ops, unsigned NumOps,
-              LinkageTypes Linkage, const Twine &Name, unsigned AddressSpace)
-      : Constant(PointerType::get(Ty, AddressSpace), VTy, Ops, NumOps),
+  GlobalValue(Type *Ty, ValueTy VTy, AllocInfo AllocInfo, LinkageTypes Linkage,
+              const Twine &Name, unsigned AddressSpace)
+      : Constant(PointerType::get(Ty, AddressSpace), VTy, AllocInfo),
         ValueType(Ty), Visibility(DefaultVisibility),
         UnnamedAddrVal(unsigned(UnnamedAddr::None)),
         DllStorageClass(DefaultStorageClass), ThreadLocal(NotThreadLocal),

llvm/include/llvm/IR/GlobalVariable.h

@@ -39,6 +39,8 @@ class DIGlobalVariableExpression;
 class GlobalVariable : public GlobalObject, public ilist_node<GlobalVariable> {
   friend class SymbolTableListTraits<GlobalVariable>;
 
+  constexpr static IntrusiveOperandsAllocMarker AllocMarker{1};
+
   AttributeSet Attrs;
 
   // Is this a global constant?
@@ -70,24 +72,31 @@ class GlobalVariable : public GlobalObject, public ilist_node<GlobalVariable> {
   GlobalVariable(const GlobalVariable &) = delete;
   GlobalVariable &operator=(const GlobalVariable &) = delete;
 
+private:
+  /// Set the number of operands on a GlobalVariable.
+  ///
+  /// GlobalVariable always allocates space for a single operands, but
+  /// doesn't always use it.
+  void setGlobalVariableNumOperands(unsigned NumOps) {
+    assert(NumOps <= 1 && "GlobalVariable can only have 0 or 1 operands");
+    NumUserOperands = NumOps;
+  }
+
+public:
   ~GlobalVariable() {
     dropAllReferences();
+
+    // Number of operands can be set to 0 after construction and initialization.
+    // Make sure that number of operands is reset to 1, as this is needed in
+    // User::operator delete
+    setGlobalVariableNumOperands(1);
   }
 
   // allocate space for exactly one operand
-  void *operator new(size_t s) {
-    return User::operator new(s, 1);
-  }
+  void *operator new(size_t s) { return User::operator new(s, AllocMarker); }
 
   // delete space for exactly one operand as created in the corresponding new operator
-  void operator delete(void *ptr){
-    assert(ptr != nullptr && "must not be nullptr");
-    User *Obj = static_cast<User *>(ptr);
-    // Number of operands can be set to 0 after construction and initialization. Make sure
-    // that number of operands is reset to 1, as this is needed in User::operator delete
-    Obj->setGlobalVariableNumOperands(1);
-    User::operator delete(Obj);
-  }
+  void operator delete(void *ptr) { User::operator delete(ptr); }
 
   /// Provide fast operand accessors
   DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value);