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

folkertdev · 2024-07-15T15:20:29Z

tracking issue #75835

in #75835 (comment) it was decided that using an abi, rather than an attribute, was the right way to go for this feature.

This PR adds that ABI and removes the #[cmse_nonsecure_entry] attribute. All relevant tests have been updated, some are now obsolete and have been removed.

Error 0775 is no longer generated. It contains the list of targets that support the CMSE feature, and maybe we want to still use this? right now a generic "this abi is not supported on this platform" error is returned when this abi is used on an unsupported platform. On the other hand, users of this abi are likely to be experienced rust users, so maybe the generic error is good enough.

rustbot · 2024-07-15T15:20:37Z

r? @lcnr

rustbot has assigned @lcnr.
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-15T15:20:39Z

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

tests/ui/cmse-nonsecure/cmse-nonsecure-entry/params-on-registers.rs

lcnr · 2024-07-16T07:44:53Z

r? compiler

tests/ui/cmse-nonsecure/cmse-nonsecure-call/callback-as-argument.rs

…rror-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.

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.

…ages, r=oli-obk `C-cmse-nonsecure-call`: improved error messages tracking issue: #81391 issue for the error messages (partially implemented by this PR): #81347 related, in that it also deals with CMSE: rust-lang/rust#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.

rustbot · 2024-07-22T08:57:51Z

HIR ty lowering was modified

cc @fmease

folkertdev · 2024-07-22T09:07:03Z

We've just added nicer error message in the same style as #127814. We believe that this resolves all steps from the tracking issue.

apiraino · 2024-07-30T13:41:24Z

r? compiler

fee1-dead · 2024-07-31T02:08:26Z

r? compiler

bors · 2024-07-31T17:49:15Z

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

rustbot · 2024-07-31T18:52:23Z

Some changes occurred in src/tools/cargo

cc @ehuss

folkertdev · 2024-07-31T19:09:25Z

something got comitted accidentally. no changes are now made to src/tools/cargo

tdittr

We reduced the scope of the PR to only cover the replacement of #[cmse_nonsecure_entry] by extern "C-cmse-nonsecure-entry" like it was requested in the tracking issue #75835 (comment) .

We also added some comments to aid the review.

tdittr · 2024-08-01T12:25:57Z

compiler/rustc_codegen_cranelift/src/abi/mod.rs

@@ -61,6 +61,9 @@ pub(crate) fn conv_to_call_conv(sess: &Session, c: Conv, default_call_conv: Call
        Conv::CCmseNonSecureCall => {
            sess.dcx().fatal("C-cmse-nonsecure-call call conv is not yet implemented");
        }
+        Conv::CCmseNonSecureEntry => {
+            sess.dcx().fatal("C-cmse-nonsecure-entry call conv is not yet implemented");


Cranelift does not support any ARM cortex-m targets anyway. So this does not change anything.

tdittr · 2024-08-01T12:32:03Z

compiler/rustc_codegen_llvm/src/abi.rs

+        if let Conv::CCmseNonSecureEntry = self.conv {
+            func_attrs.push(llvm::CreateAttrString(cx.llcx, "cmse_nonsecure_entry"))
+        }


This got moved here from compiler/rustc_codegen_llvm/src/attributes.rs

tdittr · 2024-08-01T12:35:36Z

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

-            sym::cmse_nonsecure_entry => {
-                if let Some(fn_sig) = fn_sig()
-                    && !matches!(fn_sig.skip_binder().abi(), abi::Abi::C { .. })
-                {
-                    struct_span_code_err!(
-                        tcx.dcx(),
-                        attr.span,
-                        E0776,
-                        "`#[cmse_nonsecure_entry]` requires C ABI"
-                    )
-                    .emit();
-                }
-                if !tcx.sess.target.llvm_target.contains("thumbv8m") {
-                    struct_span_code_err!(tcx.dcx(), attr.span, E0775, "`#[cmse_nonsecure_entry]` is only valid for targets with the TrustZone-M extension")
-                    .emit();
-                }
-                codegen_fn_attrs.flags |= CodegenFnAttrFlags::CMSE_NONSECURE_ENTRY
-            }


All these checks are no longer needed, as the ABI now is specified explicitly, and checks for if an ABI is supported on a target are already in place here: compiler/rustc_target/src/spec/mod.rs

tdittr · 2024-08-01T12:44:25Z

compiler/rustc_target/src/spec/mod.rs

            CCmseNonSecureCall => ["arm", "aarch64"].contains(&&self.arch[..]),
+            CCmseNonSecureEntry => ["arm", "aarch64"].contains(&&self.arch[..]),


Here we follow what C-cmse-nonsecure-call does. However we might want to check explicitly for the thumbv8m target. However this PR only wants to tackle the attribute to ABI conversion requested in the tracking issue.

We actually added this check right away, turned out to be pretty straightforward.

tdittr · 2024-08-01T13:02:34Z

tests/assembly/cmse.rs

+// CHECK-LABEL: __acle_se_entry_point
+// CHECK: bxns


This tests end-to-end if we communicated the attribute to LLVM. From the tracking issue ( #75835 ):

add a special symbol on the function which is the _acle_se prefix and the standard function name

constrain the number of parameters to avoid using the Non-Secure stack

before returning from the function, clear registers that might contain Secure information

use the BXNS instruction to return

So here we check if the label with the __acle_se_ prefix exists. And that the bxns (Branch and Exchange Non-secure) instruction is used to return.

tdittr · 2024-08-01T13:05:43Z

tests/assembly/cmse.rs

+// CHECK-LABEL: call_nonsecure
+// CHECK: blxns


Same as above, but now checking for blxns to be used. From that tracking issue ( #81391 ):

save registers needed after the call to Secure memory

clear all registers that might contain confidential information

clear the Least Significant Bit of the function address

branches using the BLXNS instruction

tdittr · 2024-08-01T13:08:38Z

tests/ui/cmse-nonsecure/cmse-nonsecure-call/callback-as-argument.rs

+#[no_mangle]
+pub extern "C-cmse-nonsecure-entry" fn test(
+    f: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32) -> u32,
+    a: u32,
+    b: u32,
+    c: u32,
+) -> u32 {
+    f(a, b, c, 42)
+}


This tests the typical use case of CMSE. test is exposed to the non-secure side, when it calls it passes along a call-back function which can be called by the secure side.

bors · 2024-08-01T15:45:19Z

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

bors · 2024-08-05T07:37:11Z

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

bors · 2024-08-07T06:32:01Z

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

verifies that the correct return instructions are emitted. Co-authored-by: Tamme Dittrich <tamme@tweedegolf.com>

folkertdev · 2024-08-15T14:27:12Z

r? @jackh726

you reviewed the structurally similar #111891, the most recent new ABI addition. Could you review this one too?

We've had some trouble finding a good reviewer here because this change, while simple in theory, touches a lot of different parts of the compiler.

Conceptually, all this does is add a new variant to the abi enum, and then follow the compiler error messages. In most files, the number of changes lines is minimal, and completely in line with surrounding code. Then, the cmse entry attribute is removed and the tests are updated. So, all changes follow naturally, but it's a lot.

bors · 2024-09-20T20:23:06Z

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

rustbot assigned lcnr Jul 15, 2024

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 15, 2024

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 15, 2024

This comment has been minimized.

Sign in to view

folkertdev force-pushed the c-cmse-nonsecure-entry branch from 06fe23a to 9d04d69 Compare July 15, 2024 15:34

This comment has been minimized.

Sign in to view

folkertdev force-pushed the c-cmse-nonsecure-entry branch from 9d04d69 to 8da0efe Compare July 15, 2024 16:10

This comment has been minimized.

Sign in to view

folkertdev commented Jul 15, 2024

View reviewed changes

tests/ui/cmse-nonsecure/cmse-nonsecure-entry/params-on-registers.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rustbot assigned fmease and unassigned lcnr Jul 16, 2024

tdittr reviewed Jul 16, 2024

View reviewed changes

tests/ui/cmse-nonsecure/cmse-nonsecure-call/callback-as-argument.rs Show resolved Hide resolved

folkertdev mentioned this pull request Jul 16, 2024

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

Merged

folkertdev force-pushed the c-cmse-nonsecure-entry branch from af4c41b to 87224ba Compare July 22, 2024 08:57

rustbot assigned fee1-dead and unassigned fmease Jul 30, 2024

rustbot assigned wesleywiser Jul 31, 2024

rustbot unassigned fee1-dead Jul 31, 2024

folkertdev force-pushed the c-cmse-nonsecure-entry branch from 87224ba to 174830a Compare July 31, 2024 18:52

folkertdev force-pushed the c-cmse-nonsecure-entry branch from 174830a to 7f3e2c9 Compare July 31, 2024 19:08

folkertdev force-pushed the c-cmse-nonsecure-entry branch 2 times, most recently from 2c83b07 to 15a0a05 Compare August 1, 2024 13:14

tdittr reviewed Aug 1, 2024

View reviewed changes

folkertdev force-pushed the c-cmse-nonsecure-entry branch from 3e1abc8 to f9ba7e8 Compare August 1, 2024 14:34

folkertdev force-pushed the c-cmse-nonsecure-entry branch from f9ba7e8 to ed11848 Compare August 1, 2024 16:01

jieyouxu mentioned this pull request Aug 4, 2024

Tracking Issue for the C-cmse-nonsecure-call ABI #81391

Open

2 tasks

folkertdev force-pushed the c-cmse-nonsecure-entry branch from ed11848 to 2fc72c6 Compare August 5, 2024 07:56

This comment has been minimized.

Sign in to view

folkertdev force-pushed the c-cmse-nonsecure-entry branch from 2fc72c6 to 9be420a Compare August 5, 2024 08:43

folkertdev force-pushed the c-cmse-nonsecure-entry branch from 9be420a to da55a39 Compare August 7, 2024 09:12

folkertdev and others added 5 commits August 15, 2024 09:55

add C-cmse-nonsecure-entry ABI

5d76fb9

remove #[cmse_nonsecure_entry]

ad7add5

add test that accepts a C-cmse-nonsecure-call function pointer

3e7916d

Add assembly test for the cmse unstable features

a6f0cc3

verifies that the correct return instructions are emitted. Co-authored-by: Tamme Dittrich <tamme@tweedegolf.com>

disallow cmse ABIs on unsupported platforms

a756451

folkertdev force-pushed the c-cmse-nonsecure-entry branch from da55a39 to a756451 Compare August 15, 2024 08:00

rustbot assigned jackh726 and unassigned wesleywiser Aug 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

folkertdev commented Jul 15, 2024

rustbot commented Jul 15, 2024

rustbot commented Jul 15, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

lcnr commented Jul 16, 2024

rustbot commented Jul 22, 2024

folkertdev commented Jul 22, 2024

apiraino commented Jul 30, 2024

fee1-dead commented Jul 31, 2024

bors commented Jul 31, 2024

rustbot commented Jul 31, 2024

folkertdev commented Jul 31, 2024

tdittr left a comment

tdittr Aug 1, 2024

tdittr Aug 1, 2024

tdittr Aug 1, 2024

tdittr Aug 1, 2024

tdittr Aug 1, 2024

tdittr Aug 1, 2024

tdittr Aug 1, 2024

tdittr Aug 1, 2024

bors commented Aug 1, 2024

bors commented Aug 5, 2024

This comment has been minimized.

bors commented Aug 7, 2024

folkertdev commented Aug 15, 2024

bors commented Sep 20, 2024

		CCmseNonSecureCall => ["arm", "aarch64"].contains(&&self.arch[..]),
		CCmseNonSecureEntry => ["arm", "aarch64"].contains(&&self.arch[..]),

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

Are you sure you want to change the base?

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

Conversation

folkertdev commented Jul 15, 2024

rustbot commented Jul 15, 2024

rustbot commented Jul 15, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

lcnr commented Jul 16, 2024

rustbot commented Jul 22, 2024

folkertdev commented Jul 22, 2024

apiraino commented Jul 30, 2024

fee1-dead commented Jul 31, 2024

bors commented Jul 31, 2024

rustbot commented Jul 31, 2024

folkertdev commented Jul 31, 2024

tdittr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Aug 1, 2024

bors commented Aug 5, 2024

This comment has been minimized.

bors commented Aug 7, 2024

folkertdev commented Aug 15, 2024

bors commented Sep 20, 2024

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

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