[HWASan] Add __hwasan_init to .preinit_array.

[morehouse]

Summary

Fixes segfaults on x86_64 caused by instrumented code running before
shadow is set up.

[eugenis]

I'm concerned that this might break android by moving hwasan initialization before libc constructor, and there are things that you can not do that early, and these things are libc-specific.
Please test on Android. LLVM buildbot is insufficient because it does not cover the interesting case of non-interceptor (platform) build.

[fmayer]

In D118171#3270800, @eugenis wrote:

I'm concerned that this might break android by moving hwasan initialization before libc constructor, and there are things that you can not do that early, and these things are libc-specific.
Please test on Android. LLVM buildbot is insufficient because it does not cover the interesting case of non-interceptor (platform) build.

I cherry picked this onto the Android LLVM toolchain, and built Android. It still boots and works okay.

[eugenis]

Could you test with a fully static binary, too?

[fmayer]

In D118171#3286064, @eugenis wrote:

Could you test with a fully static binary, too?

I statically compiled a Hello World executable and it works.

[eugenis]

This change does not handle the -shared-libsan case, and does not have any effect on Android other than in static executables. It also makes the comment in hwasan_preinit.cpp incorrect - that code is NOT linked into the main executable. I think we should replicate the "asan-preinit" driver logic for hwasan.

Note that ASan does not link the static preinit bit on Android. I believe that's a bug in the initial implementation of -shared-libsan - the code comment says "Do not link static runtime to DSOs or if compiling for Android", but that is clearly meant to refer to the full runtime library (because Android historically used shared runtime only) and not the preinit stub. See https://reviews.llvm.org/D3043.

[morehouse]

In D118171#3288089, @eugenis wrote:

This change does not handle the -shared-libsan case, and does not have any effect on Android other than in static executables. It also makes the comment in hwasan_preinit.cpp incorrect - that code is NOT linked into the main executable. I think we should replicate the "asan-preinit" driver logic for hwasan.

Note that ASan does not link the static preinit bit on Android. I believe that's a bug in the initial implementation of -shared-libsan - the code comment says "Do not link static runtime to DSOs or if compiling for Android", but that is clearly meant to refer to the full runtime library (because Android historically used shared runtime only) and not the preinit stub. See https://reviews.llvm.org/D3043.

I don't see any existing tests for HWASan + -shared-libsan. Is this a mode we really care about?

I can mimic the asan-preinit logic, but I'm not sure how to test it.

[eugenis]

It is the mode that we use on Android, so yes. LLVM buildbot runs android tests with shared runtime library + interceptors, it should reproduce the issue.

[morehouse]

• In shared library mode, link preinit stub with every DSO.

[eugenis]

LGTM, but I'd like @pcc to take a look.

[pcc]

What guarantee do we have that the __hwasan_init .preinit_array entry will be placed before the user's .preinit_array entries?

clang/lib/Driver/ToolChains/CommonArgs.cpp

842

Shouldn't it be added to HelperStaticRuntimes? Otherwise I'm not sure how this would work because the library would need to be enclosed in --whole-archive in order to be included in the link.

compiler-rt/lib/hwasan/hwasan_preinit.cpp

23

Can it be static?

[eugenis]

The driver seems to always add sanitizer runtimes first on the linker command line.

clang/lib/Driver/ToolChains/CommonArgs.cpp

842

Right, good point.

[morehouse]

• Make __local_hwasan_preinit static.
• Use HelperStaticRuntimes.

clang/lib/Driver/ToolChains/CommonArgs.cpp

842

Yes, good catch!

[pcc]

clang/lib/Driver/ToolChains/CommonArgs.cpp

842

Thanks. We'd better test on Android again because I imagine that the previous patch wouldn't have had any effect.

[fmayer]

clang/lib/Driver/ToolChains/CommonArgs.cpp

842

I patched the Android LLVM toolchain with this, build Android, it booted. Built static Hello World executable and it works.

[pcc]

LGTM

clang/lib/Driver/ToolChains/CommonArgs.cpp

if (SanArgs.needsSharedRt()) {
     }
     if (SanArgs.needsTsanRt() && SanArgs.linkRuntimes())
       SharedRuntimes.push_back("tsan");
     if (SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
       if (SanArgs.needsHwasanAliasesRt())
         SharedRuntimes.push_back("hwasan_aliases");
       else
         SharedRuntimes.push_back("hwasan");
+      if (!Args.hasArg(options::OPT_shared))
+        HelperStaticRuntimes.push_back("hwasan-preinit");
     }
   }
 
   // The stats_client library is also statically linked into DSOs.
   if (SanArgs.needsStatsRt() && SanArgs.linkRuntimes())
     StaticRuntimes.push_back("stats_client");
 
   // Always link the static runtime regardless of DSO or executable.

compiler-rt/lib/hwasan/CMakeLists.txt

set(HWASAN_RTL_SOURCES
   hwasan_thread_list.cpp
   hwasan_type_test.cpp
   )
 
 set(HWASAN_RTL_CXX_SOURCES
   hwasan_new_delete.cpp
   )
 
+set(HWASAN_RTL_PREINIT_SOURCES
+  hwasan_preinit.cpp
+  )
+
 set(HWASAN_RTL_HEADERS
   hwasan.h
   hwasan_allocator.h
   hwasan_dynamic_shadow.h
   hwasan_flags.h
   hwasan_flags.inc
   hwasan_globals.h
   hwasan_interface_internal.h
add_compiler_rt_object_libraries(RTHwasan_cxx
   CFLAGS ${HWASAN_RTL_CFLAGS}
   DEFS ${HWASAN_DEFINITIONS})
 add_compiler_rt_object_libraries(RTHwasan_dynamic
   ARCHS ${HWASAN_SUPPORTED_ARCH}
   SOURCES ${HWASAN_RTL_SOURCES} ${HWASAN_RTL_CXX_SOURCES}
   ADDITIONAL_HEADERS ${HWASAN_RTL_HEADERS}
   CFLAGS ${HWASAN_DYNAMIC_CFLAGS}
   DEFS ${HWASAN_DEFINITIONS})
+add_compiler_rt_object_libraries(RTHwasan_preinit
+  ARCHS ${HWASAN_SUPPORTED_ARCH}
+  SOURCES ${HWASAN_RTL_PREINIT_SOURCES}
+  ADDITIONAL_HEADERS ${HWASAN_RTL_HEADERS}
+  CFLAGS ${HWASAN_RTL_CFLAGS}
+  DEFS ${HWASAN_DEFINITIONS})
 
 file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/dummy.cpp "")
 add_compiler_rt_object_libraries(RTHwasan_dynamic_version_script_dummy
   ARCHS ${HWASAN_SUPPORTED_ARCH}
   SOURCES ${CMAKE_CURRENT_BINARY_DIR}/dummy.cpp
   CFLAGS ${HWASAN_DYNAMIC_CFLAGS}
   DEFS ${HWASAN_DEFINITIONS})
 
if(use_aliases)
     set(hwasan_object_lib RTHwasanAliases)
     set(hwasan_object_dyn_lib RTHwasanAliases_dynamic)
     set(hwasan_runtime clang_rt.hwasan_aliases)
   endif()
   add_compiler_rt_runtime(${hwasan_runtime}
     STATIC
     ARCHS ${arch}
     OBJECT_LIBS ${hwasan_object_lib}
+                RTHwasan_preinit
                 RTInterception
                 RTSanitizerCommon
                 RTSanitizerCommonLibc
                 RTSanitizerCommonCoverage
                 RTSanitizerCommonSymbolizer
                 RTUbsan
     CFLAGS ${hwasan_rtl_flags}
     PARENT_TARGET hwasan)

 
 foreach(arch ${HWASAN_SUPPORTED_ARCH})
   add_hwasan_runtimes(${arch} FALSE)
   if(${arch} MATCHES "x86_64")
     add_hwasan_runtimes(${arch} TRUE)
   endif()
 endforeach()
 
+add_compiler_rt_runtime(clang_rt.hwasan-preinit
+  STATIC
+  ARCHS ${HWASAN_SUPPORTED_ARCH}
+  OBJECT_LIBS RTHwasan_preinit
+  CFLAGS ${HWASAN_RTL_CFLAGS}
+  PARENT_TARGET hwasan)
+
 add_compiler_rt_resource_file(hwasan_ignorelist hwasan_ignorelist.txt hwasan)
 
 add_subdirectory("scripts")
 
 # if(COMPILER_RT_INCLUDE_TESTS)
 #   add_subdirectory(tests)
 # endif()

compiler-rt/lib/hwasan/hwasan_preinit.cpp

+//===-- hwasan_preinit.cpp ------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of HWAddressSanitizer, an address sanity checker.
+//
+// Call __hwasan_init at the very early stage of process startup.
+//===----------------------------------------------------------------------===//
+#include "hwasan_interface_internal.h"
+#include "sanitizer_common/sanitizer_internal_defs.h"
+
+#if SANITIZER_CAN_USE_PREINIT_ARRAY
+// The symbol is called __local_hwasan_preinit, because it's not intended to
+// be exported.
+// This code linked into the main executable when -fsanitize=hwaddress is in
+// the link flags. It can only use exported interface functions.
+__attribute__((section(".preinit_array"), used)) static void (
+    *__local_hwasan_preinit)(void) = __hwasan_init;
+#endif

compiler-rt/test/hwasan/TestCases/preinit_array.c

+// Test that HWASan shadow is initialized before .preinit_array functions run.
+
+// RUN: %clang_hwasan %s -o %t
+// RUN: %run %t
+
+volatile int Global;
+void StoreToGlobal() { Global = 42; }
+
+__attribute__((section(".preinit_array"), used))
+void (*__StoreToGlobal_preinit)() = StoreToGlobal;
+
+int main() { return Global != 42; }