Skip to content

Instantly share code, notes, and snippets.

@antirez

antirez/gist:1876875

Created Feb 21, 2012
Embed
What would you like to do?
/* If you compile with -O2 the code will print "Wrong behavior", compiling
* with -O0 the code will print "Correct behavior".
*
* Tested with:
* Apple clang version 2.1 (tags/Apple/clang-163.7.1) (based on LLVM 3.0svn)
* Target: x86_64-apple-darwin11.3.0
* Thread model: posix
*/
#include <stdio.h>
int getValue(long long *l) {
*l = -9223372036854775484LL;
return 1;
}
void bug(void) {
long long value, incr, oldvalue;
incr = -1000;
if (getValue(&value) != 1) return;
oldvalue = value;
value += incr;
if ((incr < 0 && value > oldvalue) || (incr > 0 && value < oldvalue)) {
printf("Correct behavior\n");
} else {
printf("Wrong behavior\n");
}
}
int main(void) {
bug();
}
@spinogrizz

This comment has been minimized.

Copy link

@spinogrizz spinogrizz commented Feb 21, 2012

Interesting bug, kudos to author!

Can confirm this bug with clang version 3.1 (tags/Apple/clang-318.0.45) (based on LLVM 3.1svn)

@MSNexploder

This comment has been minimized.

Copy link

@MSNexploder MSNexploder commented Feb 21, 2012

Bug still appears in Apples clang version 3.1.
Actually it also happens with -O1.

Stefans-MacBook-Pro  Desktop  clang --version
Apple clang version 3.1 (tags/Apple/clang-318.0.45) (based on LLVM 3.1svn)
Target: x86_64-apple-darwin11.3.0
Thread model: posix
Stefans-MacBook-Pro  Desktop  clang -O0 foo.c && ./a.out 
Correct behavior
Stefans-MacBook-Pro  Desktop  clang -O1 foo.c && ./a.out
Wrong behavior
Stefans-MacBook-Pro  Desktop  clang -O2 foo.c && ./a.out
Wrong behavior
Stefans-MacBook-Pro  Desktop  clang -O3 foo.c && ./a.out
Wrong behavior
Stefans-MacBook-Pro  Desktop  clang -O4 foo.c && ./a.out
Wrong behavior
@asl

This comment has been minimized.

Copy link

@asl asl commented Feb 21, 2012

No bug here. You code exploits undefined behavior (signed integer wrap), so compiler can do everything here. E.g. it can format your HDD or send nifty e-mail to your boss :)
Add -ftrapv cmdline you catch a trap on such UB.

@antirez

This comment has been minimized.

Copy link
Owner Author

@antirez antirez commented Feb 21, 2012

@asl: you are right, but this is my feeling about this issue:

Hi Salvatore,

If I understand this code correctly, there is a signed overflow.
Signed overflow is undefined behavior. Clang (or LLVM) apparently
takes advantage of that by optimizing (incr < 0 && value > oldvalue)
=> false.

I Dmitri,

I understand the code is undefined, but a compiler can decide to have
different practical behaviors when an undefined condition is
encountered. I think that the best way to handle this condition in the
real world is to do the math accordingly to the underlaying
architecture, so that just the result is undefined, but avoiding to
take advantage of the fact it is undefined to optimized.

I'm not stating that clang is behavior is no correct: strictly
speaking it is following the standard. However its concrete behavior
probably does not minimize the probability of real-world (possibly
broken) code fails. That said I'm going to fix my code so that it is
standard complaint and can compile fine with clang.

Thank you,
Salvatore

@malu

This comment has been minimized.

Copy link

@malu malu commented Feb 21, 2012

I'd expect clang to be able to warn about undefined behaviour.

@asl

This comment has been minimized.

Copy link

@asl asl commented Feb 21, 2012

@malu: this is not always possible. Consider e.g. here the function getValue() to be external (and not visible to compiler). The UB in line 24 may or may not be depending on the value returned. -ftrapv will add the checks for some cases of UB, so, it can be caught in the runtime.

@malu

This comment has been minimized.

Copy link

@malu malu commented Feb 21, 2012

Okay, I didn't think of external function, that'd be a problem.

@antirez

This comment has been minimized.

Copy link
Owner Author

@antirez antirez commented Feb 21, 2012

Related: /usr/include/i386/limits.h:#define LLONG_MIN (-0x7fffffffffffffffLL-1)

@antirez

This comment has been minimized.

Copy link
Owner Author

@antirez antirez commented Feb 21, 2012

I'm turning the old code into this one:

    oldvalue = value;
    if ((incr < 0 && oldvalue < 0 && incr < (LLONG_MIN-oldvalue)) ||
        (incr > 0 && oldvalue > 0 && incr > (LLONG_MAX-oldvalue))) {
        printf("Correct behavior\n");
    } else {
        printf("Wrong behavior\n");
    }
    value += incr;

Should be safe since it is guaranteed that all the parts never overflow.
LLONG_MIN-oldvalue can't overflow because oldvalue is guaranteed to be < 0
LLONG_MAX-oldvalue can't overflow because oldvalue is guaranteed to be > 0

Now trying it in practice :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment