<source>:5:13: warning: unsafe buffer access [-Wunsafe-buffer-usage]
5 | if (table[i] == v) return true;
| ^~~~~
This one is weirdly hard to get a compiler warning out of which is a fair critque, but so many of the "Look what the compiler did to my UB silently!" issues are not at all silent and would have been stopped dead with "-Wall -Wextra -Werror"
How are we supposed to know whether our code is being compiled sensibly or not, without poring over the disassembly? Just set all the warning flags and hope for the best?
Almost all of the "don't do the overly clever stuff!" proposals would throw away a huge amount of actually productive clever stuff.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wwhatever"
// code
#pragma clang diagnostic pop
There is no `if(obvious out-of-bound access) silently emit nonsense har har har` in the compiler's source code. The compiler doesn't understand intent or the program as a whole. It applies micro transformations that all make sense in isolation. And while the compiler also tries to detect erroneous programming patterns and warn about those, that's exceedingly more difficult.
And I'm not saying it should do that. I'm saying if the compiler is able to detect erroneous code, then it should emit a warning when it does so. An out of bounds access is an example of code that is basically always erroneous.
>There is no `if(obvious out-of-bound access) silently emit nonsense har har har` in the compiler's source code. The compiler doesn't understand intent or the program as a whole. It applies micro transformations that all make sense in isolation.
Yes, I understand that. However, like I said in my first response, this optimization in particular is only valid if the array is definitely accessed incorrectly. If the compiler is able to perform this optimization, there are only two possibilities: either the compiler can determine in some cases (and in this one in particular) that an array is accessed incorrectly and doesn't warn about it; or it can't determine that condition and this optimization is caused by a compiler bug and there are cases where the compiler incorrectly performs it, breaking the code. If the former is the case, then someone wrote the code to check whether an array is always accessed correctly. Either that, or nobody wrote it and the compiler deduces from even more basic principles that arrays must always be accessed by indices less than their lengths; which, I mean, that might be the case, but I seriously doubt it.
Today in most cases nobody is writing this code. Neither C nor C++ have any mandate for such detection.
There is a proposal, which could perhaps make it into C++ 26, to formally specify "erroneous behaviour" and have the compiler do something particular and warn you that what you're doing is a bad idea for the specified cases†, but it's easily possible that doesn't end up in the IS, or that compiler vendors aren't interested in implementing it. Meanwhile, if it happens at all it's up to the vendor.
† "Erroneous behaviour" is one possible approach to the uninitialized locals problem in C++. Once upon a time C says local variables can be declared and used without initializing them, this actually has Undefined Behaviour, which is very surprising for C and C++ programmers who tend to imagine that they're getting the much milder Unspecified Behaviour, but they are not. Many outfits use compiler flags to say look, when I do this, and I know sometimes I'll screw up, just give me zeroes, so that's Defined Behaviour, it's not Intended Behaviour but at least it's not Undefined. This includes all major OS vendors (Microsoft, Apple, Red Hat etc.)
Some people brought this approach to WG21, but there was pushback, if uninitialized variables are zero, then they're not really uninitialized are they? This has two consequences, 1. Performance optimisations from not initializing data evaporate; and 2. It is now "correct" to use this zero initialization behaviour because it's specified by the language standard, so maybe you can't lint on it.
Erroneous Behaviour solves (2) by saying no, it's still wrong, it's just safely wrong, the compiler can report this is wrong and it must ensure your results are zero.
Another proposal offers a syntax to solve (1) by saying explicitly in your program, "No, I'm a smart C++ programmer, do not initialize these values", akin to the markers like ~~~ you may have seen to mean "Don't initialize this" in some other languages.
No. The code does not show any undefined behavior if any of the elements of `table` is equal to `v`, because then the loop is ended by an early return. The compiler certainly did not prove that this code always has undefined behavior.
I'm failing to see what's not sensible about how that code is compiled.
The only possible way that function could return false is if you read past the end of the array and the value there happens to be different from `v`. Is it really the more sensible to rely on that, rather than fixing a known behavior in case of array overflow?
Currently that piece of code doesn't trigger a warning with -Wall. It's not even flagged with -Wextra - it needs -Weverything.
Where to draw the line is not always clear, but here's a very clear-cut example[1] where emitting a warning would be bad. If you don't want to watch the video, it's basically this:
- the code technically contains undefined behavior, but it will never be actually triggered by the program
- changing the code to remove undefined behavior forces the compiler to emit terrible code
Making the compiler yell at the programmer in this case would be terrible, but it's clearly a consequence of what you're asking.
So let me reply to your comment as if it were serious: yes, if the programmer by supernatural means knows that the "v" is always presented somewhere in the array, then this function works exactly as intended: it would always return true, and the compiler optimises it to do so as quickly as possible! But... perhaps there is some other way to pass such programmer's knowledge ("the arguments are guaranteed to be such that this loop is guaranteed to finish early") to the compiler in a more explicit way? Some sort of explicitly written assertion? A pre-condition? A contract, if you like?
See, it's very difficuly to maintain such unspoken contracts and invariants during the codebases' life because they're unspoken and unwritten. Comments barely count since compilers generally ignore them.
To be fair it's not the compiler's fault that the source program is broken - the argument is over whether the compiler is being helpful or being obtuse, and this particular case I'd argue the latter.
Thanks for the video link - it's an interesting example, but the crucial difference there, I think, is that in that case the compiler isn't doing something counter to the programmer's intent. The code isn't incorrect (assuming a non-pathological buffer size) - it's merely more convenient for the compiler when expressed with int32_t indices rather than uint32_t indices.
I do appreciate, though, that deciding what to yell about and what not to yell about is an extremely non-trivial problem.
Not as good as warning, but UBSan catches this at run-time: https://godbolt.org/z/Mdjn7h8dj
- a correct program does not access table[4]
- therefore the loop must always exit early
- the only way to exit early is to return true
int table[4];
bool exists_in_table(int v)
{
for (int i = 0; i <= 4; i++) {
if (table[i] == v) return true;
}
return false;
}
i -> 4 goto return true or exit loop and return false Since the branch in on undef behavior it is okay for the compiler to choose any branch destination or none at all (i.e. remove all further code). The compiler in this case likely chose to just remove the branch and any destinations. All that the prior code does is return true, since there is no next iter, so thats all what is left.
No, the optimization is valid if the function is always called with "v" that is actually exists in the table; in this case the function should always return true so it's only proper for the compiler to throw out the extraneous code.
And writing the loop in such way is programmer's promise/guarantee to the compiler that the function will indeed be called only in that manner. That's the essence of the UB: it's the programmer who promises to the compiler that she will perform all the necessary checks (or formal proofs of impossibilty) herself; the compiler may go forward and rely on the implied invariants and preconditions.
And this is, of course, the main problem of the UB because 95% of the time the programmer does not actually intend to make such a gurantee: she simply is unaware (for whatever reason) that there is a pre-condition (checked by nobody!) that's required for the program's correctness to hold... or it's even just a typo she made.
This is tricky, because the code is perfectly valid if it always early-exit (and I have written code like this myself that avoids bound checking by guaranteeing an early exit, when micro-optimizing), so it is hard to statically reject it. On the other hand it seems a very obvious thing to warn on.