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.

Sign up for GitHub

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

Reapply [compiler-rt] Check for and use -lunwind when linking with -nodefaultlibs #66584

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Reapply [compiler-rt] Check for and use -lunwind when linking with -nodefaultlibs #66584

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions compiler-rt/cmake/config-ix.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ if (C_SUPPORTS_NODEFAULTLIBS_FLAG)
moldname mingwex msvcrt)
list(APPEND CMAKE_REQUIRED_LIBRARIES ${MINGW_LIBRARIES})
endif()
if (NOT TARGET unwind)
# Don't check for a library named unwind, if there's a target with that name within
# the same build.
check_library_exists(unwind _Unwind_GetRegionStart "" COMPILER_RT_HAS_LIBUNWIND)
if (COMPILER_RT_HAS_LIBUNWIND)
# If we're omitting default libraries, we might need to manually link in libunwind.
# This can affect whether we detect a statically linked libc++ correctly.
list(APPEND CMAKE_REQUIRED_LIBRARIES unwind)
Copy link

Choose a reason for hiding this comment

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

Note that one of the implications of this change is that for example on x86_64 test for __i386__ symbol existence will fail if 32-bit version of libunwind is not installed:

llvm-project/compiler-rt/cmake/base-config-ix.cmake

Line 209 in 43b2b2e

test_target_arch(i386 __i386__ "-m32")

Although 32-bit libunwind does not seem to be required.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, so you have an x86_64 libunwind, and enough 32 bit libraries, so that compilation with -m32 works, but as we detected the x86_64 libunwind we now also require linking libunwind in all other tests, which makes the test for -m32 fail suddenly.

That's unfortunate...

@petrhosek Do you have any good ideas about how to handle this? In principle, I think the whole handling of multiple archs (like compiler-rt does, when building both x86_64 and i386 in the same cmake invocation) to be kind of wrong - ideally each arch build should be a separate cmake configuration of all the runtimes, so that all the tests are done specifically for that arch. But untangling those bits of compiler-rt is outside of what I want to take on.

endif()
endif()
endif ()

# CodeGen options.
Expand Down