Re-enable support for using the system libunwind #42689

omajid · 2020-09-24T16:50:07Z

You 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 variable, 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

omajid · 2020-09-24T16:50:26Z

cc @janvorli

omajid · 2020-09-24T16:51:13Z

~~I am still trying to test this end-to-end.~~

Consuming via source-build, things look good here:

$ ldd source-build/dotnet-sdk/packs/Microsoft.NETCore.App.Host.fedora.32-x64/5.0.0-preview.8.20407.11/runtimes/fedora.32-x64/native/singlefilehost | grep libunwind
        libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8 (0x00007f05853d0000)
        libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f05853b6000)
$ ldd source-build/dotnet-sdk/shared/Microsoft.NETCore.App/5.0.0-preview.8.20407.11/libcoreclr.so | grep unwind
        libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8 (0x00007f2a4f412000)
        libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f2a4f3f8000)

omajid · 2020-09-24T19:31:31Z

src/libraries/Native/Unix/configure.cmake

source-build builds each component (coreclr, libraries, installer) separately and passes a separate set of args. It can skip passing -DCLR_CMAKE_USE_SYSTEM_LIBUNWIND to libraries. But this seems like a nice to have to make it possible to build all of runtime at once using the main ./build.sh script in one call.

It is kind of unfortunate that the main build script only supports single set of cmake args that it pushes to all three sub-parts (coreclr, installer, libraries). It would be nice to enhance it to enable specifying these flags separately for coreclr, installer and libraries so that we don't have to do tricks like this. But for this PR, it is fine as is.

I filed #42711 for tracking that

This includes a backport of dotnet/runtime#42689

janvorli

LGTM modulo the nit

janvorli · 2020-09-24T22:30:19Z

src/libraries/Native/Unix/configure.cmake

It is kind of unfortunate that the main build script only supports single set of cmake args that it pushes to all three sub-parts (coreclr, installer, libraries). It would be nice to enhance it to enable specifying these flags separately for coreclr, installer and libraries so that we don't have to do tricks like this. But for this PR, it is fine as is.

janvorli · 2020-09-24T22:38:06Z

src/installer/corehost/cli/apphost/static/CMakeLists.txt

A nit - can you please delete the now empty if / endif and move the comment above to the location below where you have placed the libunwind finding?

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: dotnet#42661

ghost · 2020-09-25T00:26:51Z

Hello @janvorli!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

omajid · 2020-09-25T13:54:15Z

Thanks for reviewing and merging this!

This includes a backport of dotnet/runtime#42689

jeffschwMSFT · 2020-09-28T17:20:48Z

@omajid is this needed for .NET 5 GA?

omajid · 2020-09-28T17:50:33Z

We (Red Hat) would definitely like to make use of this feature in .NET 5 GA release. We are using it in 3.1 and this is a regression from .NET Core 3.1 to .NET 5.

Ideally we would add it to 5.0 GA (or post GA if GA is absolutely frozen for now).

If this doesn't get added to 5.0 GA, we still have some options: we can carry this as a patch in source-build or in our RPM packages. But that gets reviewed less thoroughly and becomes harder to maintain as runtime gets additional fixes.

I think we should decide to handle this issue consistently with #42094 and #42415: either all go to 5.0 or none of them go to 5.0 GA.

jeffschwMSFT · 2020-09-28T18:00:02Z

@dleeapho thoughts?

agocke · 2020-09-29T17:44:49Z

/backport to release/5.0

github-actions · 2020-09-29T17:45:59Z

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/279015832

This includes a backport of dotnet/runtime#42689

Dotnet-GitSync-Bot added the area-Infrastructure-coreclr label Sep 24, 2020

omajid commented Sep 24, 2020

View reviewed changes

omajid added a commit to omajid/dotnet-source-build that referenced this pull request Sep 24, 2020

Re-enable system libunwind support

19011ba

This includes a backport of dotnet/runtime#42689

This was referenced Sep 24, 2020

Agents canceling during dotnet builds jaredpar/runfo#47

Closed

Azure Agent timeout dotnet/roslyn#48030

Closed

janvorli approved these changes Sep 24, 2020

View reviewed changes

omajid mentioned this pull request Sep 24, 2020

Support per-component (coreclr, installer, etc) cmake args #42711

Open

omajid force-pushed the system-libunwind-again branch from 333b6b5 to 5ed5582 Compare September 25, 2020 00:23

janvorli approved these changes Sep 25, 2020

View reviewed changes

janvorli added the auto-merge label Sep 25, 2020

ghost merged commit 5130c6d into dotnet:master Sep 25, 2020

omajid deleted the system-libunwind-again branch September 25, 2020 20:56

omajid mentioned this pull request Sep 25, 2020

Re-enable system libunwind support dotnet/source-build#1761

Merged

omajid added a commit to omajid/dotnet-source-build that referenced this pull request Sep 28, 2020

Re-enable system libunwind support

17dc345

This includes a backport of dotnet/runtime#42689

github-actions bot mentioned this pull request Sep 29, 2020

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

Merged

runfoapp bot mentioned this pull request Sep 30, 2020

Mono System.Text.Json.Tests on Windows timing out #42677

Open

omajid added a commit to omajid/dotnet-source-build that referenced this pull request Oct 28, 2020

Re-enable system libunwind support

b83635a

This includes a backport of dotnet/runtime#42689

omajid added a commit to omajid/dotnet-source-build that referenced this pull request Nov 16, 2020

Re-enable system libunwind support

b766e47

This includes a backport of dotnet/runtime#42689

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

This pull request was closed.

Re-enable support for using the system libunwind #42689

Re-enable support for using the system libunwind #42689

Uh oh!

Conversation

omajid commented Sep 24, 2020

Uh oh!

omajid commented Sep 24, 2020

Uh oh!

omajid commented Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omajid Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

janvorli Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

omajid Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

janvorli left a comment

Choose a reason for hiding this comment

Uh oh!

janvorli Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

janvorli Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 25, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

omajid commented Sep 25, 2020

Uh oh!

jeffschwMSFT commented Sep 28, 2020

Uh oh!

omajid commented Sep 28, 2020

Uh oh!

jeffschwMSFT commented Sep 28, 2020

Uh oh!

agocke commented Sep 29, 2020

Uh oh!

github-actions bot commented Sep 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

omajid commented Sep 24, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.