-
Notifications
You must be signed in to change notification settings - Fork 78
Casting infinity to an intiger sometimes results in a 0 value(with optimizations enabled) #690
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
Comments
After further reduction, all of this is just a very long way of obtaining infinity. fn main() {
let mut _4 = 2623078656.0;
_4 = _4 * _4 * _4;
let val = -(_4 * _4) * _4 * (_4);
let inf: f32 = val * val;
let int = inf as u64;
println!("{inf:?} {:?} {int:?}", inf.to_bits());
} With optimizations enabled, GCC will optimize the |
In C this is UB. LLVM used to have it be UB too for a long time, requiring rustc to check for overflow before doing the actual cast, but now there is an intrinsic which saturates on overflow. I guess GCC has the same UB. |
Huh, I did not expect this to remain uncaught for so long.
https://github.com/rust-lang/rustc_codegen_gcc/blob/master/src%2Fbuilder.rs#L1212-L1216 I guess we will have to implement a workaround here too. Thankfully, it is a fairly simple thing to do. There is one thing that concerns me, tough. The x86_64 float to int casts are wrapping, right? So, with the default settings, this should have tripped way more tests. This suggests to me that there is a workaround in play already, but it is just broken. @antoyo does |
I do not think so. We set some flags like Any idea of tests that should fail because of this bug? |
I would have to check.
The more fundamental issue here is wrapping vs saturating casts. I believe
that on x86_64 C casts wrap around, while in Rust they saturate.
It seems to me like they are saturating(in `cg_gcc`), but in a subtly wrong way.
If they were wrapping that should cause almost every file generated by
rustlantis to fail.
Looking at the generated assembly, I might have a rough idea about the case
of the bug.
…On Fri, May 30, 2025, 20:10 antoyo ***@***.***> wrote:
*antoyo* left a comment (rust-lang/rustc_codegen_gcc#690)
<#690 (comment)>
@antoyo <https://github.com/antoyo> does cg_gcc set any flags that could
influence this? E.g. do you switch float casts to saturating somehow?
I do not think so. We set some flags like -fwrapv, but I don't think this
would influence this.
Any idea of tests that should fail because of this bug?
—
Reply to this email directly, view it on GitHub
<#690 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR52NDRXOW3AEQAJFCYSF3D3BCNIXAVCNFSM6AAAAAB6IKP4BOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMRTGA3DOOJVGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Analyzing the GIMPLE we generate is a bit tough - duplicate block names make it hard to see what is happening. So, it looks like the "saturation check" is present on the GIMPLE level, the problem just is... it happens after the cast. if (_37 < _38) goto then; else goto else;
then:
selectVar = 0;
goto after;
else:
_39 = loadedValue2 * loadedValue3;
_40 = -_39;
_41 = loadedValue4 * _40;
_42 = loadedValue5 * _41;
_43 = loadedValue2 * loadedValue3;
_44 = -_43;
_45 = loadedValue4 * _44;
_46 = loadedValue5 * _45;
_47 = _42 * _46;
// That seems wrong???
selectVar = (size_t) _47;
goto after;
after:
_48 = loadedValue2 * loadedValue3;
_49 = -_48;
_50 = loadedValue4 * _49;
_51 = loadedValue5 * _50;
_52 = loadedValue2 * loadedValue3;
_53 = -_52;
_54 = loadedValue4 * _53;
_55 = loadedValue5 * _54;
_56 = _51 * _55;
_57 = 1.844674297419792384e+19;
if (_56 > _57) goto then; else goto else;
then:
selectVar = 18446744073709551615; Here, it looks like if the float value > 0, then we cast it to a float, and then check if it is larger than the max int value after the fact(swaping the value out in that case). Since such a cast is UB if the floating point is out of range, GCC is able to "see" the loaded value can't possibly be larger than max(since that is UB), and removes the branch. So, it seems like all of the muck at the begining is needed to get GCC to "see" that the branch is impossible. |
I still don't get why the casts are sometimes saturating in the first place. A piece of code like this works(is saturating): for i in 0..1000 {
println!("{:?}", i as f32 as u8);
} But it should not, since a bit of equivalent C, compiled with GCC, does not(it wraps around). #include <stdio.h>
#include <float.h>
int main()
{
for (int i = 0; i < 1000; i++)printf("%u\n", (unsigned int)(unsigned char)((float)i));
return 0;
} |
I do not know if this is the problem here, but that kind of ordering issues can happen because GCC uses an AST-based IR while LLVM uses an instruction-based IR. |
I can't find where that branch is inserted. If it is not inserted by cg_gcc(I am not sure about that), then maybe it is inserted by libgccjit? |
It could be inserted in |
Never mind, it is in rustc_codegen_gcc/src/builder.rs Line 1841 in 706905b
At least the comment here suggests the order of operations(cast then check) is simply wrong. We can work around this by swapping the order of operations around. For unsigned casts, maybe could do something clever with clamps or min/max, e.g. |
So, I reimplemented the cast in a way that I think should be fully sound:
But it is still broken with optimizations. Can you see anything obviously wrong with this cast? I got fat fingers, and Github's got some short-cuts I don't know about :(. I can't reopen it myself, sadly. |
It seems like I can replicate the issue in C. I think the following code ought to be UB free, right? #include <stdint.h>
#include <stdio.h>
uint64_t womky() {
uint64_t ret;
uint64_t fti_cast_res;
float loadedValue5;
float loadedValue4;
float loadedValue3;
float loadedValue2;
float loadedValue1;
char stack_var_1[4];
char *stack_var_1_12_1;
float _19;
float _29;
stack_var_1_12_1 = &stack_var_1[0];
float _2 = 3.4028234663852885981170418348451692544e+38;
*(float *)stack_var_1_12_1 = _2;
char *stack_var_1_13_3 = &stack_var_1[0];
loadedValue1 = *((float *)stack_var_1_13_3);
char *stack_var_1_14_4 = &stack_var_1[0];
*((float *)stack_var_1_14_4) = loadedValue1;
char *stack_var_1_15_5 = &stack_var_1[0];
loadedValue2 = *((float *)stack_var_1_15_5);
char *stack_var_1_16_6 = &stack_var_1[0];
loadedValue3 = *((float *)stack_var_1_16_6);
char *stack_var_1_17_7 = &stack_var_1[0];
loadedValue4 = *((float *)stack_var_1_17_7);
char *stack_var_1_18_8 = &stack_var_1[0];
loadedValue5 = *((float *)stack_var_1_18_8);
float _9 = loadedValue2 * loadedValue3;
float _10 = -_9;
float _11 = loadedValue4 * _10;
float _12 = loadedValue5 * _11;
float _13 = loadedValue2 * loadedValue3;
float _14 = -_13;
float _15 = loadedValue4 * _14;
float _16 = loadedValue5 * _15;
float _17 = _12 * _16;
float _18 = 1.844674297419792384e+19;
if (_17 > _18)
goto gt_max;
else
goto lt_max;
lt_max:
_19 = loadedValue2 * loadedValue3;
float _20 = -_19;
float _21 = loadedValue4 * _20;
float _22 = loadedValue5 * _21;
float _23 = loadedValue2 * loadedValue3;
float _24 = -_23;
float _25 = loadedValue4 * _24;
float _26 = loadedValue5 * _25;
float _27 = _22 * _26;
float _28 = 0.0;
if (_27 > _28)
goto in_bounds;
else
goto lt_min;
in_bounds:
_29 = loadedValue2 * loadedValue3;
float _30 = -_29;
float _31 = loadedValue4 * _30;
float _32 = loadedValue5 * _31;
float _33 = loadedValue2 * loadedValue3;
float _34 = -_33;
float _35 = loadedValue4 * _34;
float _36 = loadedValue5 * _35;
float _37 = _32 * _36;
fti_cast_res = (uint64_t)_37;
goto after_cast;
gt_max:
fti_cast_res = 18446744073709551615lu;
goto after_cast;
lt_min:
fti_cast_res = 0;
goto after_cast;
after_cast:
ret = fti_cast_res;
return ret;
}
int main() {
printf("Hello World %lu", (uint64_t)(1.0 / 0.0));
return 0;
} |
Could you please push your branch somewhere and tell me exactly which reproducer produces that GIMPLE IR? |
The branch is here: https://github.com/FractalFir/rustc_codegen_gcc/tree/master Both the snippet of gimple, and the GIMPLE converted to C come from this small Rust reproducer. #[unsafe(no_mangle)]
#[inline(never)]
pub fn cast(num: f32) -> u64 {
num as u64
}
#[unsafe(no_mangle)]
fn womky() -> u64 {
let mut _4 = 340282346638528859811704183484516925440.000000;
_4 = _4;
let val = -(_4 * _4) * _4 * (_4);
let inf: f32 = (val * val);
let int = inf as u64;
int
}
fn main() {
println!("{:?}", womky());
} The |
I further minimzed the C example, removing the cast alltogether. This function returns true in -O1, but false in -O2. Since it only contains floating-point math(which is UB-free?), it should behave identically in both cases. #include <stdbool.h>
#include <stdio.h>
bool womky(void) {
float val = 3.4028234663852885981170418348451692544e+38;
float a = val * -(val * val);
float b = val * a;
return b * b > 0.0;
}
int main(void) { printf("%d", womky()); } EDIT: I think I know what is going on. Here, we get screwed over by the sign of infinity. If you run this Rust code:
You'll see that casting inf and -inf to a u64 results in different values. Here, GCC's floating-point optimizations change a positive infinity to a negative one. I think this is OK in C(not sure, tough). I don't know if this is a miscompilation according to Rust(I think so). |
There's been a lot of discussion about the float semantics in Rust. You might be able to find the answer to that question in one of those links:
|
When I looked at the bits of the float(or the float itself), they were identical.
It looks like, in this case, GCC only applied the optimizations when computing the floating-point value for the cast, but not when it got reinterpreted as bits(or passed to be displayed). In other words, the value of that float is indeterminate(is this the right term?): each time we look at it, it changes. |
This has the potential to royally screw us over. Consider a cast like this: if (val > (float) ULONG_MAX) return ULONG_MAX;
else if (val < 0) return 0;
else return (uint64_t) val; Suppose GCC optimizes the value in the 1st check to -infinity, and keeps the rest as infinity. Now, our code looks like this: if (-infinity > (float) ULONG_MAX) return ULONG_MAX;
else if (infinity < 0) return 0;
else return (uint64_t) infinity; Since negative infinity is less than else if (infinity < 0) return 0;
else return (uint64_t) infinity; Positive ininfity is more than 0, so we fall trough here: else return (uint64_t) infinity; And now, our code has UB it did not have before. On some systems, this could cause a crash. |
Manifesting this issue in C is quite easy. Here, we can see that:
#include <stdbool.h>
#include <stdio.h>
#include <string.h>
unsigned int to_bits(float f){
unsigned int ret;
memcpy(&ret,&f, sizeof(unsigned int));
return ret;
}
bool womky(void) {
float val = 3.4028234663852885981170418348451692544e+38;
float a = val * -(val * val);
float b = val * a;
float res = b * b;
printf("%f %x\n",res, to_bits(res));
return res > 0.0;
}
int main(void) { printf("%d\n", womky()); } @antoyo do you know if this behaviour(the same variable has 2 different values when accessed) is permitted by the C standard? |
I have honestly no idea. And putting the comparison in a int __attribute__ ((noinline)) is_positive(float f) {
return f > 0.0;
} gives the correct result. |
@FractalFir As far as I can tell this is just a plain GCC bug (somewhere in VRP presumably). The result of the float calculation here is required to be |
https://gcc.godbolt.org/z/4j66dWz6s has vrp1 printing something like this:
If I'm reading this right, it seems to think that |
Uh oh!
There was an error while loading. Please reload this page.
This program(which runs fine under MIRI) produces incorrect results when compiled with
cg_gcc
.The text was updated successfully, but these errors were encountered: