apparent bad interaction between math and short-circuit

[regehr]

Bugzilla Link	3354
Resolution	FIXED
Resolved on	Jan 19, 2009 19:16
Version	unspecified
OS	Linux
Attachments	testcase
CC	@lattner,@efriedma-quic

Extended Description

The attached program displays this behavior on x86 Linux:

[regehr@babel tmp30]$ llvm-gcc -O small.c -o small
small.c: In function ‘main’:
small.c:19: warning: division by zero
small.c:19: warning: division by zero
[regehr@babel tmp30]$ ./small
x=1
Floating point exception

The thing is, the divide-by-zero behaviors are all protected by short-circuiting operations, and should never fire. I claim that LLVM's -O0 behavior is what must occur at all optimization levels:

[regehr@babel tmp30]$ llvm-gcc -O0 small.c -o small
small.c: In function ‘main’:
small.c:19: warning: division by zero
small.c:19: warning: division by zero
[regehr@babel tmp30]$ ./small
x=1
x=0

The macro in the attached code is simply a macroized version of the 2nd safe multiplication fragment here:

https://www.securecoding.cert.org/confluence/display/cplusplus/INT32-CPP.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow

[lattner]

You find some very fun and interesting bugs. This one looks like it is in reassociate. Working on it!

[lattner]

Fixed here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090119/072366.html

Reassociate was moving a constant expression to a different block under the assumption that constants can't have side effects. I think that this is a reasonable expectation, so I changed x/0 to constant fold into undef as is allowed by langref.

[efriedma-quic]

Reassociate was moving a constant expression to a different block under the
assumption that constants can't have side effects. I think that this is a
reasonable expectation, so I changed x/0 to constant fold into undef as is
allowed by langref.

That's only a workaround... what if the denominator is an unfoldable constant?

[lattner]

You're right, I should use ConstantExpr::canTrap.

[lattner]

Hrm, this is even more interesting. Reassociate shouldn't move computations "up" the CFG. The before code is:

%14 = and i1 %12, %tmp4
%brmerge.demorgan = and i1 %14,
br i1 %brmerge.demorgan, label %bb6, label %bb2

the after code is:

%14 = and i1 %12,
%brmerge.demorgan = and i1 %14, %tmp4
br i1 %brmerge.demorgan, label %bb6, label %bb2

The code generator just happens to lower this into two branches, both are bad, before reassociate gets involved.

[lattner]

This is a simplifycfg bug, here's a testcase:

@​G = extern_weak global i32

define i32 @​test(i32 %tmp21, i32 %tmp24) {
%tmp25 = icmp sle i32 %tmp21, %tmp24
br i1 %tmp25, label %bb2, label %bb1

bb1: ; preds = %bb
%tmp26 = icmp sgt i32 sdiv (i32 -32768, i32 ptrtoint (i32* @​G to i32)), 0
br i1 %tmp26, label %bb6, label %bb2
bb2:
ret i32 42

bb6:
unwind
}

[lattner]

Here's a fix for that bug. There is another bug as well:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090119/072372.html

[lattner]

This patch fixes the rest of it:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090119/072389.html