Page MenuHomePhabricator

[compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully
AcceptedPublic

Authored by mgorny on Dec 19 2018, 9:17 AM.

Details

Summary

Add a code to properly test for presence of LLVMTestingSupport library
when performing a stand-alone build, and skip tests requiring it when
it is not present. Since the library is not installed, llvm-config
reported empty --libs for it and the tests failed to link with undefined
references. Skipping the two fdr_* test files is better than failing to
build, and should be good enough until we find a better solution.

NB: both installing LLVMTestingSupport and building it automatically
from within compiler-rt sources are non-trivial. The former due to
dependency on gtest, the latter due to tight integration with LLVM
source tree.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Dec 19 2018, 9:17 AM
Herald added subscribers: Restricted Project, llvm-commits. · View Herald TranscriptDec 19 2018, 9:17 AM
dberris accepted this revision.Dec 20 2018, 6:59 PM

LGTM

Thanks, @mgorny!

This revision is now accepted and ready to land.Dec 20 2018, 6:59 PM
This revision was automatically updated to reflect the committed changes.

In case compiler-rt abandons llvm-config in favor of find_package(LLVM) one day, we could drop the COMPILER_RT_HAS_LLVMTESTINGSUPPORT logic here and use imported target properties instead. It seems a cleaner solution to me and avoids issues like D57521.

justinkb reopened this revision.Nov 19 2019, 5:51 AM
justinkb added a subscriber: justinkb.

The check doesn't work correctly if LLVM is built as a dylib and the LLVMTestingSupport static library isn't installed (which it normally wouldn't be)

On my machine llvm-config --components (correctly) doesn't print testingsupport, but since llvm-config --libs "testingsupport" does return "-lLLVM-9" since "If LLVM_LINK_DYLIB is ON, the single shared library will be return for "--libs" (quote from llvm-config source). Since testingsupport never seems to get built into the dylib, we end up with the failing check.

This revision is now accepted and ready to land.Nov 19 2019, 5:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2019, 5:51 AM

That sounds like a bug in llvm-config. It shouldn't really return dylib for components that aren't part of it.

That sounds like a bug in llvm-config. It shouldn't really return dylib for components that aren't part of it.

I don't disagree. Still, the check is relying on correct behavior the tool apparently doesn't have

I've tried fixing llvm-config but that's non-trivial. If I fix it to correctly recognize which components are included in dylib, and return other libraries directly, it just uncovers other problems.

Please see if D71613 fixes your problem.