Skip to content

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Dec 2, 2023

The alignment is also computed when accessing a field of extern type at non-zero offset, so we also panic in that case.

Previously size_of_val worked because the code path there assumed that "thin pointer" means "sized". But that's not true any more with extern types. The returned size and align are just blatantly wrong, so it seems better to panic than returning wrong results. We use a non-unwinding panic since code probably does not expect size_of_val to panic.

@rustbot
Copy link
Collaborator

rustbot commented Dec 2, 2023

r? @petrochenkov

(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

@RalfJung
Copy link
Member Author

RalfJung commented Dec 2, 2023

@bjorn3 If this lands, cranelift will need a similar patch.

@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the extern-type-size-of-val branch from 0e67c75 to d65e936 Compare December 2, 2023 13:31
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Dec 2, 2023

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the extern-type-size-of-val branch from 86b647b to c672796 Compare December 2, 2023 14:11
@petrochenkov
Copy link
Contributor

r? compiler
I'm overloaded with reviews.

@rustbot rustbot assigned WaffleLapkin and unassigned petrochenkov Dec 4, 2023
@RalfJung RalfJung force-pushed the extern-type-size-of-val branch 2 times, most recently from 08933d3 to 564fb4a Compare December 4, 2023 18:08
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 10, 2023
@RalfJung RalfJung force-pushed the extern-type-size-of-val branch 2 times, most recently from 0e4630e to 5f5e6b9 Compare December 11, 2023 12:40
@RalfJung
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 11, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 11, 2023

The Miri subtree was changed

cc @rust-lang/miri

Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

Thanks!

@WaffleLapkin
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 11, 2023

📌 Commit 3415957 has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2023
@RalfJung
Copy link
Member Author

Argh, missed one file that prints a backtrace...
@bors r=WaffleLapkin

@bors
Copy link
Collaborator

bors commented Dec 12, 2023

📌 Commit f813ccd has been approved by WaffleLapkin

It is now in the queue for this repository.

@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 12, 2023
@bors
Copy link
Collaborator

bors commented Dec 12, 2023

⌛ Testing commit f813ccd with merge 7c35f1b...

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2023
…WaffleLapkin codegen: panic when trying to compute size/align of extern type The alignment is also computed when accessing a field of extern type at non-zero offset, so we also panic in that case. Previously `size_of_val` worked because the code path there assumed that "thin pointer" means "sized". But that's not true any more with extern types. The returned size and align are just blatantly wrong, so it seems better to panic than returning wrong results. We use a non-unwinding panic since code probably does not expect size_of_val to panic.
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] askama_derive test:false 4.764 Compiling askama v0.12.0 Compiling rustdoc v0.0.0 (/checkout/src/librustdoc) [RUSTC-TIMING] askama test:false 0.390 ##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled. ##[group]Clock drift check local time: Wed Dec 13 00:02:01 UTC 2023 local time: Wed Dec 13 00:02:01 UTC 2023 Session terminated, killing shell... ...killed. ##[error]The operation was canceled. Cleaning up orphan processes 
@bors
Copy link
Collaborator

bors commented Dec 13, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 13, 2023
@RalfJung
Copy link
Member Author

@bors retry The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.

@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 13, 2023
@bors
Copy link
Collaborator

bors commented Dec 13, 2023

⌛ Testing commit f813ccd with merge 2fdd9ed...

@bors
Copy link
Collaborator

bors commented Dec 13, 2023

☀️ Test successful - checks-actions
Approved by: WaffleLapkin
Pushing 2fdd9ed to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 13, 2023
@bors bors merged commit 2fdd9ed into rust-lang:master Dec 13, 2023
@rustbot rustbot added this to the 1.76.0 milestone Dec 13, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2fdd9ed): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.6% [1.6%, 1.6%] 1
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.6% [1.6%, 1.6%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 672.121s -> 672.003s (-0.02%)
Artifact size: 312.40 MiB -> 312.43 MiB (0.01%)

@RalfJung RalfJung deleted the extern-type-size-of-val branch December 13, 2023 18:33
bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Dec 18, 2023
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Dec 19, 2023
…WaffleLapkin codegen: panic when trying to compute size/align of extern type The alignment is also computed when accessing a field of extern type at non-zero offset, so we also panic in that case. Previously `size_of_val` worked because the code path there assumed that "thin pointer" means "sized". But that's not true any more with extern types. The returned size and align are just blatantly wrong, so it seems better to panic than returning wrong results. We use a non-unwinding panic since code probably does not expect size_of_val to panic.
egkoppel added a commit to popcorn-2/popcorn-2 that referenced this pull request Feb 11, 2024
egkoppel added a commit to popcorn-2/popcorn-2 that referenced this pull request Feb 11, 2024
celinval added a commit to celinval/rust-dev that referenced this pull request Jun 4, 2024
Update Rust toolchain from nightly-2023-12-13 to nightly-2023-12-14 without any other source changes. This is an automatically generated pull request. If any of the CI checks fail, manual intervention is required. In such a case, review the changes at https://github.com/rust-lang/rust from rust-lang@3340d49 up to rust-lang@eeff92a. The log for this commit range is: rust-lang@eeff92ad32 Auto merge of rust-lang#118402 - notriddle:notriddle/ranking-and-filtering, r=GuillaumeGomez rust-lang@a90372c6e8 Auto merge of rust-lang#118213 - Urgau:check-cfg-diagnostics-rustc-cargo, r=petrochenkov rust-lang@2862500152 Auto merge of rust-lang#118919 - matthiaskrgr:rollup-02udckl, r=matthiaskrgr rust-lang@bec6672984 rustdoc-search: clean up handleSingleArg type handling rust-lang@9dfcf131b3 rustdoc-search: better hashing, faster unification rust-lang@9a9695a052 rustdoc-search: use set ops for ranking and filtering rust-lang@fd1d256d61 rustdoc-search: remove the now-redundant `validateResult` rust-lang@251d1af0d2 Rollup merge of rust-lang#118906 - Kobzol:bootstrap-is-windows, r=petrochenkov rust-lang@666353e7ba Rollup merge of rust-lang#118883 - HosseinAssaran:patch-1, r=fmease rust-lang@1dd36119d0 Rollup merge of rust-lang#118871 - tmiasko:coroutine-maybe-uninit-fields, r=compiler-errors rust-lang@dbc6ec6636 Rollup merge of rust-lang#118759 - compiler-errors:bare-unit-structs, r=petrochenkov rust-lang@f6617d050d Remove dangling check-cfg ui tests files rust-lang@5345a166fe Add more suggestion to unexpected cfg names and values rust-lang@7176b8babd Auto merge of rust-lang#118894 - dtolnay:bootstrapwrite, r=onur-ozkan rust-lang@c3def263a4 Auto merge of rust-lang#118870 - Enselic:rustc_passes-query-stability, r=compiler-errors rust-lang@56d25ba5ea Auto merge of rust-lang#118500 - ZetaNumbers:tcx_hir_refactor, r=petrochenkov rust-lang@2fdd9eda0c Auto merge of rust-lang#118534 - RalfJung:extern-type-size-of-val, r=WaffleLapkin rust-lang@066e6ffa02 Fix LLD thread flag selection for Windows targets rust-lang@c5208518fa Add `TargetSelection::is_windows` method rust-lang@f651b436ce Auto merge of rust-lang#117050 - c410-f3r:here-we-go-again, r=petrochenkov rust-lang@9f1bfe53b6 Auto merge of rust-lang#118900 - workingjubilee:rollup-wkv9hq1, r=workingjubilee rust-lang@f9078a40ee Rollup merge of rust-lang#118891 - compiler-errors:async-gen-blocks, r=eholk rust-lang@4583a0134f Rollup merge of rust-lang#118889 - matthiaskrgr:compl_2023_2, r=WaffleLapkin rust-lang@df0686b629 Rollup merge of rust-lang#118887 - smoelius:patch-1, r=Nilstrieb rust-lang@2f937c720d Rollup merge of rust-lang#118886 - GuillaumeGomez:clean-up-search-vars, r=notriddle rust-lang@5308733112 Rollup merge of rust-lang#118885 - matthiaskrgr:compl_2023, r=compiler-errors rust-lang@89d4a9bee9 Rollup merge of rust-lang#118884 - matthiaskrgr:auszweimacheins, r=Nadrieril rust-lang@18e0966f39 Rollup merge of rust-lang#118873 - lukas-code:fix_waker_getter_tracking_issue_number, r=workingjubilee rust-lang@0430782d1d Rollup merge of rust-lang#118872 - GuillaumeGomez:codeblock-attr-lint, r=notriddle rust-lang@a33f1a3d3a Rollup merge of rust-lang#118864 - farnoy:masked-load-store-fixes, r=workingjubilee rust-lang@2d1d443d7f Rollup merge of rust-lang#118858 - mu001999:dead_code/clean, r=cuviper rust-lang@77d1699756 Auto merge of rust-lang#116438 - ChrisDenton:truncate, r=thomcc rust-lang@b30e94b7bb Unbreak non-unix non-windows bootstrap rust-lang@1d78ce681e Actually parse async gen blocks correctly rust-lang@2a1acc26a0 Update compiler/rustc_pattern_analysis/src/constructor.rs rust-lang@3795cc8eb0 more clippy::complexity fixes rust-lang@046f2dea33 Typo rust-lang@58327c10c5 Add a test for a codeblock with multiple invalid attributes rust-lang@f1342f30a5 Clean up variables in `search.js` rust-lang@d707461a1a clippy::complexity fixes rust-lang@6892fcd690 simplify merging of two vecs rust-lang@a2ffff0708 Change a typo mistake in the-doc-attribute.md rust-lang@f813ccd784 also add a Miri test rust-lang@edcb7aba6b also test projecting to some sized fields at non-zero offset in structs with an extern type tail rust-lang@a47416beb5 test that both size_of_val and align_of_val panic rust-lang@bb0fd665a8 Follow guidelines for lint suggestions rust-lang@98aa20b0a7 Add test for `rustX` codeblock attribute rust-lang@d3cb25f4cf Add `rustX` check to codeblock attributes lint rust-lang@24f009c5e5 Move some methods from `tcx.hir()` to `tcx` rust-lang@04f3adb4a7 fix `waker_getters` tracking issue number rust-lang@e9b16cc2c5 rustc_passes: Enforce `rustc::potential_query_instability` lint rust-lang@95b5a80f47 Fix alignment passed down to LLVM for simd_masked_load rust-lang@fb32eb3529 Clean up CodeBlocks::next code rust-lang@df227f78c6 make it more clear what comments refer to; avoid dangling unaligned references rust-lang@b9c9b3e7a2 remove a cranelift test that doesn't make sense any more rust-lang@9ef1e35166 reject projecting to fields whose offset we cannot compute rust-lang@b1613ebc43 codegen: panic when trying to compute size/align of extern type rust-lang@6c0dbb8cc6 Remove dead codes in core rust-lang@a48cebc4b8 Coroutine variant fields can be uninitialized rust-lang@d473bdfdc3 Support bare unit structs in destructuring assignments rust-lang@0278505691 Attempt to try to resolve blocking concerns rust-lang@c6f7aa0eea Make File::create work on Windows hidden files Co-authored-by: celinval <celinval@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. 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.

8 participants