Skip to content

ManuallyDrop constructed with properly initialized data via MaybeUninit fails ASan #72154

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

Closed
rjsberry opened this issue May 12, 2020 · 6 comments · Fixed by #72178
Closed

ManuallyDrop constructed with properly initialized data via MaybeUninit fails ASan #72154

rjsberry opened this issue May 12, 2020 · 6 comments · Fixed by #72178
Labels
A-sanitizers Area: Sanitizers for correctness and code quality C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rjsberry
Copy link

rjsberry commented May 12, 2020

The minimum example I can get this down to is:

use std::mem::{self, ManuallyDrop, MaybeUninit};

fn main() {
    let x: [usize; 1] = unsafe {
        let mut x: [MaybeUninit<usize>; 1] = MaybeUninit::uninit().assume_init();
        x[0] = MaybeUninit::new(0_usize);
        mem::transmute(x)
    };
    let _ = ManuallyDrop::new(x);
}
ASan output

$ RUSTFLAGS="-Z sanitizer=address" cargo run --target x86_64-unknown-linux-gnu
=================================================================
==231==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fff9245bcc0 at pc 0x5590c6b16160 bp 0x7fff9245bbd0 sp 0x7fff9245b398
WRITE of size 8 at 0x7fff9245bcc0 thread T0
    #0 0x5590c6b1615f  (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x7415f)
    #1 0x5590c6b360a4  (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x940a4)
    #2 0x5590c6b36850  (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x94850)
    #3 0x5590c6b3e707  (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x9c707)
    #4 0x5590c6b367c4  (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x947c4)
    #5 0x5590c6b36519  (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x94519)
    #6 0x7f149d0c609a  (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #7 0x5590c6aac299  (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0xa299)

Address 0x7fff9245bcc0 is located in stack of thread T0 at offset 224 in frame
#0 0x5590c6b35bbf (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x93bbf)

This frame has 20 object(s):
[32, 40) ''
[64, 72) 'slot.i.i'
[96, 104) ''
[128, 136) '_3.i'
[160, 168) ''
[192, 200) 'self.i'
[224, 232) '_2.i' <== Memory access at offset 224 is inside this variable
[256, 264) ''
[288, 296) ''
[320, 328) 'value.i'
[352, 360) ''
[384, 392) '' (line 9)
[416, 424) '' (line 5)
[448, 456) '' (line 5)
[480, 488) '_8' (line 9)
[512, 520) '_7' (line 9)
[544, 552) '_6' (line 7)
[576, 584) '_3' (line 5)
[608, 616) 'x1' (line 5)
[640, 648) 'x' (line 4)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions are supported)
SUMMARY: AddressSanitizer: stack-use-after-scope (/code/target/x86_64-unknown-linux-gnu/debug/smoke+0x7415f)
Shadow bytes around the buggy address:
0x100072483740: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100072483750: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100072483760: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100072483770: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
0x100072483780: f8 f2 f2 f2 00 f2 f2 f2 f8 f2 f2 f2 00 f2 f2 f2
=>0x100072483790: f8 f2 f2 f2 00 f2 f2 f2[f8]f2 f2 f2 00 f2 f2 f2
0x1000724837a0: f8 f2 f2 f2 00 f2 f2 f2 00 f2 f2 f2 f8 f2 f2 f2
0x1000724837b0: f8 f2 f2 f2 f8 f2 f2 f2 f8 f2 f2 f2 f8 f2 f2 f2
0x1000724837c0: f8 f2 f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 f3 f3 f3
0x1000724837d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000724837e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==231==ABORTING

Without the ManuallyDrop it is fine. The same code with regular initialization (no MaybeUninit) does not cause ASan to fail.

The above was run with rustc 1.45.0-nightly (99cb9cc 2020-05-11).

@rjsberry rjsberry added the C-bug Category: This is a bug. label May 12, 2020
@jonas-schievink jonas-schievink added the A-sanitizers Area: Sanitizers for correctness and code quality label May 12, 2020
@tmiasko
Copy link
Contributor

tmiasko commented May 13, 2020

Minimized test case (in terms of generated LLVM IR):

pub struct Wrap {
    pub t: [usize; 1]
}

impl Wrap {
    #[inline(always)]
    pub fn new(t: [usize; 1]) -> Self {
        Wrap { t }
    }
}

#[inline(always)]
pub fn assume_init() -> [usize; 1] {
    [1234]
}

fn main() {
    let x: [usize; 1] = assume_init();
    Wrap::new(x);
}

Sanitizer enables emissions of lifetime markers to detect use after scope bugs. Alloca for array in assume_init will not have one, but the one in Wrap::new will. Both functions are inlined into main, but inliner is asked NOT to add additional lifetime markers. Inliner notices that array allocas have disjoint lifetimes and merges them together. Lifetime markers from the second function invalidate operations from the first one, introducing undefined behaviour.

@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 13, 2020
@jonas-schievink
Copy link
Contributor

Is this actual UB that may be miscompiled, or just something the sanitizers complain about?

@tmiasko
Copy link
Contributor

tmiasko commented May 13, 2020

If similar incorrect transformation were performed under different circumstances that could have quite bad consequences.

At the same the impact of this particular issue is rather limited, since it requires opt-level=0 so that inliner does not introduce lifetime markers, and an enabled sanitizer so that lifetime markers are emitted during codegen (otherwise this happens only for opt-level != 0).

@RalfJung
Copy link
Member

The original code is technically speaking UB for calling assume_init on an uninitialized usize. However, as far as I know it lowers to well-defined LLVM IR, so I do not think that is the source of the problem.

@rjsberry
Copy link
Author

rjsberry commented May 15, 2020

Hi @RalfJung, I appreciate this is slightly off topic but could you clarify the UB in the original code?

FWIU the assume_init on [MaybeUninit<usize>; 1] is safe as this doesn't require proper initialization as per the MaybeUninit docs. The first element is then properly initialized and the array is transmuted to the initialized type.

@RalfJung
Copy link
Member

Ah you are right, I misread. I didn't realize the usize got initialized by the time you did assume_init.

@bors bors closed this as completed in 0ec4b06 May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sanitizers Area: Sanitizers for correctness and code quality C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants