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

Rust 1.78 Does Not Return c_void Pointer Correctly #125658

Closed
git-blame opened this issue May 28, 2024 · 8 comments
Closed

Rust 1.78 Does Not Return c_void Pointer Correctly #125658

git-blame opened this issue May 28, 2024 · 8 comments
Labels
C-gub Category: the reverse of a compiler bug is generally UB

Comments

@git-blame
Copy link

We need to use a C library/API that has an initialization routine that returns a new opaque pointer in a parameter (i.e. func(void** ctx)). To do so, we have an interface similar to this:

 extern {
    fn initialize(ctx: &*mut c_void) -> i32;
}
...
let context = ptr::null_mut();
let result = unsafe { initialize(&context) };
...
// pass context to another C function

This has always worked since at least version 1.64.0. Recently, we started seeing our Windows build failing. Debugging reveals that since 1.78.0, variable context is always 0 and does not receive the pointer value created by the C function.

Further debugging shows the following:

  • Fails on both linux and mingw version 1.78.0
  • Succeeds with debug compile, fails with --release
  • Succeeds if we add a statement that references the variable?! For example,
let result = unsafe { initialize(&context) };
debug!("ptr value is {:?}", context);

Version it worked on

It most recently worked on: 1.77

Version with regression

rustc --version --verbose:

rustc 1.78.0 (9b00956e5 2024-04-29)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: x86_64-unknown-linux-gnu
release: 1.78.0
LLVM version: 18.1.2
rustc 1.78.0 (9b00956e5 2024-04-29) (Rev1, Built by MSYS2 project)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: x86_64-pc-windows-gnu
release: 1.78.0
LLVM version: 18.1.4
@git-blame git-blame added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels May 28, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 28, 2024
@git-blame
Copy link
Author

Since passing an int* and returning a void* still works, our current fix is to write a wrapper similar to this:

 extern {
     fn initialize_wrapper(result: *mut i32) -> *mut c_void;
}
...
let mut result: i32 = 0;
let context = unsafe { initialize_wrapper(&mut result) };
// check result
...
// pass context to another C function

@ChrisDenton
Copy link
Member

extern {
    fn initialize(ctx: &*mut c_void) -> i32;
}

That looks like UB because your passing a shared reference rather than a mutable one. I.e. it should be defined as:

extern {
    fn initialize(ctx: &mut *mut c_void) -> i32;
}

@git-blame
Copy link
Author

It's just odd that it worked up to 1.77.0 and also with 1.78.0, there are still some weird conditions where it still works (see description).

@Urgau
Copy link
Member

Urgau commented May 28, 2024

I believe that what you're trying to do is UB, by taking an immutable reference and trying to assign to it you're violating the immutability requirements of that reference.

At least that's what I think is happening when I try to recreate your sample code in pure Rust, playground.

Using a mutable reference should already be a step in the right direction.

extern {
    fn initialize(ctx: &mut *mut c_void) -> i32;
}
// ...
let mut context = ptr::null_mut();
let result = unsafe { initialize(&mut context) };

@Urgau
Copy link
Member

Urgau commented May 28, 2024

It's just odd that it worked up to 1.77.0 and also with 1.78.0, there are still some weird conditions where it still works (see description).

Optimization around aliasing are (if I remember correctly) only done at higher level of optimization.

@lqd lqd removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels May 28, 2024
@git-blame
Copy link
Author

Thanks, using the &mut option works. But I'm surprised that the 1.78.0 compiler did not complain about passing immutable reference since it behaves differently than previous versions.

@Noratrieb
Copy link
Member

the compiler usually doesn't complain about doing undefined behavior because it doesn't know that you're doing undefined behavior.

@Noratrieb
Copy link
Member

here the compiler just assumed that you wouldn't modify it, and it has no way of knowing that you are modifying it. you just have to be careful. i don't think there's a tool that could have helped in this case, but in general, make sure to use tools like address sanitizer to check your unsafe code (and for unsafe code without FFI, Miri).

closing as not a bug

@Noratrieb Noratrieb closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2024
@saethlin saethlin added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 29, 2024
@workingjubilee workingjubilee added C-gub Category: the reverse of a compiler bug is generally UB and removed C-discussion Category: Discussion or questions that doesn't represent real issues. labels Jun 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-gub Category: the reverse of a compiler bug is generally UB
Projects
None yet
Development

No branches or pull requests

8 participants