[all-commits] [llvm/llvm-project] e3f936: Don't rely on undefined behavior to store how a `U...

[Daniel Paoliello via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: e3f936eb755d9ae37019ffcc7f53d71d2d58d188
      https://github.com/llvm/llvm-project/commit/e3f936eb755d9ae37019ffcc7f53d71d2d58d188
  Author: Daniel Paoliello <danpao at microsoft.com>
  Date:   2024-09-11 (Wed, 11 Sep 2024)

  Changed paths:
    M llvm/include/llvm/Analysis/MemorySSA.h
    M llvm/include/llvm/IR/Constant.h
    M llvm/include/llvm/IR/Constants.h
    M llvm/include/llvm/IR/DerivedUser.h
    M llvm/include/llvm/IR/Function.h
    M llvm/include/llvm/IR/GlobalAlias.h
    M llvm/include/llvm/IR/GlobalIFunc.h
    M llvm/include/llvm/IR/GlobalObject.h
    M llvm/include/llvm/IR/GlobalValue.h
    M llvm/include/llvm/IR/GlobalVariable.h
    M llvm/include/llvm/IR/InstrTypes.h
    M llvm/include/llvm/IR/Instruction.h
    M llvm/include/llvm/IR/Instructions.h
    M llvm/include/llvm/IR/User.h
    M llvm/lib/IR/Constants.cpp
    M llvm/lib/IR/ConstantsContext.h
    M llvm/lib/IR/Function.cpp
    M llvm/lib/IR/Globals.cpp
    M llvm/lib/IR/Instruction.cpp
    M llvm/lib/IR/Instructions.cpp
    M llvm/lib/IR/User.cpp

  Log Message:
  -----------
  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.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications