[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

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.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55891/new/

https://reviews.llvm.org/D55891



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Paul Mulders via Phabricator via cfe-commits]

justinkb added a comment.

In D55891#1751610 , @mgorny wrote:

> 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


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55891/new/

https://reviews.llvm.org/D55891



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

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


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55891/new/

https://reviews.llvm.org/D55891



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Paul Mulders via Phabricator via cfe-commits]

justinkb reopened this revision.
justinkb added a comment.
This revision is now accepted and ready to land.
Herald added a project: LLVM.

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.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55891/new/

https://reviews.llvm.org/D55891



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz added a comment.

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 .


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55891/new/

https://reviews.llvm.org/D55891



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349899: [xray] [tests] Detect and handle missing 
LLVMTestingSupport gracefully (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D55891?vs=178896&id=179275#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55891/new/

https://reviews.llvm.org/D55891

Files:
  compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
  compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt


Index: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
@@ -239,7 +239,7 @@
 # Detect if we have the LLVMXRay and TestingSupport library installed and
 # available from llvm-config.
 execute_process(
-  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray" "testingsupport"
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray"
   RESULT_VARIABLE HAD_ERROR
   OUTPUT_VARIABLE CONFIG_OUTPUT)
 if (HAD_ERROR)
@@ -254,6 +254,26 @@
   set(COMPILER_RT_HAS_LLVMXRAY TRUE)
 endif()
 
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT FALSE)
+execute_process(
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "testingsupport"
+  RESULT_VARIABLE HAD_ERROR
+  OUTPUT_VARIABLE CONFIG_OUTPUT)
+if (HAD_ERROR)
+  message(WARNING "llvm-config finding testingsupport failed with status 
${HAD_ERROR}")
+else()
+  string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT 
${CONFIG_OUTPUT})
+  list(GET CONFIG_OUTPUT 0 LDFLAGS)
+  list(GET CONFIG_OUTPUT 1 LIBLIST)
+  if (LIBLIST STREQUAL "")
+message(WARNING "testingsupport library not installed, some tests will 
be skipped")
+  else()
+set(LLVM_TESTINGSUPPORT_LDFLAGS ${LDFLAGS} CACHE STRING "Linker flags 
for LLVMTestingSupport library")
+set(LLVM_TESTINGSUPPORT_LIBLIST ${LIBLIST} CACHE STRING "Library list 
for LLVMTestingSupport")
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT TRUE)
+  endif()
+endif()
+
 # Make use of LLVM CMake modules.
 # --cmakedir is supported since llvm r291218 (4.0 release)
 execute_process(
Index: compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt
===
--- compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt
+++ compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt
@@ -1,10 +1,16 @@
-add_xray_unittest(XRayTest SOURCES
+set(TEST_SOURCES
   allocator_test.cc
   buffer_queue_test.cc
-  fdr_controller_test.cc
-  fdr_log_writer_test.cc
   function_call_trie_test.cc
   profile_collector_test.cc
   segmented_array_test.cc
   test_helpers.cc
   xray_unit_test_main.cc)
+
+if (NOT COMPILER_RT_STANDALONE_BUILD OR COMPILER_RT_HAS_LLVMTESTINGSUPPORT)
+  list(APPEND TEST_SOURCES
+fdr_controller_test.cc
+fdr_log_writer_test.cc)
+endif()
+
+add_xray_unittest(XRayTest SOURCES ${TEST_SOURCES})
Index: compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
===
--- compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
+++ compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
@@ -60,6 +60,10 @@
   if (COMPILER_RT_STANDALONE_BUILD)
 append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LDFLAGS} 
XRAY_UNITTEST_LINK_FLAGS)
 append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LIBLIST} 
XRAY_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
+  ${LLVM_TESTINGSUPPORT_LDFLAGS} XRAY_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
+  ${LLVM_TESTINGSUPPORT_LIBLIST} XRAY_UNITTEST_LINK_FLAGS)
   else()
 # We add the library directories one at a time in our CFLAGS.
 foreach (DIR ${LLVM_LIBRARY_DIR})


Index: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
@@ -239,7 +239,7 @@
 # Detect if we have the LLVMXRay and TestingSupport library installed and
 # available from llvm-config.
 execute_process(
-  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray" "testingsupport"
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray"
   RESULT_VARIABLE HAD_ERROR
   OUTPUT_VARIABLE CONFIG_OUTPUT)
 if (HAD_ERROR)
@@ -254,6 +254,26 @@
   set(COMPILER_RT_HAS_LLVMXRAY TRUE)
 endif()
 
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT FALSE)
+execute_process(
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "testingsupport"
+  RESULT_VARIABLE HAD_ERROR
+  OUTPUT_VARIABLE CONFIG_OUTPUT)
+if (HAD_ERROR)
+  message(WARNING "llvm-config 

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Dean Michael Berris via Phabricator via cfe-commits]

dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.

LGTM

Thanks, @mgorny!


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55891/new/

https://reviews.llvm.org/D55891



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, dberris, george.karpenkov.
Herald added subscribers: Sanitizers, llvm-commits.

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.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D55891

Files:
  cmake/Modules/CompilerRTUtils.cmake
  lib/xray/tests/CMakeLists.txt
  lib/xray/tests/unit/CMakeLists.txt


Index: lib/xray/tests/unit/CMakeLists.txt
===
--- lib/xray/tests/unit/CMakeLists.txt
+++ lib/xray/tests/unit/CMakeLists.txt
@@ -1,10 +1,16 @@
-add_xray_unittest(XRayTest SOURCES
+set(TEST_SOURCES
   allocator_test.cc
   buffer_queue_test.cc
-  fdr_controller_test.cc
-  fdr_log_writer_test.cc
   function_call_trie_test.cc
   profile_collector_test.cc
   segmented_array_test.cc
   test_helpers.cc
   xray_unit_test_main.cc)
+
+if (NOT COMPILER_RT_STANDALONE_BUILD OR COMPILER_RT_HAS_LLVMTESTINGSUPPORT)
+  list(APPEND TEST_SOURCES
+fdr_controller_test.cc
+fdr_log_writer_test.cc)
+endif()
+
+add_xray_unittest(XRayTest SOURCES ${TEST_SOURCES})
Index: lib/xray/tests/CMakeLists.txt
===
--- lib/xray/tests/CMakeLists.txt
+++ lib/xray/tests/CMakeLists.txt
@@ -60,6 +60,10 @@
   if (COMPILER_RT_STANDALONE_BUILD)
 append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LDFLAGS} 
XRAY_UNITTEST_LINK_FLAGS)
 append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LIBLIST} 
XRAY_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
+  ${LLVM_TESTINGSUPPORT_LDFLAGS} XRAY_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
+  ${LLVM_TESTINGSUPPORT_LIBLIST} XRAY_UNITTEST_LINK_FLAGS)
   else()
 # We add the library directories one at a time in our CFLAGS.
 foreach (DIR ${LLVM_LIBRARY_DIR})
Index: cmake/Modules/CompilerRTUtils.cmake
===
--- cmake/Modules/CompilerRTUtils.cmake
+++ cmake/Modules/CompilerRTUtils.cmake
@@ -239,7 +239,7 @@
 # Detect if we have the LLVMXRay and TestingSupport library installed and
 # available from llvm-config.
 execute_process(
-  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray" "testingsupport"
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray"
   RESULT_VARIABLE HAD_ERROR
   OUTPUT_VARIABLE CONFIG_OUTPUT)
 if (HAD_ERROR)
@@ -254,6 +254,26 @@
   set(COMPILER_RT_HAS_LLVMXRAY TRUE)
 endif()
 
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT FALSE)
+execute_process(
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "testingsupport"
+  RESULT_VARIABLE HAD_ERROR
+  OUTPUT_VARIABLE CONFIG_OUTPUT)
+if (HAD_ERROR)
+  message(WARNING "llvm-config finding testingsupport failed with status 
${HAD_ERROR}")
+else()
+  string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT 
${CONFIG_OUTPUT})
+  list(GET CONFIG_OUTPUT 0 LDFLAGS)
+  list(GET CONFIG_OUTPUT 1 LIBLIST)
+  if (LIBLIST STREQUAL "")
+message(WARNING "testingsupport library not installed, some tests will 
be skipped")
+  else()
+set(LLVM_TESTINGSUPPORT_LDFLAGS ${LDFLAGS} CACHE STRING "Linker flags 
for LLVMTestingSupport library")
+set(LLVM_TESTINGSUPPORT_LIBLIST ${LIBLIST} CACHE STRING "Library list 
for LLVMTestingSupport")
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT TRUE)
+  endif()
+endif()
+
 # Make use of LLVM CMake modules.
 # --cmakedir is supported since llvm r291218 (4.0 release)
 execute_process(


Index: lib/xray/tests/unit/CMakeLists.txt
===
--- lib/xray/tests/unit/CMakeLists.txt
+++ lib/xray/tests/unit/CMakeLists.txt
@@ -1,10 +1,16 @@
-add_xray_unittest(XRayTest SOURCES
+set(TEST_SOURCES
   allocator_test.cc
   buffer_queue_test.cc
-  fdr_controller_test.cc
-  fdr_log_writer_test.cc
   function_call_trie_test.cc
   profile_collector_test.cc
   segmented_array_test.cc
   test_helpers.cc
   xray_unit_test_main.cc)
+
+if (NOT COMPILER_RT_STANDALONE_BUILD OR COMPILER_RT_HAS_LLVMTESTINGSUPPORT)
+  list(APPEND TEST_SOURCES
+fdr_controller_test.cc
+fdr_log_writer_test.cc)
+endif()
+
+add_xray_unittest(XRayTest SOURCES ${TEST_SOURCES})
Index: lib/xray/tests/CMakeLists.txt
=