Skip to content
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

floating point intrinsics may clobber errno in unrelated code #12695

Open
thestinger opened this issue Mar 4, 2014 · 11 comments
Open

floating point intrinsics may clobber errno in unrelated code #12695

thestinger opened this issue Mar 4, 2014 · 11 comments
Labels
C-bug Category: This is a bug. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@thestinger
Copy link
Contributor

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).

@thestinger
Copy link
Contributor Author

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.

@steveklabnik
Copy link
Member

Traige: no change.

@steveklabnik
Copy link
Member

Triage: no change

@DemiMarie
Copy link
Contributor

DemiMarie commented Jul 17, 2016

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.

@Mark-Simulacrum
Copy link
Member

Nominating for lang team: Is this I-unsound? cc @aturon

@Mark-Simulacrum Mark-Simulacrum added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jul 20, 2017
@aturon aturon added P-low Low priority and removed I-nominated labels Jul 20, 2017
@aturon
Copy link
Member

aturon commented Jul 20, 2017

This is not a soundness bug per se, but rather "just" incorrect codegen. P-low.

@Mark-Simulacrum Mark-Simulacrum removed C-enhancement Category: An issue proposing an enhancement or a PR with one. I-wrong labels Jul 26, 2017
@jonas-schievink jonas-schievink added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 12, 2020
@jonas-schievink
Copy link
Contributor

Miscompilations are also soundness bugs, labeling accordingly.

@Mark-Simulacrum
Copy link
Member

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).

@hanna-kruppe
Copy link
Contributor

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).

@Mark-Simulacrum Mark-Simulacrum removed the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Apr 24, 2020
@Mark-Simulacrum Mark-Simulacrum changed the title using LLVM floating point intrinsics is unsound with glibc floating point intrinsics may clobber errno in unrelated code Jul 17, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
Complete type param/associated type in trait generic arg per arg index

- Fix rust-lang#12140
- Also fix tidy check does not work for marks in multiline
@beetrees
Copy link
Contributor

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.

@y21
Copy link
Member

y21 commented Jul 26, 2024

(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...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants