Detect UB due to mismatching declarations? #3581
Labels
A-interpreter
Area: affects the core interpreter
C-spec-question
Category: it is unclear what the intended behavior of Miri for this case is
I-misses-UB
Impact: makes Miri miss UB, i.e., a false negative (with default settings)
Comments
RalfJung
added
C-bug
|
Actually... probably this is a rustc bug, and the code shouldn't be UB in the first place. |
RalfJung
added
C-spec-question
|
Closing since this is a rustc bug, not UB. |
|
Seems like I misunderstood @nikic and maybe this has to be UB after all... 😢 |
RalfJung
added
the
I-misses-UB
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Sep 3, 2024
…try>
codegen: do not set attributes on foreign function imports
Currently we are setting all the usual attributes when translating an FFI import, like
```rust
extern {
pub fn myfun(x: NonZeroI32) -> &'static mut ();
}
```
into LLVM. However, if the same function is imported multiple times as different items in Rust, that all becomes a single LLVM declaration. The attributes set on any one of them may then apply to calls done via other imported items. For instance, the import above, *even if it is never called*, can introduce UB if there is another part of the program that also imports `myfun`, calls it via that import, and passes 0 or `myfun` returns NULL.
To fix that, this PR changes the function declarations we emit to all look like `declare `@fn()`.` The one exception are Rust's own allocator functions, which have a bunch of attributes that LLVM currently only honors in the declaration, not the call site -- though with llvm/llvm-project@1953629 we can maybe avoid that in the future.
The hope is that this fixes rust-lang#46188. The only cases I can still think of where we emit different declarations with the same name and one of them "wins" are:
- Rust's native allocation functions. Even then, the "wrong" declarations will just have no argument and return value so I don't think this should cause problems.
- Two `extern static` with the same name but different type, or an `extern static` with the same name as an imported function. Again I doubt the "type of the static" will lead LLVM to deduce things about the function or vice versa, so probably the worst that can happen is LLVM crashes. (And of course if this static actually ends up resolving to a function, that's UB unless the static has size 0. And conversely, if it resolves to a static instead of code then calling the function is UB.)
Fixes rust-lang/miri#3581 by making this not UB.
Cc `@nikic`
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Sep 4, 2024
…try>
codegen: do not set attributes on foreign function imports
Currently we are setting all the usual attributes when translating an FFI import, like
```rust
extern {
pub fn myfun(x: NonZeroI32) -> &'static mut ();
}
```
into LLVM. However, if the same function is imported multiple times as different items in Rust, that all becomes a single LLVM declaration. The attributes set on any one of them may then apply to calls done via other imported items. For instance, the import above, *even if it is never called*, can introduce UB if there is another part of the program that also imports `myfun`, calls it via that import, and passes 0 or `myfun` returns NULL.
To fix that, this PR changes the function declarations we emit to all look like `declare `@fn()`.` The one exception are Rust's own allocator functions, which have a bunch of attributes that LLVM currently only honors in the declaration, not the call site -- though with llvm/llvm-project@1953629 we can maybe avoid that in the future.
The hope is that this fixes rust-lang#46188. The only cases I can still think of where we emit different declarations with the same name and one of them "wins" are:
- Rust's native allocation functions. Even then, the "wrong" declarations will just have no argument and return value so I don't think this should cause problems.
- Two `extern static` with the same name but different type, or an `extern static` with the same name as an imported function. Again I doubt the "type of the static" will lead LLVM to deduce things about the function or vice versa, so probably the worst that can happen is LLVM crashes. (And of course if this static actually ends up resolving to a function, that's UB unless the static has size 0. And conversely, if it resolves to a static instead of code then calling the function is UB.)
Fixes rust-lang/miri#3581 by making this not UB.
Cc `@nikic`
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Sep 11, 2024
…try>
codegen: do not set attributes on foreign function imports
Currently we are setting all the usual attributes when translating an FFI import, like
```rust
extern {
pub fn myfun(x: NonZeroI32) -> &'static mut ();
}
```
into LLVM. However, if the same function is imported multiple times as different items in Rust, that all becomes a single LLVM declaration. The attributes set on any one of them may then apply to calls done via other imported items. For instance, the import above, *even if it is never called*, can introduce UB if there is another part of the program that also imports `myfun`, calls it via that import, and passes 0 or `myfun` returns NULL.
To fix that, this PR changes the function declarations we emit to all look like `declare `@fn()`.` The one exception are Rust's own allocator functions, which have a bunch of attributes that LLVM currently only honors in the declaration, not the call site -- though with llvm/llvm-project@1953629 we can maybe avoid that in the future.
The hope is that this fixes rust-lang#46188. The only cases I can still think of where we emit different declarations with the same name and one of them "wins" are:
- Rust's native allocation functions. Even then, the "wrong" declarations will just have no argument and return value so I don't think this should cause problems.
- Two `extern static` with the same name but different type, or an `extern static` with the same name as an imported function. Again I doubt the "type of the static" will lead LLVM to deduce things about the function or vice versa, so probably the worst that can happen is LLVM crashes. (And of course if this static actually ends up resolving to a function, that's UB unless the static has size 0. And conversely, if it resolves to a static instead of code then calling the function is UB.)
Fixes rust-lang/miri#3581 by making this not UB.
Cc `@nikic`
try-job: x86_64-msvc
try-job: i686-msvc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This code has UB that Miri does not detect. See rust-lang/rust#46188 for context.
It seems quite hard to detect this though... we have to somehow check all declarations of all no_mangle functions that ever get called, or something like that.
The text was updated successfully, but these errors were encountered: