diff options
author | Daniel Paoliello <danpao@microsoft.com> | 2024-09-11 11:34:26 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-09-11 11:34:26 -0700 |
commit | e3f936eb755d9ae37019ffcc7f53d71d2d58d188 (patch) | |
tree | 24824fd0e3fc43979a5883545e5b9adeefc3c1b9 /llvm/lib/IR/Constants.cpp | |
parent | be770edee59310b158bf3a30ddc2645007ab8da3 (diff) | |
download | llvm-e3f936eb755d9ae37019ffcc7f53d71d2d58d188.zip llvm-e3f936eb755d9ae37019ffcc7f53d71d2d58d188.tar.gz llvm-e3f936eb755d9ae37019ffcc7f53d71d2d58d188.tar.bz2 |
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.
Diffstat (limited to 'llvm/lib/IR/Constants.cpp')
-rw-r--r-- | llvm/lib/IR/Constants.cpp | 36 |
1 files changed, 18 insertions, 18 deletions
diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp index e32a54f..6d035d5 100644 --- a/llvm/lib/IR/Constants.cpp +++ b/llvm/lib/IR/Constants.cpp @@ -1267,9 +1267,9 @@ static Constant *getSequenceIfElementsMatch(Constant *C, } ConstantAggregate::ConstantAggregate(Type *T, ValueTy VT, - ArrayRef<Constant *> V) - : Constant(T, VT, OperandTraits<ConstantAggregate>::op_end(this) - V.size(), - V.size()) { + ArrayRef<Constant *> V, + AllocInfo AllocInfo) + : Constant(T, VT, AllocInfo) { llvm::copy(V, op_begin()); // Check that types match, unless this is an opaque struct. @@ -1282,8 +1282,9 @@ ConstantAggregate::ConstantAggregate(Type *T, ValueTy VT, } } -ConstantArray::ConstantArray(ArrayType *T, ArrayRef<Constant *> V) - : ConstantAggregate(T, ConstantArrayVal, V) { +ConstantArray::ConstantArray(ArrayType *T, ArrayRef<Constant *> V, + AllocInfo AllocInfo) + : ConstantAggregate(T, ConstantArrayVal, V, AllocInfo) { assert(V.size() == T->getNumElements() && "Invalid initializer for constant array"); } @@ -1346,8 +1347,9 @@ StructType *ConstantStruct::getTypeForElements(ArrayRef<Constant*> V, return getTypeForElements(V[0]->getContext(), V, Packed); } -ConstantStruct::ConstantStruct(StructType *T, ArrayRef<Constant *> V) - : ConstantAggregate(T, ConstantStructVal, V) { +ConstantStruct::ConstantStruct(StructType *T, ArrayRef<Constant *> V, + AllocInfo AllocInfo) + : ConstantAggregate(T, ConstantStructVal, V, AllocInfo) { assert((T->isOpaque() || V.size() == T->getNumElements()) && "Invalid initializer for constant struct"); } @@ -1388,8 +1390,9 @@ Constant *ConstantStruct::get(StructType *ST, ArrayRef<Constant*> V) { return ST->getContext().pImpl->StructConstants.getOrCreate(ST, V); } -ConstantVector::ConstantVector(VectorType *T, ArrayRef<Constant *> V) - : ConstantAggregate(T, ConstantVectorVal, V) { +ConstantVector::ConstantVector(VectorType *T, ArrayRef<Constant *> V, + AllocInfo AllocInfo) + : ConstantAggregate(T, ConstantVectorVal, V, AllocInfo) { assert(V.size() == cast<FixedVectorType>(T)->getNumElements() && "Invalid initializer for constant vector"); } @@ -1879,7 +1882,7 @@ BlockAddress *BlockAddress::get(Function *F, BasicBlock *BB) { BlockAddress::BlockAddress(Function *F, BasicBlock *BB) : Constant(PointerType::get(F->getContext(), F->getAddressSpace()), - Value::BlockAddressVal, &Op<0>(), 2) { + Value::BlockAddressVal, AllocMarker) { setOperand(0, F); setOperand(1, BB); BB->AdjustBlockAddressRefCount(1); @@ -1951,7 +1954,7 @@ DSOLocalEquivalent *DSOLocalEquivalent::get(GlobalValue *GV) { } DSOLocalEquivalent::DSOLocalEquivalent(GlobalValue *GV) - : Constant(GV->getType(), Value::DSOLocalEquivalentVal, &Op<0>(), 1) { + : Constant(GV->getType(), Value::DSOLocalEquivalentVal, AllocMarker) { setOperand(0, GV); } @@ -2009,7 +2012,7 @@ NoCFIValue *NoCFIValue::get(GlobalValue *GV) { } NoCFIValue::NoCFIValue(GlobalValue *GV) - : Constant(GV->getType(), Value::NoCFIValueVal, &Op<0>(), 1) { + : Constant(GV->getType(), Value::NoCFIValueVal, AllocMarker) { setOperand(0, GV); } @@ -2056,7 +2059,7 @@ ConstantPtrAuth *ConstantPtrAuth::getWithSameSchema(Constant *Pointer) const { ConstantPtrAuth::ConstantPtrAuth(Constant *Ptr, ConstantInt *Key, ConstantInt *Disc, Constant *AddrDisc) - : Constant(Ptr->getType(), Value::ConstantPtrAuthVal, &Op<0>(), 4) { + : Constant(Ptr->getType(), Value::ConstantPtrAuthVal, AllocMarker) { assert(Ptr->getType()->isPointerTy()); assert(Key->getBitWidth() == 32); assert(Disc->getBitWidth() == 64); @@ -2758,11 +2761,8 @@ const char *ConstantExpr::getOpcodeName() const { GetElementPtrConstantExpr::GetElementPtrConstantExpr( Type *SrcElementTy, Constant *C, ArrayRef<Constant *> IdxList, Type *DestTy, - std::optional<ConstantRange> InRange) - : ConstantExpr(DestTy, Instruction::GetElementPtr, - OperandTraits<GetElementPtrConstantExpr>::op_end(this) - - (IdxList.size() + 1), - IdxList.size() + 1), + std::optional<ConstantRange> InRange, AllocInfo AllocInfo) + : ConstantExpr(DestTy, Instruction::GetElementPtr, AllocInfo), SrcElementTy(SrcElementTy), ResElementTy(GetElementPtrInst::getIndexedType(SrcElementTy, IdxList)), InRange(std::move(InRange)) { |