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

Update illumos cross rootfs to fix CoreCLR PAL test #14978

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

AustinWise
Copy link
Contributor

Each commit description contains details about the changes; here is a summary:

  • Update binutils from 2.33.1 to 2.42 and update GCC from 8.4.0 to 13.3.0. Without this change, libstc++ would fail to find the handler when throwing the PAL_SEHException in the CoreCLR PAL test paltest_pal_sxs_test1. These versions match those used in OpenIndiana 2024.04.
  • Use the 2019 versions of pkgsrc packages. This fixes a linking problem where new versions of pkgsrc packages are compiled with GCC's stack protector which is not supported in the 2018 version of the illumos sysroot we are using.

These commits are extract from dotnet/runtime#105207

Contributes to dotnet/runtime#35362

CC @am11 @gwr

Currently the script uses an illumos sysroot from 2018 while downloading
the latest available packages from pkgsrc. These packages are currently
built on a newer illumos from 2021 that supports GCC's stack protector.
When using this rootfs to build, we get error like this:

> .tools/rootfs/x64/x86_64-sun-solaris2.10/bin/ld: .tools/rootfs/x64/lib/libz.so: undefined reference to '__stack_chk_fail@ILLUMOS_0.37'

This fixes the mismatch by using an older set of packages from 2019.
This fixes the paltest exception_handling/pal_sxs/test1. It was crashing
when throwing the PAL_SEHException. The C++ runtime was able to find EH
data for ThrowExceptionHelper but not for FailingFunction.

These versions match the versions used in OpenIndiana currently.
@am11
Copy link
Member

am11 commented Jul 25, 2024

Assuming you tested it by deleting the existing /crossrootfs/x64 in ubuntu-22.04-cross-illumos image and building runtime with it, looks good.

@AustinWise
Copy link
Contributor Author

I did most of testing in a non containerized build. I ran the PAL tests and the System.Runtime.Tests from libraries.

I also confirmed that I can build the pal tests in container. I used the following steps:

First build the rootfs on my Ubuntu 22.04 machine:

./eng/common/cross/build-rootfs.sh x64 illumos

Then I put this Dockerfile in .tools/ :

FROM mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-cross-illumos
RUN rm -r /crossrootfs/
COPY rootfs/x64 /crossrootfs/x64

Built my own image from the .tools directory:

docker build . -t austin-cross-illumos

Then used my new image to build the paltests from the root of the runtime repo:

docker run --rm -it -v$(pwd):/runtime -e ROOTFS_DIR=/crossrootfs/x64 austin-cross-illumos \
  /runtime/build.sh clr.paltests -c Debug -gcc --keepnativesymbols true -cross -os illumos

These PAL tests passed on OpenIndiana (save the tests that were failing before this change: paltest_vsnprintf_test4 and paltest_duplicatehandle_test7).

@am11
Copy link
Member

am11 commented Jul 25, 2024

Thank you! I think this is enough for testing.
I am not sure how well is sunos2.11 support on llvm right now, but a while back I had a discussion with SmartOS folks who told me that it is supported. I will try to bring it to the same plan later when I get some free cycles.

@gwr
Copy link

gwr commented Jul 25, 2024

Thank you! I think this is enough for testing. I am not sure how well is sunos2.11 support on llvm right now, but a while back I had a discussion with SmartOS folks who told me that it is supported. I will try to bring it to the same plan later when I get some free cycles.

I can answer that. We have several versions of clang delivered on OmniOS and OpenIndiana (OI).
On OI 2024.04 I see these versions: 15.0 17.0 18.1

Notably "lldb" is missing (I'm working on that as I have time).

@am11
Copy link
Member

am11 commented Jul 25, 2024

Notably "lldb" is missing (I'm working on that as I have time).

Yup that's one of the main attraction since libSOS gdb support is primarily/untested (dotnet/diagnostics#272 (comment)).

@gwr
Copy link

gwr commented Jul 25, 2024

Looks like you should squash. Otherwise LGTM. I don't see a "review" button...

@am11
Copy link
Member

am11 commented Jul 25, 2024

Squash is enabled on merge for maintainers. None of us work for Microsoft (maintainers of github.com/dotnet).

@am11
Copy link
Member

am11 commented Jul 25, 2024

cc @akoeplinger

@akoeplinger akoeplinger merged commit 485c9eb into dotnet:main Jul 25, 2024
11 checks passed
@akoeplinger
Copy link
Member

Thank you!

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.

4 participants