(impala) 03/03: IMPALA-12563: Fix UBSAN on ARM

[mi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 0d21776502538ca2ea861825f7168daa60a1e0d4
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Tue Nov 21 01:00:47 2023 +0000

    IMPALA-12563: Fix UBSAN on ARM
    
    Links gcc after all other libraries when building with UBSAN. On ARM,
    several symbols are included that aren't present in libclang_rt (enabled
    by -rtlib=compiler-rt for UBSAN builds) or in libgcc_s.so (needed with
    the alternate rtlib); linking libgcc.a after all other libraries ensures
    the symbols are present. There may be other repercussions, so this is
    only done for UBSAN builds.
    
    Skips FE tests with UBSAN on ARM due to increased use of thread-local
    storage on ARM that exceeds some implementation-defined limit.
    Setting '-XX:ThreadStackSize=16m' didn't help.
    
    Change-Id: I799bedd1cc73c852b0edb928dc71166e534918ba
    Reviewed-on: http://gerrit.cloudera.org:8080/20721
    Reviewed-by: Michael Smith <mi...@cloudera.com>
    Tested-by: Michael Smith <mi...@cloudera.com>
---
 CMakeLists.txt       | 9 +++++++++
 bin/run-all-tests.sh | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 755d31bc8..b61982d7a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -196,6 +196,15 @@ function(IMPALA_ADD_THIRDPARTY_LIB NAME HEADER STATIC_LIB SHARED_LIB)
     ADD_THIRDPARTY_LIB(${NAME} SHARED_LIB ${SHARED_LIB})
   else()
     ADD_THIRDPARTY_LIB(${NAME} STATIC_LIB ${STATIC_LIB})
+    if (CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64")
+      if ("${CMAKE_BUILD_TYPE}" STREQUAL "UBSAN" OR
+          "${CMAKE_BUILD_TYPE}" STREQUAL "UBSAN_FULL")
+        # UBSAN builds on ARM require that gcc is included last to cover several symbols
+        # omitted in libgcc_s, which is required because we use -rtlib=compiler-rt to
+        # work around https://bugs.llvm.org/show_bug.cgi?id=16404.
+        target_link_libraries(${NAME} INTERFACE gcc)
+      endif()
+    endif()
   endif()
 endfunction()
 
diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh
index b6472e5e6..1a06ca9b2 100755
--- a/bin/run-all-tests.sh
+++ b/bin/run-all-tests.sh
@@ -116,6 +116,15 @@ if [[ "${ERASURE_CODING}" = true ]]; then
   FE_TEST=false
 fi
 
+if test -v CMAKE_BUILD_TYPE && [[ "${CMAKE_BUILD_TYPE}" =~ 'UBSAN' ]] \
+    && [[ "$(uname -p)" = "aarch64" ]]; then
+  # FE tests fail on ARM with
+  #   libfesupport.so: cannot allocate memory in static TLS block
+  # https://bugzilla.redhat.com/show_bug.cgi?id=1722181 mentions this is more likely
+  # on aarch64 due to how it uses thread-local storage (TLS). There's no clear fix.
+  FE_TEST=false
+fi
+
 # Indicates whether code coverage reports should be generated.
 : ${CODE_COVERAGE:=false}