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

Control Flow Guard (CFG) on Windows stops working with -Copt-level=3 and without -Coverflow-checks=on #135963

Closed
anforowicz opened this issue Jan 24, 2025 · 4 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. O-windows Operating system: Windows

Comments

@anforowicz
Copy link
Contributor

Expected behavior: Control Flow Guard (CFG) works regardless of whether -Copt-level=3 and/or -Coverflow-checks=on are present

Actual behavior: CFG stops working with -Copt-level=3 and without -Coverflow-checks=on - see step 3 below

Repro steps:

  1. Setup: get a Windows machine + install Rust via rustup

  2. Check compiler version:

    C:\src\cfgtest>rustc --version
    rustc 1.84.0 (9fc6b4312 2025-01-07)
    
  3. Prepare the following source code that should trigger a Control Flow Guard:

Contents of `cfgtest.rs`
C:\src\cfgtest>type cfgtest.rs
use std::arch::asm;

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
const NOP_INSTRUCTION_SIZE: usize = 1;
#[cfg(target_arch = "aarch64")]
const NOP_INSTRUCTION_SIZE: usize = 4;

#[inline(never)]
fn nop_sled() {
    unsafe { asm!("nop", "nop", "ret",) }
}

#[inline(never)]
fn indirect_call(func: fn()) {
    func();
}

fn main() {
    eprintln!("start!");
    let fptr = unsafe { std::mem::transmute::<usize, fn()>(
        nop_sled as usize + NOP_INSTRUCTION_SIZE
    )};
    eprintln!("calling into a bad/middle-of-a-function pointer");
    // Generates a FAST_FAIL_GUARD_ICALL_CHECK_FAILURE if CFG triggers.
    indirect_call(fptr);
    // Should only reach here if CFG is disabled.
    eprintln!("failed");
}
  1. Compile without using -Coverflow-checks=on, but with -Copt-level=3.

    • Expected behavior: CFG should trigger a crash before reaching eprintln!("failed");
    • Actual behavior: CFG is not triggered and "failed" is printed to the console
    C:\src\cfgtest>del cfgtest.exe
    
    C:\src\cfgtest>rustc --crate-name cfgtest cfgtest.rs --crate-type bin --edition=2021 --target=x86_64-pc-windows-msvc -Copt-level=3 -Ccontrol-flow-guard -o ./cfgtest.exe
    
    C:\src\cfgtest>.\cfgtest.exe
    start!
    calling into a bad/middle-of-a-function pointer
    failed    
    
  2. Compile again, this time adding -Coverflow-checks=on.

    • Expected=actual behavior: CFG triggers a crash before reaching eprintln!("failed");
    C:\src\cfgtest>del cfgtest.exe
    
    C:\src\cfgtest>rustc --crate-name cfgtest cfgtest.rs --crate-type bin --edition=2021 -Coverflow-checks=on --target=x86_64-pc-windows-msvc -Copt-level=3 -Ccontrol-flow-guard -o ./cfgtest.exe
    
    C:\src\cfgtest>.\cfgtest.exe
    start!
    calling into a bad/middle-of-a-function pointer
    

/cc @ajpaverd who has kindly worked on enabling CFG in #68793

PS. FWIW I've discovered this unexpected behavior when trying to remove -Coverflow-checks=on from Chromium build configuration in https://crrev.com/c/6176309. Removing this led to failures in Chromium's CFG tests: see https://ci.chromium.org/ui/p/chromium/bisection/test-analysis/b/4790699691081728 and https://ci.chromium.org/ui/p/chromium/builders/ci/win-rel-cft/7593/overview.

@anforowicz anforowicz added the C-bug Category: This is a bug. label Jan 24, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 24, 2025
@anforowicz
Copy link
Contributor Author

FWIW, CFG kicks back in if I also remove -Copt-level=3:

C:\src\cfgtest>del cfgtest.exe

C:\src\cfgtest>rustc --crate-name cfgtest cfgtest.rs --crate-type bin --edition=2021 --target=x86_64-pc-windows-msvc -Ccontrol-flow-guard -o ./cfgtest.exe

C:\src\cfgtest>.\cfgtest.exe
start!
calling into a bad/middle-of-a-function pointer

@anforowicz
Copy link
Contributor Author

Maybe the test that I am using (and Chromium here) is too simplistic. I guess I should try using std::hint::black_box to avoid optimizing away the simplistic test call. Hold on...

@anforowicz
Copy link
Contributor Author

Yes, the test gives expected results after inserting std::hint::black_box here:

fn main() {
    eprintln!("start!");  
    let fptr = unsafe { std::mem::transmute::<usize, fn()>(
        nop_sled as usize + NOP_INSTRUCTION_SIZE
    )};
    eprintln!("calling into a bad/middle-of-a-function pointer");  
    // Generates a FAST_FAIL_GUARD_ICALL_CHECK_FAILURE if CFG triggers.
    // `std::hint::black_box` is used to prevent optimizing away the call.
    indirect_call(std::hint::black_box(fptr));  // <= HERE
    // Should only reach here if CFG is disabled.
    eprintln!("failed");  
}

So maybe everything here works as expected and we can just go ahead and close this issue? Sorry for the noise :-/

@workingjubilee
Copy link
Member

@anforowicz Thank you for opening an issue when you determined there might be a problem! I cannot count (literally) how many issue reports have been lost entirely because someone waits until they are absolutely 100% sure that the issue is in the compiler, and not merely until after they've done a reasonable review, instead of a perfect one.

But yes, it does seem to just be an optimization problem: sometimes the code you expect to see "run" doesn't get evaluated by the CPU, but by the abstract machine. Closing as expected behavior.

@saethlin saethlin added O-windows Operating system: Windows C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-bug Category: This is a bug. labels Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. O-windows Operating system: Windows
Projects
None yet
Development

No branches or pull requests

4 participants