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

[release/5.0] Re-enable support for using the system libunwind #42853

Merged
merged 1 commit into from
Oct 2, 2020

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 29, 2020

Backport of #42689 to release/5.0

/cc @agocke @omajid

Customer Impact

Source build uses the system libunwind, so without this change the singlefilehost cannot build.

Testing

source build has been tested with the change, and succeeds.

Risk

Low, passing existing options for the CoreCLR build to the singlefilehost build in the same way.

We can now build runtime against the system libunwind using:

    ./build.sh -cmakeargs -DCLR_CMAKE_USE_SYSTEM_LIBUNWIND

This allows Linux distributions that already ship a compatible version
of libunwind library to use that instead of carrying a duplicate in
.NET. This provides some benefits to them, including smaller build
sizes, slightly faster builds and fewer places to fix any
vulnerabilities

This functionality was already supported in .NET Core 3.1 and has
regressed since.

CoreCLR already handles `-DCLR_CMAKE_USE_SYSTEM_LIBUNWIND`, so no
changes are needed there.

The libraries build doesn't care about this cmake varibale, but cmake
itself fails if the variable is not used:

    EXEC : CMake error : [runtime/src/libraries/Native/build-native.proj]
        Manually-specified variables were not used by the project:

          CLR_CMAKE_USE_SYSTEM_LIBUNWIND

So libraries just needs to check and ignore this variable.

The singlefilehost needs to link against libunwind too. Otherwise the
linker fails to resolve symbols, making the build fail:

    /usr/bin/ld: runtime/artifacts/bin/coreclr/Linux.x64.Debug//lib/libcoreclrpal.a(seh.cpp.o): in function `UnwindContextToWinContext(unw_cursor*, _CONTEXT*)':
      dotnet/runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:176: undefined reference to `_ULx86_64_get_reg'
    /usr/bin/ld: runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:177: undefined reference to `_ULx86_64_get_reg'
    /usr/bin/ld: runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:178: undefined reference to `_ULx86_64_get_reg'
    /usr/bin/ld: runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:179: undefined reference to `_ULx86_64_get_reg'
    /usr/bin/ld: runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:180: undefined reference to `_ULx86_64_get_reg'
    /usr/bin/ld: runtime/artifacts/bin/coreclr/Linux.x64.Debug//lib/libcoreclrpal.a(seh.cpp.o): runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:181: more undefined references to `_ULx86_64_get_reg' follow
    /usr/bin/ld: runtime/artifacts/bin/coreclr/Linux.x64.Debug//lib/libcoreclrpal.a(seh.cpp.o): in function `GetContextPointer(unw_cursor*, ucontext_t*, int, unsigned long**)':
      runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:227: undefined reference to `_ULx86_64_get_save_loc'
    /usr/bin/ld: runtime/artifacts/bin/coreclr/Linux.x64.Debug//lib/libcoreclrpal.a(seh.cpp.o): in function `PAL_VirtualUnwind':
      runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:340: undefined reference to `_ULx86_64_init_local'
    /usr/bin/ld: runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:351: undefined reference to `_ULx86_64_step'
    /usr/bin/ld: runtime/src/coreclr/src/pal/src/exception/seh-unwind.cpp:360: undefined reference to `_ULx86_64_is_signal_frame'
    clang-10: error: linker command failed with exit code 1 (use -v to see invocation)

Fixes: #42661
@agocke
Copy link
Member

agocke commented Oct 1, 2020

@jeffschwMSFT This has already been approved, right?

@jeffschwMSFT
Copy link
Member

@agocke once we have verified and feel this is good, we can merge as part of tell mode for GA.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

Approved. We should take this as tell mode in GA

@agocke agocke merged commit fd61375 into release/5.0 Oct 2, 2020
@agocke agocke deleted the backport/pr-42689-to-release/5.0 branch October 2, 2020 00:47
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
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.

5 participants