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

Spurious build failures sometimes #20

Open
retep998 opened this issue Aug 20, 2020 · 9 comments
Open

Spurious build failures sometimes #20

retep998 opened this issue Aug 20, 2020 · 9 comments
Labels

Comments

@retep998
Copy link
Contributor

When building a program that uses this crate, I will sometimes get this error:

thread 'main' panicked at 'Are you sure you have RC.EXE in your $PATH?: Custom { kind: InvalidInput, error: "nul byte found in provided data" }', C:\...\Rust\registry\src\github.com-1ecc6299db9ec823\embed-resource-1.3.3\src\windows_msvc.rs:27:13

It goes away if I try to build again.

@nabijaczleweli
Copy link
Owner

Can you provide a backtrace (preferably by setting RUST_BACKTRACE=full)? I can't really do much about this myself otherwise.

@retep998
Copy link
Contributor Author

It is panicking on this line specifically, and there's only one way it gets called so I didn't think a backtrace would make much of a difference (plus getting one for a heisenbug like this...)

.expect("Are you sure you have RC.EXE in your $PATH?")

The actually useful information would be knowing which ensure_no_nuls call is failing but the backtrace can't tell us that:
https://github.com/rust-lang/rust/blob/d9d4d39612aae0b8398340bd83d592cafad8e4ec/library/std/src/sys/windows/process.rs#L60-L66

@nabijaczleweli
Copy link
Owner

nabijaczleweli commented Aug 21, 2020

If you're feeling naughty, you could edit C:\...\Rust\registry\src\github.com-1ecc6299db9ec823\embed-resource-1.3.3\src\windows_msvc.rs and add dbg!()s around the arguments to new() and args(), so you'll have the offending arguments in target\variant\build\crate-hash\output if this fails again.

@retep998
Copy link
Contributor Author

retep998 commented Aug 24, 2020

Caused by:
  process didn't exit successfully: `.../target\debug\build\...-bc1d1fa0f0973d32\build-script-build` (exit code: 101)
--- stderr
[...\Rust\registry\src\github.com-1ecc6299db9ec823\embed-resource-1.3.3\src\windows_msvc.rs:26] find_windows_sdk_rc_exe().as_ref().map_or(Path::new("rc.exe"), Path::new) = "C:\\Program Files (x86)\\Windows Kits\\10\\bin\\10.0.19041.0\\x64\\rc.exe"
[...\Rust\registry\src\github.com-1ecc6299db9ec823\embed-resource-1.3.3\src\windows_msvc.rs:27] &["/fo", &format!("{}/{}.lib", out_dir, prefix), resource] = [
    "/fo",
    ".../target\\debug\\build\\...-ee488b4\u{0}\u{0}95c5992\\out/elevated-manifest.lib",
    "elevated-manifest.rc",
]
thread 'main' panicked at 'Are you sure you have RC.EXE in your $PATH?: Custom { kind: InvalidInput, error: "nul byte found in provided data" }', ...\Rust\registry\src\github.com-1ecc6299db9ec823\embed-resource-1.3.3\src\windows_msvc.rs:26:13

out_dir somehow has nul bytes in it?! It should be ee488b44895c5992 not ee488b4\u{0}\u{0}95c5992.
I'm definitely not calling it in any weird way, just a literal embed_resource::compile("elevated-manifest.rc"); as the first line in the build script.
So now I added a dbg! to when out_dir is originally acquired to find out if env::var("OUT_DIR") is giving us garbage, or if UB later on is somehow corrupting it. I would not be surprised if vswhom is inflicting UB here.
Just gotta wait until this bug randomly decides to trigger again...

@retep998
Copy link
Contributor Author

retep998 commented Sep 3, 2020

  [...\Rust\registry\src\github.com-1ecc6299db9ec823\embed-resource-1.3.3\src\lib.rs:129] env::var("OUT_DIR") = Ok(
      ".../target\\debug\\build\\rust-cam-worker-dd42d6b834aee72b\\out",
  )
[...\Rust\registry\src\github.com-1ecc6299db9ec823\embed-resource-1.3.3\src\windows_msvc.rs:27] &["/fo", &format!("{}/{}.lib", out_dir, prefix), resource] = [
      "/fo",
      ".../target\\debug\\build\\rust-cam-worker-dd42d6b\u{0}\u{0}4aee72b\\out/elevated-manifest.lib",
      "elevated-manifest.rc",
  ]

Don't you just love to see memory corruption?
I'm going to pin the blame firmly on vswhom and work on my own replacement for all the VS finding logic.

nabijaczleweli added a commit to nabijaczleweli/vswhom.rs that referenced this issue Sep 3, 2020
nabijaczleweli added a commit to nabijaczleweli/vswhom-sys.rs that referenced this issue Sep 3, 2020
@nabijaczleweli
Copy link
Owner

that's what I get for FFIng to gamedev code, I guess; ugh

thanks for tracking this down so far, I've added notices to the vswhom crates' READMEs

@segevfiner
Copy link

Offf. That vswhom C++ code is quite a piece of work and doesn't seem to have been written with code quality in mind or good C++ practices...

If I had to guess, this line seems suspect:
https://github.com/nabijaczleweli/vswhom-sys.rs/blob/2600d1bae41a7a293379e8fd0f70af260a3ead9d/ext/vswhom.cpp#L283

value is a wchar_t and length is in bytes. Which means it's going to write two NUL bytes past the end of the string, corrupting memory as seen here.

@nabijaczleweli
Copy link
Owner

Thanks for pointing that one out. I've updated vswhom-sys with this (and the other place where it.. just doesn't even attempt to get terminated), a rebuild will pick it up.

@segevfiner
Copy link

I think modern MSVC supports ASAN BTW. Might be worth a shot as it might be able to catch such errors if any remain.

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

No branches or pull requests

3 participants