Functions called by a may_dangle Drop impl

[dtolnay]

Roughly speaking, #[may_dangle] on a generic parameter in an unsafe Drop impl promises that the drop behavior does not interact with data of that type other than possibly by dropping it.

To what extent does this restrict the Drop impl from delegating to other functions that are not annotated with #[may_dangle]?

Canonical example:

#![feature(dropck_eyepatch)]

struct Struct<'a> {
    field: &'a u8,
}

unsafe impl<#[may_dangle] 'a> Drop for Struct<'a> {
    fn drop(&mut self) {
        // may_dangle promises that we do not do the following during the course of Drop:
        // println!("{}", *self.field);

        self.no_read_field(); // is this UB?
    }
}

impl<'a> Struct<'a> {
    fn no_read_field(&self) -> usize {
        // DO NOT: println!("{}", *self.field);
        std::mem::size_of::<Self>()
    }
}

fn main() {
    fn f<'a>(s: &'a mut Struct<'a>) -> Struct<'a> {
        Struct { field: s.field }
    }
    let mut s = Struct { field: &0 };
    f(&mut s); // borrowed value does not live long enough without may_dangle
}

Such code appears to be widespread among uses of #[may_dangle] in the standard library and ecosystem.

My concern is that if inside of no_read_field we have a reference/pointer that is understood to be dereferenceable by the compiler backend as insinuated by #77, in theory the backend would be free to insert a spurious read of *self.field into no_read_field if it so desires. During a Drop in which 'a is already dangling, that seems "bad".

Is it necessary for no_read_field to understand 'a to be may_dangle as well?—

unsafe impl<#[may_dangle] 'a> Struct<'a> {
    fn no_read_field(&self) -> usize {
        // DO NOT: println!("{}", *self.field);
        std::mem::size_of::<Self>()
    }
}

Real world instance from standard library code:

• unsafe impl<#[may_dangle] T> Drop for LinkedList<T>
• calls self.pop_front_node()
• which juggles some values of type Node<T>, directly containing a field of type T

If you imagine a spurious read of that T (suppose T=&u8) inside pop_front_node this is analogous to the example above.

I found vaguely related discussions at #84 and #268 but this seems to me a pretty different case that I haven't found discussed. The salient example in both of those other issues is along the lines of #268 (comment) where a value somewhere contains an invalid bit pattern for its type, whereas the current issue involving may_dangle is an issue even if that never happens.

[bjorn3]

The no read rule is because the field may contain references to already dropped values and as such the library invariants may be invalidated. Any language invariants are still upheld and as such reading the fields in not UB in itself.

[dtolnay]

Here is a simplified example to illustrate this actually getting miscompiled. @bjorn3 If the code is indeed not UB, I guess that makes this a compiler implementation bug?

#![feature(dropck_eyepatch)]

struct Struct<'a> {
    field: &'a usize,
}

unsafe impl<#[may_dangle] 'a> Drop for Struct<'a> {
    fn drop(&mut self) {
        no_read_field(self.field);  // UB?
    }
}

fn no_read_field(_field: &usize) {}

fn main() {
    let s;
    let field = Box::new(!0);
    s = Struct { field: &field };
    let _ = s;
}

In https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=69c5361d05d2be0574971ea0cbe7e4d6, the resulting LLVM IR for no_read_field is:

; playground::no_read_field
; Function Attrs: nonlazybind uwtable
define internal void @_ZN10playground13no_read_field17h935601555ec44661E(i64* align 8 dereferenceable(8) %_field) unnamed_addr #2 !dbg !807 {
start:
  %_field.dbg.spill = alloca i64*, align 8
  store i64* %_field, i64** %_field.dbg.spill, align 8
  call void @llvm.dbg.declare(metadata i64** %_field.dbg.spill, metadata !811, metadata !DIExpression()), !dbg !812
  ret void, !dbg !813
}

Notice i64* align 8 dereferenceable(8) %_field.

In the LLVM language reference, dereferenceable means:

"A pointer that is dereferenceable can be loaded from speculatively without a risk of trapping."

Yet _field is definitely not that, when called from Struct::drop. The Box allocation is already gone by that point.

[bjorn3]

I see what you mean. Currently it shouldn't be a problem when the reference is not directly passed but as part of a struct that is not a simple wrapper, but it isn't guaranteed I think.

[RalfJung]

@dtolnay great question! In principle, the answer is -- yes, #[may_dangle] needs to be propagated to all the functions called by drop. If this was done consistently and properly, the entire mechanism could even be made safe. The only reason it is unsafe currently is that this propagation is not done.

Regarding your concrete examples, the first one is fine since &self is a reference to a reference, and dereferencable is not (currently) applied recursively -- and indeed if I have my way wrt #77, it will not be applied recursively. The second example is indeed UB, but notice that the "bad" code is inside drop. Conceptually, 'a as a lifetime is not alive during drop, and so drop must make sure to not touch any reference with that lifetime -- and yet it does exactly that.

[dtolnay]

Thank you -- that clears it up for me! may_dangle is more subtle than I realized, specifically the difference between manipulating &'dangling T vs &&'dangling T.

I will start paying attention for this and filing issues. I filed gnzlbg/slice_deque#93 and vcombey/fallible_collections#18 to start off.