Skip to content

fn(u64) -> u64 can be cast as i64, resulting in gibberish #50796

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

Closed
jonathanstrong opened this issue May 16, 2018 · 9 comments
Closed

fn(u64) -> u64 can be cast as i64, resulting in gibberish #50796

jonathanstrong opened this issue May 16, 2018 · 9 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jonathanstrong
Copy link

jonathanstrong commented May 16, 2018

minimal example:

fn f(x: u64) -> u64 { x * 2 }

fn main() {
    let g = f as i64;
    println!("{}", g); // 94295280847184
}

playground

Ordinarily rust is very strict about casts so I was surprised by this behavior when, because of a typo, I was trying to cast a fn as an i64. It compiled fine and resulted in garbage numbers that were difficult to track down. I assume this is a bug and not intended behavior.

First encountered the problem on nightly 1.27 2018-05-13 but it is also present in stable 1.26, per the playground example.

@sfackler
Copy link
Member

The number is the address of the function.

@jonathanstrong
Copy link
Author

ah. well here is one voice urging to discontinue this shortcut as it is quite a footgun by rust standards.

@josephlr
Copy link
Contributor

You're running into one of the type casts that isn't a coercion.

These exist so pointers (and function pointers) can be converted into usize values (for similar use cases to uintptr_t in C/C++). The weird thing is that rust allows for casting from pointers to integer types that are not usize (like u8). It's hard to think of a reasonable use case for turning a pointer into a byte. See this example for what I'm talking about.

Regardless, changing this behavior is non backwards-compatible. So the documentation on casts/coercions might need to be updated (it's not in the book's second edition). I also guess restricting what as does could be done at an epoch change, but that's a rather significant change.

@ishitatsuyuki
Copy link
Contributor

ishitatsuyuki commented May 16, 2018

So, what we could do is to issue a warning for casts like this.

The most verbose way would be f as *const fn(u64) -> u64 as usize as i64, although I'm not sure what parts should be omitted from this cast.

EDIT: function names getting directly converted into raw pointers seems the confusing point here.

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 16, 2018
@oli-obk
Copy link
Contributor

oli-obk commented May 16, 2018

There's an open issue in clippy for this issue: rust-lang/rust-clippy#2588

@ollie27
Copy link
Member

ollie27 commented May 16, 2018

The most verbose way would be f as *const fn(u64) -> u64 as usize as i64

I hope you meant f as fn(u64) -> u64 as usize as i64. Unfortunately f as *const fn(u64) -> u64 does compile but it's yet another nonsense cast from fn(u64) -> u64 to *const fn(u64) -> u64 that we should really lint against.

@jonathanstrong
Copy link
Author

in the meantime, I saw a post a few weeks ago about custom lints. For those who have some familiarity with this, how difficult would it be (for a knowledgeable rust dev, just no experience writing plugins) to write a plugin that prohibits this specific cast? It never occurred to me this could happen and I'm concerned there could be other places where it did.

@oli-obk
Copy link
Contributor

oli-obk commented May 16, 2018

@jonathanstrong this particular lint should be fairly easy, as you essentially just want to prevent casting functions to anything but usize or function pointers.

I would definitely suggest to add it to clippy, because the bar is much lower for new lints. Adding new lints to rustc requires an RFC. (also clippy builds faster than rustc 😉 )

The clippy-issue referenced above has already some small hint as to where to start. If you have more questions just ask on the issue or in the #clippy irc channel.

@remram44
Copy link
Contributor

This got implemented in clippy (rust-clippy#2814) so it can probably be closed here.

@oli-obk oli-obk closed this as completed Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants