PhantomData behavior is very surprising w.r.t. lack of Drop implications

[SoniEx2]

I tried this code:

use core::cell::Cell;
use core::marker::PhantomData as PDOrBox;
//use std::boxed::Box as PDOrBox;
struct Foo<'a> {
  selfref: Cell<Option<&'a Foo<'a>>>,
}
impl<'a> Drop for Foo<'a> {
  fn drop(&mut self) {
  }
}
fn make_selfref<'a>(x: &'a PDOrBox<Foo<'a>>) {}
fn make_pdorbox<'a>() -> PDOrBox<Foo<'a>> {
    unimplemented!()
}
fn main() {
  let x = make_pdorbox();
  make_selfref(&x);
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=14dee41511095d6e940069db08cdc23f

I expected to see this happen: since a PhantomData claims to act like a T including for the purposes of borrowck/dropck then this should absolutely compile-fail. just as it does if you use a Box, or T directly!

Instead, this happened: it doesn't!

this is clearly unsound.

Meta

rustc --version --verbose:

playground

Backtrace

<backtrace>

[SkiFire13]

this is clearly unsound

It isn't clear to me though. I don't see how this could ever be exploited to create unsoundness.

It seems the error happens because Box<T> actually drops T, which counts as a use of the T, while PhantomData<T> doesn't, so it compiles.

[SoniEx2]

see also https://docs.rs/selfref/0.2.4/selfref/

(maybe we should just ship it with the PhantomData and let y'all deal with it instead. but we hear that counts as "releasing exploits into the wild" and some ppl are very litigious about that stuff and we don't really wanna deal with that just yet.)

it's also worth noting that it fails to compile if you use a T directly, and PhantomData is supposed to act like a T, is it not? so why does it fail to act like a T?

[SkiFire13]

PhantomData is supposed to act like a T

It's supposed to make a struct act as if it contains a T, it doesn't act itself as a T.

[SoniEx2]

why can't/shouldn't it act itself as a T when used directly?

[Monadic-Cat]

PhantomData is less like Box<T> and more like [T; 0].
Using your example, this compiles:

use core::cell::Cell;
//use core::marker::PhantomData as PDOrBox;
//use std::boxed::Box as PDOrBox;

type PDOrBox<T> = [T; 0];

struct Foo<'a> {
  selfref: Cell<Option<&'a Foo<'a>>>,
}
impl<'a> Drop for Foo<'a> {
  fn drop(&mut self) {
  }
}
fn make_selfref<'a>(x: &'a PDOrBox<Foo<'a>>) {}
fn make_pdorbox<'a>() -> PDOrBox<Foo<'a>> {
    unimplemented!()
}
fn main() {
  let x = make_pdorbox();
  make_selfref(&x);
}

I haven't stared at this long enough to decide whether this is correct behavior for PhantomData<T>, but I think it's a bit of a stretch to call it "unsound" without qualification- it's clearly not an "unsound" API in the sense of "we wrap something unsafe but still cause UB in some cases" which is usual in Rust.

[SoniEx2]

it's unsound as in "if you use this with unsafe, you'll have soundness issues". and the worst part is that there's no appropriate alternative that can be used on stable. there's no "PhantomDrop", no "PhantomBox", no anything like that.

[Lee-Janggun]

AFAIK, PhantomData are special markers to place in structs to aid rustc on doing some analysis. When it is used by itself, it should really do nothing as it is a zero sized type, so should be optimized away.

it's unsound as in "if you use this with unsafe, you'll have soundness issues". and the worst part is that there's no appropriate alternative that can be used on stable. there's no "PhantomDrop", no "PhantomBox", no anything like that.

So for this, I really don't get the point. For the first sentence, are you saying that using PhantomData in any unsafe code is UB? For the second, are you recommending new types that "act" as if they are Box (which I have no idea what it would mean)?

[SoniEx2]

a PhantomActuallyData<T: ?Sized> that acts like a T except for size. which should be just PhantomData but for some reason isn't.

acts like a T for borrowck, dropck, Drop impl, Copy/Clone, projections, etc etc.

[Lee-Janggun]

1. What would be the motivation for such type?
2. Can such type ever be sound? Since it has size 0, in actual compilation it will be optimized away, and in actual execution there will be nothing. For example if you had a PhantomActuallyData<Cell<u64>> and called get() on it to get the inner value, what would the value of this at runtime?

Anything related to such PhantomActuallyData will need to be make sure to be no-ops at runtime, and I don't know if that is possible or even needed.

Although this itself is slightly off-topic to the original question. Regarding it, I think you are trying to give PhantomData semantic requirements it was never meant to have, hence why you thought the above example was unsound.

[nbdd0121]

The difference is solely because PhantomData does not implement Drop.

[SoniEx2]

@Lee-Janggun the actual receiver is Cell<u64> not PhantomActuallyData<Cell<u64>> so you can't actually call get() on it unless it explicitly supported it. but the point is that it should work just like a Foo<T> for some Foo except this one happens to be zero-sized for any T (including T: !Sized).

@nbdd0121 it needs the #[may_dangle] like Box has, otherwise it would have the wrong semantics. (are we allowed to break the Copy^Drop rule for PhantomData? because that'd be awesome!)

[nbdd0121]

Anyway, PhantomData is not unsound, but surely it is not doing what the document suggests. You can't produce UB without unsafe code, it's just that your unsafe code depends on the property described in the document.

I think it'll be a breaking change to modify the behaviour of PhantomData does though, because if PhantomData<T> were truly behave like T, the need of a drop glue will make things containing PhantomData ineligible to NLL.

Probably we should take this an opportunity to stablise #[may_dangle] or some alternative instead.

[SkiFire13]

Note that PhantomData's documentation specifically talks about using it as a field, not by itself like in this issue.
Quoting from the documentation (emphasis mine):

Zero-sized type used to mark things that “act like” they own a T.

Adding a PhantomData<T> field to your type tells the compiler that your type acts as though it stores a value of type T, even though it doesn’t really. This information is used when computing certain safety properties.

Does this issue also affect PhantomData when used as a field?

[SoniEx2]

yes.