Fix transmute::<T, U> where T requires a bigger alignment than U #38670

dotdash · 2016-12-29T01:49:52Z

For transmute::<T, U> we simply pointercast the destination from a U
pointer to a T pointer, without providing any alignment information,
thus LLVM assumes that the destination is aligned to hold a value of
type T, which is not necessarily true. This can lead to LLVM emitting
machine instructions that assume said alignment, and thus cause aborts.

To fix this, we need to provide the actual alignment to store_operand()
and in turn to store() so they can set the proper alignment information
on the stores and LLVM can emit the proper machine instructions.

Fixes #32947

rust-highfive · 2016-12-29T01:50:04Z

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

For transmute::<T, U> we simply pointercast the destination from a U pointer to a T pointer, without providing any alignment information, thus LLVM assumes that the destination is aligned to hold a value of type T, which is not necessarily true. This can lead to LLVM emitting machine instructions that assume said alignment, and thus cause aborts. To fix this, we need to provide the actual alignment to store_operand() and in turn to store() so they can set the proper alignment information on the stores and LLVM can emit the proper machine instructions. Fixes rust-lang#32947

eddyb · 2016-12-31T16:00:33Z

src/librustc_trans/base.rs

-        let (data, extra) = load_fat_ptr(bcx, src, t);
-        store_fat_ptr(bcx, data, extra, dst, t);
-    } else {
-        store_ty(bcx, load_ty(bcx, src, t), dst, t);


Are these two other cases unreachable through MIR trans? Are they useless/problematic?

I might have overlooked a corner case in coerce_unsized_into, but I can't figure out how to actually trigger that code path. Using Rc seems not to go to memcpy_ty, and I somehow failed to make a custom type that allows unsized coercions. Will try next year

@eddyb in your r=me remark, you're saying that you want something like a comment right here saying "there used to be other cases handled here, see discussion at #38670 ?)

I don't think a comment is necessary (as opposed to the case where special cases would be added). The change makes sense if the other cases are not reachable due to how MIR trans would be structured, I just wanted to make sure we're certain that's the case.

There are two places where memcpy_ty is used. One is store_operand, which I think should never hit anything but the memcpy case here, because the other two are handled via Immediate and Pair operands. The other is coerce_unsized_into which may call this with arbitrary types, but I think that's a pretty uncommon operation to begin with, and keeping those two cases here means adding align parameters to at more places.

eddyb · 2016-12-31T16:02:07Z

r=me with an explanation on the simplification of memcpy_ty.

eddyb · 2017-01-04T12:06:35Z

@bors r+

bors · 2017-01-04T12:06:36Z

📌 Commit 71a11a0 has been approved by eddyb

bors · 2017-01-04T14:26:00Z

⌛ Testing commit 71a11a0 with merge d40d01b...

Fix transmute::<T, U> where T requires a bigger alignment than U For transmute::<T, U> we simply pointercast the destination from a U pointer to a T pointer, without providing any alignment information, thus LLVM assumes that the destination is aligned to hold a value of type T, which is not necessarily true. This can lead to LLVM emitting machine instructions that assume said alignment, and thus cause aborts. To fix this, we need to provide the actual alignment to store_operand() and in turn to store() so they can set the proper alignment information on the stores and LLVM can emit the proper machine instructions. Fixes #32947

bors · 2017-01-04T16:31:46Z

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing d40d01b to master...

Version 1.16.0 (2017-03-16) =========================== Language -------- * Lifetimes in statics and consts default to `'static`. [RFC 1623] * [The compiler's `dead_code` lint now accounts for type aliases][38051]. * [Uninhabitable enums (those without any variants) no longer permit wildcard match patterns][38069] * [Clean up semantics of `self` in an import list][38313] * [`Self` may appear in `impl` headers][38920] * [`Self` may appear in struct expressions][39282] Compiler -------- * [`rustc` now supports `--emit=metadata`, which causes rustc to emit a `.rmeta` file containing only crate metadata][38571]. This can be used by tools like the Rust Language Service to perform metadata-only builds. * [Levenshtein based typo suggestions now work in most places, while previously they worked only for fields and sometimes for local variables][38927]. Together with the overhaul of "no resolution"/"unexpected resolution" errors (#[38154]) they result in large and systematic improvement in resolution diagnostics. * [Fix `transmute::<T, U>` where `T` requires a bigger alignment than `U`][38670] * [rustc: use -Xlinker when specifying an rpath with ',' in it][38798] * [`rustc` no longer attempts to provide "consider using an explicit lifetime" suggestions][37057]. They were inaccurate. Stabilized APIs --------------- * [`VecDeque::truncate`] * [`VecDeque::resize`] * [`String::insert_str`] * [`Duration::checked_add`] * [`Duration::checked_sub`] * [`Duration::checked_div`] * [`Duration::checked_mul`] * [`str::replacen`] * [`str::repeat`] * [`SocketAddr::is_ipv4`] * [`SocketAddr::is_ipv6`] * [`IpAddr::is_ipv4`] * [`IpAddr::is_ipv6`] * [`Vec::dedup_by`] * [`Vec::dedup_by_key`] * [`Result::unwrap_or_default`] * [`<*const T>::wrapping_offset`] * [`<*mut T>::wrapping_offset`] * `CommandExt::creation_flags` * [`File::set_permissions`] * [`String::split_off`] Libraries --------- * [`[T]::binary_search` and `[T]::binary_search_by_key` now take their argument by `Borrow` parameter][37761] * [All public types in std implement `Debug`][38006] * [`IpAddr` implements `From<Ipv4Addr>` and `From<Ipv6Addr>`][38327] * [`Ipv6Addr` implements `From<[u16; 8]>`][38131] * [Ctrl-Z returns from `Stdin.read()` when reading from the console on Windows][38274] * [std: Fix partial writes in `LineWriter`][38062] * [std: Clamp max read/write sizes on Unix][38062] * [Use more specific panic message for `&str` slicing errors][38066] * [`TcpListener::set_only_v6` is deprecated][38304]. This functionality cannot be achieved in std currently. * [`writeln!`, like `println!`, now accepts a form with no string or formatting arguments, to just print a newline][38469] * [Implement `iter::Sum` and `iter::Product` for `Result`][38580] * [Reduce the size of static data in `std_unicode::tables`][38781] * [`char::EscapeDebug`, `EscapeDefault`, `EscapeUnicode`, `CaseMappingIter`, `ToLowercase`, `ToUppercase`, implement `Display`][38909] * [`Duration` implements `Sum`][38712] * [`String` implements `ToSocketAddrs`][39048] Cargo ----- * [The `cargo check` command does a type check of a project without building it][cargo/3296] * [crates.io will display CI badges from Travis and AppVeyor, if specified in Cargo.toml][cargo/3546] * [crates.io will display categories listed in Cargo.toml][cargo/3301] * [Compilation profiles accept integer values for `debug`, in addition to `true` and `false`. These are passed to `rustc` as the value to `-C debuginfo`][cargo/3534] * [Implement `cargo --version --verbose`][cargo/3604] * [All builds now output 'dep-info' build dependencies compatible with make and ninja][cargo/3557] * [Build all workspace members with `build --all`][cargo/3511] * [Document all workspace members with `doc --all`][cargo/3515] * [Path deps outside workspace are not members][cargo/3443] Misc ---- * [`rustdoc` has a `--sysroot` argument that, like `rustc`, specifies the path to the Rust implementation][38589] * [The `armv7-linux-androideabi` target no longer enables NEON extensions, per Google's ABI guide][38413] * [The stock standard library can be compiled for Redox OS][38401] * [Rust has initial SPARC support][38726]. Tier 3. No builds available. * [Rust has experimental support for Nvidia PTX][38559]. Tier 3. No builds available. * [Fix backtraces on i686-pc-windows-gnu by disabling FPO][39379] Compatibility Notes ------------------- * [Uninhabitable enums (those without any variants) no longer permit wildcard match patterns][38069] * In this release, references to uninhabited types can not be pattern-matched. This was accidentally allowed in 1.15. * [The compiler's `dead_code` lint now accounts for type aliases][38051]. * [Ctrl-Z returns from `Stdin.read()` when reading from the console on Windows][38274] * [Clean up semantics of `self` in an import list][38313] [37057]: rust-lang/rust#37057 [37761]: rust-lang/rust#37761 [38006]: rust-lang/rust#38006 [38051]: rust-lang/rust#38051 [38062]: rust-lang/rust#38062 [38062]: rust-lang/rust#38622 [38066]: rust-lang/rust#38066 [38069]: rust-lang/rust#38069 [38131]: rust-lang/rust#38131 [38154]: rust-lang/rust#38154 [38274]: rust-lang/rust#38274 [38304]: rust-lang/rust#38304 [38313]: rust-lang/rust#38313 [38314]: rust-lang/rust#38314 [38327]: rust-lang/rust#38327 [38401]: rust-lang/rust#38401 [38413]: rust-lang/rust#38413 [38469]: rust-lang/rust#38469 [38559]: rust-lang/rust#38559 [38571]: rust-lang/rust#38571 [38580]: rust-lang/rust#38580 [38589]: rust-lang/rust#38589 [38670]: rust-lang/rust#38670 [38712]: rust-lang/rust#38712 [38726]: rust-lang/rust#38726 [38781]: rust-lang/rust#38781 [38798]: rust-lang/rust#38798 [38909]: rust-lang/rust#38909 [38920]: rust-lang/rust#38920 [38927]: rust-lang/rust#38927 [39048]: rust-lang/rust#39048 [39282]: rust-lang/rust#39282 [39379]: rust-lang/rust#39379 [`<*const T>::wrapping_offset`]: https://doc.rust-lang.org/std/primitive.pointer.html#method.wrapping_offset [`<*mut T>::wrapping_offset`]: https://doc.rust-lang.org/std/primitive.pointer.html#method.wrapping_offset [`Duration::checked_add`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_add [`Duration::checked_div`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_div [`Duration::checked_mul`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_mul [`Duration::checked_sub`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_sub [`File::set_permissions`]: https://doc.rust-lang.org/std/fs/struct.File.html#method.set_permissions [`IpAddr::is_ipv4`]: https://doc.rust-lang.org/std/net/enum.IpAddr.html#method.is_ipv4 [`IpAddr::is_ipv6`]: https://doc.rust-lang.org/std/net/enum.IpAddr.html#method.is_ipv6 [`Result::unwrap_or_default`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or_default [`SocketAddr::is_ipv4`]: https://doc.rust-lang.org/std/net/enum.SocketAddr.html#method.is_ipv4 [`SocketAddr::is_ipv6`]: https://doc.rust-lang.org/std/net/enum.SocketAddr.html#method.is_ipv6 [`String::insert_str`]: https://doc.rust-lang.org/std/string/struct.String.html#method.insert_str [`String::split_off`]: https://doc.rust-lang.org/std/string/struct.String.html#method.split_off [`Vec::dedup_by_key`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.dedup_by_key [`Vec::dedup_by`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.dedup_by [`VecDeque::resize`]: https://doc.rust-lang.org/std/collections/vec_deque/struct.VecDeque.html#method.resize [`VecDeque::truncate`]: https://doc.rust-lang.org/std/collections/vec_deque/struct.VecDeque.html#method.truncate [`str::repeat`]: https://doc.rust-lang.org/std/primitive.str.html#method.repeat [`str::replacen`]: https://doc.rust-lang.org/std/primitive.str.html#method.replacen [cargo/3296]: rust-lang/cargo#3296 [cargo/3301]: rust-lang/cargo#3301 [cargo/3443]: rust-lang/cargo#3443 [cargo/3511]: rust-lang/cargo#3511 [cargo/3515]: rust-lang/cargo#3515 [cargo/3534]: rust-lang/cargo#3534 [cargo/3546]: rust-lang/cargo#3546 [cargo/3557]: rust-lang/cargo#3557 [cargo/3604]: rust-lang/cargo#3604 [RFC 1623]: https://github.com/rust-lang/rfcs/blob/master/text/1623-static.md

rust-highfive assigned pnkfelix Dec 29, 2016

dotdash force-pushed the transmute_align branch 2 times, most recently from a8b8d21 to 65b7c2b Compare December 30, 2016 16:37

dotdash force-pushed the transmute_align branch from 65b7c2b to 71a11a0 Compare December 31, 2016 15:32

eddyb reviewed Dec 31, 2016

View reviewed changes

bors mentioned this pull request Jan 4, 2017

Rustdoc: disambiguate Implementors when the type name is not unique #38414

Merged

bors merged commit 71a11a0 into rust-lang:master Jan 4, 2017

bors mentioned this pull request Jan 4, 2017

Additional cleanup to rustc_trans #38756

Merged

dotdash deleted the transmute_align branch January 31, 2018 11:34

jethrogb mentioned this pull request Aug 2, 2019

Initial constant-time implementation of Base64 for generic & AVX2 fortanix/b64-ct#1

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix transmute::<T, U> where T requires a bigger alignment than U #38670

Fix transmute::<T, U> where T requires a bigger alignment than U #38670

dotdash commented Dec 29, 2016 •

edited

Loading

rust-highfive commented Dec 29, 2016

eddyb Dec 31, 2016

dotdash Dec 31, 2016

pnkfelix Jan 3, 2017

eddyb Jan 3, 2017

dotdash Jan 4, 2017

eddyb commented Dec 31, 2016

eddyb commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

Fix transmute::<T, U> where T requires a bigger alignment than U #38670

Fix transmute::<T, U> where T requires a bigger alignment than U #38670

Conversation

dotdash commented Dec 29, 2016 • edited Loading

rust-highfive commented Dec 29, 2016

eddyb Dec 31, 2016

Choose a reason for hiding this comment

dotdash Dec 31, 2016

Choose a reason for hiding this comment

pnkfelix Jan 3, 2017

Choose a reason for hiding this comment

eddyb Jan 3, 2017

Choose a reason for hiding this comment

dotdash Jan 4, 2017

Choose a reason for hiding this comment

eddyb commented Dec 31, 2016

eddyb commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

dotdash commented Dec 29, 2016 •

edited

Loading