floating point intrinsics may clobber errno in unrelated code

Opened by Daniel Micay at 2024-07-26 10:40:26

These intrinsics assume errno is not set, but it may be set. In theory, one of these intrinsics could be reordered between a call to a C function setting errno and a check of the errno value in Rust. Rust could ignore this problem, as it will rarely (if ever) occur. However, safety mandates that we either supply our own math library known to not set errno on math errors (glibc is a black sheep in this regard) or stop using these unless something like -fno-math-errno is passed (as clang handles it on Linux).

I think the best solution will be providing a Rust-specific math library. It's a bit sad, but changing this due to glibc's math library will significantly hurt performance and we need something else on Windows anyway.
Daniel Micay at 2014-03-05 02:06:30
Traige: no change.
Steve Klabnik at 2015-04-28 18:12:24
Triage: no change
Steve Klabnik at 2016-06-06 21:49:06
What about Julia's OpenLibM?

I would suggest using something like -fno-math-errno by default. For one, it allows for some math operations to be inlined.
Demi Marie Obenour at 2016-07-17 00:43:15
Nominating for lang team: Is this I-unsound? cc @aturon
Mark Rousskov at 2017-07-20 00:20:16
This is not a soundness bug per se, but rather "just" incorrect codegen. P-low.
Aaron Turon at 2017-07-20 19:12:50
Miscompilations are also soundness bugs, labeling accordingly.
Jonas Schievink at 2020-01-12 15:12:06
I cannot find evidence today of the claim that LLVM's floating point intrinsics require errno to not be set -- indeed, looking for errno in the LangRef seems to indicate that they all guarantee that errno would not be set (or make no mention of it).

OTOH, it does seem true that if we're lowering to libm or otherwise, errno can be set -- and that may mask a later access to errno (as mentioned, due to reordering or just lack of knowledge). But I think to some extent I'd argue that's not a soundness bug, as the codegen should be correct, just not do what you expect (i.e. there should not be a memory error here, though there could be a logic bug).
Mark Rousskov at 2020-04-24 21:47:46
Yeah, I don't think this is a soundness issue. It just means we effectively don't support interacting with C errno from Rust (at least not while using these math intrinsics anywhere in the general vicinity).
Hanna Kruppe at 2020-04-24 22:02:34

On x86_64-unknown-linux-gnu, the following code (playground):

#[inline(never)]
fn some_c_function() {
	// This could be any function that sets errno. For demonstration purposes, set errno directly.
	// This is the function used to get a pointer to errno on Linux, see the URL below for other platforms:
	// https://github.com/rust-lang/rust/blob/aa877bc71c8c8082122bee23d17c8669f30f275d/library/std/src/sys/pal/unix/os.rs#L52
	extern "C" {
		fn __errno_location() -> *mut i32;
	}
	unsafe { *__errno_location() = 1; }
}

fn main() {
    let inf = std::hint::black_box(f64::INFINITY);
    some_c_function();
    // LLVM will hoist `inf % 1.0` to here.
    loop {
    	// This should display an error code of 1 ("Operation not permitted"), but will display error
    	// code 33 ("Numerical argument out of domain") instead when compiled with optimisations.
        dbg!(std::io::Error::last_os_error());
        if std::hint::black_box(true) {
            break;
        }
        std::hint::black_box(inf % 1.0);
    }
}

should print

[src/main.rs:19:9] std::io::Error::last_os_error() = Os {
    code: 1,
    kind: PermissionDenied,
    message: "Operation not permitted",
}

but when compiled with optimisations, will instead print

[src/main.rs:19:9] std::io::Error::last_os_error() = Os {
    code: 33,
    kind: Uncategorized,
    message: "Numerical argument out of domain",
}

despite inf % 1.0 (which is compiled to a fmod libc function call) never getting executed at the source code level. This is because LLVM asssumes the frem LLVM IR instruction is pure and therefore hoists it to between the some_c_function() call and the last_os_error() call.

beetrees at 2024-07-26 00:09:35

(That PR should not have closed this issue. It updates a submodule with commits that reference another repo's issue that happens to have the same issue number as this one...)
Timo at 2024-07-26 10:40:26