[GWP-ASan] Attempt to fix Android/ARM platforms.

[hctim]

Summary

This patch re-enabled GWP-ASan for Android and other ARM platforms. It does two
things:

1. Adds -fno-emulated-tls, as the clang driver still default compiles using emulated TLS on Android.
2. Also uses sanitizer common unwinder on Android, as glibc backtrace() isn't available.

This patch should be monitored closely when submitting. I don't have the ARM32
machines that the patches initially broke to verify that this fixes the
problem, but it's a good attempt.

[eugenis]

LGTM

This adds sanitizer_common libraries to scudo_minimal (but not to scudo_standalone). Is it OK with you, Kostya?

[cryptoad]

It's OK with me. Part of the reason for splitting out the symbolizer was some memory overhead as well, which we probably want to look into.
Is COMPILER_RT_HAS_GWP_ASAN going to be true by default for Android? If so we probably should loop in the toolchain team so that there is no surprise on their next toolchain update.

[cryptoad]

Can this be abandoned? Looks stale.

[hctim]

In D64457#1762210, @cryptoad wrote:

Can this be abandoned? Looks stale.

I'll need to take a look and rethink this - is planned changes okay to get it off your dashboard?

[cryptoad]

In D64457#1762220, @hctim wrote:

In D64457#1762210, @cryptoad wrote:

Can this be abandoned? Looks stale.

I'll need to take a look and rethink this - is planned changes okay to get it off your dashboard?

Works thanks!

[hctim]

• Updates.

[hctim]

Reviving this a little. check-gwp_asan in my aarch64 crostool now WAI, with some warnings during compilation from ar and ranlib: unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000. Looks like this is the PAC/BTI properties from D62609, but doesn't really hurt us here.

@cryptoad - I hear through the grapevine that you're having similar initial-exec TLS alignment issues in the linker (as per the inline comment in this patch). Is that what's stopping you from adding scudo_standalone to aarch64/Android on the upstream (LLVM) bots?

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

This alignment is manually required, as otherwise with my NDK (r18b) the bionic linker complains when running the test that the executable's TLS segment is underaligned: alignment is 8, needs to be at least 64 for ARM64 Bionic. Apparently this is fixed in the new NDKv21, but we can't update our bots to that at this time as the migration is nontrivial :( (NDKv21 breaks other things in standalone compiler-rt).

[merge_guards_bot]

Unit tests: fail. 62146 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 1 errors and 0 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[hctim]

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

@pcc - do you think this is a reasonable solution to get it to build without pushing the new NDK/API version to the bots?

[pcc]

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

I don't recall, did just upgrading the NDK break things or did things only break when you upgraded the API level as well? If the former, maybe you could add --target=aarch64-linux-android30 and so on to cflags/ldflags for the gwp-asan targets?

[eugenis]

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

Simply updating the NDK breaks things. It could very well be trivial, I did not look into it at all beyond that fact that the bot turned red.

[hctim]

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

Are we happy with this workaround for now folks?

[pcc]

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

This is fine for now but please add a comment.

[eugenis]

LGTM

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

If we pack the thread local variables to put them on the same cacheline, I'd say this change is not only acceptable but even desirable.

[hctim]

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

258 ↗

(On Diff #240298)

We may take up more static TLS space, but given we should probably be part of the initial set anyway, that's not a problem.

[smeenai]

Is there anything holding this up?

On our end, we're able to build GWP-ASAN and Scudo for Android with this change. We haven't actually tried running them yet though :D

[smeenai]

I filed https://bugs.llvm.org/show_bug.cgi?id=45014 for an Android compiler-rt compilation error I ran into for API 29. (I needed to build with API 29 because it's only API 29's libc.so that has __tls_get_addr and __aeabi_read_tp, which are needed for non-emulated TLS.)

[hctim]

(no longer relevant - much fresher patch should be coming soon to do the same thing)

compiler-rt/cmake/config-ix.cmake

 endif()
 
 if(OS_NAME MATCHES "Linux")
   set(ALL_FUZZER_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64})
 else()
   set(ALL_FUZZER_SUPPORTED_ARCH ${X86_64} ${ARM64})
 endif()
 
-set(ALL_GWP_ASAN_SUPPORTED_ARCH ${X86} ${X86_64})
+set(ALL_GWP_ASAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64} ${ARM32})
 if(APPLE)
   set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64})
 else()
   set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64} ${ARM32} ${PPC64})
 endif()
 set(ALL_MSAN_SUPPORTED_ARCH ${X86_64} ${MIPS64} ${ARM64} ${PPC64})
 set(ALL_HWASAN_SUPPORTED_ARCH ${X86_64} ${ARM64})
 set(ALL_PROFILE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${PPC64}

 endif()
 
 # Note: Fuchsia and Windows are not currently supported by GWP-ASan. Support
 # is planned for these platforms. Darwin is also not supported due to TLS
 # calling malloc on first use.
 # TODO(hctim): Enable this on Android again. Looks like it's causing a SIGSEGV
 # for Scudo and GWP-ASan, further testing needed.
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND GWP_ASAN_SUPPORTED_ARCH AND
-    OS_NAME MATCHES "Linux")
+    OS_NAME MATCHES "Android|Linux")
   set(COMPILER_RT_HAS_GWP_ASAN TRUE)
 else()
   set(COMPILER_RT_HAS_GWP_ASAN FALSE)
 endif()
 pythonize_bool(COMPILER_RT_HAS_GWP_ASAN)

compiler-rt/lib/gwp_asan/CMakeLists.txt

set(GWP_ASAN_HEADERS
   options.h
   options.inc
   random.h
 )
 
 # Ensure that GWP-ASan meets the delegated requirements of some supporting
 # allocators. Some supporting allocators (e.g. scudo standalone) cannot use any
 # parts of the C++ standard library.
-set(GWP_ASAN_CFLAGS -fno-rtti -fno-exceptions -nostdinc++ -pthread)
+set(GWP_ASAN_CFLAGS -fno-rtti -fno-exceptions -nostdinc++ -pthread
+    -fno-emulated-tls)
 append_list_if(COMPILER_RT_HAS_FPIC_FLAG -fPIC GWP_ASAN_CFLAGS)
 append_list_if(COMPILER_RT_HAS_OMIT_FRAME_POINTER_FLAG -fno-omit-frame-pointer
                GWP_ASAN_CFLAGS)
 append_list_if(COMPILER_RT_HAS_OMIT_FRAME_POINTER_FLAG
                -mno-omit-leaf-frame-pointer GWP_ASAN_CFLAGS)
 
 # Remove -stdlib= which is unused when passing -nostdinc++.
 string(REGEX REPLACE "-stdlib=[a-zA-Z+]*" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})

compiler-rt/lib/scudo/CMakeLists.txt

set(SCUDO_MINIMAL_OBJECT_LIBS
   RTSanitizerCommonNoTermination
   RTSanitizerCommonLibc
   RTInterception)
 
 if (COMPILER_RT_HAS_GWP_ASAN)
   # Currently, Scudo uses the GwpAsan flag parser. This backs onto the flag
   # parsing mechanism of sanitizer_common. Once Scudo has its own flag parsing,
   # and parses GwpAsan options, you can remove this dependency.
-  list(APPEND SCUDO_MINIMAL_OBJECT_LIBS RTGwpAsan RTGwpAsanOptionsParser
-                                        RTGwpAsanBacktraceLibc)
-  list(APPEND SCUDO_CFLAGS -DGWP_ASAN_HOOKS)
+  list(APPEND SCUDO_MINIMAL_OBJECT_LIBS RTGwpAsan RTGwpAsanOptionsParser)
+
+  # Install GWP-ASan stubs. Also, ensure that TLS is not emulated on this
+  # platform (Android still compiles with emulated TLS by default).
+  list(APPEND SCUDO_CFLAGS -DGWP_ASAN_HOOKS -fno-emulated-tls)
+
+  # Android does not have the libc backtrace() support (from execinfo.h). In
+  # this case, we use the sanitizer common variant.
+  if (ANDROID)
+    list(APPEND SCUDO_MINIMAL_OBJECT_LIBS RTGwpAsanBacktraceSanitizerCommon
+         RTSanitizerCommonSymbolizer)
+  else()
+    list(APPEND SCUDO_MINIMAL_OBJECT_LIBS RTGwpAsanBacktraceLibc)
+  endif()
 endif()
 
 set(SCUDO_OBJECT_LIBS ${SCUDO_MINIMAL_OBJECT_LIBS})
 set(SCUDO_DYNAMIC_LIBS ${SCUDO_MINIMAL_DYNAMIC_LIBS})
 
 if (FUCHSIA)
   list(APPEND SCUDO_CFLAGS -nostdinc++)
   list(APPEND SCUDO_DYNAMIC_LINK_FLAGS -nostdlib++)