4 ms·
So that "next = a + 1" is essentially a note to the compiler, promising that "++a" won't overflow? To me that seems really reckless! If you need that note, mak
by iainmerrick 9y ago
So that "next = a + 1" is essentially a note to the compiler, promising that "++a" won't overflow?
To me that seems really reckless! If you need that note, make it a compiler pragma or something, rather than trying to sneak it in by repurposing some existing code.
Now if overflow were implementation-defined, the compiler would have to assume that INT_MAX + 1 wraps to INT_MIN (say). So as written, the "if (a == INT_MAX)" line would stay. That's what I would expect to happen on reading the code, and I'm confident that that's what most reasonable people would expect too.
If you want to skip the "if (a == INT_MAX)" check, just take it out.
I'm sure optimizations like this are important for squeezing a few extra percent out of the standard benchmarks. But for most ordinary programmers they're just a minefield you have to tiptoe carefully around.
- simias 9y agoI wouldn't write code like this on purpose, rather I'd genuinely need "next" for some other computation and the compiler could then use that to infer that I know "a" not to overflow. If I wanted to make it an "annotation" then I'd use "assert(a < INT_MAX);" which would serve the same function to the compiler and make sense to the next coder reading it (or me in two months). Actually "assert" is probably not the right one since it could get removed with NDEBUG, but something like that. This is a very simple example of course, but maybe this "increment" code is actually used in many other parts of the codebase where the test actually makes sense. It's not rare for instance to start a C function with "if (param == NULL) return;", just in case. If the compiler inlines the function and sees that the caller dereferences "param" it can assume that it's not null and remove the test. But you're right that it makes it easy to shoot yourself in the foot. I remember a strange bug in the linux kernel source that was "hidden" by such an optimization, the code looked something like that: int bar(somestruct *ptr) { int somevalue = ptr->val; if (ptr == NULL) { return -EINVAL; } // Rest of the function here } The real code was a little more complicated than that obviously, but at a glance it might look that the function won't run if "ptr" is NULL. GCC however noticed the dereference before the test and happily decided that "of course this coder knows what they're doing, ptr can't be NULL!" and removed it. It can take a while to debug code like that.
- iainmerrick 9y agoThat code ought to be a compile error. "On this code path, your code assumes ptr is null and not-null at the same time!" Not all UB can be detected at compile time, but when it can be detected, it should be flagged to the user.
- simias 9y agoBut what if it's the result of some inlining or simply macro expansion? It's a tough call to make.
- inetknght 9y agoIf it's from inlining, then the compiler has a bug. If it's from a macro, then stop using macros in a broken fashion. Let it be an error.
- jjnoakes 9y agoHow would that be a compiler bug? Assuming g() is never called with a NULL pointer, this code should not produce any compile-time errors: int f(int *x) { if (x == NULL) return 0; return *x; } int g(int *p) { return (*p) + f(p); }
- kbenson 9y agoThis is essentially what caused the Cap'n'Proto remote vulnerability[1] 6 months back. The compiler optimized away an overflow check that directed control to an error handling path, eliding the code that checked for the exact error. Fun. 1: https://news.ycombinator.com/item?id=14163111 https://news.ycombinator.com/item?id=14163111