[PATCH] D75065: [cmake] Strip quotes in compiler-rt/lib/crt and explicitly throw error if check executable fails

[Zhizhou Yang via Phabricator via llvm-commits]

zhizhouy updated this revision to Diff 247777.
zhizhouy retitled this revision from "[cmake] Strip quotes in compiler-rt/lib/crt" to "[cmake] Strip quotes in compiler-rt/lib/crt and explicitly throw error if check executable fails".
zhizhouy added a comment.

> (1) I'm not sure why PGO options are passed to compiler-rt. It's probably harmless for -fprofile-use or -fprofile-instr-use as we only use the profile. My concern is we might instrument profdata runtime routine. This can create recursion that leads to run time issue.

I created a new patch to not pass PGO related options to compiler-rt: D75499 <https://reviews.llvm.org/D75499>

> (2) As I mentioned in (1), passing an invalid profile does not suppose to cause error in section. Silently ignoring the option should be a noop. The error seems to be incheck_cxx_section_exists() logic. In this sense, this patch does not seem to be complete to me: For example, if we pass an invalid profile path (after stripping the "), will it still fail?

Seems that if the profile path is not correct, it will be filtered before getting passed into compiler-rt, so your example should not happen. But it is not good to fail silently, so I added a check for it.


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

https://reviews.llvm.org/D75065

Files:
  compiler-rt/lib/crt/CMakeLists.txt


Index: compiler-rt/lib/crt/CMakeLists.txt
===================================================================
--- compiler-rt/lib/crt/CMakeLists.txt
+++ compiler-rt/lib/crt/CMakeLists.txt
@@ -43,6 +43,10 @@
     endif()
   endforeach()
 
+  # Strip quotes from the compile command, as the compiler is not expecting
+  # quoted arguments (potential quotes added from D62063).
+  string(REPLACE "\"" "" test_compile_command "${test_compile_command}")
+
   string(REPLACE " " ";" test_compile_command "${test_compile_command}")
 
   execute_process(
@@ -52,6 +56,12 @@
     ERROR_VARIABLE TEST_ERROR
   )
 
+  # Explicitly throw a fatal error message if test_compile_command fails.
+  if(TEST_RESULT)
+    message(FATAL_ERROR "${TEST_ERROR}")
+    return()
+  endif()
+
   execute_process(
     COMMAND ${CMAKE_OBJDUMP} -h "${TARGET_NAME}/CheckSectionExists.o"
     RESULT_VARIABLE CHECK_RESULT


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75065.247777.patch
Type: text/x-patch
Size: 892 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200303/dbdec2bb/attachment.bin>