[compiler-rt] 7a9ebe9 - [msan] Intercept qsort, qsort_r.

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 26 12:37:34 PST 2019


Hi,

This patch causes build failures of the sanitizers with watchOS enabled (and maybe tvOS), similar to the failure with the iOS simulator.

I went ahead and disabled QSORT interception for both in 54c522420347e58aa7bae1892cf5c5672b57c875. It would be great if you could have a look to double check if that fix makes sense.

Cheers,
Florian

> On Dec 23, 2019, at 20:35, Evgenii Stepanov via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 
> Author: Evgenii Stepanov
> Date: 2019-12-23T11:34:49-08:00
> New Revision: 7a9ebe95125ea87a494d0c18f44f10bd70e12188
> 
> URL: https://github.com/llvm/llvm-project/commit/7a9ebe95125ea87a494d0c18f44f10bd70e12188
> DIFF: https://github.com/llvm/llvm-project/commit/7a9ebe95125ea87a494d0c18f44f10bd70e12188.diff
> 
> LOG: [msan] Intercept qsort, qsort_r.
> 
> Summary:
> This fixes qsort-related false positives with glibc-2.27.
> I'm not entirely sure why they did not show up with the earlier
> versions; the code seems similar enough.
> 
> Reviewers: vitalybuka
> 
> Subscribers: #sanitizers, llvm-commits
> 
> Tags: #sanitizers, #llvm
> 
> Differential Revision: https://reviews.llvm.org/D71740
> 
> Added: 
>    compiler-rt/test/msan/qsort.cpp
> 
> Modified: 
>    compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
>    compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
> 
> Removed: 
> 
> 
> 
> ################################################################################
> diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
> index af301a1a689c..2d9636a9c879 100644
> --- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
> +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
> @@ -9639,6 +9639,75 @@ 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
> +
> static void InitializeCommonInterceptors() {
> #if SI_POSIX
>   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
> @@ -9940,6 +10009,8 @@ 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 61a6b82ef818..08937122e0bc 100644
> --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
> +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
> @@ -90,6 +90,12 @@
> # define SI_IOS 0
> #endif
> 
> +#if SANITIZER_IOSSIM
> +# define SI_IOSSIM 1
> +#else
> +# define SI_IOSSIM 0
> +#endif
> +
> #if SANITIZER_FUCHSIA
> # define SI_NOT_FUCHSIA 0
> #else
> @@ -575,5 +581,7 @@
> #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)
> +#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
> new file mode 100644
> index 000000000000..eb8697011867
> --- /dev/null
> +++ b/compiler-rt/test/msan/qsort.cpp
> @@ -0,0 +1,73 @@
> +// 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;
> +}
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list