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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to