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/Function.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/Function.cpp')
-rw-r--r-- | llvm/lib/IR/Function.cpp | 5 |
1 files changed, 2 insertions, 3 deletions
diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp index afef893..82ff4e1 100644 --- a/llvm/lib/IR/Function.cpp +++ b/llvm/lib/IR/Function.cpp @@ -402,7 +402,7 @@ Function *Function::createWithDefaultAttr(FunctionType *Ty, LinkageTypes Linkage, unsigned AddrSpace, const Twine &N, Module *M) { - auto *F = new Function(Ty, Linkage, AddrSpace, N, M); + auto *F = new (AllocMarker) Function(Ty, Linkage, AddrSpace, N, M); AttrBuilder B(F->getContext()); UWTableKind UWTable = M->getUwtable(); if (UWTable != UWTableKind::None) @@ -501,8 +501,7 @@ static unsigned computeAddrSpace(unsigned AddrSpace, Module *M) { Function::Function(FunctionType *Ty, LinkageTypes Linkage, unsigned AddrSpace, const Twine &name, Module *ParentModule) - : GlobalObject(Ty, Value::FunctionVal, - OperandTraits<Function>::op_begin(this), 0, Linkage, name, + : GlobalObject(Ty, Value::FunctionVal, AllocMarker, Linkage, name, computeAddrSpace(AddrSpace, ParentModule)), NumArgs(Ty->getNumParams()), IsNewDbgInfoFormat(UseNewDbgInfoFormat) { assert(FunctionType::isValidReturnType(getReturnType()) && |