[sanitizer] Use -Wl,-z,global on Android

[cryptoad]

Summary

Use -Wl,-z,global for all Sanitizer shared libraries on
Android. We want them to be in the global group
(https://android.googlesource.com/platform/bionic/+/master/android-changes-for-ndk-developers.md#changes-to-library-search-order)
to avoid any alloc/dealloc mismatch between the libc allocator & said library.

audioserver was one of the binary that exhibited the problem with Scudo,
this seems to fix it.

[edited for accuracy]

[cryptoad]

Updating comment.

[eugenis]

lib/scudo/CMakeLists.txt

24 ↗

(On Diff #155044)

ASAN_DYNAMIC_LINK_FLAGS <- is this a typo?

Can this be done in a common code for all sanitizers that intercept malloc?

Also, I don't think github is the canonical source for AOSP, could you update the link in the patch description to https://android.googlesource.com/platform/bionic/+/master/android-changes-for-ndk-developers.md ?

[cryptoad]

s/ASAN/SCUDO because I failed at copy/pasting :(

[cryptoad]

lib/scudo/CMakeLists.txt

24 ↗

(On Diff #155044)

Doh! Thanks for catching the error.
Name updated, description updated.
Regarding the last point, you'd want an update to SANITIZER_COMMON_LINK_FLAGS based on interception? (not sure if it's possible, just trying to clarify).

[eugenis]

lib/scudo/CMakeLists.txt

24 ↗

(On Diff #155044)

SANITIZER_COMMON_LINK_FLAGS can't depend on interception, I think.

In fact, adding -z,global unconditionally should not break anything as far as I can see.
Could you move this code to the top-level (compiler-rt) CMakeLists, and remove it from asan and hwasan?

[cryptoad]

Moving the "global" link flag to the common CMake code so that it can benefit
all the shared libraries. Remove the previous instances of said code in ASan
and HWAsan.

[cryptoad]

lib/scudo/CMakeLists.txt

24 ↗

(On Diff #155044)

Done, will test locally to make sure it all works out.

[eugenis]

LGTM, thanks!

CMakeLists.txt

if (ANDROID)
     append_list_if(COMPILER_RT_HAS_GCC_LIB gcc SANITIZER_COMMON_LINK_LIBS)
   else()
     append_list_if(COMPILER_RT_HAS_GCC_S_LIB gcc_s SANITIZER_COMMON_LINK_LIBS)
   endif()
 endif()
 
 append_list_if(COMPILER_RT_HAS_LIBC c SANITIZER_COMMON_LINK_LIBS)
 
+if(ANDROID)
+# Put the Sanitizer shared libraries in the global group. For more details, see
+# android-changes-for-ndk-developers.md#changes-to-library-search-order
+  if (COMPILER_RT_HAS_Z_GLOBAL)
+    list(APPEND SANITIZER_COMMON_LINK_FLAGS -Wl,-z,global)
+  endif()
+endif()
+
 if("${CMAKE_SYSTEM_NAME}" STREQUAL "Fuchsia")
   list(APPEND SANITIZER_COMMON_LINK_FLAGS -Wl,-z,defs,-z,now,-z,relro)
   list(APPEND SANITIZER_COMMON_LINK_LIBS zircon)
 endif()
 
 if (SANITIZER_CXX_ABI_LIBNAME STREQUAL "libc++")
   if (SANITIZER_CXX_ABI_INTREE)
     if (NOT LIBCXXABI_ENABLE_STATIC_UNWINDER AND (TARGET unwind_shared OR HAVE_LIBUNWIND))

lib/asan/CMakeLists.txt

 
 set(ASAN_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 set(ASAN_COMMON_DEFINITIONS ${COMPILER_RT_ASAN_SHADOW_SCALE_DEFINITION})
 
 append_rtti_flag(OFF ASAN_CFLAGS)
 
 set(ASAN_DYNAMIC_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS})
 
-if(ANDROID)
-# On Android, -z global does not do what it is documented to do.
-# On Android, -z global moves the library ahead in the lookup order,
-# placing it right after the LD_PRELOADs. This is used to compensate for the fact
-# that Android linker does not look at the dependencies of the main executable
-# that aren't dependencies of the current DSO when resolving symbols from said DSO.
-# As a net result, this allows running ASan executables without LD_PRELOAD-ing the
-# ASan runtime library.
-# The above is applicable to L MR1 or newer.
-  if (COMPILER_RT_HAS_Z_GLOBAL)
-    list(APPEND ASAN_DYNAMIC_LINK_FLAGS -Wl,-z,global)
-  endif()
-endif()
-
 set(ASAN_DYNAMIC_DEFINITIONS
   ${ASAN_COMMON_DEFINITIONS} ASAN_DYNAMIC=1)
 append_list_if(WIN32 INTERCEPTION_DYNAMIC_CRT ASAN_DYNAMIC_DEFINITIONS)
 
 set(ASAN_DYNAMIC_CFLAGS ${ASAN_CFLAGS})
 append_list_if(COMPILER_RT_HAS_FTLS_MODEL_INITIAL_EXEC
   -ftls-model=initial-exec ASAN_DYNAMIC_CFLAGS)
 append_list_if(MSVC /DEBUG ASAN_DYNAMIC_LINK_FLAGS)

lib/hwasan/CMakeLists.txt

 set(HWASAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF HWASAN_RTL_CFLAGS)
 append_list_if(COMPILER_RT_HAS_FPIC_FLAG -fPIC HWASAN_RTL_CFLAGS)
 # Prevent clang from generating libc calls.
 append_list_if(COMPILER_RT_HAS_FFREESTANDING_FLAG -ffreestanding HWASAN_RTL_CFLAGS)
 
 set(HWASAN_DYNAMIC_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS})
 
-if(ANDROID)
-# On Android, -z global does not do what it is documented to do.
-# On Android, -z global moves the library ahead in the lookup order,
-# placing it right after the LD_PRELOADs. This is used to compensate for the fact
-# that Android linker does not look at the dependencies of the main executable
-# that aren't dependencies of the current DSO when resolving symbols from said DSO.
-# As a net result, this allows running ASan executables without LD_PRELOAD-ing the
-# ASan runtime library.
-# The above is applicable to L MR1 or newer.
-  if (COMPILER_RT_HAS_Z_GLOBAL)
-    list(APPEND HWASAN_DYNAMIC_LINK_FLAGS -Wl,-z,global)
-  endif()
-endif()
-
 set(HWASAN_DYNAMIC_CFLAGS ${HWASAN_RTL_CFLAGS})
 append_list_if(COMPILER_RT_HAS_FTLS_MODEL_INITIAL_EXEC
   -ftls-model=initial-exec HWASAN_DYNAMIC_CFLAGS)
 append_list_if(MSVC /DEBUG HWASAN_DYNAMIC_LINK_FLAGS)
 
 set(HWASAN_DYNAMIC_LIBS ${SANITIZER_CXX_ABI_LIBRARY} ${SANITIZER_COMMON_LINK_LIBS})
 
 append_list_if(COMPILER_RT_HAS_LIBDL dl HWASAN_DYNAMIC_LIBS)