Skip to content

Upgrade the fortanix-sgx-abi dependency #143272

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 26, 2025
Merged

Upgrade the fortanix-sgx-abi dependency #143272

merged 1 commit into from
Jul 26, 2025
+4 −5

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Jul 1, 2025

0.6.1 removes the compiler-builtins dependency, part of RUST-142265. The breaking change from 0.5 to 0.6 is for an update to the insecure_time API [1].

I validated that ./x c library --target x86_64-fortanix-unknown-sgx completes successfully with this change.

Link: fortanix/rust-sgx@a34e976 [1]

@rustbot
Copy link
Collaborator

rustbot commented Jul 1, 2025

r? @thomcc

rustbot has assigned @thomcc.
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 rustbot added O-SGX Target: SGX S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 1, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 1, 2025

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@tgross35
Copy link
Contributor Author

tgross35 commented Jul 1, 2025

@jethrogb mind taking a look at this?

@tgross35
Upgrade the fortanix-sgx-abi dependency
978e11b 
0.6.1 removes the `compiler-builtins` dependency, part of RUST-142265.
The breaking change from 0.5 to 0.6 is for an update to the
`insecure_time` API [1].

I validated that `./x c library --target x86_64-fortanix-unknown-sgx`
completes successfully with this change.

Link: fortanix/rust-sgx@a34e976 [1]
@tgross35 tgross35 mentioned this pull request Jul 1, 2025
Open
@jethrogb
Copy link
Contributor

jethrogb commented Jul 2, 2025

Unfortunately, we're missing documentation for the modified insecure_time call so I don't know if this is correct.

@tgross35
Copy link
Contributor Author

tgross35 commented Jul 4, 2025 •
edited
Loading

Is this unimplemented!() the actual function, or does it get hooked somewhere? I'm having some difficulty tracing this back https://github.com/fortanix/rust-sgx/blob/adcc6cdb981bcf2101437edbd3ef6144246621d3/intel-sgx/fortanix-sgx-abi/src/lib.rs#L584. I'm thinking that if it is actually an unconditional panic, the function body for insecure_time here could become unimplemented!() rather than calling raw::insecure_time so we can upgrade without concern of the API change.

@jethrogb
Copy link
Contributor

jethrogb commented Jul 4, 2025

fortanix-sgx-abi just contains definitions describing the ABI. The implementations exist elsewhere:

https://github.com/rust-lang/rust/blob/master/library/std/src/sys/pal/sgx/abi/usercalls/raw.rs
https://github.com/fortanix/rust-sgx/blob/master/intel-sgx/enclave-runner/src/usercalls/abi.rs

But without documentation it's not really possible to determine whether the implementation is correct.

@tgross35
Copy link
Contributor Author

@jethrogb in that case, would it be possible to do a new patch release from 0.5 that includes fortanix/rust-sgx#770 but doesn't have the API change? Unfortunately, sgix 0.5 having the crates.io dependency on compiler-builtins is preventing some refactoring.

@jethrogb
Copy link
Contributor

We've finalized the documentation and can conclude from that it's fine to ignore the second tuple element. I approve this PR as is.

@tgross35
Copy link
Contributor Author

Oh, awesome! Thank you for confirming.

r? libs

@rustbot rustbot assigned jhpratt and unassigned thomcc Jul 25, 2025
@jhpratt
Copy link
Member

jhpratt commented Jul 25, 2025

@bors r=jhpratt,jethrogb

@bors
Copy link
Collaborator

bors commented Jul 25, 2025

📌 Commit 978e11b has been approved by jhpratt,jethrogb

It is now in the queue for this repository.

@bors 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 25, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 26, 2025
@tgross35
Rollup merge of rust-lang#143272 - tgross35:bump-fortanix, r=jhpratt,…
5f4dac3 
…jethrogb

Upgrade the `fortanix-sgx-abi` dependency

0.6.1 removes the `compiler-builtins` dependency, part of RUST-142265. The breaking change from 0.5 to 0.6 is for an update to the `insecure_time` API [1].

I validated that `./x c library --target x86_64-fortanix-unknown-sgx` completes successfully with this change.

Link: fortanix/rust-sgx@a34e976 [1]
@tgross35 tgross35 mentioned this pull request Jul 26, 2025
Closed
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 26, 2025
@tgross35
Rollup merge of rust-lang#143272 - tgross35:bump-fortanix, r=jhpratt,…
777e329 
…jethrogb

Upgrade the `fortanix-sgx-abi` dependency

0.6.1 removes the `compiler-builtins` dependency, part of RUST-142265. The breaking change from 0.5 to 0.6 is for an update to the `insecure_time` API [1].

I validated that `./x c library --target x86_64-fortanix-unknown-sgx` completes successfully with this change.

Link: fortanix/rust-sgx@a34e976 [1]
@tgross35 tgross35 mentioned this pull request Jul 26, 2025
Closed
bors added a commit that referenced this pull request Jul 26, 2025
@bors
Auto merge of #144485 - tgross35:rollup-3e11y2g, r=tgross35
ca0755f 
Rollup of 9 pull requests

Successful merges:

 - #140871 (Don't lint against named labels in `naked_asm!`)
 - #141663 (rustdoc: add ways of collapsing all impl blocks)
 - #143272 (Upgrade the `fortanix-sgx-abi` dependency)
 - #143585 (`loop_match`: suggest extracting to a `const` item)
 - #143698 (Fix unused_parens false positive)
 - #143859 (Guarantee 8 bytes of alignment in Thread::into_raw)
 - #144042 (Verify llvm-needs-components are not empty and match the --target value)
 - #144160 (tests: debuginfo: Work around or disable broken tests on powerpc)
 - #144431 (Disable has_reliable_f128_math on musl targets)

r? `@ghost`
`@rustbot` modify labels: rollup
@tgross35 tgross35 mentioned this pull request Jul 26, 2025
Merged
bors added a commit that referenced this pull request Jul 26, 2025
@bors
Auto merge of #144490 - tgross35:rollup-ps0utme, r=tgross35
8708f3c 
Rollup of 9 pull requests

Successful merges:

 - #140871 (Don't lint against named labels in `naked_asm!`)
 - #141663 (rustdoc: add ways of collapsing all impl blocks)
 - #143272 (Upgrade the `fortanix-sgx-abi` dependency)
 - #143585 (`loop_match`: suggest extracting to a `const` item)
 - #143698 (Fix unused_parens false positive)
 - #143859 (Guarantee 8 bytes of alignment in Thread::into_raw)
 - #144160 (tests: debuginfo: Work around or disable broken tests on powerpc)
 - #144412 (Small cleanup: Use LocalKey<Cell> methods more)
 - #144431 (Disable has_reliable_f128_math on musl targets)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3b99668 into rust-lang:master Jul 26, 2025
10 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 26, 2025
rust-timer added a commit that referenced this pull request Jul 26, 2025
@rust-timer
Unrolled build for #143272
0ff24a1 
Rollup merge of #143272 - tgross35:bump-fortanix, r=jhpratt,jethrogb

Upgrade the `fortanix-sgx-abi` dependency

0.6.1 removes the `compiler-builtins` dependency, part of RUST-142265. The breaking change from 0.5 to 0.6 is for an update to the `insecure_time` API [1].

I validated that `./x c library --target x86_64-fortanix-unknown-sgx` completes successfully with this change.

Link: fortanix/rust-sgx@a34e976 [1]
@tgross35 tgross35 deleted the bump-fortanix branch July 28, 2025 05:47
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 30, 2025
@tgross35
Rollup merge of rust-lang#143272 - tgross35:bump-fortanix, r=jhpratt,…
94e12a6 
…jethrogb

Upgrade the `fortanix-sgx-abi` dependency

0.6.1 removes the `compiler-builtins` dependency, part of RUST-142265. The breaking change from 0.5 to 0.6 is for an update to the `insecure_time` API [1].

I validated that `./x c library --target x86_64-fortanix-unknown-sgx` completes successfully with this change.

Link: fortanix/rust-sgx@a34e976 [1]
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 30, 2025
@bors
Auto merge of rust-lang#144490 - tgross35:rollup-ps0utme, r=tgross35
50b68d9 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#140871 (Don't lint against named labels in `naked_asm!`)
 - rust-lang#141663 (rustdoc: add ways of collapsing all impl blocks)
 - rust-lang#143272 (Upgrade the `fortanix-sgx-abi` dependency)
 - rust-lang#143585 (`loop_match`: suggest extracting to a `const` item)
 - rust-lang#143698 (Fix unused_parens false positive)
 - rust-lang#143859 (Guarantee 8 bytes of alignment in Thread::into_raw)
 - rust-lang#144160 (tests: debuginfo: Work around or disable broken tests on powerpc)
 - rust-lang#144412 (Small cleanup: Use LocalKey<Cell> methods more)
 - rust-lang#144431 (Disable has_reliable_f128_math on musl targets)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jhpratt jhpratt

Labels
O-SGX Target: SGX S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.90.0
Development

Successfully merging this pull request may close these issues.
6 participants
@tgross35 @rustbot @jethrogb @jhpratt @bors @thomcc