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

repr(packed) calls drop with unaligned pointer #43306

Closed
RalfJung opened this issue Jul 18, 2017 · 4 comments
Closed

repr(packed) calls drop with unaligned pointer #43306

RalfJung opened this issue Jul 18, 2017 · 4 comments
Labels
C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

The following test fails:

struct NeedAlign(u64);

impl Drop for NeedAlign {
    fn drop(&mut self) {
        assert_eq!(self as *mut _ as usize % 8, 0);
    }
}


#[repr(packed)]
#[derive(Copy, Clone)]
struct Unalign<T>(T);

struct Breakit {
    x: u8,
    y: Unalign<NeedAlign>,
}

fn main() {
    println!("before");
    {
        let x = NeedAlign(0);
    }
    println!("middle");
    {
        let x = Breakit { x : 0, y : Unalign(NeedAlign(0)) };
    }
    println!("after");
}

This could lead to UB, because code for drop is generated under the assumption that the pointer is aligned.

Related to #27060, but not solved by making taking references to field of packed structs unsafe.
We furthermore have to forbid using types which implement Drop in a packed struct.

Open question:
How is this supposed to handle generic types, like in the example above? Error on declaration (T may implement Drop), or only error when Unalign is actually used with a type that implements Drop?
IIRC unions also have a no-drop restriction, so this should probably use the same rules.

@retep998 retep998 added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jul 18, 2017
@arielb1
Copy link
Contributor

arielb1 commented Jul 18, 2017

The union/destructor restriction is more like a lint than a strong restriction.

I think we might want a "finer" solution here, because people might have unaligned Box<T>/Rc<T>/etc., and we won't want them to be "use unsafe for instant UB".

@RalfJung
Copy link
Member Author

people might have unaligned Box/Rc/etc., and we won't want them to be "use unsafe for instant UB".

Well but these are already UB right now, aren't they? What is the alternative to forbidding them?

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 28, 2017
@RalfJung
Copy link
Member Author

This is fixed by #44884, isn't it?

@arielb1
Copy link
Contributor

arielb1 commented Jan 15, 2018

Sure. Fixed, with a test: src/test/run-pass/packed-struct-drop-aligned.rs

@arielb1 arielb1 closed this as completed Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants