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

build.rs panics with debug assertions in the standard library enabled #422

Closed
jyn514 opened this issue Jan 21, 2021 · 14 comments · Fixed by #445
Closed

build.rs panics with debug assertions in the standard library enabled #422

jyn514 opened this issue Jan 21, 2021 · 14 comments · Fixed by #445

Comments

@jyn514
Copy link

jyn514 commented Jan 21, 2021

thread '<unnamed>' panicked at 'attempt to create unaligned or null slice', /home/joshua/rustc/library/core/src/slice/raw.rs:90:5
  17:     0x7f1ba4aab4bd - core::panicking::panic::h8cbc293f6a10d9b7
                               at /home/joshua/rustc/library/core/src/panicking.rs:50:5
  18:     0x7f1ba49b435a - core::slice::raw::from_raw_parts::h53f99ecbe714ca76
                               at /home/joshua/rustc/library/core/src/slice/raw.rs:90:5
  19:     0x7f1ba49b69ce - windows_winmd::parsed::blob::Blob::read_utf16::hab64d71897c4ec66
                               at /home/joshua/.local/lib/cargo/git/checkouts/windows-rs-4d19330348d493fa/256538e/crates/winmd/src/parsed/blob.rs:83:32
  20:     0x7f1ba45ef2a8 - windows_gen::constant::Constant::gen::h92df2277ccc1ec96
                               at /home/joshua/.local/lib/cargo/git/checkouts/windows-rs-4d19330348d493fa/256538e/crates/gen/src/constant.rs:49:65
  21:     0x7f1ba45c6658 - windows_gen::type_definition::TypeDefinition::gen::hf233f3f33c3c1c9a
                               at /home/joshua/.local/lib/cargo/git/checkouts/windows-rs-4d19330348d493fa/256538e/crates/gen/src/type_definition.rs:85:34
  22:     0x7f1ba451ccfe - windows_gen::type_tree::TypeTree::gen::{{closure}}::h20e29892e9070001
                               at /home/joshua/.local/lib/cargo/git/checkouts/windows-rs-4d19330348d493fa/256538e/crates/gen/src/type_tree.rs:119:22

This is with an incremental build with a newly built compiler, so there's a small possibility it could be rust-lang/rust#76720. But I'm reporting it just in case it's a bug in the build script itself.

The command I ran was

cargo +stage1 rustc --profile=check -- -Z time-passes

where +stage1 is rust-lang/rust@5f74ab4.

@jyn514
Copy link
Author

jyn514 commented Jan 21, 2021

rm -r target did fix the issue (and now I get linker errors about the windows runtime instead).

@kennykerr
Copy link
Collaborator

The panic is interesting, but not very actionable unless there's a consistent repro. It doesn't look familiar.

You'll have a hard time linking on Linux due to the lack of libs, until we have support for DLL imports in Rust. We have someone looking into that.

@jyn514
Copy link
Author

jyn514 commented Jan 21, 2021

This is probably not worth looking into then. The bit about DLL imports looks very cool, I didn't realize that was possible :)

@jyn514 jyn514 closed this as completed Jan 21, 2021
@jyn514
Copy link
Author

jyn514 commented Jan 25, 2021

I found why this only panicked locally, that's a debug_assert! and the standard library is distributed in release mode: https://github.com/rust-lang/rust/blob/84864bfea9c00fb90a1fa6e3af1d8ad52ce8f9ec/library/core/src/slice/raw.rs#L90

@jyn514 jyn514 reopened this Jan 25, 2021
@jyn514 jyn514 changed the title build.rs panics on linux (sometimes) build.rs panics with debug assertions in the standard library enabled Jan 25, 2021
@bjorn3
Copy link

bjorn3 commented Jan 25, 2021

Blob::read_utf16 can create an unaligned &[u16]:

pub fn read_utf16(&self) -> String {
let bytes = self.reader.files[self.file_index as usize].bytes[self.offset..].as_ptr();
unsafe {
String::from_utf16(std::slice::from_raw_parts(
bytes as *const u16,
self.size / 2,
))
.unwrap()
}
}

This is UB.

@jyn514
Copy link
Author

jyn514 commented Jan 25, 2021

The issue is that self.offset may not be a multiple of 16, right?

@rylev
Copy link
Contributor

rylev commented Jan 25, 2021

Yes, and it can also be null.

@bjorn3
Copy link

bjorn3 commented Jan 25, 2021

If either files[...].bytes is not aligned to 2 bytes or self.offset is not a multiple of 2, the resulting &[u16] will be unaligned.

@kennykerr
Copy link
Collaborator

I'd have to check the ECMA spec, but if it is null then it probably just means it should be an empty string. I'm not sure whether it is guaranteed to be aligned, but we can always harden it to deal with that case.

Is there a reliable repro for this?

@jyn514
Copy link
Author

jyn514 commented Jan 25, 2021

@kennykerr you'd have to compile a custom rustc, by default these assertions are disabled. There are instructions at https://rustc-dev-guide.rust-lang.org/building/how-to-build-and-run.html, or @rylev already has a local compiler built.

@jyn514
Copy link
Author

jyn514 commented Jan 25, 2021

@kennykerr
Copy link
Collaborator

There is a bug in the source metadata where many constants are missing values. microsoft/win32metadata#88 This may be what is causing the null case.

I'm not sure what would be causing the unaligned case. The ECMA spec says that all data must be stored with proper alignment. It's possible this is not being preserved when the PE file (winmd) is copied into a Vec.

@kennykerr
Copy link
Collaborator

Field constants are the only place where UTF16 values are stored within the winmd file. For the most part, strings are always UTF8.

@kennykerr
Copy link
Collaborator

And thanks @jyn514 - Ryan said he'll take a look at fixing this.

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.

4 participants