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

Build on CBL-mariner host with rootfs #84148

Merged
merged 88 commits into from
Apr 12, 2023
Merged

Build on CBL-mariner host with rootfs #84148

merged 88 commits into from
Apr 12, 2023

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Mar 30, 2023

This changes the linux builds to use the CBL-mariner build images added in dotnet/dotnet-buildtools-prereqs-docker#832. All of these builds are now cross-builds with a rootfs (including x64, and x64 musl).

  • The new images intentionally don't have binutils, so our build jobs have been updated to use llvm-based tools. This includes lld when we run nativeaot over our tests. This leads to a slight size regression, I believe due to ILCompiler not supporting --gc-sections with lld (see Add LinkerFlavor to NativeAOT #83558 (comment)). cc @MichalStrehovsky
  • This change turns off PGO optimization because it was hitting a compiler crash when consuming and old version of profile data produced by clang9 instrumented builds (https://bugzilla.redhat.com/show_bug.cgi?id=1827282). The intention is to quickly flow the change into our optimization repo, then collect and ingest the new data so we can turn optimization back on.
  • The Mono LLVMAot test build is kept on the old build images, because they run the aot compiler which depends on binutils. We can probably move it to the same plan as the other legs, but I'm not familiar with the context here. cc @akoeplinger. We would at least need to fix AS_NAME here:
    #define AS_NAME "as"
  • Mono arm64 product build is kept on the old images due to Unable to compile mono arm64 with clang12 #84503.

Contributes to #83428.

janvorli and others added 20 commits February 9, 2023 11:19
This change enables cross-building for Alpine x64 on other x64 distro.
Necessary because pkg-config on mariner is a shim for
pkgconf, which doesn't support override variables.
- Use lld for LTO instead of ld.gold
- Set cross toolchain options for x64 mono build
- Prefer llvm-objcopy for mono symbol stripping
Our custom LLVM build seems to have different defaults when targeting
ARM than the clang-12 available in ubuntu.
Probably related to
https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/commit/b7363248b115339c4fb838fcd3ae43671eedae0a.

Passing the -march explicitly should not hurt.
Allow text relocations. The lld defaults differ from ld, which will add
text relocations on demand by default.

See https://maskray.me/blog/2020-12-19-lld-and-gnu-linker-incompatibilities
To locate gcc toolchain in target sysroot, when host arch matches target arch.
@ghost
Copy link

ghost commented Mar 30, 2023

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: sbomer
Assignees: sbomer
Labels:

area-Infrastructure-coreclr

Milestone: -

eng/Subsets.props Outdated Show resolved Hide resolved
src/coreclr/pgosupport.cmake Outdated Show resolved Hide resolved
@am11
Copy link
Member

am11 commented Apr 13, 2023

Since this is merged with the LinkerFlavor=lld change, despite the discussion in #84493, let me reiterate:

lld in NativeAOT was supposed to be experimental / optional. Switching CI to lld wholesale, while default linker in NativeAOT build integration for user is still bfd, is a strange move. The size of bfd linker in binary form is 1.5 MB on linux-arm64, while I can see there are number of unused tools present in the CBL image as part of llvm-toolchian. We are not saving or simplifying anything by omitting the tiny bfd linker binary from the CBL image. bfd is the default linker on mainstream distros and an important component to skip from official .NET builder images.

Also, this is less about "the size of published binary increases with lld compared to bfd" and more about "testing the product in CI with what the end-user on mainstream distros gets as default". lld is not installed as part of llvm package on Debian, Alpine etc. either, it is a a completely separate package for reasons. lld is a "compatibility linker" for bfd. See https://maskray.me/blog/2020-12-19-lld-and-gnu-linker-incompatibilities which explain that while they are trying to close the gaps in some areas with newer versions of lld, they intend to diverge from bfd in others areas.

We should revert LinkerFlavor=lld changes made in this PR for CI or at least understand the implications before switching to something which was added as experimental a few weeks ago.

BTW, runtime-libraries enterprise-linux leg, which was skipped in this PR, started failing:

clang(0,0): error : (NETCORE_ENGINEERING_TELEMETRY=Build) invalid linker name in argument '-fuse-ld=lld'

https://dev.azure.com/dnceng-public/public/_build/results?buildId=237624&view=logs&j=03b95830-93c2-599b-7e4d-1efb940adc28&t=9abf97cd-b4a3-5a26-6cf2-d05ce865f2ef

BruceForstall pushed a commit to dotnet/jitutils that referenced this pull request Apr 14, 2023
Fixes dotnet/runtime#84780. Tested by hooking
up the changes locally with runtime's `jitformat.py` and running
`python3 ./src/coreclr/scripts/jitformat.py -c /runtime/src/coreclr -o
linux -a x64` in our mariner build container.

This might break scenarios where jitutils are used outside of ci on a
non-mariner build image. Per discussion with @BruceForstall let's make a
quick fix to unblock ci, and if this is problematic we could for example
add an extra argument that controls whether the `-cross` argument is
passed to the runtime build.

For context, the `-cross` argument is necessary after
dotnet/runtime#84148 because our linux build
images have change from centos 7 to CBL-mariner, and the linux builds
are now cross-builds that use a rootfs (even x64 and x64-musl).
@rzikm
Copy link
Member

rzikm commented Apr 19, 2023

This PR (or a related one) breaks System.Net.Security.Enterprise.Test builds (https://dev.azure.com/dnceng-public/public/_build/results?buildId=243440&view=logs&j=03b95830-93c2-599b-7e4d-1efb940adc28&t=9abf97cd-b4a3-5a26-6cf2-d05ce865f2ef&l=3907).

clang : error : invalid linker name in argument '-fuse-ld=lld' [/repo/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj]

The docker image used (mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04) has clang-9 but no lld binary installed. Will that be fixed by the TODO at
https://github.com/sbomer/runtime/blob/92d1f2217649103e3f3e5121b8ad35dca5a81bbb/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj#L62? Or is some additional detection needed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants