flag_check.exe being generated in source tree

[wmmc88]

Under certain circumstances, a flag_check.exe appears in the root of crates using cc in their build scripts. This appears to be a regression of an issue that was fixed by #246. I've confirmed this issue is present in all versions starting from 1.2.7 to at least 1.2.15, so it seems like something in 1.2.7 is causing this regression.

In order to trigger this issue, I needed to have the following in my .cargo/config.toml:

[target.'cfg(target_os = "windows")']
rustflags = [
  # -Ccontrol-flow-guard: Enable Control Flow Guard (https://doc.rust-lang.org/rustc/codegen-options/index.html#control-flow-guard, https://learn.microsoft.com/en-us/cpp/build/reference/guard-enable-control-flow-guard).
  "-C",
  "control-flow-guard",
]

Given the diff of 1.2.7 and the fact I need to provide -Ccontrol-flow-guard to get this to trigger, I'm guessing something is causing is_flag_supported_inner to run, and that that is erroneously dropping the linked artifact in root of the crate

A minimal repro is available at https://github.com/wmmc88/minimal-repros/tree/cc-flag-check

[wmmc88]

An even smaller repro:

build.rs

fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
    dbg!(cc::Build::new().is_flag_supported("-Ccontrol-flow-guard")?);
    Ok(())
}

[NobodyXu]

I think the bug might be triggered by

cc-rs/src/flags.rs

Line 314 in 181c03d

push_if_supported(format!("-mguard={cc_val}").into());

[NobodyXu]

Under certain circumstances, a flag_check.exe appears in the root of crates using cc in their build scripts

Do you mean that it appears outside target?

Or is that it generates a binary within target?

[wmmc88]

Under certain circumstances, a flag_check.exe appears in the root of crates using cc in their build scripts

Do you mean that it appears outside target?

Or is that it generates a binary within target?

It appears outside target, in the same folder as the crates cargo.toml

[ChrisDenton]

I recall seeing something like this before. I believe that was caused by not setting the output directory when invoking the C compiler. So it would use the current directory which, if cc is in a build script, will be the project's root.

[wmmc88]

This line introduced in #1336 (@ChrisDenton's pr) seems to be when the bug started manifesting. commenting out that line on main seems to cause the bug to no longer manifest. Looks like its because before the addition of that line, msvc would just fail with:

flag_check.c
LINK : error LNK2001: unresolved external symbol mainCRTStartup
flag_check.exe : fatal error LNK1120: 1 unresolved externals

and it would erroneously report the flag as not supported (which is what #1336 fixed)

I believe that was caused by not setting the output directory when invoking the C compiler.

Agree with this. I was wondering why this wasn't already the case, but seems like before #1336, it just never got to the point of compiling successfully ever

[ChrisDenton]

Ah, thanks for investigating! Yep that makes sense.

We could set the current_dir when calling the C compiler or if we rely on that for other reason then there's /Fe (compiler option) or /OUT (linker option)

[wmmc88]

Ah, thanks for investigating! Yep that makes sense.

We could set the current_dir when calling the C compiler or if we rely on that for other reason then there's /Fe (compiler option) or /OUT (linker option)

Okay, I think setting current_dir is probably the best way. I'll submit a pr w/ some tests :)

[ChrisDenton]

Oh, I just realised we do already set /Fo already so I did a quick experiment with adding /Fe too (#1414). But do feel free to submit a PR. I'm not sure if my test is the best approach as flag_check is quite self-contained (it's the only place we create an exe to my knowledge).

[wmmc88]

Ok I've submitted my patch(#1415 ) using current_dir in case any other artifacts get generated by msvc (now or in the future). We probably don't ever want any of that to ever generate outside of OUT_DIR anyways...

I also added a quick "is the git working tree clean" check for any CI jobs that produce artifacts so to prevent any more future regressions of this bug

[ChrisDenton]

Great! Thanks for both reporting and fixing this.