update fortanix tests #144395

folkertdev · 2025-07-24T12:16:41Z

Firstly, as far as I can tell, no CI job actually runs any of the fortanix tests? Maybe I'm missing the job that runs these tests though?

In any case, the assembly tests now use minicore, meaning that they will run regardless of the host architecture (specifically, they will run during a standard PR CI build).

The run-make test is actually broken, and I'd propose to make it just cargo build rather than cargo run. We can have a separate test for actually running the program, if desired.

Also this test is subject to #128733, so I'd like to re-evaluate what parts of the C/C++ compilation are actually required or useful.

cc @jethrogb @raoulstrackx @aditijannu

r? @jieyouxu

rustbot · 2025-07-24T12:16:47Z

jieyouxu is currently at their maximum review capacity.
They may take a while to respond.

rustbot · 2025-07-24T12:16:49Z

This PR modifies run-make tests.

cc @jieyouxu

tests/assembly-llvm/x86_64-fortanix-unknown-sgx-lvi-generic-load.rs

folkertdev · 2025-07-24T12:18:57Z

tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs

-    // - C/C++ code compiled as part of Rust crates
-    // For those different kinds, we do have very small code examples that should be
-    // mitigated in some way. Mostly we check that ret instructions should no longer be present.
+    cargo().arg("-v").arg("run").arg("--target").arg(target()).current_dir("enclave").run();


Can this instead be cargo build? That means to run the test one would not need the whole fortanix toolchain setup.

Also, when I do make that change, this test actually fails. So, that again suggests this test does not actually run on CI. When this test was introduced in #77008, nothing is changed to CI to make it run, so unless that was added before I don't think this ever actually worked.

Yes cargo build should work just as good.

In order to mitigate LVI, all binary code needs to be mitigated. This test includes code generation for Rust code, inline assembly, global assembly and C/C++ code compiled as part of Rust crates. For the latter there need to be environment variables set:

CC_x86_64_fortanix_unknown_sgx=clang-11 \ CFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ CXX_x86_64_fortanix_unknown_sgx=clang++-11 \ CXXFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \

we do that in src/ci/docker/host-x86_64/dist-various-2/Dockerfile. When testing this locally, you may not have this set.

So, it is my understanding that the dist- jobs do not actually run any tests. From https://rustc-dev-guide.rust-lang.org/tests/ci.html?highlight=try#ci-workflow:

Dist jobs build a full release of the compiler for a specific platform, including all the tools we ship through rustup; Those builds are then uploaded to the rust-lang-ci2 S3 bucket and are available to be locally installed with the rustup-toolchain-install-master tool. The same builds are also used for actual releases: our release process basically consists of copying those artifacts from rust-lang-ci2 to the production endpoint and signing them.

and

Non-dist jobs run our full test suite on the platform

Hence, I don't think the fortanix tests ever ran on CI

In order to mitigate LVI, all binary code needs to be mitigated. This test includes code generation for Rust code, inline assembly, global assembly and C/C++ code compiled as part of Rust crates

This is just up to the programmer though right? In the sense that you'd need to set a bunch of rather non-obvious flags for this to work at all. So I think testing the interaction with C and especially C++ is quite low.

Also, clang-11 is ancient, why is that still used? It looks like modern clang has -mlvi-hardening, but since llvm 11 the output of llvm IR has changed quite a bit so just using a more recent LLVM makes some of the tests fail.

I'm afraid that description about dist jobs not running any tests isn't quite accurate nowadays, because opt-dist do run tests (cc @Kobzol)

Well, yeah, but only on x64 Linux and MSVC, nowhere else.

jieyouxu · 2025-07-24T14:46:26Z

triagebot edited your pings out, I'll reping manually:

FYI @jethrogb @raoulstrackx @aditijannu

jieyouxu · 2025-07-24T14:48:46Z

I'll have to wait to hear back from the SGX target maintainers regarding the tests, as I don't have the context either.

@rustbot blocked (waiting to hear back)

tests/assembly-llvm/x86_64-fortanix-unknown-sgx-lvi-generic-load.rs

raoulstrackx · 2025-07-25T08:41:16Z

tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs

-    // - C/C++ code compiled as part of Rust crates
-    // For those different kinds, we do have very small code examples that should be
-    // mitigated in some way. Mostly we check that ret instructions should no longer be present.
+    cargo().arg("-v").arg("run").arg("--target").arg(target()).current_dir("enclave").run();


Yes cargo build should work just as good.

raoulstrackx · 2025-07-25T08:45:39Z

tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs

-    // - C/C++ code compiled as part of Rust crates
-    // For those different kinds, we do have very small code examples that should be
-    // mitigated in some way. Mostly we check that ret instructions should no longer be present.
+    cargo().arg("-v").arg("run").arg("--target").arg(target()).current_dir("enclave").run();


In order to mitigate LVI, all binary code needs to be mitigated. This test includes code generation for Rust code, inline assembly, global assembly and C/C++ code compiled as part of Rust crates. For the latter there need to be environment variables set:

CC_x86_64_fortanix_unknown_sgx=clang-11 \ CFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ CXX_x86_64_fortanix_unknown_sgx=clang++-11 \ CXXFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \

we do that in src/ci/docker/host-x86_64/dist-various-2/Dockerfile. When testing this locally, you may not have this set.

tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs

jieyouxu · 2025-07-25T08:54:11Z

@rustbot author

rustbot · 2025-07-25T08:54:15Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

jethrogb · 2025-07-25T10:31:23Z

@folkertdev

Firstly, as far as I can tell, no CI job actually runs any of the fortanix tests?

We're a tier 2 target. So the Rust project doesn't run any tests. Fortanix has internal CI that watches the Rust repo.

folkertdev · 2025-07-25T11:09:06Z

We're a tier 2 target. So the Rust project doesn't run any tests.

Right, that makes some sense historically then. Though there is a lot of variance in what tests do get run for tier-2 targets. e.g. s390x-unknown-linux-gnu gets exercised quite a bit despite being tier 2.

For tests/assembly-llvm/x86_64-fortanix-unknown-sgx-lvi-generic-load.rs and friends, we can just run the test now with minicore from any target.

For the run-make test: why is it in-tree at all when it's not actually executed?

jethrogb · 2025-07-25T11:28:00Z

If the Rust project's policy has changed is this regard we would very much like the test suite to be run for this target as well.

folkertdev · 2025-07-25T11:47:42Z

I don't think it's changed for run-make, but for many of the other tests (e.g. assembly) we do test for more targets than just tier 1.

But run-make requires more setup, and is usually more fragile, so I do think those are limited to tier 1 targets. And then I'm still not sure why the fortanix test is in-tree.

Kobzol · 2025-07-25T12:29:32Z

As a general answer, I doubt there is a clear reason for this. The way we run tests is quite arbitrary in many ways, and there's a lot of historical cruft. So the answer can very possibly be "it's by accident".

jethrogb · 2025-07-25T12:41:12Z

I'm still not sure why the fortanix test is in-tree.

Why wouldn't the test be in-tree? Citing from the target tier policy:

The target must provide documentation for the Rust community explaining how to build for the target using cross-compilation, and explaining how to run tests for the target.

I realize this doesn't require it to be in tree, but that does seem like a logical place for it.

In addition, having it in-tree makes the most sense for future promotion to tier 1.

jieyouxu · 2025-07-25T13:20:24Z

The test changes look good to me, but since the run-make test is not exercised in CI, can one of the target maintainers @jethrogb @raoulstrackx @aditijannu confirm it works locally on the target as expected and that the test changes look good to you as well?

folkertdev · 2025-07-25T13:34:39Z

ftr even with clang-11 and the cflags from the docker file I can't get this to run locally

=== STDERR ===
unw_getcontext.checks:2:8: error: CHECK: expected string not found in input
CHECK: lfence
       ^
<stdin>:6:33: note: scanning from here
000000000008cb31 <unw_getcontext>:
                                ^
<stdin>:7:19: note: possible intended match here
 8cb31: f3 0f 1e fa endbr64
                  ^

Input file: <stdin>
Check file: unw_getcontext.checks

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           1:  
           2: enclave/target/x86_64-fortanix-unknown-sgx/debug/enclave: file format elf64-x86-64 
           3:  
           4: Disassembly of section .text: 
           5:  
           6: 000000000008cb31 <unw_getcontext>: 
check:2'0                                     X~~ error: no match found
           7:  8cb31: f3 0f 1e fa endbr64 
check:2'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:2'1                       ?          possible intended match
           8:  8cb35: 48 89 07 movq %rax, (%rdi) 
check:2'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           9:  8cb38: 48 89 5f 08 movq %rbx, 0x8(%rdi) 
check:2'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          10:  8cb3c: 48 89 4f 10 movq %rcx, 0x10(%rdi) 
check:2'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          11:  8cb40: 48 89 57 18 movq %rdx, 0x18(%rdi) 
check:2'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          12:  8cb44: 48 89 7f 20 movq %rdi, 0x20(%rdi) 
check:2'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           .
           .
           .
>>>>>>
------------------------------------------



failures:
    [run-make] tests/run-make/x86_64-fortanix-unknown-sgx-lvi

(the llvm/clang is from https://www.kernel.org/pub/tools/llvm/)

Make it more idiomatic with the new run-make infra

rustbot assigned jieyouxu Jul 24, 2025

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs 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 24, 2025

folkertdev commented Jul 24, 2025

View reviewed changes

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2025

raoulstrackx reviewed Jul 25, 2025

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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jul 25, 2025

use minicore for fortanix assembly tests

182a19e

folkertdev force-pushed the fortanix-run-make-test branch from e240476 to afe48b4 Compare July 25, 2025 10:58

jieyouxu 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 25, 2025

update fortanix run-make test

659ca90

Make it more idiomatic with the new run-make infra

folkertdev force-pushed the fortanix-run-make-test branch from afe48b4 to 659ca90 Compare July 25, 2025 13:43

update fortanix tests #144395

Are you sure you want to change the base?

update fortanix tests #144395

Conversation

folkertdev commented Jul 24, 2025

Uh oh!

rustbot commented Jul 24, 2025

Uh oh!

rustbot commented Jul 24, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Jul 24, 2025

Uh oh!

jieyouxu commented Jul 24, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jieyouxu commented Jul 25, 2025

Uh oh!

rustbot commented Jul 25, 2025

Uh oh!

jethrogb commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

folkertdev commented Jul 25, 2025

Uh oh!

jethrogb commented Jul 25, 2025

Uh oh!

folkertdev commented Jul 25, 2025

Uh oh!

Kobzol commented Jul 25, 2025

Uh oh!

jethrogb commented Jul 25, 2025

Uh oh!

jieyouxu commented Jul 25, 2025

Uh oh!

folkertdev commented Jul 25, 2025

Uh oh!

Uh oh!

jieyouxu Jul 25, 2025 •

edited

Loading

jethrogb commented Jul 25, 2025 •

edited

Loading