[compiler-rt] 8fcce5a - Revert "[msan] Intercept qsort, qsort_r."

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 27 11:27:11 PST 2019


Author: Reid Kleckner
Date: 2019-12-27T11:24:07-08:00
New Revision: 8fcce5ac73d49981656d9126e6c88391c1f6bf01

URL: https://github.com/llvm/llvm-project/commit/8fcce5ac73d49981656d9126e6c88391c1f6bf01
DIFF: https://github.com/llvm/llvm-project/commit/8fcce5ac73d49981656d9126e6c88391c1f6bf01.diff

LOG: Revert "[msan] Intercept qsort, qsort_r."

This reverts commit 7a9ebe95125ea87a494d0c18f44f10bd70e12188, and
dependent commit 54c522420347e58aa7bae1892cf5c5672b57c875, which
disables qsort interception for some iOS platforms.

After this change, the -Nolibc sanitizer common test binary crashes on
startup on my regular Linux workstation, as well as on our bots:
https://ci.chromium.org/p/chromium/builders/try/linux_upload_clang/740

 ********************
  Failing Tests (1):
       SanitizerCommon-Unit ::
       ./Sanitizer-x86_64-Test/SanitizerCommon.NolibcMain

Loading it up in gdb shows that it crashes during relocation processing,
which suggests that some glibc loader versions do not support the
THREADLOCAL data added in this interceptor.

Added: 
    

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
    compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

Removed: 
    compiler-rt/test/msan/qsort.cpp


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
index 8f365ee30854..58e4226eb0cc 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -9644,75 +9644,6 @@ INTERCEPTOR(int, getentropy, void *buf, SIZE_T buflen) {
 #define INIT_GETENTROPY
 #endif
 
-#if SANITIZER_INTERCEPT_QSORT
-// Glibc qsort uses a temporary buffer allocated either on stack or on heap.
-// Poisoned memory from there may get copied into the comparator arguments,
-// where it needs to be dealt with. But even that is not enough - the results of
-// the sort may be copied into the input/output array based on the results of
-// the comparator calls, but directly from the temp memory, bypassing the
-// unpoisoning done in wrapped_qsort_compar. We deal with this by, again,
-// unpoisoning the entire array after the sort is done.
-//
-// We can not check that the entire array is initialized at the beginning. IMHO,
-// it's fine for parts of the sorted objects to contain uninitialized memory,
-// ex. as padding in structs.
-typedef int (*qsort_compar_f)(const void *, const void *);
-static THREADLOCAL qsort_compar_f qsort_compar;
-static THREADLOCAL SIZE_T qsort_size;
-int wrapped_qsort_compar(const void *a, const void *b) {
-  COMMON_INTERCEPTOR_UNPOISON_PARAM(2);
-  COMMON_INTERCEPTOR_INITIALIZE_RANGE(a, qsort_size);
-  COMMON_INTERCEPTOR_INITIALIZE_RANGE(b, qsort_size);
-  return qsort_compar(a, b);
-}
-
-INTERCEPTOR(void, qsort, void *base, SIZE_T nmemb, SIZE_T size,
-            qsort_compar_f compar) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, qsort, base, nmemb, size, compar);
-  qsort_compar_f old_compar = qsort_compar;
-  qsort_compar = compar;
-  SIZE_T old_size = qsort_size;
-  qsort_size = size;
-  REAL(qsort)(base, nmemb, size, wrapped_qsort_compar);
-  qsort_compar = old_compar;
-  qsort_size = old_size;
-  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, base, nmemb * size);
-}
-#define INIT_QSORT COMMON_INTERCEPT_FUNCTION(qsort)
-#else
-#define INIT_QSORT
-#endif
-
-#if SANITIZER_INTERCEPT_QSORT_R
-typedef int (*qsort_r_compar_f)(const void *, const void *, void *);
-static THREADLOCAL qsort_r_compar_f qsort_r_compar;
-static THREADLOCAL SIZE_T qsort_r_size;
-int wrapped_qsort_r_compar(const void *a, const void *b, void *arg) {
-  COMMON_INTERCEPTOR_UNPOISON_PARAM(3);
-  COMMON_INTERCEPTOR_INITIALIZE_RANGE(a, qsort_r_size);
-  COMMON_INTERCEPTOR_INITIALIZE_RANGE(b, qsort_r_size);
-  return qsort_r_compar(a, b, arg);
-}
-
-INTERCEPTOR(void, qsort_r, void *base, SIZE_T nmemb, SIZE_T size,
-            qsort_r_compar_f compar, void *arg) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, qsort_r, base, nmemb, size, compar, arg);
-  qsort_r_compar_f old_compar = qsort_r_compar;
-  qsort_r_compar = compar;
-  SIZE_T old_size = qsort_r_size;
-  qsort_r_size = size;
-  REAL(qsort_r)(base, nmemb, size, wrapped_qsort_r_compar, arg);
-  qsort_r_compar = old_compar;
-  qsort_r_size = old_size;
-  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, base, nmemb * size);
-}
-#define INIT_QSORT_R COMMON_INTERCEPT_FUNCTION(qsort_r)
-#else
-#define INIT_QSORT_R
-#endif
-
 #include "sanitizer_common_interceptors_netbsd_compat.inc"
 
 static void InitializeCommonInterceptors() {
@@ -10016,8 +9947,6 @@ static void InitializeCommonInterceptors() {
   INIT_CRYPT;
   INIT_CRYPT_R;
   INIT_GETENTROPY;
-  INIT_QSORT;
-  INIT_QSORT_R;
 
   INIT___PRINTF_CHK;
 }

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
index 4cc69af1241d..61a6b82ef818 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -90,24 +90,6 @@
 # define SI_IOS 0
 #endif
 
-#if SANITIZER_IOSSIM
-# define SI_IOSSIM 1
-#else
-# define SI_IOSSIM 0
-#endif
-
-#if SANITIZER_WATCHOS
-# define SI_WATCHOS 1
-#else
-# define SI_WATCHOS 0
-#endif
-
-#if SANITIZER_TVOS
-# define SI_TVOS 1
-#else
-# define SI_TVOS 0
-#endif
-
 #if SANITIZER_FUCHSIA
 # define SI_NOT_FUCHSIA 0
 #else
@@ -593,8 +575,5 @@
 #define SANITIZER_INTERCEPT_ATEXIT SI_NETBSD
 #define SANITIZER_INTERCEPT_PTHREAD_ATFORK SI_NETBSD
 #define SANITIZER_INTERCEPT_GETENTROPY SI_FREEBSD
-#define SANITIZER_INTERCEPT_QSORT \
-  (SI_POSIX && !SI_IOSSIM && !SI_WATCHOS && !SI_TVOS)
-#define SANITIZER_INTERCEPT_QSORT_R (SI_LINUX && !SI_ANDROID)
 
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

diff  --git a/compiler-rt/test/msan/qsort.cpp b/compiler-rt/test/msan/qsort.cpp
deleted file mode 100644
index eb8697011867..000000000000
--- a/compiler-rt/test/msan/qsort.cpp
+++ /dev/null
@@ -1,73 +0,0 @@
-// RUN: %clangxx_msan -O0 -g %s -o %t && %run %t
-
-#include <assert.h>
-#include <errno.h>
-#include <glob.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <sanitizer/msan_interface.h>
-
-constexpr size_t kSize1 = 27;
-constexpr size_t kSize2 = 7;
-
-bool seen2;
-
-void dummy(long a, long b, long c, long d, long e) {}
-
-void poison_stack_and_param() {
-  char x[10000];
-  int y;
-  dummy(y, y, y, y, y);
-}
-
-__attribute__((always_inline)) int cmp(long a, long b) {
-  if (a < b)
-    return -1;
-  else if (a > b)
-    return 1;
-  else
-    return 0;
-}
-
-int compar2(const void *a, const void *b) {
-  assert(a);
-  assert(b);
-  __msan_check_mem_is_initialized(a, sizeof(long));
-  __msan_check_mem_is_initialized(b, sizeof(long));
-  seen2 = true;
-  poison_stack_and_param();
-  return cmp(*(long *)a, *(long *)b);
-}
-
-int compar1(const void *a, const void *b) {
-  assert(a);
-  assert(b);
-  __msan_check_mem_is_initialized(a, sizeof(long));
-  __msan_check_mem_is_initialized(b, sizeof(long));
-
-  long *p = new long[kSize2];
-  // kind of random
-  for (int i = 0; i < kSize2; ++i)
-    p[i] = i * 2 + (i % 3 - 1) * 3;
-  qsort(p, kSize1, sizeof(long), compar2);
-  __msan_check_mem_is_initialized(p, sizeof(long) * kSize2);
-  delete[] p;
-
-  poison_stack_and_param();
-  return cmp(*(long *)a, *(long *)b);
-}
-
-int main(int argc, char *argv[]) {
-  long *p = new long[kSize1];
-  // kind of random
-  for (int i = 0; i < kSize1; ++i)
-    p[i] = i * 2 + (i % 3 - 1) * 3;
-  poison_stack_and_param();
-  qsort(p, kSize1, sizeof(long), compar1);
-  __msan_check_mem_is_initialized(p, sizeof(long) * kSize1);
-  assert(seen2);
-  delete[] p;
-  return 0;
-}


        


More information about the llvm-commits mailing list