Found a bug - maybe?

Clang Frontend
#1

A picture is worth a thousand words (and by picture, I mean shell output). This is with

clark@clark-server ~/test $ clang++ --version
clang version 1.1 (branches/release_27)
Target: x86_64-pc-linux-gnu
Thread model: posix
clark@clark-server ~/test $ cat foo.cpp
#include
#include

struct ParseFailed {};

int main()
{
bool succeeded = false;

const char buffer = {
0x00, 0x11
};

try {
if(buffer > (buffer - 4))
throw ParseFailed();

succeeded = false;
} catch(ParseFailed) {
succeeded = true;
}

if(succeeded)
printf(“SUCCESS!\n”);
else
printf(“FAILURE!\n”);
}

clark@clark-server ~/test $ clang++ -O0 foo.cpp && ./a.out
SUCCESS!
clark@clark-server ~/test $ clang++ -O2 foo.cpp && ./a.out
FAILURE!

What should I do? This little oddity popped up in one of my unit tests while running my codebase through clang.

#2

Here’s a reduced test case. Strangely enough, if I change buffer to be of size 1, this code works fine.

#3

AMDG

Clark Gaebel wrote:

<snip>
            const char buffer = {
                    0x00, 0x11
            };

            try {
                    if(buffer > (buffer - 4))
  
Looks like undefined behavior.

In Christ,
Steven Watanabe

#4

(buffer - 4) causes undefined behavior, per [expr.add]: "if the
expression P points to the i-th element of an array ob ject, the
expressions (P)+N (equivalently, N+(P)) and (P)-N (where N has the
value n) point to, respectively, the i + n-th and i - n-th elements of
the array object, provided they exist."

Instcombine actually does the damage here, turning

define i32 @main() {
entry:
  %retval = alloca i32, align 4 ; <i32*> [#uses=2]
  store i32 0, i32* %retval
  br i1 icmp ugt (i8* getelementptr inbounds ([2 x i8]*
@_ZZ4mainE6buffer, i32 0, i32 0), i8* getelementptr inbounds ([2 x
i8]* @_ZZ4mainE6buffer, i32 0, i32 -4)), label %if.then, label
%if.else

if.then: ; preds = %entry
  %call = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds ([10
x i8]* @.str, i32 0, i32 0)) ; <i32> [#uses=0]
  br label %if.end

if.else: ; preds = %entry
  %call1 = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds
([10 x i8]* @.str1, i32 0, i32 0)) ; <i32> [#uses=0]
  br label %if.end

if.end: ; preds = %if.else, %if.then
  %0 = load i32* %retval ; <i32> [#uses=1]
  ret i32 %0
}

into

define i32 @main() {
entry:
  %retval = alloca i32, align 4 ; <i32*> [#uses=2]
  store i32 0, i32* %retval
  br i1 false, label %if.then, label %if.else

if.then: ; preds = %entry
  br label %if.end

if.else: ; preds = %entry
  %call1 = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds
([10 x i8]* @.str1, i32 0, i32 0)) ; <i32> [#uses=0]
  br label %if.end

if.end: ; preds = %if.else, %if.then
  %0 = load i32* %retval ; <i32> [#uses=1]
  ret i32 %0
}

#5

(buffer - 4) causes undefined behavior, per [expr.add]: "if the
expression P points to the i-th element of an array ob ject, the
expressions (P)+N (equivalently, N+(P)) and (P)-N (where N has the
value n) point to, respectively, the i + n-th and i - n-th elements of
the array object, provided they exist."

Right, a portable way to do this is to do the comparison on "(uintptr_t)P" instead of on P.

-Chris

#6

Although undefined (in which case, a warning on -pedantic would be
nice) this code passes fine on gcc, msvc, and icc, and seems like
something extremely small (and potentially fatal) to do differently.
I'll change it to the portable way, but this is definately not
expected.