4 ms·

> LLVM will now remove stores to constant memory (since this is a contradiction) under the assumption the code in question must be dead. This has proven to be p

> LLVM will now remove stores to constant memory (since this is a contradiction) under the assumption the code in question must be dead. This has proven to be problematic for some C/C++ code bases which expect to be able to cast away ‘const’. This is (and has always been) undefined behavior, but up until now had not been actively utilized for optimization purposes in this exact way. For more information, please see: bug 42763 and post commit discussion. This will break all sorts of code. Often we have things that are const for callers of an API but not necessarily for the implementation of an API. Other cases are where an API should have used const but didn't and then got partly constified, and there are places where you still need to cast away const-ness.

pubby 7y ago
I don't see this change breaking the examples you have given. You can always modify non-const objects. What you can't do is modify const objects through a pointer to non-const. e.g. ((char*)"fooo")[0] = 1 is broken code but int x = 0; int const* y = &x; *((int*)y) = 1; is not.
- cryptonector 7y ago
  That's not what I got from the quoted paragraph. I'll have to chase the links to be sure.
  - jcranmer 7y ago
    In LLVM terms, it's referring to these kinds of stores: @vval = constant double 1.0, align 8 define void @foo(double *val) { store double %val, double @cval, align 8 | At the LLVM level, the "const" qualifier to pointers and other non-global values get reflected nowhere in the type system. But if you declare a const global variable, that variable gets placed in a read-only data section.
    - cryptonector 7y ago
      Got it. Thanks!
moefh 7y ago
I think it will break a lot less code than you think. The C standard has a subtlety with regards to const in that it allows you to (for example) cast away the const-ness of a pointer and use it to write to it, as long as the original memory was not const (i.e., it was gratuitously cast to const). That's used by a lot of code, and LLVM still respects that. This optimization (AFAIK) only removes stores that are disallowed by the standard, which is when the memory itself you're trying to store to was actually declared const. This would most likely cause a crash anyway, the memory would in most cases be inside a read-only segment. The bug report mentioned is from code inside a kernel, not from a normal executable that would be loaded with proper memory protection (like making read-only sections actually read-only), which is probably why the code worked without the optimization. See the example in[1]: - The function g casts away the const to write to the pointed memory, the generated assembly does what you'd expect (it contains a store). - The function f tries to write to memory the compiler knows is actually const, clang optimizes the store away. Trying to actually write there would crash if the program was loaded in a normal OS, since the memory is inside a read-only segment. [1] https://godbolt.org/z/as3Mh5 https://godbolt.org/z/as3Mh5
- account42 7y ago
  You can have const variables on the stack. Those will not have hardware write protection but it's still undefined to write to them.
d33 7y ago
I guess the question is: will it silently optimize them away or will it be a compile error to use such expressions?
- DerekL 7y ago
  It can't be a compile error. It's not an error for a program to contain such code, it's only an error if that code executes.
  - leni536 7y ago
    That's not strictly true. Undefined behavior includes unsuccessful compilation as well [0]. But for this the compiler has to prove that the offending code is executed every time the program is executed. I think compilers don't bother with this though. [0] https://eel.is/c++draft/intro.defs#defns.undefined-sentence-3 https://eel.is/c++draft/intro.defs#defns.undefined-sentence-...
rurban 7y ago
Optimizing away even more calls of memset, memcpy or memmove is the completely wrong way. gcc went down this bug and called it "optimization", now llvm did the same. Can we at least keep at least one sane working compiler? At least emit a prominent warning when "optimizing" away lib calls. This helps only artificial benchmarks and will hurt everybody. If an assignment to const memory is illegal, treat it as error, and don't silently remove it. Fear we'll have to blacklist clang-9 also, not just gcc-9.
- mlindner 7y ago
  How about you fix your code to not rely on undefined behavior specific to a compiler version? Poor coding standards is not an excuse.
- sanxiyn 7y ago
  > This helps only artificial benchmarks and will hurt everybody. Any source of this claim? As I understand, on SPEC 2006 lib call optimizations make 5% improvement with GCC, the least artificial benchmark among SPEC 2006.
- d33 7y ago
  I believe rurban might have some point - for example, consider this CCC talk: https://www.youtube.com/watch?v=0WzjAKABSDk https://www.youtube.com/watch?v=0WzjAKABSDk The point is that memset is often critical for security and compilers don't always do a good job of optimizing it out. If they strike a false positive, information leak vulnerabilities are very likely. It's also pretty difficult to detect those failures without recompiling entire program with a modified compiler, as shown in the talk.
ndesaulniers 7y ago
So I've been bitten by this twice already, including just today. 1. This was the last bug preventing us from booting a MIPS Linux kernel. 2. An out of tree MediaTek display driver in Android was causing a kernel panic. To be clear, this is explicitly undefined behavior in C. But this optimization to me feels like semantics from C bleeding into LLVM IR a little.
- account42 7y ago
  It's hardly C semantics that const memory should not ever be written to, otherwise there would be no need for the distinction at the LLVM ir level.

pubby 7y ago

I don't see this change breaking the examples you have given. You can always modify non-const objects. What you can't do is modify const objects through a pointer to non-const. e.g. ((char*)"fooo")[0] = 1 is broken code but int x = 0; int const* y = &x; *((int*)y) = 1; is not.

cryptonector 7y ago
That's not what I got from the quoted paragraph. I'll have to chase the links to be sure.
- jcranmer 7y ago
  In LLVM terms, it's referring to these kinds of stores: @vval = constant double 1.0, align 8 define void @foo(double *val) { store double %val, double @cval, align 8 | At the LLVM level, the "const" qualifier to pointers and other non-global values get reflected nowhere in the type system. But if you declare a const global variable, that variable gets placed in a read-only data section.
  - cryptonector 7y ago
    Got it. Thanks!

moefh 7y ago

I think it will break a lot less code than you think. The C standard has a subtlety with regards to const in that it allows you to (for example) cast away the const-ness of a pointer and use it to write to it, as long as the original memory was not const (i.e., it was gratuitously cast to const). That's used by a lot of code, and LLVM still respects that. This optimization (AFAIK) only removes stores that are disallowed by the standard, which is when the memory itself you're trying to store to was actually declared const. This would most likely cause a crash anyway, the memory would in most cases be inside a read-only segment. The bug report mentioned is from code inside a kernel, not from a normal executable that would be loaded with proper memory protection (like making read-only sections actually read-only), which is probably why the code worked without the optimization. See the example in[1]: - The function g casts away the const to write to the pointed memory, the generated assembly does what you'd expect (it contains a store). - The function f tries to write to memory the compiler knows is actually const, clang optimizes the store away. Trying to actually write there would crash if the program was loaded in a normal OS, since the memory is inside a read-only segment. [1] https://godbolt.org/z/as3Mh5 https://godbolt.org/z/as3Mh5

account42 7y ago
You can have const variables on the stack. Those will not have hardware write protection but it's still undefined to write to them.

d33 7y ago

I guess the question is: will it silently optimize them away or will it be a compile error to use such expressions?

DerekL 7y ago
It can't be a compile error. It's not an error for a program to contain such code, it's only an error if that code executes.
- leni536 7y ago
  That's not strictly true. Undefined behavior includes unsuccessful compilation as well [0]. But for this the compiler has to prove that the offending code is executed every time the program is executed. I think compilers don't bother with this though. [0] https://eel.is/c++draft/intro.defs#defns.undefined-sentence-3 https://eel.is/c++draft/intro.defs#defns.undefined-sentence-...

rurban 7y ago

Optimizing away even more calls of memset, memcpy or memmove is the completely wrong way. gcc went down this bug and called it "optimization", now llvm did the same. Can we at least keep at least one sane working compiler? At least emit a prominent warning when "optimizing" away lib calls. This helps only artificial benchmarks and will hurt everybody. If an assignment to const memory is illegal, treat it as error, and don't silently remove it. Fear we'll have to blacklist clang-9 also, not just gcc-9.

mlindner 7y ago
How about you fix your code to not rely on undefined behavior specific to a compiler version? Poor coding standards is not an excuse.
sanxiyn 7y ago
> This helps only artificial benchmarks and will hurt everybody. Any source of this claim? As I understand, on SPEC 2006 lib call optimizations make 5% improvement with GCC, the least artificial benchmark among SPEC 2006.
d33 7y ago
I believe rurban might have some point - for example, consider this CCC talk: https://www.youtube.com/watch?v=0WzjAKABSDk https://www.youtube.com/watch?v=0WzjAKABSDk The point is that memset is often critical for security and compilers don't always do a good job of optimizing it out. If they strike a false positive, information leak vulnerabilities are very likely. It's also pretty difficult to detect those failures without recompiling entire program with a modified compiler, as shown in the talk.

ndesaulniers 7y ago

So I've been bitten by this twice already, including just today. 1. This was the last bug preventing us from booting a MIPS Linux kernel. 2. An out of tree MediaTek display driver in Android was causing a kernel panic. To be clear, this is explicitly undefined behavior in C. But this optimization to me feels like semantics from C bleeding into LLVM IR a little.

account42 7y ago
It's hardly C semantics that const memory should not ever be written to, otherwise there would be no need for the distinction at the LLVM ir level.