AddressSanitizer: false positives with structs/tuples that end with Zero Sized Types #39882
Comments
|
|
|
Very likely. Here's a more direct repro: fn main () {
let mut x = (4, ());
unsafe {
std::ptr::write(&mut x, (3, ()));
}
} |
|
That'd do it! Should we report this upstream? |
|
cc @japaric |
|
@dimbleby Thanks for the reduced test case.
This sounded nonsensical to me so I went to look at the generated assembly: (Sorry, this is ARM assembly as that's what I'm most familiar with)
``` rust
#[no_mangle]
pub fn _start() -> ! {
let mut x = (24, ());
unsafe {
ptr::write(&mut x, (42, ()));
}
} and compared it with a
``` rust
#[no_mangle]
pub fn _start() -> ! {
let mut x = 24;
unsafe {
ptr::write(&mut x, 42);
}
} but apart from some extra stack usage there's no semantic difference and certainly neither of these programs access uninitialized memory. Next, I looked at the LLVM IR and there I found something interesting: fn main() {
let mut x = (24, ());
unsafe {
ptr::write(&mut x, (42, ()));
}
}This is the ASan error message for that program: Looking at the two, I have concluded that this chunk of IR is what triggers the false positive in ASan: This is effectively getting a pointer to the I intent to report this upstream but, first, I'd like to know why rustc is emitting those two lines in the LLVM IR (cc @eddyb) so I can include that in the report. Is it to catch misuses of zero sized types? As in fn main() {
let x = ();
let y = unsafe { *(&x as *const () as *const i32) };
// println!("{}", y); // optional
}In general, it seems that ASan & MSan interact poorly with zero sized types: #include <stdio.h>
typedef struct {} Nil;
int main() {
Nil nil = {};
int x = *((int *)&nil);
printf("%p\n", (void *)&nil);
printf("%p\n", (void *)&x);
printf("%d\n", x);
} |
|
So those seem like bugs in the sanitizers, really. OTOH, we could avoid generating loads and stores for ZSTs, I suppose we're just not checking often enough. EDIT: To be a bit more clear, that's just generic code not being special-cased all over trans. |
japaric
changed the title
|
Updated the issue description to better reflect the problem and the possible fix and also documented a workaround. |
|
cc @kcc Is it an ASan bug that a load to a zero sized type at the end of a buffer is caught by the sanitizer? Or should we just not generate loads for such things? |
|
It seems like both of those things are true. |
|
Fair. |
Fixes rust-fuzz#25 Does not compile currently as it exits with > ERROR: AddressSanitizer: heap-buffer-overflow […] > ACCESS of size 0 cf. rust-lang/rust#39882
shepmaster
changed the title
|
I believe this should have a high priority since ASan is completely useless if such code is generated for std. I wasn't able to determine how it generates store instructions, but this should be simple for devs who're familiar with LLVM. |
|
@eddyb yes, reproduced locally 2017-05-03 build. |
|
Ah, I see, the |
|
A note on implementing: apart from alloca, copy_intrinsic should also be fixed. There should also be a test similar to this, on read, read_unaligned, write and write_unaligned. |
|
@istankovic That test could house |
|
@eddyb wrong name :P Yeah, we can put all the things to that test, but I wanted to separate because that file is labeled with MIR. |
|
Ah, I see what's happening, the test was added when MIR trans was opt-in, whereas this is a long-standing problem with the rest of trans that got carried over. |
|
I just hit the same bug in extern crate serde;
#[macro_use]
extern crate serde_json;
fn main() {
let val = json!({
"Hello": "world"
});
println!("{}", serde_json::to_string(&val).unwrap());
}To clarify: the program aborts in |
|
Via play.integer32.com I found that That specific |
|
@eddyb Thanks! I will try it when it's merged. |
rustc_trans: do not store pair fields if they are ZSTs. Should help with #39882 even if it's not a complete fix AFAICT.
|
(cc #39699 tracking issue for sanitizers) |
|
Sorry, I forgot about this. That PR helped me. Thank you! |
Mark-Simulacrum
added
the
A-sanitizers
53: Add fuzz target for html5ever r=frewsxcv Fixes #25 Does not compile currently as it exits with > ERROR: AddressSanitizer: heap-buffer-overflow […] > ACCESS of size 0 cf. rust-lang/rust#39882
|
As of 1.21.0-nightly (f774bce 2017-08-12), x86_64-unknown-linux-gnu:
Could anyone produce an updated test case, or declare that this has actually been fixed? |
|
@kennytm I think this is OK to close. My test case: Which is good on my current nightly, but I haven't compared the assembly/MIR output because:
|
|
Just a data point: It's fixed in my case. |
|
Triage: close? @Mark-Simulacrum |
Update
STR
Meta
Cause
We produce loads of the zero sized types in LLVM-IR.
Workaround
Append a non zero sized type to your struct so the ZST is not the last field of the struct. You could do this only if a feature is enabled to avoid changing the API of your crate.
Then test with:
Fix
Remove those loads by specializing the treatment of zero sized types in trans. As per @eddyb comment.
Original report
program:
rustc version: 1.17.0-nightly (62eb605 2017-02-15)
build:
result:
Of course it's possible that this is a false positive, though my experience in C and C++ has been that it's very rare indeed for the address sanitiser to produce such.
The text was updated successfully, but these errors were encountered: