`C-cmse-nonsecure-call`: improved error messages #127814

folkertdev · 2024-07-16T15:26:21Z

tracking issue: #81391
issue for the error messages (partially implemented by this PR): #81347
related, in that it also deals with CMSE: #127766

When using the C-cmse-nonsecure-call ABI, both the arguments and return value must be passed via registers. Previously, when violating this constraint, an ugly LLVM error would be shown. Now, the rust compiler itself will print a pretty message and link to more information.

when the `C-cmse-nonsecure-call` ABI is used, arguments and return values must be passed via registers. Failing to do so (i.e. spilling to the stack) causes an LLVM error down the line, but now rustc will properly emit an error a bit earlier in the chain

rustbot · 2024-07-16T15:26:28Z

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-07-16T15:26:30Z

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

GuillaumeGomez

Diagnostic error explanation looks good to me.

compiler/rustc_codegen_ssa/src/mir/block.rs

oli-obk · 2024-07-16T15:40:22Z

Can you swap the commits so we can see the diagnostics as a diff on your actual commit? Or are the LLVM diagnostics too annoying?

tdittr · 2024-07-16T16:05:48Z

Can you swap the commits so we can see the diagnostics as a diff on your actual commit? Or are the LLVM diagnostics too annoying?

We did consider this, but LLVM will only return the first error it encounters. Also rebasing this is not straight forward, since we moved some of the tests which git rebase really does not seem to like.

the error is only generated for functions that are actually codegen'd

Co-authored-by: Tamme Dittrich <[email protected]>

tdittr · 2024-07-16T16:48:21Z

For the name of the new mod we were not sure, so we just picked one, suggestions are very welcome.

folkertdev · 2024-07-16T16:49:15Z

compiler/rustc_codegen_ssa/src/mir/cmse.rs

+/// Check conditions on inputs and outputs that the cmse ABIs impose: arguments and results MUST be
+/// returned via registers (i.e. MUST NOT spill to the stack). LLVM will also validate these
+/// conditions, but by checking them here rustc can emit nicer error messages.
+pub fn validate_cmse_abi<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(


in the near future, this will also validate the C-cmse-nonsecure-entry abi which will be added by #127766.

folkertdev · 2024-07-16T17:39:44Z

CI fails with

failures:

---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0798 (line 18059) stdout ----
Test compiled successfully, but it's marked `compile_fail`.
---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0798 (line 18073) stdout ----
Test compiled successfully, but it's marked `compile_fail`.

failures:
    /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0798 (line 18059)
    /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0798 (line 18073)

test result: FAILED. 1041 passed; 2 failed; 55 ignored; 0 measured; 0 filtered out; finished in 8.37s

How can I run those tests locally?

EDIT: ./x.py test --stage 1 src/tools/error_index_generator seems to do it but is slow. using stage 0 is faster but does that do everything?

compiler/rustc_error_codes/src/error_codes/E0798.md

tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-via-stack.stderr

folkertdev · 2024-07-17T19:09:27Z

Something that stops working with the logic as implemented (and I think that is correct) is using a repr(Rust) union that is effectively a foundational type.

https://godbolt.org/z/oaTdf9cYE

My reasoning is that repr(Rust) is unstable, and so while it is currently given to LLVM as a foundational type, that is not guaranteed. We're future-proofing the implementation here by not allowing it. Using a union with just one foundational field does not work in C. repr(transparent) on unions seems far away from being stabilized, but if it guarantees that a one-field union has the same representation as that field then that would work.

rustbot · 2024-07-18T10:10:31Z

HIR ty lowering was modified

cc @fmease

folkertdev · 2024-07-18T10:27:20Z

we've now implemented the logic in hir_analyze, removed code from codegen_ssa, and error when generics are used in function pointer types with the C-cmse-nonsecure-call abi

it was moved to hir_analysis in the previous commit

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs

folkertdev · 2024-07-18T12:51:36Z

@rustbot ready

oli-obk · 2024-07-18T12:56:47Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs

    let mut accum = 0u64;

-    for arg_def in fn_sig.inputs().iter() {
+    for (index, arg_def) in fn_sig.inputs().iter().enumerate() {
        let layout = tcx.layout_of(ParamEnv::reveal_all().and(*arg_def.skip_binder()))?;


I think at this point it may be easier to just emit the errors in is_valid_cmse_inputs and _output directly. Then you can also report a "generics not allowed" error with a span pointing at the offending thing.

we have 2 reasons not to do that

arg_def doesn't have any span information. it's effectively just a Ty

in the (near, hopefully) future, the C-cmse-nonsecure-entry abi will require the same validation, and we'd like to reuse this code, so passing in BareFn-specific data will not work

oli-obk · 2024-07-19T15:15:56Z

@bors r+ rollup

bors · 2024-07-19T15:16:06Z

📌 Commit c2894a4 has been approved by oli-obk

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#127295 (CFI: Support provided methods on traits) - rust-lang#127814 (`C-cmse-nonsecure-call`: improved error messages) - rust-lang#127949 (fix: explain E0120 better cover cases when its raised) - rust-lang#127966 (Use structured suggestions for unconstrained generic parameters on impl blocks) - rust-lang#127976 (Lazy type aliases: Diagostics: Detect bivariant ty params that are only used recursively) - rust-lang#127978 (Avoid ref when using format! for perf) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#127814 - folkertdev:c-cmse-nonsecure-call-error-messages, r=oli-obk `C-cmse-nonsecure-call`: improved error messages tracking issue: rust-lang#81391 issue for the error messages (partially implemented by this PR): rust-lang#81347 related, in that it also deals with CMSE: rust-lang#127766 When using the `C-cmse-nonsecure-call` ABI, both the arguments and return value must be passed via registers. Previously, when violating this constraint, an ugly LLVM error would be shown. Now, the rust compiler itself will print a pretty message and link to more information.

folkertdev added 2 commits July 16, 2024 15:58

add more tests for cmse-nonsecure-call stack spills

1e86064

rustbot assigned oli-obk Jul 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 Jul 16, 2024

folkertdev changed the title ~~C cmse nonsecure call error messages~~ C-cmse-nonsecure-call: improved error messages Jul 16, 2024

GuillaumeGomez approved these changes Jul 16, 2024

View reviewed changes

oli-obk reviewed Jul 16, 2024

View reviewed changes

compiler/rustc_codegen_ssa/src/mir/block.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

folkertdev and others added 2 commits July 16, 2024 18:38

make function pub in error_codes markdown file

36d2371

the error is only generated for functions that are actually codegen'd

move cmse ABI validation into its own module

c7ff46c

Co-authored-by: Tamme Dittrich <[email protected]>

folkertdev commented Jul 16, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

another attempt at fixing the examples in the error codes .md

1a79606

folkertdev commented Jul 16, 2024

View reviewed changes

compiler/rustc_error_codes/src/error_codes/E0798.md Outdated Show resolved Hide resolved

stop running code samples in the error code .md

09b620d

oli-obk reviewed Jul 17, 2024

View reviewed changes

tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-via-stack.stderr Outdated Show resolved Hide resolved

add test for repr(transparent) enum

5f0f690

add test for unions and repr(transparent) with ZST fields

8a3dd7f

This comment has been minimized.

Sign in to view

move CMSE validation to hir_analysis

7b63734

remove cmse validation from rustc_codegen_ssa

6b6b842

it was moved to hir_analysis in the previous commit

folkertdev force-pushed the c-cmse-nonsecure-call-error-messages branch from 096cc19 to 6b6b842 Compare July 18, 2024 10:42

oli-obk requested changes Jul 18, 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 Jul 18, 2024

improve error reporting

c2894a4

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 Jul 18, 2024

oli-obk reviewed Jul 18, 2024

View reviewed changes

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 Jul 19, 2024

matthiaskrgr mentioned this pull request Jul 19, 2024

Rollup of 6 pull requests #127982

Merged

bors merged commit 3b20150 into rust-lang:master Jul 19, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jul 19, 2024

folkertdev mentioned this pull request Jul 22, 2024

add extern "C-cmse-nonsecure-entry" fn #127766

Merged

This was referenced Sep 5, 2024

ICE: has escaping bound vars, so it cannot be wrapped in a dummy binder #129983

Closed

ICE: Layout::compute: unexpected type _ #130104

Open

folkertdev mentioned this pull request Sep 9, 2024

fix ICE in CMSE type validation #130064

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`C-cmse-nonsecure-call`: improved error messages #127814

`C-cmse-nonsecure-call`: improved error messages #127814

folkertdev commented Jul 16, 2024 •

edited

Loading

rustbot commented Jul 16, 2024

rustbot commented Jul 16, 2024

GuillaumeGomez left a comment

oli-obk commented Jul 16, 2024

tdittr commented Jul 16, 2024

This comment has been minimized.

tdittr commented Jul 16, 2024 •

edited

Loading

folkertdev Jul 16, 2024

This comment has been minimized.

folkertdev commented Jul 16, 2024 •

edited

Loading

folkertdev commented Jul 17, 2024

rustbot commented Jul 18, 2024

folkertdev commented Jul 18, 2024

This comment has been minimized.

folkertdev commented Jul 18, 2024

oli-obk Jul 18, 2024

folkertdev Jul 18, 2024

oli-obk commented Jul 19, 2024

bors commented Jul 19, 2024

C-cmse-nonsecure-call: improved error messages #127814

C-cmse-nonsecure-call: improved error messages #127814

Conversation

folkertdev commented Jul 16, 2024 • edited Loading

rustbot commented Jul 16, 2024

rustbot commented Jul 16, 2024

GuillaumeGomez left a comment

Choose a reason for hiding this comment

oli-obk commented Jul 16, 2024

tdittr commented Jul 16, 2024

This comment has been minimized.

tdittr commented Jul 16, 2024 • edited Loading

folkertdev Jul 16, 2024

Choose a reason for hiding this comment

This comment has been minimized.

folkertdev commented Jul 16, 2024 • edited Loading

folkertdev commented Jul 17, 2024

rustbot commented Jul 18, 2024

folkertdev commented Jul 18, 2024

This comment has been minimized.

folkertdev commented Jul 18, 2024

oli-obk Jul 18, 2024

Choose a reason for hiding this comment

folkertdev Jul 18, 2024

Choose a reason for hiding this comment

oli-obk commented Jul 19, 2024

bors commented Jul 19, 2024

`C-cmse-nonsecure-call`: improved error messages #127814

`C-cmse-nonsecure-call`: improved error messages #127814

folkertdev commented Jul 16, 2024 •

edited

Loading

tdittr commented Jul 16, 2024 •

edited

Loading

folkertdev commented Jul 16, 2024 •

edited

Loading