dyn Trait comparison should not include the vtable pointer

This was previously raised in #103763 (comment). Currently, comparing a pair of *const dyn Trait raw pointers for equality will check that both the address and vtable are equal.

This is problematic because the vtable of a type is not guaranteed to be unique for each type: different types can share the same vtable and the same type can have its vtable instantiated multiple times.

To illustrate this issue, consider the following program which prints true in release mode and false in debug mode because LLVM is merging the two identical vtables (playground).

struct A;
struct B;

trait T {}
impl T for A {}
impl T for B {}

fn main() {
	let ab = (A, B);
    let a = &ab.0 as *const dyn T;
    let b = &ab.1 as *const dyn T;
    println!("{}", a == b);
    println!("{}", a as *const u8 == b as *const u8);
}

It is also possible to achieve the reverse effect where two dyn Trait pointers to the same value (with the same base type) compare unequal because a separate vtable was instantiated with each pointer (e.g. due to codegen units or separate crates).

In conclusion, vtables are completely useless as a source of truth for pointer equality, so we should only consider the pointer address when comparing dyn Trait raw pointers. We currently document this as a footgun (#103567), but it would be better to eliminate this footgun entirely from the language.

This was already discussed in lang triage meeting on 2022-11-08 (notes: https://hackmd.io/@rust-lang-team/SyDavQuri) when Arc::ptr_eq was discussed.

In conclusion, vtables are completely useless as a source of truth for pointer equality

I agree with this in the narrow, since for equality it's not useful.

But for inequality, one can rely on it today, no? For example,

#[derive(Debug)]
#[repr(transparent)]
struct Foo(u8);

let f = Foo(1);
let p1: *const dyn Debug = &f;
let p2: *const dyn Debug = &f.0;

assert_ne!(p1, p2); // always passes today

So is it possible that people could be relying on this side of it already?

That's only true because u8 and Foo have different Debug impls. However it is perfectly valid for the compiler to merge 2 vtables if they are identical:

trait T {}
impl T for u8 {}
impl T for Foo {}

#[repr(transparent)]
struct Foo(u8);

let f = Foo(1);
let p1: *const dyn T = &f;
let p2: *const dyn T = &f.0;

assert_ne!(p1, p2); // FAILS IN RELEASE MODE, passes in debug mode

We could change that by making vtable not unnamed_addr.

True, but that doesn't solve the issue where multiple instances of the same vtable are emitted in different codegen units. I feel that overall this is a footgun in the language which people are already having to work around by explicitly casting to *mut u8.

Discussed in @rust-lang/lang today. Conclusions:

The proposal as stated (just compare data pointers) means that x == y being true no longer means "these values are semantically equivalent". We feel that would be a breaking change, in that there could be sound code that checks whether x == y that would be broken.
We agree that the existing semantics of x == y (basically: if true, it means equivalent, but it may fail "randomly") are not desirable.
We think the desired semantics are that the data pointer is the same and the vtables are for the same type, but we understand that making vtables unique may not be viable across dynamic linking boundaries and so forth.
Are there other ways forward here? Do vtables always contain type-id, for example?

x == y can return true even if the values are not semantically equivalent. See my example in #106447 (comment) where the values compare equal despite having different types.

So today x == y provides no information: it can have false positives if the vtables are merged, and false negatives if multiple instances of the same vtable are emitted.

Well, x == y does mean that they will behave equivalently. The type might be different, but the behaviour is since the code is the same.

@Amanieu

The type might be different, but the behaviour is since the code is the same.

I'm not sure that's true at the semantic level. The behavior is the same given the same bit patterns, but (if I understand correctly) hypothetically you could have two types with the same bit patterns that have differing meanings, and the same machine code would therefore have differing semantic meanings. Plus, we are now exposing implementation details of an impl at the level of language semantics.

For trait objects I can't come up with a realistic example other than what @Amanieu showed above, though.

I would add that merging of functions is not a purely hypothetical concern. For instance, I've seen it break backtraces where a function that calls abort() gets merged with a function in a completely unrelated file (another part of libc, or something written in another language) that does the same thing. So you get something like this:

Abort
  1: abort in libc.so
  2: some_random_function in some_random_lib.so
  3: function_that_never_calls_the_above in your_code
  4: something_else in your_code
  ...
  N: main

This can be super confusing, especially when combined with inlining, when lacking line numbers (or they're unreliable) in a backtrace, and when (3) is a large function! And while it doesn't threaten language semantics to break backtraces (they're always best effort), it's an example of a situation where reasoning only about a function's behavior is not sufficient to recover the semantic meaning.

So today x == y provides no information: it can have false positives if the vtables are merged, and false negatives if multiple instances of the same vtable are emitted.

Fair, but I think it is close to the sense that I specified. It seems like we could prevent this merging by including e.g. type-id in the vtable.

I think that x == y should be entirely deterministic (assuming both x and y point to live objects). This means that it should always produce a correct result, no matter what optimization level is used or how the compiler splits codegen units.

Practically speaking, this leaves 2 options:

Only compare the data pointer, not the vtable. This is what I am proposing here.
Include type-ids in the vtable and do type comparison by comparing the type-ids instead of the vtable pointer. This comes at a cost in vtable size though.

Either option is fine and would make dyn trait comparison have actually sane results.

@scottmcm

@scottmcm is opening a new issue to track the lint.

On a personal note, I would much prefer to fix the existing behavior by adding more info into the vtable.

Issue for adding a lint: #117717

Auto merge of rust-lang#117758 - Urgau:lint_pointer_trait_comparisons…

Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…

fix(opcode/Ifacmp): compare objects by data pointer only

Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…

Navigation Menu

Repository navigation

dyn Trait comparison should not include the vtable pointer #106447

28 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

dyn Trait comparison should not include the vtable pointer #106447

Description

Activity

nbdd0121 commented on Apr 6, 2023

scottmcm commented on Apr 18, 2023

Amanieu commented on Apr 21, 2023

nbdd0121 commented on Apr 21, 2023

Amanieu commented on Apr 21, 2023

nikomatsakis commented on May 2, 2023

Amanieu commented on May 3, 2023

nbdd0121 commented on May 3, 2023

tmandry commented on May 3, 2023

tmandry commented on May 3, 2023

nikomatsakis commented on May 9, 2023

Amanieu commented on May 11, 2023

28 remaining items

nikomatsakis commented on Nov 8, 2023

nikomatsakis commented on Nov 8, 2023

scottmcm commented on Nov 8, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions