Find the Integer Bug

Not all of the functions below are correct. The first person to leave a comment containing a minimal fix to a legitimate bug will get a small prize. I’m posting this not because the bug is particularly subtle or interesting but rather because I wrote this code for a piece about integer overflow and thought — wrongly, as usual — that I could get this stuff right the first time.

By “legitimate bug” I mean a bug that would cause the function to execute undefined behavior or return an incorrect result using GCC or Clang on Linux on x86 or x64 — I’m not interested in unexpected implementation-defined integer truncation behaviors, patches for failures under K&R C on a VAX-11, or style problems. For convenience, here are GCC’s implementation-defined behaviors for integers. I don’t know that Clang has a section in the manual about this but in general we expect its integers to behave like GCC’s.

checked_add_2 seems suspicious. The intermediate value of ((uint32_t)a + (uint32_t)b) will never be negative, however, it might go outside of the range of the int32_t it’s being assigned to if a or b are sufficiently large or negative, which is undefined behaviour, right?

I’m not sure of how you’d fix it though, short of just scrapping that function.

Hi Ryan, I don’t want to give things away but the cast from unsigned to signed is not a problem due to the guarantees provided in the linked GCC documentation: “For conversion to a type of width N, the value is reduced modulo 2^N to be within range of the type; no signal is raised.”

checked_add_2 seems to return an incorrect result if both a and b are INT_MIN, as in that case the unsigned addition overflows into 0, but the overflow check fails, as it only checks for r > 0 or r < 0.

Ryan, perhaps the GCC text would be better phrased as “conversions from a larger to smaller signed int are performed by lopping off the unneeded high-order bits” and then “conversions from unsigned to signed of the same size will change the interpretation but not the bits.”

With the help of the ECLAIR system I found the same counterexample to correctness found by Andrew and that that is the only problematic case. Moreover, the system quickly proves that it is not necessary to change the second comparison of `r’ with 0: a strict less than is OK. Finally, the code does not have any undefined behavior: only the implementation-defined behavior John said he is not interested in: