[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

[Marco Elver via Phabricator via cfe-commits]

melver reclaimed this revision.
melver added a comment.

> 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.

Thanks for clarifying -- this makes it clear it's "automatic storage". But I think we may still get conflicting views because `-ftrivial-auto-var-init` speaks about "automatic variables", and strictly speaking `__builtin_alloca()` is not a "variable".

> 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.

I agree that having alloca() initialized is desirable normally.

The problem is that it is not at all obvious if `-ftrivial-auto-var-init` implies "automatic storage" or "automatic variables", only. (I mixed that up above as well!)

The latter is what (I think) led GCC folks to claim (and convinced me, too) that `__builtin_alloca()` is out-of-scope. That flag talks too much about "automatic stack variables".

If, however, you declare that it's the former, that's fine too. We just have to be very careful with how we document this -- do we want to replace "automatic stack variable" with "automatic stack storage" in documentation? We have to admit that this part is too easy to misinterpret, which is how we ended up here.

I'm going to reopen this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115440