[scudo] Refactor of CRC32 and ARM runtime CRC32 detection

Summary:
ARM & AArch64 runtime detection for hardware support of CRC32 has been added
via check of the AT_HWVAL auxiliary vector.

Following Michal's suggestions in D28417, the CRC32 code has been further
changed and looks better now. When compiled with full relro (which is strongly
suggested to benefit from additional hardening), the weak symbol for
computeHardwareCRC32 is read-only and the assembly generated is fairly clean
and straight forward. As suggested, an additional optimization is to skip
the runtime check if SSE 4.2 has been enabled globally, as opposed to only
for scudo_crc32.cpp.

scudo_crc32.h has no purpose anymore and was removed.

Reviewers: alekseyshl, kcc, rengolin, mgorny, phosek

Reviewed By: rengolin, mgorny

Subscribers: aemerson, rengolin, llvm-commits

Differential Revision: https://reviews.llvm.org/D28574

llvm-svn: 292409

compiler-rt/cmake/config-ix.cmake

@@ -31,6 +31,7 @@ check_cxx_compiler_flag(-fno-lto             COMPILER_RT_HAS_FNO_LTO_FLAG)
 check_cxx_compiler_flag("-Werror -msse3" COMPILER_RT_HAS_MSSE3_FLAG)
 check_cxx_compiler_flag("-Werror -msse4.2"   COMPILER_RT_HAS_MSSE4_2_FLAG)
 check_cxx_compiler_flag(--sysroot=.          COMPILER_RT_HAS_SYSROOT_FLAG)
+check_cxx_compiler_flag("-Werror -mcrc"      COMPILER_RT_HAS_MCRC_FLAG)
 
 if(NOT WIN32 AND NOT CYGWIN)
   # MinGW warns if -fvisibility-inlines-hidden is used.

compiler-rt/lib/scudo/CMakeLists.txt

@@ -16,10 +16,17 @@ set(SCUDO_SOURCES
   scudo_termination.cpp
   scudo_utils.cpp)
 
+# Enable the SSE 4.2 instruction set for scudo_crc32.cpp, if available.
 if (COMPILER_RT_HAS_MSSE4_2_FLAG)
   set_source_files_properties(scudo_crc32.cpp PROPERTIES COMPILE_FLAGS -msse4.2)
 endif()
 
+# Enable the AArch64 CRC32 feature for scudo_crc32.cpp, if available.
+# Note that it is enabled by default starting with armv8.1-a.
+if (COMPILER_RT_HAS_MCRC_FLAG)
+  set_source_files_properties(scudo_crc32.cpp PROPERTIES COMPILE_FLAGS -mcrc)
+endif()
+
 if(COMPILER_RT_HAS_SCUDO)
   foreach(arch ${SCUDO_SUPPORTED_ARCH})
     add_compiler_rt_runtime(clang_rt.scudo

compiler-rt/lib/scudo/scudo_allocator.cpp

@@ -15,7 +15,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "scudo_allocator.h"
-#include "scudo_crc32.h"
 #include "scudo_utils.h"
 
 #include "sanitizer_common/sanitizer_allocator_interface.h"
@@ -73,15 +72,21 @@ static uptr Cookie;
 // at compilation or at runtime.
 static atomic_uint8_t HashAlgorithm = { CRC32Software };
 
-// Helper function that will compute the chunk checksum, being passed all the
-// the needed information as uptrs. It will opt for the hardware version of
-// the checksumming function if available.
-INLINE u32 hashUptrs(uptr Pointer, uptr *Array, uptr ArraySize, u8 HashType) {
-  u32 Crc;
-  Crc = computeCRC32(Cookie, Pointer, HashType);
-  for (uptr i = 0; i < ArraySize; i++)
-    Crc = computeCRC32(Crc, Array[i], HashType);
-  return Crc;
+SANITIZER_WEAK_ATTRIBUTE u32 computeHardwareCRC32(u32 Crc, uptr Data);
+
+INLINE u32 computeCRC32(u32 Crc, uptr Data, u8 HashType) {
+  // If SSE4.2 is defined here, it was enabled everywhere, as opposed to only
+  // for scudo_crc32.cpp. This means that other SSE instructions were likely
+  // emitted at other places, and as a result there is no reason to not use
+  // the hardware version of the CRC32.
+#if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+  return computeHardwareCRC32(Crc, Data);
+#else
+  if (computeHardwareCRC32 && HashType == CRC32Hardware)
+    return computeHardwareCRC32(Crc, Data);
+  else
+    return computeSoftwareCRC32(Crc, Data);
+#endif  // defined(__SSE4_2__)
 }
 
 struct ScudoChunk : UnpackedHeader {
@@ -108,11 +113,11 @@ struct ScudoChunk : UnpackedHeader {
     ZeroChecksumHeader.Checksum = 0;
     uptr HeaderHolder[sizeof(UnpackedHeader) / sizeof(uptr)];
     memcpy(&HeaderHolder, &ZeroChecksumHeader, sizeof(HeaderHolder));
-    u32 Hash = hashUptrs(reinterpret_cast<uptr>(this),
-                         HeaderHolder,
-                         ARRAY_SIZE(HeaderHolder),
-                         atomic_load_relaxed(&HashAlgorithm));
-    return static_cast<u16>(Hash);
+    u8 HashType = atomic_load_relaxed(&HashAlgorithm);
+    u32 Crc = computeCRC32(Cookie, reinterpret_cast<uptr>(this), HashType);
+    for (uptr i = 0; i < ARRAY_SIZE(HeaderHolder); i++)
+      Crc = computeCRC32(Crc, HeaderHolder[i], HashType);
+    return static_cast<u16>(Crc);
   }
 
   // Checks the validity of a chunk by verifying its checksum.

compiler-rt/lib/scudo/scudo_allocator.h

@@ -18,6 +18,10 @@
 
 #include "sanitizer_common/sanitizer_allocator.h"
 
+#if !SANITIZER_LINUX
+# error "The Scudo hardened allocator is currently only supported on Linux."
+#endif
+
 #include <atomic>
 
 namespace __scudo {

compiler-rt/lib/scudo/scudo_crc32.cpp

@@ -12,13 +12,13 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include "sanitizer_common/sanitizer_internal_defs.h"
+
 // Hardware CRC32 is supported at compilation via the following:
 // - for i386 & x86_64: -msse4.2
 // - for ARM & AArch64: -march=armv8-a+crc or -mcrc
 // An additional check must be performed at runtime as well to make sure the
 // emitted instructions are valid on the target host.
-#include "scudo_crc32.h"
-#include "scudo_utils.h"
 
 #if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 # ifdef __SSE4_2__
@@ -34,20 +34,9 @@
 namespace __scudo {
 
 #if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
-INLINE u32 computeHardwareCRC32(u32 Crc, uptr Data) {
+u32 computeHardwareCRC32(u32 Crc, uptr Data) {
   return CRC32_INTRINSIC(Crc, Data);
 }
-
-u32 computeCRC32(u32 Crc, uptr Data, u8 HashType) {
-  if (HashType == CRC32Hardware) {
-    return computeHardwareCRC32(Crc, Data);
-  }
-  return computeSoftwareCRC32(Crc, Data);
-}
-#else
-u32 computeCRC32(u32 Crc, uptr Data, u8 HashType) {
-  return computeSoftwareCRC32(Crc, Data);
-}
 #endif  // defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 
 }  // namespace __scudo

compiler-rt/lib/scudo/scudo_crc32.h

@@ -1,30 +0,0 @@
-//===-- scudo_crc32.h -------------------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-///
-/// Header for scudo_crc32.cpp.
-///
-//===----------------------------------------------------------------------===//
-
-#ifndef SCUDO_CRC32_H_
-#define SCUDO_CRC32_H_
-
-#include "sanitizer_common/sanitizer_internal_defs.h"
-
-namespace __scudo {
-
-enum : u8 {
-  CRC32Software = 0,
-  CRC32Hardware = 1,
-};
-
-u32 computeCRC32(u32 Crc, uptr Data, u8 HashType);
-
-}  // namespace __scudo
-
-#endif  // SCUDO_CRC32_H_

compiler-rt/lib/scudo/scudo_utils.cpp

@@ -20,8 +20,9 @@
 #if defined(__x86_64__) || defined(__i386__)
 # include <cpuid.h>
 #endif
-
-#include <cstring>
+#if defined(__arm__) || defined(__aarch64__)
+# include <sys/auxv.h>
+#endif
 
 // TODO(kostyak): remove __sanitizer *Printf uses in favor for our own less
 //                complicated string formatting code. The following is a
@@ -82,7 +83,7 @@ CPUIDRegs getCPUFeatures() {
 }
 
 #ifndef bit_SSE4_2
-#define bit_SSE4_2 bit_SSE42  // clang and gcc have different defines.
+# define bit_SSE4_2 bit_SSE42  // clang and gcc have different defines.
 #endif
 
 bool testCPUFeature(CPUFeature Feature)
@@ -97,6 +98,25 @@ bool testCPUFeature(CPUFeature Feature)
   }
   return false;
 }
+#elif defined(__arm__) || defined(__aarch64__)
+// For ARM and AArch64, hardware CRC32 support is indicated in the
+// AT_HWVAL auxiliary vector.
+
+#ifndef HWCAP_CRC32
+# define HWCAP_CRC32 (1<<7)  // HWCAP_CRC32 is missing on older platforms.
+#endif
+
+bool testCPUFeature(CPUFeature Feature) {
+  uptr HWCap = getauxval(AT_HWCAP);
+
+  switch (Feature) {
+    case CRC32CPUFeature:
+      return !!(HWCap & HWCAP_CRC32);
+    default:
+      break;
+  }
+  return false;
+}
 #else
 bool testCPUFeature(CPUFeature Feature) {
   return false;

compiler-rt/lib/scudo/scudo_utils.h

@@ -53,7 +53,11 @@ struct Xorshift128Plus {
   u64 State[2];
 };
 
-// Software CRC32 functions, to be used when hardware support is not detected.
+enum : u8 {
+  CRC32Software = 0,
+  CRC32Hardware = 1,
+};
+
 u32 computeSoftwareCRC32(u32 Crc, uptr Data);
 
 }  // namespace __scudo