instcombine miscompiles pointer comparison

[llvmbot]

Bugzilla Link	1850
Resolution	FIXED
Resolved on	Apr 24, 2010 07:06
Version	2.1
OS	Linux
Attachments	test case
Reporter	LLVM Bugzilla Contributor
CC	@jayfoad

Extended Description

The attached test case is based on gcc.c-torture/execute/20010329-1.c in the GCC testsuite. If I compile it with "llvm-gcc -O1 -S -emit-llvm pre.c", I get this:

define i32 @​main() {
entry:
tail call void @​abort( )
unreachable
}

This is a bit surprising, because the test looks like it should obviously pass (i.e. call exit(0) instead of abort()). You could probably argue that the results of the pointer comparisons are undefined, because the pointers don't point into any valid object, but I still think it would be nice if the code behaved in the obvious way.

I've investigated a bit, and the GCC front end is generating code a bit like this:

    %x = inttoptr i64 2147483649 to i8* ; 0x80000001
    %x1 = getelementptr i8* %x, i64 2147483648 ; 0x80000000
    %cond = icmp ule i8* %x1, inttoptr (i64 5 to i8*)

It's a bit strange that it adds 0x80000000 to x instead of subtracting it, but if pointers are 32 bit then the result will be the same.

Then, after -instcombine, this is folded to false, presumably because the operations have been evaluated using 64-bit arithmetic. So maybe -instcombine is wrong to assume that pointers are 64 bit.

[llvmbot]

assigned to @lattner

[llvmbot]

Jay, what versions of llvm and llvm-gcc are you using? I can't reproduce the bug. With the freshly updated (r44790) llvm and llvm-gcc-4.2. On a Linux/x86 platform I get:

$ llvm-gcc -O1 -S -emit-llvm pre.c -o -
; ModuleID = 'pre.c'
target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32"
target triple = "i686-pc-linux-gnu"

define i32 @​main() {
entry:
tail call void @​exit( i32 0 ) noreturn nounwind
unreachable
}

declare void @​exit(i32) noreturn nounwind

[llvmbot]

Jay, what versions of llvm and llvm-gcc are you using?

I'm using the LLVM 2.1 release with the GCC 4.0 front end.

[lattner]

llvm-gcc produces good output now, but instcombine still miscompiles this testcase:

define i1 @​test() {
%x = inttoptr i64 2147483649 to i8* ; 0x80000001
%x1 = getelementptr i8* %x, i64 2147483648 ; 0x80000000
%cond = icmp ule i8* %x1, inttoptr (i64 5 to i8*)
ret i1 %cond
}

[lattner]

Further reduced:

target datalayout =
"e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32"
target triple = "i686-pc-linux-gnu"

define i1 @​test() {
%cond = icmp ule i8* inttoptr (i64 4294967297 to i8*), inttoptr (i64 5 to i8*)
ret i1 %cond
}

[lattner]

Fixed, patch here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20071210/056149.html

Thanks!

-Chris

[llvmbot]

Thanks, that fix works for me.

[llvmbot]

This bit of the fix looks obviously wrong, because an IntToPtr only has one operand:

•  if ((CE0->getOpcode() == Instruction::IntToPtr &&
  

•       CE0->getOperand(0)->getType() == IntPtrTy &&
  

•       CE0->getOperand(1)->getType() == IntPtrTy) ||
  

[llvmbot]

This bit of the fix looks obviously wrong

This seems like the obvious fix, and it works for me:

--- Analysis/ConstantFolding.cpp (revision 8837)
+++ Analysis/ConstantFolding.cpp (working copy)
@@ -308,10 +308,10 @@
// there is a truncation or extension that we aren't modeling.
if ((CE0->getOpcode() == Instruction::IntToPtr &&
CE0->getOperand(0)->getType() == IntPtrTy &&

•       CE0->getOperand(1)->getType() == IntPtrTy) ||
  

•       Ops[1]->getOperand(0)->getType() == IntPtrTy) ||
       (CE0->getOpcode() == Instruction::PtrToInt &&
        CE0->getType() == IntPtrTy &&
  

•       CE0->getOperand(0)->getType() == CE0->getOperand(1)->getType())) {
  

•       CE0->getOperand(0)->getType() == Ops[1]->getOperand(0)->getType())) {
     Constant *NewOps[] = {
       CE0->getOperand(0), cast<ConstantExpr>(Ops[1])->getOperand(0)
     };
  

[lattner]

Excellent catch, you're exactly right:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20071210/056218.html