Hacker News Reader

Comment by jerf | Hacker News Reader

You don't really understand the UB problem if you model this in your head as some sort of simple set of rules that everybody really should have known all along, and it's not a big deal to fix the UB in your code. You have to make sure you're modeling it as A: a set of rules so arcane and byzantine that virtually nobody understands them, and to a first approximation, no non-trivial well-defined C program has ever been written and B: that this was a de facto change in C's nature... in a very real way, the language changed out from underneath people, without them expecting it.

You can't just shrug this away with "Well, if you want performance...", because people in fact don't want abstract "performance"... they want the language they were truly writing in to perform well, not for what is de facto a different dialect of the language to suddenly appear and replace the language they were using.

View on HN

> You can't just shrug this away with "Well, if you want performance...", because people in fact don't want abstract "performance"... they want the language they were truly writing in to perform well, not for what is de facto a different dialect of the language to suddenly appear and replace the language they were using.

Is wanting a for loop setting an array to zero to optimize into memset optimizing the language they were truly writing in? I think it is. But that optimization frequently depends on undefined behavior.

UB exploitation usually exists because people filed bugs on compilers complaining that they didn't optimize some case they expected to optimize.

Yes, this is a good optimization, since it efficiently does what the programmer intended. The bad optimization is removing the security-essential loop altogether when the compiler notices that result appears unused, and sensitive information is left susceptible to later attack.

UB exploitation usually exists because people filed bugs on compilers complaining that they didn't optimize some case they expected to optimize.

I doubt that anyone has ever filed a bug saying "I explicitly wrote a loop to zero memory, but the compiler failed to optimize it out." If you know of one, please point to it. I think you are throwing out the baby (intentional C) with the bathwater (autogenerated C++).

> I doubt that anyone has ever filed a bug saying "I explicitly wrote a loop to zero memory, but the compiler failed to optimize it out."

That optimization is a natural consequence of SROA and DCE. If you claim you don't want those optimizations, I don't know what to tell you. Those optimizations are some of the most basic, critical optimizations any modern C/C++ compiler does and throwing them away can easily result in at least a 2x performance loss.

The optimization actually has very little to do with SROA. In LLVM there is a special loop idiom recognition pass designed to detect memset / memcpy loops that works after ordinary SSA conversion.

> I doubt that anyone has ever filed a bug saying "I explicitly wrote a loop to zero memory, but the compiler failed to optimize it out."

I suspect it's less that they asked for that than they asked for some generic optimization on loops, and the implementation works great 99% of the time, and 100% of the test cases, but because it was approached from a performance perspective the idea that sometimes we want to change the state of the memory, even though it has no affect on a correctly functioning program, isn't considered. Zeroing memory in this way has nothing to do with correctly running programs, the only use is to defend against your program malfunctioning and exposing the data, or some external actor looking at memory. Neither of those have to do with the normal functionality of the code the compiler is running, but is is important. Unfortunately since it deals with things that have nothing to do with the actual instructions needed to make the program function, it's easy to overlook.

> because it was approached from a performance perspective the idea that sometimes we want to change the state of the memory, even though it has no affect on a correctly functioning program, isn't considered.

A lot of dead stores happen because of post-inlined, post-constant-propagated (etc.) code. Nobody writes that code by hand; it occurs due to inlining and other aggressive types of optimizations. You need those optimizations for performance; IPO is a huge win.

It's like the old joke: "Who writes that kind of code? Macros do."

I understand, I'm not really taking a stance. The question, really, is should the compiler be responsible for making sure your unused bits and ends of memory that are provably irrelevant to the functioning of the program that is being compiled stay exactly as you left them? I think it's interesting because the situation is a bit untenable on both ends of the spectrum.

If we disallow all those optimizations, we end up with hugely wasteful programs in the current C ecosystem. Enforcing this is useless, the market will route around your best intentions, because no company that's making money will be willing to cede a huge performance gap to their competitors.

If we require code be annotated to bypass optimizations, we run afoul of the fact that it's impossible to know what optimizations might be developed in the future that will affect portions of our code that might be security sensitive.

The problem is insidious and deep. Imagine an architecture that allows specialized instructions to speed up zeroing chunks of memory, but does so by hardware mapping different pages around behind the scenes (to swap in a pre-zeroed page for a filled page). Correctly detecting that the architecture has an efficient method for zeroing large chunks of space could be detected and used by the compiler, but the hardware is working against us because the original memory may still be accessible to an external actor. C is ensuring the memory addresses you requested are zeroed, it's just not ensuring that the data that existed within them is no longer anywhere on the system.

The C standard already allows the compiler to remove calls to memset if the memory isn't used afterwards by the program (essentially dead-store elimination). C11 provides memset_s to clear memory for security reasons.

You can easily write a for loop that optimizes to memset without relying on undefined behavior. The only potential problem is that the loop bound evaluation could require a memory access, but this is simple to work around.

Well, not if the compiler can't prove that the stores don't alias the induction variable. TBAA is the easiest and most reliable way to prove that, which is UB exploitation.

No, that is not the most reliable way to prove that property. The most reliable way is to ensure that the loop bound and induction variable are local variables that have not had their address taken.

Well, yes, in the trivial case, but not if the induction variable and/or loop bound come in as references (as in templated code, etc).

There's also the issue of the store changing the array base, as in Chris Lattner's example on "What Every C Programmer Should Know".

You can always make the induction variable and loop bound local variables, and the induction variable is almost always a local variable anyways. It was the WebKit coding style for years to use local variables for loop bounds for this reason.

This is the example you're talking about in "What Every C Programmer Should Know":

  float *P;
  void zero_array() {
    int i;
    for (i = 0; i < 10000; ++i)
      P[i] = 0.0f;
  }

If you make the global variable static, then it gets converted to a memset with -fno-strict-aliasing, because the 'P' variable never has its address taken. If you used LTO with sufficient linkage information, the same thing would happen. Similarly, if you pass the array as an argument then the loop gets converted to a bzero call just fine without -fno-strict-aliasing:

  float *P;
  void zero_array_impl(float* a) {
    int i;
    for (i = 0; i < 10000; ++i)
      a[i] = 0.0f;
  }
  void zero_array() {
    zero_array_impl(P);
  }

I don't think this is a very good example for these reasons.

> You don't really understand the UB problem if you model this in your head as some sort of simple set of rules that everybody really should have known all along

What does this have to do with my comment, at all?

> You can't just shrug this away with

Who is shrugging?

You don't understand a complex problem until you understand both sides. The article only presents one side. I am presenting the other. It doesn't mean I think the problem is simple or easy.

To add to this, a lot of what compilers decide is undefined isn't bizarre corner case code that rarely happens, but common and useful techniques that the compiler will happily demolish without warning. The 'defined' workarounds, however, are usually difficult, less safe, harder to understand, and often slower. This is without getting into the nightmare that is LTO finding 'undefined' behavior across program boundaries.

> The 'defined' workarounds, however, are usually difficult, less safe, harder to understand, and often slower.

That has not been my experience. Do you have an example of this?

There were examples in the linked message board post [1]. Note how there is now a paragraph describing why the code is written with 4 casts instead of comparing pointers naively. Multiply this by hundreds of instances of similar code in a large code base.

[1] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libx...

Why not just write a function is_ptr_less that does all the casts.

P.S: In C++ std::less is guaranteed to do the right thing(tm).

Sure - all sorts of stuff with type-punning in unions because much harder to understand when it's transformed into a whole series of casts to/from void/char , with the loss of checking that one gets from a union. Even without unions, aliasing rules can screw you over in surprising ways. Look at the free list example linked in the post for another one.

Slower - the various methods of checking for integer overflow are slower than x = a + b; if (a < x) {/overflow*/}. It can also be easier to just use a slower, or more memory hungry way of doing things than a better but needlessly complicated method.

At my current job we've sacrificed memory (and in this case, performance as a consequence) because the complexity cost of keeping some critical code in the defined behavior realm was too risky.

Type punning in unions is best done using memcpy. Compilers know what it does and optimize it: http://blog.regehr.org/archives/959

As for integer overflow at least gcc and clang have built-in functions that generate optimal code by checking the CPU's overflow flag: http://clang.llvm.org/docs/LanguageExtensions.html#checked-a... and https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins...

C99 Techincal Corrigendum 3 clarifies that union-based type punning is valid by amending 6.5.2.3 with this footnote:

If the member used to access the contents of a union object is not the same as the member last used to store a value in the object, the appropriate part of the object representation of the value is reinterpreted as an object representation in the new type as described in 6.2.6 (a process sometimes called “type punning”). This might be a trap representation.

(copied from a comment on blog.regehr.org)

This is actually a really common area of misunderstanding. But ultimately, one of the main uses of unions is to provide multiple ways to access the same data.

In C, yes, type-punning with union is valid. However, in C++, it is considered undefined behavior and may get removed by the compiler if it sees fit.

Thanks for the clarification. Do the new C++ standards explicitly discuss this anywhere?

There's this stackoverflow qanswer: http://stackoverflow.com/questions/11373203/accessing-inacti...

where answers try to distill the standard into something legible by humans. Unsurprisingly, they come to different conclusions.

The overall interpretation in some situations, if you look at things just right, you can do type punning in C++. But it's pretty risky since you're a LOC away from the compiler deleting your function.

Or, you can't really do it at all safely. Which I would conclude from the above anyways.

because people in fact don't want abstract "performance"

That's true, if performance were the only consideration, we would all be writing assembly.

To a large degree the language didn't change (some of the undefined behaviours we are discussing were in C89), rather the compiler vendors started to enforce parts of it.

Reply on news.ycombinator.com