[CMake][LibFuzzer] Match symbol visibility setting between LibFuzzer object files and unit tests.

[delcypher]

Summary

This fixes inconsistent symbol visibility. This shows up as a linker
warning if r336238 (43f633564e338a6dde83d49a48e5bfcbfdce292c) is
reverted.

ld: warning: direct access in function 'fuzzer::CleanseCrashInput(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, fuzzer::fuzzer_allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&, fuzzer::FuzzingOptions const&)' from file '/Volumes/data/dev/llvm/upstream/master/builds/projects/compiler-rt/lib/fuzzer/tests/libRTFuzzerTest.x86_64.a(FuzzerDriver.cpp.o)' to global weak symbol 'fuzzer::Command::ignoreRemainingArgs()::kIgnoreRemaining' from file 'FuzzerTestObjects.FuzzerUnittest.cpp.x86_64.o' means the weak symbol cannot be overridden
 at runtime. This was likely caused by different translation units being compiled with different visibility settings.

r336238 just hid the issue rather than fixing the real issue. On macOS
and other platforms we usually compile with -fvisibility=hidden but
the unit tests were compiled without this flag.

[delcypher]

@yln @kubamracek Ping.

[yln]

LGTM. Compiling all linker inputs with the same visibility is the right approach.

Please check my understanding:
Before r336238 the kIgnoreRemaining constant got inlined, both into libFuzzer and the unit test. So we had two definitions, one with hidden and one with default visibility resulting in a linker warning. If we (just for testing purposes) compile libFuzzer without hidden visibility, we would get a "duplicate symbol" linker error. With this patch we have two hidden definitions, which is fine.

[delcypher]

In D58055#1410888, @yln wrote:

LGTM. Compiling all linker inputs with the same visibility is the right approach.

Please check my understanding:
Before r336238 the kIgnoreRemaining constant got inlined, both into libFuzzer and the unit test.

I wouldn't say the constant got inlined, instead its the fact we have a single method that is compiled with different visibility and that method uses static storage. It seems that the static storage is implemented on Darwin as a weak global.

So we had two definitions, one with hidden and one with default visibility resulting in a linker warning. If we (just for testing purposes) compile libFuzzer without hidden visibility, we would get a "duplicate symbol" linker error. With this patch we have two hidden definitions, which is fine.

I'm not 100% sure about this. I'm not sure what the linker means by "direct access" here.

[yln]

Okay, got it. I didn't read the warning carefully enough.
LGTM.

lib/fuzzer/tests/CMakeLists.txt

 endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND
    COMPILER_RT_LIBCXX_PATH AND
    COMPILER_RT_LIBCXXABI_PATH)
   list(APPEND LIBFUZZER_UNITTEST_CFLAGS -nostdinc++)
 endif()
 
+if ("-fvisibility=hidden" IN_LIST LIBFUZZER_CFLAGS)
+  # Match visibility settings.
+  list(APPEND LIBFUZZER_UNITTEST_CFLAGS "-fvisibility=hidden")
+endif()
+
 if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST FUZZER_SUPPORTED_ARCH)
   # libFuzzer unit tests are only run on the host machine.
   set(arch ${COMPILER_RT_DEFAULT_TARGET_ARCH})
 
   set(LIBFUZZER_TEST_RUNTIME RTFuzzerTest.${arch})
   if(APPLE)
     set(LIBFUZZER_TEST_RUNTIME_OBJECTS
       $<TARGET_OBJECTS:RTfuzzer.osx>)