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

RFC for fn ptr validity #197

Merged
merged 5 commits into from
Jul 7, 2022
Merged

RFC for fn ptr validity #197

merged 5 commits into from
Jul 7, 2022

Conversation

RalfJung
Copy link
Member

This is a draft RFC based on the consensus in #72. I hope to get some feedback from the WG before officially proposing it.

@Lokathor
Copy link
Contributor

I think this is the right link for the rendered form

@Lokathor
Copy link
Contributor

Very minor technicality: ARM uses 2 or 4 alignment depending on instruction set.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 25, 2019

Very minor technicality: ARM uses 2 or 4 alignment depending on instruction set.

That means 2 is the only alignment we can assume in general, right?

@hanna-kruppe
Copy link

There are ARM chips which don't implement any ISA with 2-byte alignment. If targeting only those, you could assume 4 byte alignment (and also wouldn't need to worry about the LSB being used to indicate Thumb mode), provided you find or define a target triple that is only compatible with those cores and not with any that implement Thumb or Thumb 2. That seems very shaky, tbh.

Copy link
Contributor

@gnzlbg gnzlbg left a comment

Choose a reason for hiding this comment

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

LGTM.

Co-Authored-By: Ixrec <[email protected]>
@RalfJung
Copy link
Member Author

@rkruppe are you saying we should explain that in the RFC?

@hanna-kruppe
Copy link

No, I don't think it's that important. To resolve @Lokathor's nit, it would be enough to talk about something like "most popular ARM targets" (cores in recent memory without Thumb or other form of 16 bit instructions are a rarity, Thumb is >20 years old and code density really sucks with only 32 bit instructions).

@Lokathor
Copy link
Contributor

Lokathor commented Aug 25, 2019

Yeah I was thinking something like "ARM targets have a code alignment of 2 or 4, depending on platform details".

I wanted the alignment to seem even less certain, to drive home that non-null is the only realistic guarantee we can give. (though I didn't articulate that well at first)

@RalfJung
Copy link
Member Author

"2 or 4" is just as certain as "2" though in terms of guarantees.

But sure, I can adjust the text for this.

@RalfJung RalfJung added the A-validity Topic: Related to validity invariants label Aug 28, 2019
@RalfJung
Copy link
Member Author

RalfJung commented Aug 28, 2019

Reading through this again, I am getting worried about how this relates to the open questions around provenance in LLVM semantics. Specifically, we might end up having to declare that it is UB to transmute data to fn() that has provenance, but does not form a complete pointer (e.g. when the first 4 bytes are from a pointer but the last 4 are not).

(For integers, we have the escape hatch of declaring that to result in poison, which is okay if we also permit uninitialized integers. But we cannot permit uninitialized function pointers.)

That would however mean that there are situations where transmuting a non-0 byte string to fn() is UB. The problem is not at all unique to fn(); e.g. bool has the same problem (if a byte is 0x01 but carries provenance, we might have to make it UB to transmute that to bool).

Unfortunately it is entirely unclear what LLVM even should do for such cases, let alone what it will do. That makes it very hard for us here to make any commitment.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 7, 2022

Almost 3 years later I think I am finally ready to commit to some answers for the provenance questions above. :) I think the presence of provenance should never cause UB on transmute. So indeed any (initialized) non-0 bit pattern is fine for fn.

Actually calling the function pointer might require provenance to be intact, though specifically for function pointers even that is in question. But anyway the RFC draft explicitly calls this out as out-of-scope.

I am not sure if that RFC draft is still of any use, since I don't know if we want to have such fine-grained RFCs. But reference/src/validity/function-pointers.md seems good to have, and having this old PR linger is certainly doing more harm than good. So, I'll merge it. :)

@RalfJung RalfJung merged commit e21202c into rust-lang:master Jul 7, 2022
@RalfJung RalfJung deleted the fn-ptr branch July 7, 2022 19:50
@RalfJung RalfJung mentioned this pull request Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-validity Topic: Related to validity invariants
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants