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

Assignments leave their place partially-destroyed if the destructor panics #30380

Closed
arielb1 opened this issue Dec 14, 2015 · 5 comments
Closed
Assignees
Labels
A-destructors Area: destructors (Drop, ..) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@arielb1
Copy link
Contributor

arielb1 commented Dec 14, 2015

STR

struct Bomb;
struct Observer<'a>(&'a mut (String, Bomb));

impl Drop for Bomb {
    fn drop(&mut self) {
        panic!("panicking destructors ftw!");
    }
}

impl<'a> Drop for Observer<'a> {
    fn drop(&mut self) {
        let _spray = "0wned".to_owned();
        println!("{}", &self.0 .0);
    }
}

fn foo(b: &mut Observer) {
    *b.0 = ("~".to_owned(), Bomb);
}

fn main() {
    let mut bomb = ("clear".to_owned(), Bomb);
    let mut observer = Observer(&mut bomb);
    foo(&mut observer);
}

Results

The assignment to *b.0 within foo calls the drop-glue for the value inside. The new tuple ("~", Bomb) is created, and then the drop glue for the old value of b is executed. It first frees the original string, and then attempts to calls Bomb's destructor. As the latter destructor panics, the function unwinds without storing a value in the place of the missing String, leaving a &mut reference that points to an invalid value, which can later be observed by a destructor or recover.

Fixes

The new value for the destination is available the whole time - the panic handler can just write it in.

@arielb1 arielb1 added I-wrong T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 14, 2015
@pnkfelix
Copy link
Member

cc me

@arielb1
Copy link
Contributor Author

arielb1 commented Dec 14, 2015

It must be noted that the assigned location is supposed to be borrowed mutably while the drop glue is called, so underhanded tricks used by the drop-glue to get a reference to that location (e.g. #30228, which led to the discovery of this issue) are just aliasing-UB.

@nikomatsakis
Copy link
Contributor

cc me.

@pnkfelix pnkfelix self-assigned this Dec 17, 2015
@pnkfelix
Copy link
Member

triage: P-medium

@pnkfelix pnkfelix added the P-medium Medium priority label Dec 17, 2015
@pnkfelix pnkfelix added A-destructors Area: destructors (Drop, ..) and removed I-nominated labels Jan 7, 2016
@arielb1
Copy link
Contributor Author

arielb1 commented May 12, 2016

This is causing problems to my non-zeroing-drop work.

@bstrie bstrie added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label May 25, 2016
arielb1 added a commit to arielb1/rust that referenced this issue May 27, 2016
this introduces a DropAndReplace terminator as a fix to rust-lang#30380. That terminator
is suppsoed to be translated by desugaring during drop elaboration, which is
not implemented in this commit, so this breaks `-Z orbit` temporarily.
arielb1 added a commit to arielb1/rust that referenced this issue May 27, 2016
bors added a commit that referenced this issue May 29, 2016
[MIR] non-zeroing drop

This enables non-zeroing drop through stack flags for MIR.

Fixes #30380.
Fixes #5016.
bors added a commit that referenced this issue May 30, 2016
[MIR] non-zeroing drop

This enables non-zeroing drop through stack flags for MIR.

Fixes #30380.
Fixes #5016.
arielb1 added a commit to arielb1/rust that referenced this issue Jun 3, 2016
this introduces a DropAndReplace terminator as a fix to rust-lang#30380. That terminator
is suppsoed to be translated by desugaring during drop elaboration, which is
not implemented in this commit, so this breaks `-Z orbit` temporarily.
arielb1 added a commit to arielb1/rust that referenced this issue Jun 3, 2016
bors added a commit that referenced this issue Jun 5, 2016
[MIR] non-zeroing drop

This enables non-zeroing drop through stack flags for MIR.

Fixes #30380.
Fixes #5016.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-destructors Area: destructors (Drop, ..) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants