Enable weak hooks on darwin

[fjricci]

Summary

By default, darwin requires a definition for weak interface functions at
link time. Adding the '-U' link flag with each weak function allows these
weak interface functions to be used without definitions, which mirrors
behavior on linux and windows.

[kubamracek]

-undefined dynamic_lookup

This means the linker won't warn/error out on any undefined symbol. Sounds very dangerous to me and it's likely to cause a lot of development headaches.

Can we solve this in a different way? E.g. a macro that would expand to a empty function body on Darwin?

[compnerd]

Its actually not that dangerous, given that ELF by default does this for any dynamic linking. However, it is possible to be more granular and specify the symbols which are okay to be undefined using the -U <name> option to the linker. It is slightly nicer than the blanket approach, but at a higher maintainence cost (any time the sanitizers change the hooks, it would need to be updated. But, that doesnt seem too bad.

[fjricci]

Upload correct commit

[fjricci]

Individually lists each undefined symbol as undefined in the linker invocation.

[fjricci]

Enable on darwin

[kubamracek]

Looks better, but I'd rather see the list of weak symbols in each subdirectory (asan/, ubsan/, etc.), because each sanitizer has its own list. It looks wrong to list sanitizer-specific symbols in a global CMake file. Can this also be stored in a separate file and not directly in CMake (with a dependency on that file)?

Can we also make the Linux linker use this list? I only looked briefly at man ld, but it seems that --dynamic-list may achieve the same?

Overall, I support this change, but I want to avoid situations where a weak symbol is forgotten to be added to the list, and the Linux build will work fine, but Darwin will be broken.

[compnerd]

Ish. Most linkers can read the arguments from a file. The GNU linker uses @file syntax as does link (Windows). ld64 can read a set of files to link via -file_list but not all the arguments AFAIK.

COFF does not allow any undefined symbols anyways, and this patch addresses MachO. On ELFish targets, -z defs will prevent undefined symbols even in DSOs, but I don't think that applies to STB_WEAK symbols.

FWIW, I think that getting the same behavior across the two is a bit beyond the scope of this change.

[fjricci]

As far as I can tell, @compnerd is correct with respect to STB_WEAK symbols on Linux. I'll upload a refactored change targeting Darwin with the weak symbols pushed down into the sanitizer-specific directories.

[fjricci]

Refactor symbols into text files only included in required link steps

[fjricci]

Accidentally committed with another commit. Reverting.

[kubamracek]

LGTM with a nit.

Note that D28359 was trying to solve a similar problem with weak linking on Windows, so maybe there are more changes coming, but this patch doesn't need to be blocked on that.

cmake/Modules/SanitizerUtils.cmake

53

nit: add a comment saying that this is specific to Darwin/Mach-O

lib/sanitizer_common/sanitizer_internal_defs.h

35

What about FreeBSD? Just curious.

[compnerd]

lib/sanitizer_common/sanitizer_internal_defs.h

35

I believe that FreeBSD uses ELF, so it should be able to support weak linkage. However, if its untested ...

[fjricci]

cmake/Modules/SanitizerUtils.cmake

53

Will do before commit.

lib/sanitizer_common/sanitizer_internal_defs.h

35

I don't have any way to run this on FreeBSD, so I'll leave this for future work.

[fjricci]

Adds undefined symbols to test cases as well

[fjricci]

Also add function comment

[fjricci]

I realized during testing that the symbols must also be added to some test cases directly. Re-uploaded with those changes, to give the opportunity for review before submitting.

[fjricci]

Fix typo

[kubamracek]

There are still a few typos where LINK_FLAGS (with the underscore) is used.

Besides that, the patch looks good.

cmake/Modules/SanitizerUtils.cmake

foreach(arch ${ARG_ARCHS})
     install(FILES $<TARGET_FILE:${target_name}>.syms
             DESTINATION ${COMPILER_RT_LIBRARY_INSTALL_DIR})
     if(ARG_PARENT_TARGET)
       add_dependencies(${ARG_PARENT_TARGET} ${target_name}-symbols)
     endif()
   endforeach()
 endmacro()
 
+function(add_weak_symbols libname linkflags)
+  file(STRINGS "${COMPILER_RT_SOURCE_DIR}/lib/${libname}/weak_symbols.txt" WEAK_SYMBOLS)
+  set(local_linkflags ${${linkflags}})
+  foreach(SYMBOL ${WEAK_SYMBOLS})
+    set(local_linkflags ${local_linkflags} -Wl,-U,${SYMBOL})
+  endforeach()
+  set(${linkflags} ${local_linkflags} PARENT_SCOPE)
+endfunction()
+
 macro(add_sanitizer_rt_version_list name)
   set(vers ${CMAKE_CURRENT_BINARY_DIR}/${name}.vers)
   cmake_parse_arguments(ARG "" "" "LIBS;EXTRA" ${ARGN})
   set(args)
   foreach(arg ${ARG_EXTRA})
     list(APPEND args "--extra" ${arg})
   endforeach()
   foreach(arg ${ARG_LIBS})

lib/asan/CMakeLists.txt

add_compiler_rt_object_libraries(RTAsan_dynamic_version_script_dummy
     CFLAGS ${ASAN_DYNAMIC_CFLAGS}
     DEFS ${ASAN_DYNAMIC_DEFINITIONS})
 endif()
 
 # Build ASan runtimes shipped with Clang.
 add_compiler_rt_component(asan)
 
 if(APPLE)
+  add_weak_symbols("asan" WEAK_SYMBOL_LINKFLAGS)
+  add_weak_symbols("ubsan" WEAK_SYMBOL_LINKFLAGS)
+  add_weak_symbols("sanitizer_common" WEAK_SYMBOL_LINKFLAGS)
+
   add_compiler_rt_runtime(clang_rt.asan
     SHARED
     OS ${SANITIZER_COMMON_SUPPORTED_OS}
     ARCHS ${ASAN_SUPPORTED_ARCH}
     OBJECT_LIBS RTAsan_dynamic
                 RTInterception
                 RTSanitizerCommon
                 RTSanitizerCommonLibc
                 RTLSanCommon
                 RTUbsan
     CFLAGS ${ASAN_DYNAMIC_CFLAGS}
+    LINKFLAGS ${WEAK_SYMBOL_LINKFLAGS}
     DEFS ${ASAN_DYNAMIC_DEFINITIONS}
     PARENT_TARGET asan)
 else()
   # Build separate libraries for each target.
 
   set(ASAN_COMMON_RUNTIME_OBJECT_LIBS
     RTInterception
     RTSanitizerCommon

lib/asan/weak_symbols.txt

+___asan_default_options
+___asan_default_suppressions
+___asan_on_error

lib/sanitizer_common/sanitizer_internal_defs.h

 #elif SANITIZER_GO
 # define SANITIZER_INTERFACE_ATTRIBUTE
 # define SANITIZER_WEAK_ATTRIBUTE
 #else
 # define SANITIZER_INTERFACE_ATTRIBUTE __attribute__((visibility("default")))
 # define SANITIZER_WEAK_ATTRIBUTE  __attribute__((weak))
 #endif
 
-#if (SANITIZER_LINUX || SANITIZER_WINDOWS) && !SANITIZER_GO
+#if (SANITIZER_LINUX || SANITIZER_MAC || SANITIZER_WINDOWS) && !SANITIZER_GO
 # define SANITIZER_SUPPORTS_WEAK_HOOKS 1
 #else
 # define SANITIZER_SUPPORTS_WEAK_HOOKS 0
 #endif
 
 // We can use .preinit_array section on Linux to call sanitizer initialization
 // functions very early in the process startup (unless PIC macro is defined).
 // FIXME: do we have anything like this on Mac?

lib/sanitizer_common/weak_symbols.txt

+___sanitizer_free_hook
+___sanitizer_malloc_hook
+___sanitizer_symbolize_code
+___sanitizer_symbolize_data
+___sanitizer_symbolize_demangle
+___sanitizer_symbolize_flush

lib/stats/CMakeLists.txt

 include_directories(..)
 
 add_custom_target(stats)
 set_target_properties(stats PROPERTIES FOLDER "Compiler-RT Misc")
 
 if(APPLE)
   set(STATS_LIB_FLAVOR SHARED)
+
+  add_weak_symbols("asan" WEAK_SYMBOL_LINKFLAGS)
+  add_weak_symbols("ubsan" WEAK_SYMBOL_LINKFLAGS)
+  add_weak_symbols("sanitizer_common" WEAK_SYMBOL_LINKFLAGS)
 else()
   set(STATS_LIB_FLAVOR STATIC)
+
+  set(WEAK_SYMBOL_LINKFLAGS)
 endif()
 
 add_compiler_rt_runtime(clang_rt.stats
   ${STATS_LIB_FLAVOR}
   ARCHS ${SANITIZER_COMMON_SUPPORTED_ARCH}
   OS ${SANITIZER_COMMON_SUPPORTED_OS}
   SOURCES stats.cc
   OBJECT_LIBS RTSanitizerCommon
               RTSanitizerCommonLibc
   CFLAGS ${SANITIZER_COMMON_CFLAGS}
+  LINKFLAGS ${WEAK_SYMBOL_LINKFLAGS}
   PARENT_TARGET stats)
 
 add_compiler_rt_runtime(clang_rt.stats_client
   STATIC
   ARCHS ${SANITIZER_COMMON_SUPPORTED_ARCH}
   OS ${SANITIZER_COMMON_SUPPORTED_OS}
   SOURCES stats_client.cc
   CFLAGS ${SANITIZER_COMMON_CFLAGS}
+  LINKFLAGS ${WEAK_SYMBOL_LINKFLAGS}
   PARENT_TARGET stats)

lib/tsan/CMakeLists.txt

if(APPLE)
   set(TSAN_ASM_SOURCES rtl/tsan_rtl_amd64.S)
   # Xcode will try to compile this file as C ('clang -x c'), and that will fail.
   if (${CMAKE_GENERATOR} STREQUAL "Xcode")
     enable_language(ASM)
   else()
     # Pass ASM file directly to the C++ compiler.
     set_source_files_properties(${TSAN_ASM_SOURCES} PROPERTIES LANGUAGE C)
   endif()
+
+  add_weak_symbols("ubsan" WEAK_SYMBOL_LINKFLAGS)
+  add_weak_symbols("sanitizer_common" WEAK_SYMBOL_LINKFLAGS)
+
   add_compiler_rt_runtime(clang_rt.tsan
     SHARED
     OS ${TSAN_SUPPORTED_OS}
     ARCHS ${TSAN_SUPPORTED_ARCH}
     SOURCES ${TSAN_SOURCES} ${TSAN_CXX_SOURCES} ${TSAN_ASM_SOURCES}
     OBJECT_LIBS RTInterception
                 RTSanitizerCommon
                 RTSanitizerCommonLibc
                 RTUbsan
     CFLAGS ${TSAN_RTL_CFLAGS}
+    LINKFLAGS ${WEAK_SYMBOL_LINKFLAGS}
     PARENT_TARGET tsan)
   add_compiler_rt_object_libraries(RTTsan_dynamic
     OS ${TSAN_SUPPORTED_OS}
     ARCHS ${TSAN_SUPPORTED_ARCH}
     SOURCES ${TSAN_SOURCES} ${TSAN_CXX_SOURCES} ${TSAN_ASM_SOURCES}
     CFLAGS ${TSAN_RTL_CFLAGS})
 
   # Build and check Go runtime.

lib/ubsan/CMakeLists.txt

if(APPLE)
   if(COMPILER_RT_HAS_UBSAN)
     # Initializer of standalone UBSan runtime.
     add_compiler_rt_object_libraries(RTUbsan_standalone
       OS ${SANITIZER_COMMON_SUPPORTED_OS}
       ARCHS ${UBSAN_SUPPORTED_ARCH}
       SOURCES ${UBSAN_STANDALONE_SOURCES}
       CFLAGS ${UBSAN_STANDALONE_CFLAGS})
 
+    add_weak_symbols("ubsan" WEAK_SYMBOL_LINKFLAGS)
+    add_weak_symbols("sanitizer_common" WEAK_SYMBOL_LINKFLAGS)
+
     add_compiler_rt_runtime(clang_rt.ubsan
       SHARED
       OS ${SANITIZER_COMMON_SUPPORTED_OS}
       ARCHS ${UBSAN_SUPPORTED_ARCH}
       OBJECT_LIBS RTUbsan
                   RTUbsan_standalone
                   RTSanitizerCommon
                   RTSanitizerCommonLibc
+      LINKFLAGS ${WEAK_SYMBOL_LINKFLAGS}
       PARENT_TARGET ubsan)
   endif()
 
 else()
   # Common parts of UBSan runtime.
   add_compiler_rt_object_libraries(RTUbsan
     ARCHS ${UBSAN_COMMON_SUPPORTED_ARCH}
     SOURCES ${UBSAN_SOURCES} CFLAGS ${UBSAN_CFLAGS})

lib/ubsan/weak_symbols.txt

+___ubsan_default_options