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

initial pass at function pointers #45

Merged
merged 4 commits into from Jan 31, 2019
Merged

initial pass at function pointers #45

merged 4 commits into from Jan 31, 2019

Conversation

strega-nil
Copy link

No description provided.

However, null values are not supported by the Rust function pointer types --
just like references, the expectation is that you use `Option` to create
nullable pointers. `Option<fn(Args...) -> Ret>` will have the exact same ABI
as `fn(Args...) -> Ret`, but additionally allows null pointer values.
Copy link
Member

Choose a reason for hiding this comment

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

This should likely reference the enum chapter once one exists; I guess the rules here will be exactly the same as the ones for reference types?

Copy link
Author

Choose a reason for hiding this comment

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

Yep!

A function pointer is the address of a function,
and has function pointer type.
The pointer is implicit in the `fn` type,
and `fn` types are implicitly `'static`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what it means to be 'static. If "T is 'static" means T: 'static, then this is not strictly true. For example,

fn(&'a u32): 'static

does not hold. This is fallout from the "syntactic definition of outlives". Such a type is not entirely trivial to make though.

I think the point is that fn pointer types do not carry their own lifetime, so they are always assumed to point to a statically defined function with no state of its own, right? Maybe we can word that more directly.

@RalfJung
Copy link
Member

RalfJung commented Nov 17, 2018

On Zulip, @ubsan was asking about function pointer transmutes:

Can you transmute<fn(*const T), fn(*const ())>(x)(p)?

(I assume we have T: Sized here.)

I would say we can modularize the problem (but this goes deep into validity invariant territory):

  1. The actual cast is always okay, because the validity invariant for function pointers does not talk about the signature. Maybe it just says "non-NULL", maybe it also says "points to executable memory", but I think it should not take the signature into account.
  2. The call, then, is when things may become UB if the signatures are wrong. We probably need some definition of "ABI compatible" here: Which "mismatches" we allow between the caller and callee signature will depend not just on layout but also on ABI concerns. For example, CTFE/miri currently say that either the type must be exactly the same, or else both types must have Scalar ABI that may differ only in its valid_range. In your example, the second condition is satisfied. If the mismatch is not allowed, we have UB.
  3. The behavior of allowed argument mismatches, then, is the same as that of a transmute, of union-based type punning, and of type punning through changing pointer types: In your case, I would say your code is equivalent to x(mem::transmute<*const T, *const ()>(p)).

@hanna-kruppe
Copy link

The behavior of allowed argument mismatches, then, is the same as that of a transmute, of union-based type punning, and of type punning through changing pointer types: In your case, I would say your code is equivalent to x(mem::transmute<*const T, *const ()>(p)).

That doesn't work because type punning is just about in-memory representation (and notions that build on it, like validity) but here the call ABI matters as well. For example, one can easily transmute between integers and floats, but trying transmute<fn(f32) -> f32, fn(i32) -> i32>(f32::sin)(42) needs to be UB: on many targets the caller would pass the parameter in an integer register but the callee expects it in a floating point register.

@RalfJung
Copy link
Member

RalfJung commented Nov 17, 2018

here the call ABI matters as well.

That's why there is an ABI compat check first, declaring full UB unless the types are ABI-compatible "enough". The third point only comes to bear if the check in the second passes. (This is why I said "behavior of allowed mismatches", which I just added emphasis to.)

@hanna-kruppe
Copy link

Oops, don't know how I missed that 🤐

@RalfJung
Copy link
Member

@ubsan will you have time to do another pass over this, incorporating comments, until our next meeting in two weeks?

@strega-nil
Copy link
Author

@RalfJung yeah! I'll do it this weekend.

@nikomatsakis
Copy link
Contributor

Things remaining to be done before merging:

Therefore, this is _only_ a safety invariant,
not a validity invariant;
as long as one doesn't call a function pointer which points to freed memory,
it is not undefined behavior.
Copy link
Member

Choose a reason for hiding this comment

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

I'd just say something like: "Function pointers have no lifetime. They must point to an allocated block of code when being called."

The rest of this belongs to #72, doesn't it?

@RalfJung
Copy link
Member

Open an issue on the topic of "when can fn pointers be transmured"

Done: #81

@avadacatavra avadacatavra merged commit 4d188f1 into rust-lang:master Jan 31, 2019
```

```rust
pub struct Cons {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the lack of #[repr(C)] accidental or intentional?

Copy link
Contributor

@Lokathor Lokathor Jun 24, 2021

Choose a reason for hiding this comment

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

probably accidental, however you should probably open a new issue about this, because a years old PR is a little too out of the way to get much attention.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shall I stop using this algorithm (spot suspicious place in a git-based book -> go to repo -> go to that file -> blame -> comment) in general?

Github UI seems to provide nice way of attaching comments to specific lines in files, so I expected those comments would be delivered precisely to those who deal with that particular file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well as I said this PR is years old. The person who wrote it doesn't even really work on Rust any more.

We are happy to have people taking an interest in fixing anything amiss, it's just that a new issue is generally preferred over continued discussion on a merged or closed PR, particularly an older one. That way the question and/or comment will stay on the list of things to resolve rather than be potentially forgotten by accident. Many people working on Rust have so many watched issues and projects that individual notifications/emails can easily get lost among them all.

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.

None yet

9 participants