7 ms·
>For example, a common thing programmers expect is that you can test for signed integer overflow by checking whether the result is less than one of the operands
by philosopher1234 3y ago
>For example, a common thing programmers expect is that you can test for signed integer overflow by checking whether the result is less than one of the operands, as in this program:
#include <stdio.h>
int f(int x) {
if(x+100 < x)
printf("overflow\n");
return x+100;
}
>Clang optimizes away the if statement. The justification is that since signed integer overflow is undefined behavior, the compiler can assume it never happens, so x+100 must never be less than x. Ironically, this program would correctly detect overflow on both ones'-complement and two's-complement machines if the compiler would actually emit the check.
My god…
- boryas 3y ago-fwrapv and it's all good :)
- not2b 3y agoIt really isn't optional as a C developer to turn on warnings and get your code warning-free. This will eliminate the most common unbounded behavior issues, like uninitialized variables, though unfortunately not all of them.
- gavinhoward 3y agoThis is why I implemented two's-complement with unsigned types, that don't have UB on overflow.
- kragen 3y agolike x > 2147483547u in this case?
- gavinhoward 3y agoCorrect. It will still overflow, but it's not UB and won't be optimized away.
- kragen 3y agoby 'overflow' do you mean 'set the high bit'
- gavinhoward 3y agoCorrect. I have routines that take the unsigned values and interpret them as signed, two's-complement values. So you would write this: val = temp1 + temp2; if (y_ssize_lt(val, temp1)) y_trap(); That `y_ssize_lt()` function computes whether `val` is less than `temp1` as though they were signed, two's-complement integers. But because they are actually unsigned, the compiler cannot be malicious and delete that code.
- kragen 3y agoi see, thanks maybe we should develop a nonmalicious compiler
- gavinhoward 3y agohttps://groups.google.com/g/boring-crypto/c/48qa1kWignU/m/o8GGp2K1DAAJ https://groups.google.com/g/boring-crypto/c/48qa1kWignU/m/o8...
- pclmulqdq 3y agoThis is why people who put "C/C++" on their resume don't usually know either language. In C++, integers are defined to be two's complement now, so that check is acceptable.
- rsc 3y agoNot true, as the post explains.
- pclmulqdq 3y agoC++20 made the switch. See P1236. It appears C23 has followed suit.
- e4m2 3y agoIt made the switch, sure, but overflow remains undefined.
- rsc 3y agoIf P1236 (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1236r1.html https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p12...) is really what it says ("alternative wording") then I don't believe that's true. Certainly P0907R3 (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0907r3.html https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p09...) is clear: > Status-quo If a signed operation would naturally produce a value that is not within the range of the result type, the behavior is undefined. The author had hoped to make this well-defined as wrapping (the operations produce the same value bits as for the corresponding unsigned type), but WG21 had strong resistance against this. My understanding is that the 2s complement change in C++20 ended up defining bitwise operations but not arithmetic.
- uecker 3y agoTwo's complement does not mean signed overflow became defined. So the check is still wrong. And no, I do not think making it defined would lead to more correct programs. You then simply have really difficult wrap-around bugs instead of signed overflow which you can be found more easily with UB sanitizer or turn into traps at run-time.
- tempodox 3y agoThis has to be the worst part of UB: That the compiler can assume it never happens. This way UB can affect your program even if the code that would exhibit the behavior is never executed.
- AlotOfReading 3y agoWhat the article misses is that the code wouldn't work on all hardware. Take MIPS for example, where the signed overflow would generate a hardware exception that the OS might have implemented to do anything from nothing to killing the generating process. C was never standardized solely on the basis of what's most performant. The vast majority of explicit UB is there because someone knew of a system or situation where that assumption wasn't true.
- lelanthran 3y ago> C was never standardized solely on the basis of what's most performant. The vast majority of explicit UB is there because someone knew of a system or situation where that assumption wasn't true. So? They could have called it implementation defined. The reasoning you present was broken then, as it is broken now. Your MIPS example displays no reason for UB to exist.
- AlotOfReading 3y agoImplementation defined means it has a defined behavior on every implementation. In the MIPS case defining that behavior would force the compiler to generate strictly signed instructions for signed values and define how the runtime platform handles these interrupts, rather than leaving the compiler free to generate "whatever works". Look, a lot of UB is frankly stupid and shouldn't be in a modern language spec, including signed overflow. I'm not defending that, only giving an example where you have to break compatibility to eliminate it.
- lelanthran 3y ago> In the MIPS case defining that behavior would force the compiler to generate strictly signed instructions for signed values and define how the runtime platform handles these interrupts, I respectfully disagree: in this particular scenario, it's enough for the compiler vendor to write "generates an interrupt on overflow" with no indication of what the handling should be, and still be well within implementation-defined behaviour. After all, the standard specifies what `raise()` does, and what `signal()` does, but doesn't specify how the runtime is going to handle `raise(signum)` when the program has not yet called `signal(signum, fptr)`. This scenario you presented displays exactly why UB should be replaced with IB in the standard. I've yet to see one good reason (including performance reasons) for why the dereferencing of a NULL pointer (for example) cannot be documented for the implementation. With UB, we have real-world examples of a NULL pointer dereference causing compilers to omit code resulting in a program that continued to run but with security implications. If changed to IB, the compiler would be forced to emit that code and let the dereference crash the program (much better).
- lelanthran 3y agoI think what really annoys me is that this looks like actual malice on the part of the standards writers, and less severe malice on the part of the compiler authors. I know I should attribute it to stupidity instead but ... The standards writers could have made all UB implementation defined, but they didn't. The compiler authors could have made all Uab implementation defined, and they didn't. Take uninitialised memory as an example, or integer overflow: The standard could have said "will have a result as documented by the implementation". They didn't. The implementation can choose a specific behaviour such as "overflow results depend on the underlying representation" or "uninitialised values will evaluate to an unpredictable result." But nooooo... The standard keeps adding more instances of UB in each revision, and the compiler authors refuse to take a stand on what overflow or uninitialised values should result in. Changing the wording so that all UB is now implementation defined does not affect blegacy code at all, except by forcing the implementation to document the behaviour and preventing a compiler from simply optimising out code.
- nottorp 3y ago> less severe malice on the part of the compiler authors. This is optimization run amok and I'd call it malice. If C/C++ are designed to let me shoot myself in the foot, the compiler should let me shoot myself in the foot instead of erasing my code.
- torstenvl 3y agoThe standard does make all definable UB implementation defined. The compiler writers intentionally misread the standard to allow these kinds of optimizations. C17 Draft, § 3.4.3: "Possible undefined behavior ranges from... . . . ignoring the situation completely with unpredictable results, to . . . . . . behaving during translation or program execution in a documented manner characteristic of the environment (with or without the issuance of a diagnostic message) . . . . . . to terminating a translation or execution (with the issuance of a diagnostic message)." Compiler writers like to language-lawyer this in two ways. First, they interpret "possible" to mean that this is a non-exhaustive list, but that isn't how interpretation of such a document typically works. Expressio unius est exclusio alterius -- the express inclusion of one is the exclusion of others -- means that any list is exhaustive unless explicitly indicated otherwise. In other words, the reality is that, according to the standard, these are the only possible options. Second, they interpret "ignoring the situation completely" to mean "actively seeking out the situation and deleting whole sections of code based on that." This is quite self-evidently a dishonest interpretation.
- pdw 3y agoThis is why I hate it when people describe C as "portable assembler". The Usenet comp.lang.c FAQ was already thirty years ago warning people not to write overflow checks like this.
- coliveira 3y agoYou cannot check for overflow like this, because you're causing it. There are other ways to do this without adding first: #include <limits.h> int safe_add(int a, int b) { if (a > 0 && b > INT_MAX - a) { /* deal with overflow... */ } else if (a < 0 && b < INT_MIN - a) { /* deal underflow.. */ } return a + b; }
- zer8k 3y agoI'm not convinced a "common" C program would ever do such a thing. Even the "Effective C" book (one of the best books on C, imo) discusses why this is wrong and why you should take advantage of `limits.h` for bounds checking. This is just bad programming. The compiler, like usual, is correct because you're not in reality checking anything. You've made an obviously non-sensical statement that the overflowed value will be less than the value. Compiler optimizes it away. You can argue the semantics of UB here but this particular UB is borderline a solved problem in any practical sense. To be fair, a static analyzer should be able to catch this.
- _dain_ 3y ago>You've made an obviously non-sensical statement— Therefore it should fail to compile. You can even spin it as a performance enhancement: the whole program can be optimized away!
- layer8 3y agoThe problem is, that’s not how the logical inference in a compiler/optimizer works. It’s very difficult to translate such an optimization back to “this statement has been optimized away”, in the general case.
- _dain_ 3y agoIf it's so difficult to figure out if the consequences of their optimization game are sensible or not, then they shouldn't play it in the first place. Who actually wants it to behave this way, other than compiler engineers competing on microbenchmarks? Who is this language even for?
- layer8 3y agoIt’s a side effect of desirable optimizations for UB-free code. You can’t have both at the same time.
- 3y ago
- raverbashing 3y agoIf anyone tried to pull this kind of language lawyering in my job they would be fired Maybe it's about time to fire the C compiler developers