Skip to content
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.

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

support building on illumos systems #1854

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

iliana
Copy link
Contributor

@iliana iliana commented Sep 16, 2024

Issues:

n/a

Description of changes:

The team that works on awslabs/tough wants to support using the aws-lc-rs Rust crate (awslabs/tough#824). As one of several downstream consumers of tough outside of AWS, we were asked for feedback. Oxide uses tough as part of our product which runs on the illumos operating system.

When linking Rust code using aws-lc-rs, the linker cannot find several aws_lc_* symbols:

Undefined            first referenced
 symbol                  in file
aws_lc_0_21_1_bn_gather5            /home/iliana/git/omicron/target/debug/deps/libaws_lc_sys-ff0ab07b18206e1f.rlib(bcm.c.o)
aws_lc_0_21_1_rsaz_1024_red2norm_avx2 /home/iliana/git/omicron/target/debug/deps/libaws_lc_sys-ff0ab07b18206e1f.rlib(bcm.c.o)
aws_lc_0_21_1_aesni_gcm_encrypt     /home/iliana/git/omicron/target/debug/deps/libaws_lc_sys-ff0ab07b18206e1f.rlib(bcm.c.o)
aws_lc_0_21_1_chacha20_poly1305_seal /home/iliana/git/omicron/target/debug/deps/libaws_lc_sys-ff0ab07b18206e1f.rlib(e_chacha20poly1305.c.o)
aws_lc_0_21_1_chacha20_poly1305_open /home/iliana/git/omicron/target/debug/deps/libaws_lc_sys-ff0ab07b18206e1f.rlib(e_chacha20poly1305.c.o)
[ ... this goes on for some hundreds of lines ... ]

uname -p on illumos systems returns i386 on 64-bit machines. This resulted in assembly code not being linked into the AWS-LC library due to architecture misdetection. To determine the native instruction set, isainfo -n is used instead.

Once these cryptic errors were out of the way, we got a more normal linking error indicating some missing libraries; symbols that are in the libc on other platforms are in separate libraries on illumos.

I'm sending this PR on behalf of my coworker, @jclulow, but I can help handle any requested changes.

Call-outs:

n/a

Testing:

Tested on an illumos system via aws-lc-rs by patching the crate in Cargo.toml with:

[patch.crates-io.aws-lc-sys]
git = "https://github.com/oxidecomputer/aws-lc-rs"
branch = "illumos/aws-lc-sys/v0.21.1"

which updates the AWS-LC submodule to this commit.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.53%. Comparing base (2f18797) to head (5f213d3).
Report is 173 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1854      +/-   ##
==========================================
+ Coverage   78.34%   78.53%   +0.19%     
==========================================
  Files         580      583       +3     
  Lines       97255    98809    +1554     
  Branches    13945    14162     +217     
==========================================
+ Hits        76190    77602    +1412     
- Misses      20444    20580     +136     
- Partials      621      627       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@justsmth justsmth self-requested a review September 17, 2024 11:25
justsmth
justsmth previously approved these changes Sep 17, 2024
CMakeLists.txt Outdated Show resolved Hide resolved
# "isainfo -n", which prints "the name of the native instruction set used by
# portable applications"; e.g., "amd64".
#
execute_process(COMMAND /usr/bin/isainfo -n OUTPUT_VARIABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Reference for isainfo: https://illumos.org/man/1/isainfo

@andrewhop andrewhop self-requested a review September 17, 2024 17:16
CMakeLists.txt Outdated Show resolved Hide resolved
@justsmth
Copy link
Contributor

Sorry, this didn't make it into the AWS-LC v1.35.0 release. Hopefully, we'll put up another release soon. (We typically align new releases of aws-lc-sys with the latest release of AWS-LC.)

@iliana
Copy link
Contributor Author

iliana commented Sep 17, 2024

We at least have Cargo's [patch.crates-io]! :)

@justsmth justsmth merged commit 468ca3f into aws:main Sep 25, 2024
108 of 110 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants