Page MenuHomePhabricator

Fix stack_trace_compressor builds for Clang < 6.0
ClosedPublic

Authored by hctim on Fri, Aug 23, 1:33 PM.

Details

Summary

Clang 4.* doesn't supply -fsanitize=fuzzer, and Clang 5.* doesn't supply
-fsanitize=fuzzer-no-link. Generally, in LLVM, fuzz targets are added through
the add_llvm_fuzzer build rule, which can't be used in compiler-rt (as it has
to be able to be standalone built).

Instead of adding tooling to add a dummy main (which kind of defeats the
purpose of these fuzz targets), we instead build the fuzz target only when the
Clang version is >= 6.*.

Diff Detail

Repository
rL LLVM

Event Timeline

hctim created this revision.Fri, Aug 23, 1:33 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFri, Aug 23, 1:33 PM
Herald added subscribers: llvm-commits, Restricted Project, mgorny. · View Herald Transcript
tejohnson accepted this revision.Fri, Aug 23, 3:29 PM

LGTM. Fixes my build issue!

This revision is now accepted and ready to land.Fri, Aug 23, 3:29 PM
This revision was automatically updated to reflect the committed changes.

@hctim

On Ubuntu Xenial with cmake 3.5.1, it fails with:

Make Error at projects/compiler-rt/lib/gwp_asan/CMakeLists.txt:108 (if):
  if given arguments:

    "COMPILER_RT_BUILD_LIBFUZZER" "AND" "GNU" "STREQUAL" "Clang" "AND" "CMAKE_CXX_COMPILER_VERSION" "VERSION_GREATER_EQUAL" "6.0"

  Unknown arguments specified

Any idea how to fix this? Thanks

@hctim

On Ubuntu Xenial with cmake 3.5.1, it fails with:

Make Error at projects/compiler-rt/lib/gwp_asan/CMakeLists.txt:108 (if):
  if given arguments:

    "COMPILER_RT_BUILD_LIBFUZZER" "AND" "GNU" "STREQUAL" "Clang" "AND" "CMAKE_CXX_COMPILER_VERSION" "VERSION_GREATER_EQUAL" "6.0"

  Unknown arguments specified

Any idea how to fix this? Thanks

Looks like cmake evaluates the VERSION_GREATER_EQUAL before the AND (https://cmake.org/cmake/help/v3.5/command/if.html#condition-syntax), which presumably leads to this issue since GNU must not have CMAKE_CXX_COMPILER_VERSION defined. According to that cmake documentation, parentheses could be added to force the correct evaluation order. Can you check if the following fixes your issue:

if (COMPILER_RT_BUILD_LIBFUZZER AND
    ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") AND
    CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 6.0)

If that still fails then I guess it will be necessary to use nested ifs.

@tejohnson Nested if ? :)

CMake Error at projects/compiler-rt/lib/gwp_asan/CMakeLists.txt:108 (if):
  if given arguments:

    "COMPILER_RT_BUILD_LIBFUZZER" "AND" "(" "GNU" "STREQUAL" "Clang" ")" "AND" "CMAKE_CXX_COMPILER_VERSION" "VERSION_GREATER_EQUAL" "6.0"

  Unknown arguments specified
dyung added a subscriber: dyung.EditedMon, Aug 26, 2:51 AM

@tejohnson Nested if ? :)

CMake Error at projects/compiler-rt/lib/gwp_asan/CMakeLists.txt:108 (if):
  if given arguments:

    "COMPILER_RT_BUILD_LIBFUZZER" "AND" "(" "GNU" "STREQUAL" "Clang" ")" "AND" "CMAKE_CXX_COMPILER_VERSION" "VERSION_GREATER_EQUAL" "6.0"

  Unknown arguments specified

We are seeing the same failure in our internal upstream builder using CMake 3.4.3.

Looking at the CMake documentation, the problem might be the use of VERSION_GREATER_EQUAL.. It appears that binary test operator was not added to CMake until 3.7.

So the best fix would probably be to use two comparisons, maybe something like this:

((CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL 6.0) OR (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0))

bjope added a subscriber: bjope.Mon, Aug 26, 4:03 AM

FYI: I made a fixup here https://reviews.llvm.org/rL369891

I see here that you suspect other problems (around usage of AND). The problem I solved was related to older cmake versions not supporting VERSION_GREATER_EQUAL.

@tejohnson Nested if ? :)

I meant something like:
if (...)

if (...)

but in any case this wasn't the problem at all.

FYI: I made a fixup here https://reviews.llvm.org/rL369891

I see here that you suspect other problems (around usage of AND). The problem I solved was related to older cmake versions not supporting VERSION_GREATER_EQUAL.

Got it thanks.

@tejohnson Nested if ? :)

I meant something like:

if (...)
  if (...)

Sure, I meant that we could try with this solution :)