Skip to content

Stack overflow in memcmp on x86_64 when used in extern "C" memcmp implementation with -Zbuild-std #487

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

Closed
sunfishcode opened this issue Aug 9, 2022 · 3 comments · Fixed by #488

Comments

@sunfishcode
Copy link
Member

This Rust testcase:

#![feature(rustc_private)] // for compiler-builtins

extern crate compiler_builtins;

static A: [u8; 119] = [0u8; 119];
static B: [u8; 119] = [1u8; 119];

#[no_mangle]
unsafe extern "C" fn memcmp(a: *const std::ffi::c_void, b: *const std::ffi::c_void, len: usize) -> i32 {
    compiler_builtins::mem::memcmp(a.cast(), b.cast(), len)
}

fn main() {
    println!("Hello, world!");

    unsafe {
        dbg!(compiler_builtins::mem::memcmp(A.as_ptr(), B.as_ptr(), 119));
    }
}

runs successfully with Rust nightly 2022-08-07, fails in Rust nightly 2022-08-08 on x86_64-unknown-linux-gnu with -Zbuild-std:

$ cargo +nightly-2022-08-07 run --quiet -Zbuild-std --target=x86_64-unknown-linux-gnu
Hello, world!
[src/main.rs:17] compiler_builtins::mem::memcmp(A.as_ptr(), B.as_ptr(), 119) = -1
$ cargo +nightly-2022-08-08 run --quiet -Zbuild-std --target=x86_64-unknown-linux-gnu
Hello, world!

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)
$

It overflows the stack with unbounded recursion, with iterations looking like this:

#980 0x000055555557c45b in test::memcmp (a=0x7fffff817f60, b=0x7fffff817f80, len=32) at src/main.rs:10
#981 0x00005555556bd96a in core::array::equality::{impl#9}::spec_eq<u128, u128, 2> (a=0x7fffff817f60, b=0x7fffff817f80)
    at .../nightly-2022-08-08-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/array/equality.rs:152
#982 0x00005555556bd9a3 in core::array::equality::{impl#9}::spec_ne<u128, u128, 2> (a=0x7fffff817f60, b=0x7fffff817f80)
    at .../nightly-2022-08-08-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/array/equality.rs:156
#983 0x00005555556bd6a3 in core::array::equality::{impl#0}::ne<u128, u128, 2> (self=0x7fffff817f60, other=0x7fffff817f80)
    at .../nightly-2022-08-08-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/array/equality.rs:16
#984 0x00005555556c0820 in compiler_builtins::mem::impls::compare_bytes::cmp<[u128; 2], u128, compiler_builtins::mem::impls::compare_bytes::{closure_env#4}> (a=0x7fffff8182a0, 
    b=0x7fffff8182c0, n=32, f=...) at src/mem/x86_64.rs:147
#985 compiler_builtins::mem::impls::compare_bytes::{closure#5} (a=0x7fffff8182a0, b=0x7fffff8182c0, n=32) at src/mem/x86_64.rs:173
#986 0x00005555556bdff0 in compiler_builtins::mem::impls::compare_bytes (a=0x7fffff8182a0, b=0x7fffff8182c0, n=32) at src/mem/x86_64.rs:182
#987 compiler_builtins::mem::memcmp (s1=0x7fffff8182a0, s2=0x7fffff8182c0, n=32) at src/mem/mod.rs:54
#988 0x000055555557c45b in test::memcmp (a=0x7fffff8182a0, b=0x7fffff8182c0, len=32) at src/main.rs:10

It looks like Rust 2022-08-08 included rust-lang/rust#100218, which updated compiler-builtins from 1.73 to 1.78, which pulled in #471. This target has SSE2, so unless I'm mistaken about what's happening here, it appears the observation "targets with SSE(2) do not seem to generate a call to memcmp even in debug mode" doesn't hold in this situation.

@Amanieu
Copy link
Member

Amanieu commented Aug 9, 2022

The actual condition in rustc seems to be to call memcmp if the object size if more than 2*pointer_size.

@Demindiro Could you look into this? We might have to remove the c32 from your memcmp implementation since we can't compare [u128; 2] directly without using memcmp.

Demindiro added a commit to Demindiro/compiler-builtins that referenced this issue Aug 10, 2022
@Demindiro
Copy link
Contributor

With rustc 1.65.0-nightly (34a6cae28 2022-08-09)) and RUSTFLAGS="-C opt-level=0" cargo bench memcmp_rust I cannot reproduce the issue.

Still, relying on the whims of the compiler is clearly a poor idea so I'll just remove it.

@Amanieu
Copy link
Member

Amanieu commented Aug 10, 2022

Fixed in compiler-builtins 0.1.79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants