Skip to content

Fix element ownership tracking with may_dangle - #594

Merged
alejandro-vaz merged 1 commit into
servo:v1from
astral-sh:charlie/codex-fix-may-dangle
Sep 12, 2026
Merged

Fix element ownership tracking with may_dangle#594
alejandro-vaz merged 1 commit into
servo:v1from
astral-sh:charlie/codex-fix-may-dangle

Conversation

@charliermarsh

Copy link
Copy Markdown
Contributor

With may_dangle enabled, we currently allow references read by an element's destructor to expire before the vector is dropped, causing a use-after-free.

Track ownership of A::Item with PhantomData so drop checking rejects this, including spilled vectors with zero inline capacity. Plain borrowed references without destructors retain the intended may_dangle behavior.

@alejandro-vaz

Copy link
Copy Markdown
Collaborator

this and #596 won't make it to v1.16.1, but we can make a second patch later

@charliermarsh

Copy link
Copy Markdown
Contributor Author

Cool, thanks for the heads up

@alejandro-vaz

Copy link
Copy Markdown
Collaborator

yeah I was looking at this the other day whilst doing some refactoring and it looked weird to me

does may_dangle even make sense there?? is it correct??

I have an intuition it is not, though I can't tell for sure

we are reading the references in order to run the destructors, so the reference must be valid and the element not dropped, which means that dropping the element before the smallvec drops leads to us reading a dangling reference (even if later we aren't double freeing)

I haven't personally used may_dangle until now so I'm somewhat confused

@alejandro-vaz alejandro-vaz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that phantomdata was exactly what @bolshoytoster removed on v2 if I remember correctly

@bolshoytoster

Copy link
Copy Markdown
Contributor

I did not know about this behaviour. v2 should be fine though, since TaggedLen still stores a PhantomData<T>.

@alejandro-vaz alejandro-vaz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I was looking at this the other day whilst doing some refactoring and it looked weird to me

does may_dangle even make sense there?? is it correct??

I have an intuition it is not, though I can't tell for sure

we are reading the references in order to run the destructors, so the reference must be valid and the element not dropped, which means that dropping the element before the smallvec drops leads to us reading a dangling reference (even if later we aren't double freeing)

I haven't personally used may_dangle until now so I'm somewhat confused

@alejandro-vaz alejandro-vaz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just researched how may_dangle actually works

it is correct now

thanks for contributing

@alejandro-vaz
alejandro-vaz merged commit cda4b73 into servo:v1 Sep 12, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants