Pass FnAbi to find_mir_or_eval_fn #133103

tiif · 2024-11-16T11:46:07Z

rust-lang/miri#4013 needs information from FnAbi, hence it is passed to find_mir_or_eval_fn.

r? @RalfJung

rustbot · 2024-11-16T11:46:17Z

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

compiler/rustc_const_eval/src/interpret/machine.rs

RalfJung · 2024-11-24T09:04:40Z

@rustbot author

tiif · 2024-11-27T18:34:42Z

@RalfJung There are some refactorings needed to remove ExternAbi from find_mir_or_eval_fn. Specifically, all functions that call this.check_shim will need to be updated. Could I confirm if this direction is what we are going for?

RalfJung · 2024-11-27T20:47:48Z

Specifically, all functions that call this.check_shim will need to be updated.

Yeah, they should pass fn_abi to check_shim, and all the ExternAbi::C { unwind: false } become abi::Conv::C.

src/tools/miri/src/shims/x86/mod.rs

src/tools/miri/src/shims/tls.rs

src/tools/miri/src/shims/windows/foreign_items.rs

bors · 2024-12-09T06:18:11Z

☔ The latest upstream changes (presumably #134052) made this pull request unmergeable. Please resolve the merge conflicts.

tiif · 2024-12-10T16:08:12Z

Interesting... for the same test, running on Linux will pass, but running on Windows will fail because of incompatible ABI.

For future reference:

./x.py test miri --test-args tests/pass/align.rs --target x86_64-pc-windows-gnu failed with calling a function with ABI X86Stdcall using caller ABI C
./x.py test miri --test-args tests/pass/align.rs --target x86_64-unknown-linux-gnu pass.

This means that for the exact same function, whether we should use Conv::C or Conv::X86Stdcall depends on the target we are running? Or maybe I just did something bad somewhere...

EDIT: Ok nevermind, it's just I used Conv::X86Stdcall when it supposed to be Conv::C, even for the same tests, it hits different path for Windows and Linux.

RalfJung · 2024-12-10T17:41:56Z

This means that for the exact same function, whether we should use Conv::C or Conv::X86Stdcall depends on the target we are running? Or maybe I just did something bad somewhere...

Yes that makes sense, Windows has some special rules here.

src/tools/miri/src/shims/windows/foreign_items.rs

tiif · 2024-12-14T17:38:40Z

I might continue to fix some cosmetic issues such as stray new lines, but this PR is ready for review

@rustbot ready

RalfJung

Looks pretty good. :) Just some minor comments.

RalfJung · 2024-12-15T08:34:51Z

src/tools/miri/src/helpers.rs

+        if fn_abi.conv != exp_abi {
            throw_ub_format!(
                "calling a function with ABI {} using caller ABI {}",


Just use debug formatting for the error for now. We do the same in interpret/call.rs already.

The proper fix here would be a impl Display for Conv somewhere near this. If you want you can do that in a future PR. :)

Sure, I can open one after this.

src/tools/miri/src/shims/foreign_items.rs

src/tools/miri/src/shims/unix/foreign_items.rs

src/tools/miri/src/shims/unix/solarish/foreign_items.rs

src/tools/miri/src/shims/unix/thread.rs

src/tools/miri/src/shims/windows/foreign_items.rs

RalfJung · 2024-12-15T08:41:11Z

@rustbot author

Pass FnAbi to find_mir_or_eval_fn rust-lang/miri#4013 needs information from ``FnAbi``, hence it is passed to ``find_mir_or_eval_fn``. r? ``@RalfJung``

bors · 2024-12-17T22:13:04Z

☔ The latest upstream changes (presumably #134381) made this pull request unmergeable. Please resolve the merge conflicts.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#133265 (Add a range argument to vec.extract_if) - rust-lang#133801 (Promote powerpc64le-unknown-linux-musl to tier 2 with host tools) - rust-lang#134323 (coverage: Dismantle `map_data.rs` by moving its responsibilities elsewhere) - rust-lang#134378 (An octuple of polonius fact generation cleanups) - rust-lang#134408 (Regression test for RPIT inheriting lifetime from projection) - rust-lang#134423 (bootstrap: use specific-purpose ui test path for `test_valid` self-test) - rust-lang#134426 (Fix typo in uint_macros.rs) Failed merges: - rust-lang#133103 (Pass FnAbi to find_mir_or_eval_fn) r? `@ghost` `@rustbot` modify labels: rollup

tiif · 2024-12-18T04:24:21Z

I will need some time to figure out how to resolve the merge conflict, and will request for r+ again after resolving it.

tiif · 2024-12-18T04:33:27Z

Hmm this is weird, the import is valid and I didn't change anything from attr, but got this error:

 --> compiler/rustc_hir/src/hir.rs:5:5
  |
5 | use rustc_ast::attr::AttributeExt;
  |     ^^^^^^^^^^^^^^^^^------------
  |     |                |
  |     |                help: a similar name exists in the module: `Attribute`
  |     no `AttributeExt` in `attr`

error[E0432]: unresolved import `rustc_ast::UnsafeBinderCastKind`
  --> compiler/rustc_hir/src/hir.rs:14:57
   |
14 |     ImplPolarity, IsAuto, Movability, Mutability, UnOp, UnsafeBinderCastKind,
   |                                                         ^^^^^^^^^^^^^^^^^^^^ no `UnsafeBinderCastKind` in the root

error[E0432]: unresolved import `rustc_ast::attr::AttributeExt`
  --> compiler/rustc_hir/src/lang_items.rs:10:5
   |
10 | use rustc_ast::attr::AttributeExt;
   |     ^^^^^^^^^^^^^^^^^------------
   |     |                |
   |     |                help: a similar name exists in the module: `Attribute`
   |     no `AttributeExt` in `attr`

For more information about this error, try `rustc --explain E0432`.

@oli-obk since you reviewed #134381, do you know what's going on here and how to resolve this?

oli-obk · 2024-12-18T04:44:46Z

That's weird. Sounds like a cache issue where it doesn't think a crate has changed when it acrually has. Worst case try x clean

tiif · 2024-12-18T04:53:49Z

x clean works, thanks!

bors · 2024-12-18T06:17:56Z

☔ The latest upstream changes (presumably #134243) made this pull request unmergeable. Please resolve the merge conflicts.

RalfJung · 2024-12-19T15:21:40Z

@bors r+

oli-obk · 2024-12-19T16:29:40Z

@bors ping

bors · 2024-12-19T16:29:43Z

😪 I'm awake I'm awake

oli-obk · 2024-12-19T16:29:53Z

@bors r=RalfJung

bors · 2024-12-19T16:29:56Z

📌 Commit fd8b983 has been approved by RalfJung

It is now in the queue for this repository.

Rollup of 6 pull requests Successful merges: - rust-lang#126118 (docs: Mention `spare_capacity_mut()` in `Vec::set_len`) - rust-lang#132830 (Rename `elem_offset` to `element_offset`) - rust-lang#133103 (Pass FnAbi to find_mir_or_eval_fn) - rust-lang#134321 (Hide `= _` as associated constant value inside impl blocks) - rust-lang#134518 (fix typos in the example code in the doc comments of `Ipv4Addr::from_bits()`, `Ipv6Addr::from_bits()` & `Ipv6Addr::to_bits()`) - rust-lang#134521 (Arbitrary self types v2: roll loop.) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#133103 - tiif:fnabi, r=RalfJung Pass FnAbi to find_mir_or_eval_fn rust-lang/miri#4013 needs information from ``FnAbi``, hence it is passed to ``find_mir_or_eval_fn``. r? `@RalfJung`

rustbot assigned RalfJung Nov 16, 2024

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 Nov 16, 2024

RalfJung reviewed Nov 24, 2024

View reviewed changes

compiler/rustc_const_eval/src/interpret/machine.rs Outdated Show resolved Hide resolved

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 Nov 24, 2024

This comment has been minimized.

Sign in to view

tiif commented Nov 30, 2024

View reviewed changes

src/tools/miri/src/shims/x86/mod.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

tiif commented Dec 3, 2024

View reviewed changes

src/tools/miri/src/shims/tls.rs Outdated Show resolved Hide resolved

tiif commented Dec 3, 2024

View reviewed changes

src/tools/miri/src/shims/windows/foreign_items.rs Show resolved Hide resolved

rust-cloud-vms bot force-pushed the fnabi branch from bc545aa to 79dce6a Compare December 7, 2024 17:28

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the fnabi branch from 8404ac5 to 64e95f8 Compare December 10, 2024 16:46

tiif commented Dec 11, 2024

View reviewed changes

src/tools/miri/src/shims/windows/foreign_items.rs Show resolved Hide resolved

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 14, 2024

RalfJung reviewed Dec 15, 2024

View reviewed changes

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 15, 2024

jieyouxu mentioned this pull request Dec 17, 2024

Rollup of 9 pull requests #134430

Closed

jieyouxu mentioned this pull request Dec 17, 2024

Rollup of 8 pull requests #134431

Closed

matthiaskrgr mentioned this pull request Dec 17, 2024

Rollup of 7 pull requests #134439

Merged

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 17, 2024

rust-cloud-vms bot force-pushed the fnabi branch from 9c7593e to 45fc89a Compare December 18, 2024 04:30

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the fnabi branch from 45fc89a to 5454c95 Compare December 19, 2024 13:57

Pass FnAbi to find_mir_or_eval_fn

fd8b983

rust-cloud-vms bot force-pushed the fnabi branch from 5454c95 to fd8b983 Compare December 19, 2024 14:11

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 19, 2024

jhpratt mentioned this pull request Dec 20, 2024

Rollup of 6 pull requests #134550

Merged

bors merged commit a53204f into rust-lang:master Dec 20, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass FnAbi to find_mir_or_eval_fn #133103

Pass FnAbi to find_mir_or_eval_fn #133103

tiif commented Nov 16, 2024

rustbot commented Nov 16, 2024

RalfJung commented Nov 24, 2024

tiif commented Nov 27, 2024 •

edited

Loading

This comment has been minimized.

RalfJung commented Nov 27, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Dec 9, 2024

tiif commented Dec 10, 2024 •

edited

Loading

RalfJung commented Dec 10, 2024 via email

tiif commented Dec 14, 2024

RalfJung left a comment

RalfJung Dec 15, 2024

tiif Dec 16, 2024

RalfJung commented Dec 15, 2024

bors commented Dec 17, 2024

tiif commented Dec 18, 2024

tiif commented Dec 18, 2024 •

edited

Loading

oli-obk commented Dec 18, 2024

This comment has been minimized.

tiif commented Dec 18, 2024

bors commented Dec 18, 2024

RalfJung commented Dec 19, 2024

oli-obk commented Dec 19, 2024

bors commented Dec 19, 2024

oli-obk commented Dec 19, 2024

bors commented Dec 19, 2024

Pass FnAbi to find_mir_or_eval_fn #133103

Pass FnAbi to find_mir_or_eval_fn #133103

Conversation

tiif commented Nov 16, 2024

rustbot commented Nov 16, 2024

RalfJung commented Nov 24, 2024

tiif commented Nov 27, 2024 • edited Loading

This comment has been minimized.

RalfJung commented Nov 27, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Dec 9, 2024

tiif commented Dec 10, 2024 • edited Loading

RalfJung commented Dec 10, 2024 via email

tiif commented Dec 14, 2024

RalfJung left a comment

Choose a reason for hiding this comment

RalfJung Dec 15, 2024

Choose a reason for hiding this comment

tiif Dec 16, 2024

Choose a reason for hiding this comment

RalfJung commented Dec 15, 2024

bors commented Dec 17, 2024

tiif commented Dec 18, 2024

tiif commented Dec 18, 2024 • edited Loading

oli-obk commented Dec 18, 2024

This comment has been minimized.

tiif commented Dec 18, 2024

bors commented Dec 18, 2024

RalfJung commented Dec 19, 2024

oli-obk commented Dec 19, 2024

bors commented Dec 19, 2024

oli-obk commented Dec 19, 2024

bors commented Dec 19, 2024

tiif commented Nov 27, 2024 •

edited

Loading

tiif commented Dec 10, 2024 •

edited

Loading

tiif commented Dec 18, 2024 •

edited

Loading