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.

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

Toolchain upgrade to nightly-2024-05-29 failed #3218

Closed
github-actions bot opened this issue May 31, 2024 · 11 comments · Fixed by #3225
Closed

Toolchain upgrade to nightly-2024-05-29 failed #3218

github-actions bot opened this issue May 31, 2024 · 11 comments · Fixed by #3225

Comments

@github-actions
Copy link
Contributor

Updating Rust toolchain from nightly-2024-05-28 to nightly-2024-05-29 requires source changes.
The failed automated run can be found here.
Please review the changes at https://github.com/rust-lang/rust from rust-lang/rust@84b40fc up to rust-lang/rust@da159eb. The log for this commit range is:
rust-lang/rust@da159eb331 Auto merge of #125682 - weihanglo:update-cargo, r=weihanglo
rust-lang/rust@037577a95a Update cargo
rust-lang/rust@274499dd0f Auto merge of #125665 - matthiaskrgr:rollup-srkx0v1, r=matthiaskrgr
rust-lang/rust@faabc74625 Rollup merge of #125637 - nnethercote:rustfmt-fixes, r=GuillaumeGomez
rust-lang/rust@de2bf3687b Rollup merge of #125598 - compiler-errors:proof-tree-builder, r=lcnr
rust-lang/rust@86f6fae93d Rollup merge of #125590 - ChrisDenton:mingw-ci-3, r=Kobzol
rust-lang/rust@78b4cafa6e Rollup merge of #125573 - GuillaumeGomez:migrate-allow-warnings-cmdline-stability, r=jieyouxu
rust-lang/rust@713c852a2f Rollup merge of #117671 - kjetilkjeka:nvptx_c_abi_avoid_direct, r=davidtwco
rust-lang/rust@8c4db851a7 Auto merge of #122662 - Mark-Simulacrum:optional-drop, r=bjorn3
rust-lang/rust@f494036530 Make ProofTreeBuilder actually generic over interner
rust-lang/rust@50a5da16b8 EvalCtxt::tcx() -> EvalCtxt::interner()
rust-lang/rust@7717a306b2 Auto merge of #124650 - oli-obk:pattern_types_syntax, r=nnethercote
rust-lang/rust@ac7e836834 Bless codegen test
rust-lang/rust@eae5031ecb Cache whether a body has inline consts
rust-lang/rust@ddc5f9b6c1 Create const block DefIds in typeck instead of ast lowering
rust-lang/rust@e5cba17b84 Use the HIR instead of mir_keys for determining whether something will have a MIR body.
rust-lang/rust@53e3c3271f Make body-visiting logic reusable
rust-lang/rust@be94ca0bcd Remove a CTFE check that was only ever used to ICE
rust-lang/rust@f1b0ca08a4 Don't format tests/run-make/*/rmake.rs.
rust-lang/rust@4702a1c345 Fix comments.
rust-lang/rust@404d47ec20 Migrate run-make/allow-warnings-cmdline-stability to rmake.rs
rust-lang/rust@56733372b8 Add assert_not_contains to run-make-support library
rust-lang/rust@f989d2f625 Auto merge of #125649 - workingjubilee:rollup-zwoum3k, r=workingjubilee
rust-lang/rust@4aaf9f645e Rollup merge of #125647 - tspiteri:track-lazy_cell_consume, r=workingjubilee
rust-lang/rust@01aa2e8511 Rollup merge of #125640 - fmease:plz-no-stringify, r=estebank
rust-lang/rust@941bf8bee1 Rollup merge of #125551 - clarfonthey:ip-bits, r=jhpratt
rust-lang/rust@fb95fda87f Rollup merge of #125343 - lcnr:eagerly-normalize-added-goals, r=compiler-errors
rust-lang/rust@8e89f83cbb Rollup merge of #125089 - Urgau:non_local_def-suggestions, r=estebank
rust-lang/rust@402a649e75 update tracking issue for lazy_cell_consume
rust-lang/rust@27cdc0df4e Don't suggest turning non-char-literal exprs of ty char into string literals
rust-lang/rust@c0d600385b Auto merge of #125636 - workingjubilee:bump-backtrace-0.3.72, r=workingjubilee
rust-lang/rust@98bfd54b0a eagerly normalize when adding goals
rust-lang/rust@13ce229042 refactor analyse visitor to instantiate states in order
rust-lang/rust@4d5a9bcb86 change selection test to run-pass
rust-lang/rust@87599ddd86 add debug_assert to alias-relate
rust-lang/rust@3ec9d8db27 Sync libstd deps with backtrace
rust-lang/rust@00b759530e Bump backtrace to 0.3.72
rust-lang/rust@d86e122941 Auto merge of #125609 - diondokter:opt-size-char-count, r=thomcc
rust-lang/rust@bcfa67d50d Remove out-of-date comment.
rust-lang/rust@71213fd607 Auto merge of #125539 - matthiaskrgr:cräsh, r=jieyouxu
rust-lang/rust@c7d300442f non_local_defs: point the parent item when appropriate
rust-lang/rust@98273ec612 non_local_defs: point to Self and Trait to give more context
rust-lang/rust@b71952904d non_local_defs: suggest removing leading ref/ptr to make the impl local
rust-lang/rust@ab23fd8dea non_local_defs: improve main without a trait note
rust-lang/rust@d3dfe14b53 non_local_defs: be more precise about what needs to be moved
rust-lang/rust@402580bcd5 non_local_defs: improve exception note for impl and macro_rules!
rust-lang/rust@22095fbd8d non_local_defs: use labels to indicate what may need to be moved
rust-lang/rust@26b873d030 non_local_defs: use span of the impl def and not the impl block
rust-lang/rust@de1c122950 non_local_defs: improve some notes around trait, bounds, consts
rust-lang/rust@06c6a2d9d6 non_local_defs: switch to more friendly primary message
rust-lang/rust@5ad4ad7aee non_local_defs: move out from #[derive(LintDiagnostic)] to manual impl
rust-lang/rust@4c002fce9d Omit non-needs_drop drop_in_place in vtables
rust-lang/rust@bae945201f remove fixed crashes, add fixed crashes to tests, add new cashed found in the meantime
rust-lang/rust@e5d100363a crashes: increment the number of tracked ones
rust-lang/rust@05fa647dc7 Always use the general case char count
rust-lang/rust@f931290949 Update description of install-mingw
rust-lang/rust@19cfe8d9e6 Add "Setup Python" action to github-hosted runners
rust-lang/rust@f63931bc1a Cleanup custom mingw in CI
rust-lang/rust@0d42cf7afe Stabilise ip_bits feature
rust-lang/rust@ead02ba0f1 NVPTX: Avoid PassMode::Direct for args in C abi

@qinheping
Copy link
Contributor

The root cause of the failure is

rust-lang/rust@e5cba17b84 Use the HIR instead of mir_keys for determining whether something will have a MIR body.

Struct constructors have body in mir---has_body return true according to mir_keys---, but have no body according to HIR. So this commit will make the body of struct constructor instances None, and hence panic: https://github.com/model-checking/kani/blob/main/kani-compiler/src/kani_middle/transform/mod.rs#L78.

In #3219, I tried to ignore all struct constructors that have no body in the reachability module. But I don't think it is a good fix.

@celinval, can you confirm if the inconsistency of such has_body check is intended? If no, I will create an issue on Rust side.

@celinval
Copy link
Contributor

celinval commented Jun 3, 2024

@oli-obk can you please confirm?

@qinheping
Copy link
Contributor

Here is an example, the constructor of HidesAPointer has body in mir but not hir.

struct HidesAPointer(*mut u32);

fn hidden_pointer(h: HidesAPointer) {}

fn main() {
    let mut a = 0;
    hidden_pointer(HidesAPointer(&mut a))
}

@oli-obk
Copy link

oli-obk commented Jun 3, 2024

Hm yea, I think I screwed up the logic there. Altough I wonder how that's not an issue for rustc at all

@tautschnig
Copy link
Member

Hm yea, I think I screwed up the logic there. Altough I wonder how that's not an issue for rustc at all

is_mir_available doesn't seem to be used a lot, maybe the cases of constructors just never matter for those callers?

@oli-obk
Copy link

oli-obk commented Jun 4, 2024

yea, I gave it a look and it seems we already special case those callers everywhere.

Why do you need is_mir_available anyway? doesn't Kani just -Zalways-emit-mir?

tautschnig added a commit to tautschnig/kani that referenced this issue Jun 4, 2024
Changes required due to:
- rust-lang/rust@a34c26e7ec Make body_owned_by return the body directly.
- rust-lang/rust@333458c2cb Uplift TypeRelation and Relate
- rust-lang/rust@459ce3f6bb Add an intrinsic for `ptr::metadata`

Resolves: model-checking#3218
@celinval
Copy link
Contributor

celinval commented Jun 4, 2024

Kani invokes stable-mir's Instace::has_body(), which invokes is_mir_available the same way the rustc collector checks which instance has body. Even with -Zalways-emit-mir, the has_body() should return false for cases such as extern functions, intrinsics, shims, and other things.

@oli-obk is there anything special with struct constructors?

@tautschnig
Copy link
Member

@oli-obk Further to what @celinval said: we invoke Instance::body() where we know that such a MIR body will be available, but we now get spuriously get None returned after rust-lang/rust@e5cba17b84 for some (?) constructors. Given those constructors don't seem to have a body just yet in HIR, this is unsurprising, but it still seems wrong for a function called is_mir_available to compute its result off of HIR instead of MIR. We need those Body objects further down the line, so we can't special-case constructors. Why was this change deemed necessary (and right) in the first place, and can it please be reverted?

@oli-obk
Copy link

oli-obk commented Jun 5, 2024

Yea, let's revert it (and add a SMIR test). It was a step in an attempt to eliminate mir_keys entirely, as its existance is in direct conflict with query feeding of MIR queries

@celinval
Copy link
Contributor

celinval commented Jun 5, 2024

@oli-obk are you planning to revert it or do you need any help?

@tautschnig
Copy link
Member

For the record: @oli-obk took care of this in rust-lang/rust#126077, waiting for it to be merged.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 7, 2024
…=BoxyUwU

Revert "Use the HIR instead of mir_keys for determining whether something will have a MIR body."

This reverts commit e5cba17.

turns out SMIR still needs it (model-checking/kani#3218). I'll create a full plan and MCP for what I intended this to be a part of. Maybe my plan is nonsense anyway.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 7, 2024
Rollup merge of rust-lang#126077 - oli-obk:revert_is_mir_available, r=BoxyUwU

Revert "Use the HIR instead of mir_keys for determining whether something will have a MIR body."

This reverts commit e5cba17.

turns out SMIR still needs it (model-checking/kani#3218). I'll create a full plan and MCP for what I intended this to be a part of. Maybe my plan is nonsense anyway.
tautschnig added a commit that referenced this issue Jun 11, 2024
Changes required due to:
- rust-lang/rust@a34c26e7ec Make body_owned_by return the body directly.
- rust-lang/rust@333458c2cb Uplift TypeRelation and Relate
- rust-lang/rust@459ce3f6bb Add an intrinsic for `ptr::metadata`
- rust-lang/rust@7e08f80b34 Split smir `Const` into `TyConst` and
`MirConst`
- rust-lang/rust@eb584a23bf offset_of: allow (unstably) taking the
offset of slice tail fields
- rust-lang/rust@16e8803579 Update cargo

Resolves: #3218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants