feat: [NODE-1566] Build with hermetic cc toolchains #3508

Bownairo · 2025-01-17T23:42:38Z

If we upgrade our build container, the dynamic libraries linked to our published binaries upgrade, too. We would like to link against older libraries in order to support more targets, and would like to upgrade our build container to use more modern tools.

In order to decouple the bazel build from our build container, build using hermetic_cc_toolchains pinned to an older libc, and patch up the rough edges.

Bownairo

~~The determinism tests are failing due to differences in one of the binaries:~~
~~> Binary files A/ic-boundary and B/ic-boundary differ~~

~~I still need to take a look. Have any ideas?~~

I haven't been able to reproduce the diff in ic-boundary, and when I went to inspect the files, they actually were the same. I did find a separate issue in infogetty where sandbox paths were leaking into the binary, but this was fixed by stripping it.

hs/spec_compliance/defs.bzl

Bownairo · 2025-03-04T05:06:44Z

Hey @dfinity/product-security! I had run into some trouble with the AFL fuzzers, and came up with these two solutions. Before I pull this out of draft and open this up to the other teams, could you take a look and let me know what you think?

Build AFL fuzzers without hermetic toolchains: ~~eero/hermetic-toolchain...eero/hermetic-toolchain-afl~~ (19166e3)
To instrument with AFL we use the AFL wrappers here. Using the hermetic toolchains makes it so that these variables are not respected, and the binaries are built without instrumentation.

It is a really simple change to avoid using the hermetic toolchain for these targets, but it means the build path for the fuzzing targets diverges.

Patch dependencies to support AFL fuzzers on zig: ~~eero/hermetic-toolchain...eero/hermetic-toolchain-afl-zig~~ (20c1e0e)
If we do want to build with zig (which backs the hermetic toolchains), I had to make a few patches to respect the AFL wrapper variables, and work around link args that zig cc does not support. This also creates another variant of lockfiles to build with fuzzing flags, but not the bundled libfuzzer.

This change is more complicated, but keeps the fuzzers in line with the built binaries. This also makes the fuzzers slower, as the LTO variant is not supported (it seems the required flags are not even on the radar for zig).

edit: Updated links to more stable commits.

venkkatesh-sekar · 2025-03-04T13:51:51Z

Hey @dfinity/product-security! I had run into some trouble with the AFL fuzzers, and came up with these two solutions. Before I pull this out of draft and open this up to the other teams, could you take a look and let me know what you think?

Build AFL fuzzers without hermetic toolchains: eero/hermetic-toolchain...eero/hermetic-toolchain-afl To instrument with AFL we use the AFL wrappers here. Using the hermetic toolchains makes it so that these variables are not respected, and the binaries are built without instrumentation.

It is a really simple change to avoid using the hermetic toolchain for these targets, but it means the build path for the fuzzing targets diverges.

Patch dependencies to support AFL fuzzers on zig: eero/hermetic-toolchain...eero/hermetic-toolchain-afl-zig If we do want to build with zig (which backs the hermetic toolchains), I had to make a few patches to respect the AFL wrapper variables, and work around link args that zig cc does not support. This also creates another variant of lockfiles to build with fuzzing flags, but not the bundled libfuzzer.

This change is more complicated, but keeps the fuzzers in line with the built binaries. This also makes the fuzzers slower, as the LTO variant is not supported (it seems the required flags are not even on the radar for zig).

Thanks @Bownairo for this awesome effort, we can go ahead with option A as it's the simplest one and the build path diverging should have very little effect on the bugs the fuzzers should find itself.

bazel/external_crates.bzl

Bownairo · 2025-03-05T23:16:33Z

~~It seems the rustix patches don't apply after the latest repin. #4188 should be the fix?~~

andrewbattat

🥳

bazel/fuzz_testing.bzl

WORKSPACE.bazel

bazel/cc_rs.patch

bazel/hermetic_cc_toolchain.patch

cpp/BUILD.bazel

hs/spec_compliance/defs.bzl

In #3508 we change the name of the underlying `ic-ref-test` binary, but keep the target the same. This breaks tests that have been depending on the binary name directly. This change uses the target path to reference the binary, instead.

Bownairo · 2025-03-27T08:46:51Z

@mraszyk would you be able to take a look at the other side of #4515?

mraszyk · 2025-03-27T08:55:31Z

@mraszyk would you be able to take a look at the other side of #4515?

What do you mean by "the other side of #4515"?

Bownairo · 2025-03-27T09:00:40Z

@mraszyk would you be able to take a look at the other side of #4515?

What do you mean by "the other side of #4515"?

Ah sorry for being unclear, #4515 set us up to change the binary name in hs/spec_compliance/BUILD.bazel, through hs/spec_compliance/defs.bzl. We wrap these targets in order to transition with disable_hermetic_cc_binary that keeps the haskell build using their current build path, with the host system toolchains.

mraszyk

Rubber-stamp as I'm not a bazel expert.

nmattia

Very exciting! 🥳

This reverts commit [810ddde](810ddde). This new toolchain is causing build performance issues. The root cause is unclear, see uber/hermetic_cc_toolchain#215.

…#4621) This reverts commit [810ddde](810ddde). This new toolchain is causing build performance issues. The root cause is unclear, see uber/hermetic_cc_toolchain#215. --------- Co-authored-by: IDX GitHub Automation <[email protected]>

Following #3508 and #4621 What has changed since last time? - dd17358 - Share the zig cache across targets to improve build speed. `zig cc` builds system components on demand, and the shared cache means these are only built once. - 80f68ab + 31d8822 - Strip debug_info, but preserve symbols, to fix traces. `zig cc` can only strip everything, or nothing at all. First patch `rules_rust` to disable the default stripping, then patch `hermetic_cc_toolchain` to strip only `debug_info`, outside of the main call to `zig cc`. --------- Co-authored-by: Nicolas Mattia <[email protected]> Co-authored-by: IDX GitHub Automation <[email protected]>

github-actions bot added the feat label Jan 17, 2025

Bownairo force-pushed the eero/hermetic-toolchain branch 3 times, most recently from 0d20bba to 5b5b249 Compare January 23, 2025 22:01

Bownairo force-pushed the eero/hermetic-toolchain branch 2 times, most recently from f5bfd00 to b8bf5b9 Compare February 4, 2025 02:08

Bownairo changed the title ~~feat: Build with hermetic cc toolchains~~ feat: [NODE-1566] Build with hermetic cc toolchains Feb 4, 2025

Bownairo force-pushed the eero/hermetic-toolchain branch 3 times, most recently from d10101c to 2c71eb6 Compare February 10, 2025 19:04

Bownairo added the CI_ALL_BAZEL_TARGETS Runs all bazel targets and uploads them to S3 label Feb 10, 2025

Bownairo force-pushed the eero/hermetic-toolchain branch from 2c71eb6 to 5a7bebe Compare February 10, 2025 21:21

Bownairo commented Feb 11, 2025

View reviewed changes

hs/spec_compliance/defs.bzl Show resolved Hide resolved

Bownairo force-pushed the eero/hermetic-toolchain branch from d6748ad to 5504f28 Compare February 12, 2025 19:47

Bownairo force-pushed the eero/hermetic-toolchain branch 2 times, most recently from 7624edc to 396609a Compare March 3, 2025 21:26

Bownairo requested a review from a team March 4, 2025 05:07

venkkatesh-sekar reviewed Mar 4, 2025

View reviewed changes

bazel/external_crates.bzl Show resolved Hide resolved

Bownairo force-pushed the eero/hermetic-toolchain branch from 201cdc8 to 95c5f5d Compare March 5, 2025 21:09

andrewbattat approved these changes Mar 6, 2025

View reviewed changes

Bownairo force-pushed the eero/hermetic-toolchain branch 4 times, most recently from 34e9296 to c9a2286 Compare March 7, 2025 02:14

venkkatesh-sekar approved these changes Mar 7, 2025

View reviewed changes

bazel/fuzz_testing.bzl Outdated Show resolved Hide resolved

nmattia reviewed Mar 7, 2025

View reviewed changes

WORKSPACE.bazel Show resolved Hide resolved

bazel/cc_rs.patch Show resolved Hide resolved

bazel/hermetic_cc_toolchain.patch Show resolved Hide resolved

cpp/BUILD.bazel Outdated Show resolved Hide resolved

hs/spec_compliance/defs.bzl Show resolved Hide resolved

use hermetic toolchains

02f7a03

IDX GitHub Automation and others added 4 commits March 23, 2025 21:53

Automatically updated Cargo*.lock

e9ad1a5

Buildifier

e594a01

Disable hermetic toolchains for macOS

12ba397

Disable tmp speedup for zig

5ab38a7

Bownairo mentioned this pull request Mar 25, 2025

fix: Pass ic-ref-test directly #4515

Merged

Wrap rules_haskell defs at a lower level

f2d0847

Bownairo added 3 commits March 26, 2025 09:13

Merge branch 'master' into eero/hermetic-toolchain

9d33dfc

Fix release stripping

33f03b0

Fix release stripping for macOS

7fec839

mraszyk approved these changes Mar 27, 2025

View reviewed changes

nmattia approved these changes Mar 27, 2025

View reviewed changes

Bownairo and others added 3 commits March 27, 2025 10:47

Drop lockfiles

356cb8f

Merge branch 'master' into eero/hermetic-toolchain

9016c4a

Automatically updated Cargo*.lock

57fbca4

Bownairo added this pull request to the merge queue Mar 27, 2025

Merged via the queue into master with commit 810ddde Mar 27, 2025
20 checks passed

Bownairo deleted the eero/hermetic-toolchain branch March 27, 2025 12:11

Bownairo restored the eero/hermetic-toolchain branch March 27, 2025 15:46

Bownairo deleted the eero/hermetic-toolchain branch March 27, 2025 15:46

nmattia mentioned this pull request Apr 1, 2025

poor build performance compared to host system toolchain uber/hermetic_cc_toolchain#215

Closed

Bownairo mentioned this pull request May 6, 2025

feat: [NODE-1566] Build with hermetic cc toolchains (again) #4848

Merged

feat: [NODE-1566] Build with hermetic cc toolchains #3508

feat: [NODE-1566] Build with hermetic cc toolchains #3508

Uh oh!

Conversation

Bownairo commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bownairo left a comment • 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!

Uh oh!

Bownairo commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

venkkatesh-sekar commented Mar 4, 2025

Uh oh!

Uh oh!

Bownairo commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewbattat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Bownairo commented Mar 27, 2025

Uh oh!

mraszyk commented Mar 27, 2025

Uh oh!

Bownairo commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mraszyk left a comment

Choose a reason for hiding this comment

Uh oh!

nmattia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Bownairo commented Jan 17, 2025 •

edited

Loading

Bownairo left a comment •

edited

Loading

Bownairo commented Mar 4, 2025 •

edited

Loading

Bownairo commented Mar 5, 2025 •

edited

Loading

Bownairo commented Mar 27, 2025 •

edited

Loading