This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Port to 3.1 - Fix bad configure tests #28106
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was an extra -c in the CMAKE_REQUIRED_FLAGS set for testing HAVE_UNW_GET_ACCESSORS and HAVE_UNW_GET_SAVE_LOC that was breaking build of coreclr under homebrew. The option was somehow making these checks behave on ARM Linux, eveb though it is not clear to me why, as it was just causing this option to be passed to the compiler twice at different positions of the command line of the cmake tests. This change fixes it by using check_symbol_exists instead of check_c_source_compiles, since just removing the duplicite -c was resulting in the check failing on ARM / ARM64 Linux due to a missing symbol from libunwind during linking
janvorli
added
area-Infrastructure-coreclr
Servicing-consider
jeffschwMSFT
approved these changes
Oct 5, 2020
|
@Anipik can you consider this when we merge changes for the next servicing release? Thanks |
jeffschwMSFT
added
Servicing-approved
jkotas
approved these changes
Oct 5, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Port of dotnet/runtime#42756
There was an extra -c in the CMAKE_REQUIRED_FLAGS set for testing
HAVE_UNW_GET_ACCESSORS and HAVE_UNW_GET_SAVE_LOC that was breaking
build of coreclr under homebrew. The option was somehow making
these checks behave on ARM Linux, even though it is not clear to
me why, as it was just causing this option to be passed to the
compiler twice at different positions of the command line of
the cmake tests.
This change fixes it by using check_symbol_exists instead of
check_c_source_compiles, since just removing the duplicite -c
was resulting in the check failing on ARM / ARM64 Linux due
to a missing symbol from libunwind during linking.
Customer impact
We've got this issue reported by a customer who was trying to add
brewrecipe for packaging.NET Core 3.1 on OSX.
Regression?
Yes, introduced in 2.1
Testing
I've manually verified that the HAVE_UNW_GET_ACCESSORS and HAVE_UNW_GET_SAVE_LOC (and UNWIND_CONTEXT_IS_UCONTEXT_T that was not broken, but it is in the block of code influenced
by the change) are generated correctly for all the support target platforms / architectures.
Risk
Very low, the change influences only a compile time detection of a presence of two libunwind
functions.