Fix CFI: f32 and f64 are encoded incorrectly for cross-language CFI #115151
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
r? @bjorn3 |
rcvalle
changed the title
compiler-errors
requested changes
Aug 24, 2023
compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs
Outdated
Show resolved
Hide resolved
| // Rust's f32 and f64 single (32-bit) and double (64-bit) precision floating-point types | ||
| // have IEEE-754 binary32 and binary64 floating-point layouts, respectively. | ||
| // | ||
| // (See https://rust-lang.github.io/unsafe-code-guidelines/layout/scalars.html#fixed-width-floating-point-types.) |
There was a problem hiding this comment.
Would it be better to link to something describing the f and d prefixes that the encoding expects for IEEE-754 floats?
There was a problem hiding this comment.
Humm... those are just the Itanium C++ encoding for these builtin types (see https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-builtin). Instead of referencing it, I think it makes more sense to explain why they can be directly used here (similarly to how it's explained in line 446 for ty::Bool). What do you think?
Fix rust-lang#115150 by encoding f32 and f64 correctly for cross-language CFI. I missed changing the encoding for f32 and f64 when I introduced the integer normalization option in rust-lang#105452 as integer normalization does not include floating point. `f32` and `f64` should be always encoded as `f` and `d` since they are both FFI safe when their representation are the same (i.e., IEEE 754) for both the Rust compiler and Clang.
rcvalle
force-pushed
the
rust-cfi-fix-115150
branch
from
d9f2b60
to
5d6e2d7
Compare
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
|
Thank you for your time, @compiler-errors! Much appreciated. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 25, 2023
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#114754 (Name what ln_gamma does) - rust-lang#115081 (Allow overwriting ExpnId for concurrent decoding) - rust-lang#115151 (Fix CFI: f32 and f64 are encoded incorrectly for cross-language CFI) - rust-lang#115169 (remove some unnecessary ignore-debug clauses) - rust-lang#115190 (Add comment to the push_trailing function) r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #115150 by encoding f32 and f64 correctly for cross-language CFI. I missed changing the encoding for f32 and f64 when I introduced the integer normalization option in #105452 as integer normalization does not include floating point.
f32andf64should be always encoded asfanddsince they are both FFI safe when their representation are the same (i.e., IEEE 754) for both the Rust compiler and Clang.