[compiler-rt] 7a9ebe9 - [msan] Intercept qsort, qsort_r.
Evgenii Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 26 12:50:36 PST 2019
Hi,
thanks, this looks fine. We could add a cmake check for thread-local
support in the future, because ex. for iossim it depends on the value
of -mmacosx-version-min. But this is good enough for now.
On Thu, Dec 26, 2019 at 12:37 PM Florian Hahn <florian_hahn at apple.com> wrote:
>
> 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