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

Objects can be dropped an arbitrary number of times in safe code #25549

Closed
Thiez opened this issue May 17, 2015 · 9 comments · Fixed by #25595
Closed

Objects can be dropped an arbitrary number of times in safe code #25549

Thiez opened this issue May 17, 2015 · 9 comments · Fixed by #25595
Labels
I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@Thiez
Copy link
Contributor

Thiez commented May 17, 2015

The following function can drop any object arbitrary number of times, using only safe code:

fn drop_n_times<T: Drop>(val: T, times: u32) {
    struct Holder<T: ?Sized>(T);
    let container = Holder(val);
    for _ in 0..times {
        &(*(&container as &Holder<Drop>)).0;
    }
}

struct Foo;
impl Drop for Foo {
    fn drop(&mut self) { println!("dropping"); }
}

fn main() {
    drop_n_times(Foo, 3);
}
dropping
dropping
dropping
dropping

Probably bad :)
Found while researching #25515

@Thiez Thiez changed the title Types can be dropped an arbitrary number of times in safe code Objects can be dropped an arbitrary number of times in safe code May 17, 2015
@sfackler sfackler added I-wrong I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. labels May 18, 2015
@sfackler
Copy link
Member

Nominating for P-high

@Manishearth
Copy link
Member

cc @pnkfelix

@Manishearth
Copy link
Member

Too bad this doesn't just cancel out the Leakocalypse 😛

@pnkfelix
Copy link
Member

The following function can drop any object arbitrary number of times, using only safe code:

(well, any object that implements Drop. But yes, I see the point being made here.)

@Thiez
Copy link
Contributor Author

Thiez commented May 18, 2015

@pnkfelix look, no Drop required. I guess it doesn't work for types that are already unsized.

fn drop_n_times<T>(val: T, times: u32) {
    struct Holder<T: ?Sized>(T);
    let container = Holder([val]);
    for _ in 0..times {
        &(*(&container as &Holder<[T]>)).0;
    }
}

@pnkfelix
Copy link
Member

to be clear, I am looking at this now as well. :)

@dotdash
Copy link
Contributor

dotdash commented May 18, 2015

I guess #22815 is related.

@pnkfelix
Copy link
Member

@dotdash I hope it is unrelated. In particular I wrote this on that ticket:

My hope is that the cases listed on this ticket (#22815) do not reflect cases where we have unsound behavior, but merely cases where we can in some cases produce lower quality code than we would otherwise.

@dotdash
Copy link
Contributor

dotdash commented May 18, 2015

@pnkfelix Yeah, it's unrelated. The type_needs_drop call happens for Drop, which does need drop.

dotdash added a commit to dotdash/rust that referenced this issue May 19, 2015
When taking the address of an unsized field we generate a rvalue datum
for the field and then convert it to an lvalue datum. At that point,
cleanup is scheduled for the field, leading to multiple drop calls.

The problem is that we generate an rvalue datum for the field, since the
pointer does not own the data and there's already cleanup scheduled
elsewhere by the true owner. Instead, an lvalue datum must be created.

Thanks to @eddyb for identifying the underlying cause and suggesting the
correct fix.

Fixes rust-lang#25549.
dotdash added a commit to dotdash/rust that referenced this issue May 19, 2015
When taking the address of an unsized field we generate a rvalue datum
for the field and then convert it to an lvalue datum. At that point,
cleanup is scheduled for the field, leading to multiple drop calls.

The problem is that we generate an rvalue datum for the field, since the
pointer does not own the data and there's already cleanup scheduled
elsewhere by the true owner. Instead, an lvalue datum must be created.

Thanks to @eddyb for identifying the underlying cause and suggesting the
correct fix.

Fixes rust-lang#25549
Fixes rust-lang#25515
bors added a commit that referenced this issue May 20, 2015
When taking the address of an unsized field we generate a rvalue datum
for the field and then convert it to an lvalue datum. At that point,
cleanup is scheduled for the field, leading to multiple drop calls.

The problem is that we generate an rvalue datum for the field, since the
pointer does not own the data and there's already cleanup scheduled
elsewhere by the true owner. Instead, an lvalue datum must be created.

Thanks to @eddyb for identifying the underlying cause and suggesting the
correct fix.

Fixes #25549.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants