4 ms·

The issue is that once you hit undefined behavior, the optimizer is allowed to do whatever it wants - the proverbial launch nukes, crash, raid your fridge, etc.

The issue is that once you hit undefined behavior, the optimizer is allowed to do whatever it wants - the proverbial launch nukes, crash, raid your fridge, etc. More likely, it could just not execute that instruction or any instruction that depends on it, because that's the fastest way to preserve undefined behavior.

zAy0LfpBZLC8mAC 9y ago
> The issue is that once you hit undefined behavior, the optimizer is allowed to do whatever it wants - the proverbial launch nukes, crash, raid your fridge, etc. That's not quite how it works, or at least not what usually happens. Optimizers don't look for undefined behaviour to then do whatever, they look for the set of defined behaviours and then try to generate the cheapest code that behaves correctly in all those cases--and if that means that undefined cases do weird stuff, that's an acceptable side effect. That in particular means that the potential for undefined behaviour to occur at runtime does not make the program undefined. If you ask the user to input a number and you then use that number as an index into some array without any input validation, the user could enter an out-of-range index, in which case the behaviour of the program would be undefined. But if the user inputs a valid index, the program must not exhibit any undefined behaviour. Therefore, yes, if any of the bytes are not inside any valid C object, then that would produce undefined behaviour. Which is precisely why the compiler would be allowed to combine the loads into one 32-bit load: As the code, if it takes that path, is going to load all four bytes, and accessing a byte outside any objects would result in undefined behaviour, the compiler may assume that all accesses are within valid objects, and therefore loading them all in one is correctness-preserving.
- Dylan16807 9y ago
  > Optimizers don't look for undefined behaviour to then do whatever, they look for the set of defined behaviours and then try to generate the cheapest code that behaves correctly in all those cases--and if that means that undefined cases do weird stuff, that's an acceptable side effect. > if any of the bytes are not inside any valid C object, then that would produce undefined behaviour The compiler might also assume that every one of these loads will be exactly 4 bytes apart. Even worse, it might know that int32_t allocations will always be aligned, so it will assume that every load will be 4-byte-aligned. It then might output code that loses track of the bottom two bits, or SSE instructions that only work with certain alignments. This isn't theoretical, this exact kind of bug has happened before, on a normal non-malicious compiler: http://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-x86.html http://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-... https://news.ycombinator.com/item?id=12889855 https://news.ycombinator.com/item?id=12889855 It's true that the buffer overrun issue is the same for either version; I misread what was being replaced. But the alignment issue is specific to this version, and can't be ignored.
  - zAy0LfpBZLC8mAC 9y ago
    Yes, that is all true for the suggested replacement code, but you quoted the statement "Yes, I know it could be unaligned, but this is x86 where it would still be faster than 4 separate reads.", which referred only to what the compiler should do, and the compiler combining char reads into 32 bit loads obviously is not allowed to assume that the resulting loads are aligned ...
    - Dylan16807 9y ago
      The sentence I quoted refers just to the technique of using 32-bit loads. That applies equally to your compiler suggestion and the C code you wrote. So I think it's a perfectly fine sentence to quote when pointing out a flaw of the C code, which is while unaligned accesses are fine in a vacuum, doing them with a wrong-typed pointer is unsafe.

zAy0LfpBZLC8mAC 9y ago

> The issue is that once you hit undefined behavior, the optimizer is allowed to do whatever it wants - the proverbial launch nukes, crash, raid your fridge, etc. That's not quite how it works, or at least not what usually happens. Optimizers don't look for undefined behaviour to then do whatever, they look for the set of defined behaviours and then try to generate the cheapest code that behaves correctly in all those cases--and if that means that undefined cases do weird stuff, that's an acceptable side effect. That in particular means that the potential for undefined behaviour to occur at runtime does not make the program undefined. If you ask the user to input a number and you then use that number as an index into some array without any input validation, the user could enter an out-of-range index, in which case the behaviour of the program would be undefined. But if the user inputs a valid index, the program must not exhibit any undefined behaviour. Therefore, yes, if any of the bytes are not inside any valid C object, then that would produce undefined behaviour. Which is precisely why the compiler would be allowed to combine the loads into one 32-bit load: As the code, if it takes that path, is going to load all four bytes, and accessing a byte outside any objects would result in undefined behaviour, the compiler may assume that all accesses are within valid objects, and therefore loading them all in one is correctness-preserving.

Dylan16807 9y ago
> Optimizers don't look for undefined behaviour to then do whatever, they look for the set of defined behaviours and then try to generate the cheapest code that behaves correctly in all those cases--and if that means that undefined cases do weird stuff, that's an acceptable side effect. > if any of the bytes are not inside any valid C object, then that would produce undefined behaviour The compiler might also assume that every one of these loads will be exactly 4 bytes apart. Even worse, it might know that int32_t allocations will always be aligned, so it will assume that every load will be 4-byte-aligned. It then might output code that loses track of the bottom two bits, or SSE instructions that only work with certain alignments. This isn't theoretical, this exact kind of bug has happened before, on a normal non-malicious compiler: http://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-x86.html http://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-... https://news.ycombinator.com/item?id=12889855 https://news.ycombinator.com/item?id=12889855 It's true that the buffer overrun issue is the same for either version; I misread what was being replaced. But the alignment issue is specific to this version, and can't be ignored.
- zAy0LfpBZLC8mAC 9y ago
  Yes, that is all true for the suggested replacement code, but you quoted the statement "Yes, I know it could be unaligned, but this is x86 where it would still be faster than 4 separate reads.", which referred only to what the compiler should do, and the compiler combining char reads into 32 bit loads obviously is not allowed to assume that the resulting loads are aligned ...
  - Dylan16807 9y ago
    The sentence I quoted refers just to the technique of using 32-bit loads. That applies equally to your compiler suggestion and the C code you wrote. So I think it's a perfectly fine sentence to quote when pointing out a flaw of the C code, which is while unaligned accesses are fine in a vacuum, doing them with a wrong-typed pointer is unsafe.