-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Global Variable Optimizer] Incorrectly lift a modulo computation outside an if statement #46922
Comments
I think the problem is in instcombine, the reuse of a condition is coupled with the swap of the branch targets, that seems wrong: https://godbolt.org/z/YMvxr6 |
*** IR Dump Before Global Variable Optimizer *** @i = internal unnamed_addr global i32 1, align 4 ; Function Attrs: nounwind uwtable if.then2: ; preds = %entry if.end7: ; preds = %if.then2, %entry declare dso_local void @use(i32) local_unnamed_addr #1 attributes #0 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" } !llvm.module.flags = !{#0} !0 = !{i32 1, !"wchar_size", i32 4} @i = internal unnamed_addr global i1 false, align 4 ; Function Attrs: nounwind uwtable if.then2: ; preds = %entry if.end7: ; preds = %if.then2, %entry declare dso_local void @use(i32) local_unnamed_addr #1 attributes #0 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" } !llvm.module.flags = !{#0} !0 = !{i32 1, !"wchar_size", i32 4} |
Nevermind, ^ is not true. |
Looks like IRBuilder is creating a ConstantExpr that's not safe to speculatively evaluate. Is that supposed to be possible? |
Yes, unfortunately. This generally isn't a problem unless the instruction that uses the constantexpr is moved, like GlobalOpt is doing here. |
Constant::canTrap can be used to query whether a constant is one of these possibly-trapping constants. |
I used C-Reduce to shrink the original test case that found this bug, using clang 12.0.1. I ended up with this test case: #include <stdio.h> static long *a, *b; $ clang-12 -O2 miscompile.c $ ./a.out Despite the warning, I believe the program is UB-free, in particular because "main" doesn't call the function "h". Does this look like an instance of the same bug? |
Looks like the same bug, yes. |
Thanks. So does it seem like the problem (in my reduced example) is that function "h", and as a result "e", is somehow being speculatively evaluated even though it's not called from main? |
Not called, inter-procedural constant propagation replaces d[0] with a select between 0 and the only value that was ever written in d[0].
|
Thanks for the explanation, that's really interesting. |
Patch proposal: |
Should be fixed with: |
Extended Description
#include <stdio.h>
static long r = 0;
static long *m[1] = {&r};
static int i = 1;
static int *pi = &i;
int main() {
unsigned x = 0;
if (i) { // Evaluates to true
++x; // Guaranteed to be reached, and afterwards x == 1
int test = pi == (int*) 1;
}
printf("x = %d\n", x);
// At this point, x == 1
if (!x) { // Evaluates to false
i = 7UL % (m[0] == (long*) 1); // Unreachable. (It would have UB if it were reachable, becuase of a mod by zero.)
}
}
Get "floating point exception" with -02, Ubuntun, and clang-6/Clang-10.
The bug is no appearing with -O0 flag.
Restor the bug:
clang-10 -O2 -w test1.c
./a.out
Floating point exception (core dumped)
clang-10 -O0 -w test1.c
./a.out
x = 1
I tried with two versions, both got the bugs:
The text was updated successfully, but these errors were encountered: