[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D115440#3183778 <https://reviews.llvm.org/D115440#3183778>, @melver wrote:

> GCC devs say that initializing explicit alloca() is a bug, because they aren't "automatic storage": https://lkml.kernel.org/r/20211209201616.GU614@gate.crashing.org
> .. which is also the reason why GCC's behaviour differs here at the moment.
>
> I actually agree with that, and alloca auto-init behaviour needs a new -mllvm param.

GCC developer Segher says:

> The space allocated by alloca is not an automatic variable, so of course it is not affected by this compiler flag.  And it should not, this flag is explicitly for *small fixed-size* stack variables (initialising others can be much too expensive).
>
>> C. Introduce a new __builtin_alloca_uninitialized().
>
> That is completely backwards.  That is the normal behaviour of alloca already.  Also you can get __builtin_alloca inserted by the compiler (for a variable length array for example), and you typically do not want those initialised either, for the same reasons.

Segher is simply wrong.

Let's consult libc's own documentation https://www.gnu.org/software/libc/manual/html_node/Variable-Size-Automatic.html

It states:

> Automatic Storage with Variable Size
> The function alloca supports a kind of half-dynamic allocation in which blocks are allocated dynamically but freed automatically.

The manpage for `alloca(3)`, states:

> This temporary space is automatically freed when the function that called alloca() returns to its caller".

Refer to C17 which says:

> Storage durations of objects
> An object has a storage duration that determines its lifetime. There are four storage durations: static, thread, automatic, and allocated.

Clearly `alloca` is neither static, nor thread, nor allocated (see "memory management functions" too). It's automatic storage.

Further, Segher misunderstand the purpose of automatic initialization. The goal is explicitly to change implementation-defined behavior from "the compiler leaves stack and register slots with whatever value it happened to overlap with (potentially leading to info leaks or UaF)" to "the compiler always overwrites previous values that were in stack and register slots before they are reused (preventing a large number of info leaks and UaF)". Saying "the normal behaviour of alloca already" is correct, but it ignores the objective of the flag: to explicitly *change* that behavior. The flag opts-in to that implementation-defined behavior change. And it has a very specific purpose w.r.t. security. This has measurable results in terms of CVEs avoided.

Additionally the flag is not explicitly for small fixed-sized variables, that was stated very clearly when it was committed, and the documentation is unambiguous about this fact. If initialization is too expensive then developers needs to opt-out with mechanisms such as `__attribute__((uninitialized))`. The support for VLAs and `alloca` was done very much on purpose, and if developers turn on variable auto-init then they reasonably expect that all automatic variables are automatically initialized, including VLAs and `alloca`. The patch you propose here is a good idea, because there's currently no way to opt-out for `alloca`. We should adopt a solution such as this one, and synchronize with GCC so that developers can use the same code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115440/new/

https://reviews.llvm.org/D115440