llvm dev - Aug 2019 - Possible bug in llvm::EmitGEPOffset

I am investigating a bad codegen bug for an out-of-tree target.

I found out that llvm::EmitGEPOffset (file include\llvm\Analysis\Utils)
will create a mul with nuw flag if the GEP isInBounds is true:

      Op = Builder->CreateMul(Op, ConstantInt::get(IntPtrTy, Size),
                              GEP->getName()+".idx", isInBounds
/*NUW*/);

But what if Op is a variable that contains a negative number. In that case
the nuw flag is wrong right?
Because the mul will eventually be changed to
     %101 = shl nuw i32 %93, 2
 %93 being a variable holding a negative value (clearly the 2 high bit are
not 0)
This will cause a miscompile eventually.

I think llvm::EmitGEPOffset is being too aggressive in setting nuw here.
Or am I missing something?

(I fixed the problem locally by calling EmitGEPOffset with NoAssumptions true)
in InstructionCombining.cpp)
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190821/a4a6a506/attachment.html>

On Thu, 22 Aug 2019 at 04:01, Francois Pichet via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> But what if Op is a variable that contains a negative number. In that case
the nuw flag is wrong right?
I think so. I believe "nsw" would be correct though, since GEPs are
defined in terms of in-bounds signed arithmetic.

Cheers.

Tim.

I agree. I've reported this issue a while back: 
https://bugs.llvm.org/show_bug.cgi?id=42699
It should use NSW instead of NUW. This bug was introduced in 2011.

Nuno

-----Original Message-----
From: Francois Pichet
Sent: Thursday, August 22, 2019 4:01 AM
To: llvm-dev at lists.llvm.org
Subject: [llvm-dev] Possible bug in llvm::EmitGEPOffset


I am investigating a bad codegen bug for an out-of-tree target.

I found out that llvm::EmitGEPOffset (file include\llvm\Analysis\Utils) will 
create a mul with nuw flag if the GEP isInBounds is true:

      Op = Builder->CreateMul(Op, ConstantInt::get(IntPtrTy, Size),
                              GEP->getName()+".idx", isInBounds
/*NUW*/);

But what if Op is a variable that contains a negative number. In that case 
the nuw flag is wrong right?
Because the mul will eventually be changed to
     %101 = shl nuw i32 %93, 2
%93 being a variable holding a negative value (clearly the 2 high bit are 
not 0)
This will cause a miscompile eventually.

I think llvm::EmitGEPOffset is being too aggressive in setting nuw here.
Or am I missing something?

(I fixed the problem locally by calling EmitGEPOffset with NoAssumptions = 
true) in InstructionCombining.cpp)


_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev