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

Code written in fn_ptr_trait is optimized away #116937

Closed
cathaysia opened this issue Oct 19, 2023 · 7 comments
Closed

Code written in fn_ptr_trait is optimized away #116937

cathaysia opened this issue Oct 19, 2023 · 7 comments

Comments

@cathaysia
Copy link
Contributor

rustc version:

rustc 1.73.0-nightly (500647f 2023-07-27)

I'm currently writing ffi related code. Some of the codes use function pointers from the C language. There are a small number of function pointers that need to be judged to be null pointers, so I used fn_ptr_trait to get the function address. This works fine in debug mode. However, in release mode all relevant code is optimized away. How can I stop this behavior?

@cathaysia cathaysia added the C-bug Category: This is a bug. label Oct 19, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 19, 2023
@cathaysia
Copy link
Contributor Author

and I get this:
image

@Urgau
Copy link
Member

Urgau commented Oct 19, 2023

As the the incorrect_fn_null_checks lint¹ is saying function pointers cannot ever be null. It's UB for them to be null.

To make them nullable follows what the lint is saying and wrapped your fn pointer into an Option like this:

- extern "C" { fn get_fn() -> extern "C" fn() -> i8; }
+ extern "C" { fn get_fn() -> Option<extern "C" fn() -> i8>; }

¹ was renamed to useless_ptr_null_checks in a newer version

@rustbot labels -needs-triage -C-bug

@rustbot rustbot removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-bug Category: This is a bug. labels Oct 19, 2023
@asquared31415
Copy link
Contributor

And relatedly, the code that runs if they are null would only run if UB occurred, so the optimizer is deleting it all in the name of speed.

@cathaysia
Copy link
Contributor Author

I can understand what the Rust optimizer is doing, and I agree with it, after all, rs requires that the function pointer cannot be null.

In fact, the "Option" mentioned in lint is not available, because cbindgen cannot work with such a signature, it will generate an Option_xxx type in function signature, but Option does not exist in C.

But on another note, it's a common fact from C that function pointers may be null. rs Can special optimization be done to extern "C" fn to avoid invalidating the null pointer check, or adding another #[nullable] function signature should also be possible?

@cathaysia
Copy link
Contributor Author

Currently I have bypassed this problem with read_volatile. If rs is not going to "fix" this problem, it would be nice to add proper documentation to the addr() function to guide users to other methods.

@cathaysia
Copy link
Contributor Author

ref: #40913
It looks like I have some misunderstanding about Option. It failed the last time I used it, I need to retest it tomorrow.

@Urgau
Copy link
Member

Urgau commented Oct 19, 2023

You only have to express the Option<extern "C" fn(...) -> ... in the Rust signature as C allows function pointer to be nullable.

Currently I have bypassed this problem with read_volatile.

This is still UB. rustc/LLVM might not be able to optimize it away now but it certainly can and could do it in the future. Option in the signature is the only way to make it sound.

In fact, the "Option" mentioned in lint is not available, because cbindgen cannot work with such a signature, it will generate an Option_xxx type in function signature, but Option does not exist in C.

This seems to be a bug to me. That should be reported to the cbindgen project.

@nikic nikic closed this as not planned Won't fix, can't repro, duplicate, stale Oct 19, 2023
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

No branches or pull requests

5 participants