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.

Sign up for GitHub

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

Jump to bottom

codegen, miri: fix computing the offset of an unsized field in a packed struct #118540

Merged
merged 1 commit into from
Dec 4, 2023
Merged

codegen, miri: fix computing the offset of an unsized field in a packed struct #118540

merged 1 commit into from
Dec 4, 2023

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Dec 2, 2023

#[repr(packed)] strikes again.

Fixes #118537
Fixes rust-lang/miri#3200

@bjorn3 I assume cranelift needs the same fix.

@rustbot
Copy link
Collaborator

rustbot commented Dec 2, 2023

r? @TaKO8Ki

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 2, 2023

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

RalfJung
RalfJung commented Dec 2, 2023
View reviewed changes
compiler/rustc_codegen_ssa/src/mir/place.rs
@@ -154,14 +153,6 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
}
_ if field.is_sized() => return simple(),
ty::Slice(..) | ty::Str | ty::Foreign(..) => return simple(),
ty::Adt(def, _) => {
if def.repr().packed() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have tried to preserve this fast-path for some packed structs, but I don't think that's worth it.

@RalfJung RalfJung force-pushed the unsized-packed-offset branch from 75729bd to ef15a81 Compare December 3, 2023 07:26
@apiraino
Copy link
Contributor

apiraino commented Dec 4, 2023

r? compiler

@rustbot rustbot assigned b-naber and unassigned TaKO8Ki Dec 4, 2023
@TaKO8Ki
Copy link
Member

TaKO8Ki commented Dec 4, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 4, 2023

📌 Commit ef15a81 has been approved by TaKO8Ki

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 4, 2023

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 4, 2023
@TaKO8Ki TaKO8Ki mentioned this pull request Dec 4, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 4, 2023
@bors
Auto merge of rust-lang#118602 - TaKO8Ki:rollup-njcouns, r=TaKO8Ki
e281163 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#118495 (Restrict what symbols can be used in `#[diagnostic::on_unimplemented]` format strings)
 - rust-lang#118540 (codegen, miri: fix computing the offset of an unsized field in a packed struct)
 - rust-lang#118551 (more targeted errors when extern types end up in places they should not)
 - rust-lang#118573 (rustc: Harmonize `DefKind` and `DefPathData`)
 - rust-lang#118586 (Improve example in `slice::windows()` doc)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 87625db into rust-lang:master Dec 4, 2023
11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 4, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 4, 2023
@rust-timer
Unrolled build for rust-lang#118540
325ce11 
Rollup merge of rust-lang#118540 - RalfJung:unsized-packed-offset, r=TaKO8Ki

codegen, miri: fix computing the offset of an unsized field in a packed struct

`#[repr(packed)]`  strikes again.

Fixes rust-lang#118537
Fixes rust-lang/miri#3200

`@bjorn3` I assume cranelift needs the same fix.
@RalfJung RalfJung deleted the unsized-packed-offset branch December 4, 2023 18:09
This was referenced Dec 13, 2023
Merged
Closed
@RalfJung
Copy link
Member Author

@WaffleLapkin should this one also be marked as "relnotes"?

@WaffleLapkin
Copy link
Member

@RalfJung I think so

@WaffleLapkin WaffleLapkin added the relnotes Marks issues that should be documented in the release notes of the next release. label Dec 14, 2023
@RalfJung
Copy link
Member Author

For the relnotes this should probably be grouped with #118538. One fixes the logic for computing the offset of an unsized field in a packed struct, the other fixes the logic for computing the size and alignment of an unsized packed struct.

bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Dec 18, 2023
@bjorn3
fix computing the offset of an unsized field in a packed struct
973dd56 
cc rust-lang/rust#118540
Fixes #1435
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Feb 18, 2024
@he32
Add a package for rust version 1.76.0.
0e9a2a1 
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.76.0 (2024-02-08)
==========================

Language
--------
- [Document Rust ABI compatibility between various types]
  (rust-lang/rust#115476)
- [Also: guarantee that char and u32 are ABI-compatible]
  (rust-lang/rust#118032)
- [Warn against ambiguous wide pointer comparisons]
  (rust-lang/rust#117758)

Compiler
--------
- [Lint pinned `#[must_use]` pointers (in particular, `Box<T>`
  where `T` is `#[must_use]`) in `unused_must_use`.]
  (rust-lang/rust#118054)
- [Soundness fix: fix computing the offset of an unsized field in
  a packed struct]
  (rust-lang/rust#118540)
- [Soundness fix: fix dynamic size/align computation logic for
  packed types with dyn Trait tail]
  (rust-lang/rust#118538)
- [Add `$message_type` field to distinguish json diagnostic outputs]
  (rust-lang/rust#115691)
- [Enable Rust to use the EHCont security feature of Windows]
  (rust-lang/rust#118013)
- [Add tier 3 {x86_64,i686}-win7-windows-msvc targets]
  (rust-lang/rust#118150)
- [Add tier 3 aarch64-apple-watchos target]
  (rust-lang/rust#119074)
- [Add tier 3 arm64e-apple-ios & arm64e-apple-darwin targets]
  (rust-lang/rust#115526)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------
- [Add a column number to `dbg!()`]
  (rust-lang/rust#114962)
- [Add `std::hash::{DefaultHasher, RandomState}` exports]
  (rust-lang/rust#115694)
- [Fix rounding issue with exponents in fmt]
  (rust-lang/rust#116301)
- [Add T: ?Sized to `RwLockReadGuard` and `RwLockWriteGuard`'s Debug impls.]
  (rust-lang/rust#117138)
- [Windows: Allow `File::create` to work on hidden files]
  (rust-lang/rust#116438)

Stabilized APIs
---------------
- [`Arc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.unwrap_or_clone)
- [`Rc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.unwrap_or_clone)
- [`Result::inspect`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect)
- [`Result::inspect_err`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect_err)
- [`Option::inspect`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.inspect)
- [`type_name_of_val`]
  (https://doc.rust-lang.org/stable/std/any/fn.type_name_of_val.html)
- [`std::hash::{DefaultHasher, RandomState}`]
  (https://doc.rust-lang.org/stable/std/hash/index.html#structs)
  These were previously available only through `std::collections::hash_map`.
- [`ptr::{from_ref, from_mut}`]
  (https://doc.rust-lang.org/stable/std/ptr/fn.from_ref.html)
- [`ptr::addr_eq`](https://doc.rust-lang.org/stable/std/ptr/fn.addr_eq.html)

Cargo
-----

See [Cargo release notes]
(https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-176-2024-02-08).

Rustdoc
-------
- [Don't merge cfg and doc(cfg) attributes for re-exports]
  (rust-lang/rust#113091)
- [rustdoc: allow resizing the sidebar / hiding the top bar]
  (rust-lang/rust#115660)
- [rustdoc-search: add support for traits and associated types]
  (rust-lang/rust#116085)
- [rustdoc: Add highlighting for comments in items declaration]
  (rust-lang/rust#117869)

Compatibility Notes
-------------------
- [Add allow-by-default lint for unit bindings]
  (rust-lang/rust#112380)
  This is expected to be upgraded to a warning by default in a future Rust
  release. Some macros emit bindings with type `()` with user-provided spans,
  which means that this lint will warn for user code.
- [Remove x86_64-sun-solaris target.]
  (rust-lang/rust#118091)
- [Remove asmjs-unknown-emscripten target]
  (rust-lang/rust#117338)
- [Report errors in jobserver inherited through environment variables]
  (rust-lang/rust#113730)
  This [may warn](rust-lang/rust#120515)
  on benign problems too.
- [Update the minimum external LLVM to 16.]
  (rust-lang/rust#117947)
- [Improve `print_tts`](rust-lang/rust#114571)
  This change can break some naive manual parsing of token trees
  in proc macro code which expect a particular structure after
  `.to_string()`, rather than just arbitrary Rust code.
- [Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint]
  (rust-lang/rust#117984)
- [Vec's allocation behavior was changed when collecting some iterators]
  (rust-lang/rust#110353)
  Allocation behavior is currently not specified, nevertheless
  changes can be surprising.
  See [`impl FromIterator for Vec`]
  (https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#impl-FromIterator%3CT%3E-for-Vec%3CT%3E)
  for more details.
- [Properly reject `default` on free const items]
  (rust-lang/rust#117818)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@b-naber b-naber

Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.76.0
7 participants
@RalfJung @rustbot @apiraino @TaKO8Ki @bors @WaffleLapkin @b-naber