Skip to content

Commit ddf897f

Browse files
committed
[msan] Check qsort input.
Summary: Qsort interceptor suppresses all checks by unpoisoning the data in the wrapper of a comparator function, and then unpoisoning the output array as well. This change adds an explicit run of the comparator on all elements of the input array to catch any sanitizer bugs. Reviewers: vitalybuka Subscribers: #sanitizers, llvm-commits Tags: #sanitizers, #llvm Differential Revision: https://reviews.llvm.org/D71780
1 parent 59878ec commit ddf897f

File tree

2 files changed

+23
-0
lines changed

2 files changed

+23
-0
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9665,6 +9665,15 @@ INTERCEPTOR(void, qsort, void *base, SIZE_T nmemb, SIZE_T size,
96659665
qsort_compar_f compar) {
96669666
void *ctx;
96679667
COMMON_INTERCEPTOR_ENTER(ctx, qsort, base, nmemb, size, compar);
9668+
// Run the comparator over all array elements to detect any memory issues.
9669+
for (SIZE_T i = 0; i < nmemb; ++i) {
9670+
void *p = (void *)((char *)base + i * size);
9671+
COMMON_INTERCEPTOR_UNPOISON_PARAM(2);
9672+
// Compare each element with itself to trigger an equality check, which
9673+
// typically requires the comparator to look as many of the object fields as
9674+
// possible.
9675+
compar(p, p);
9676+
}
96689677
qsort_compar_f old_compar = qsort_compar;
96699678
qsort_compar = compar;
96709679
SIZE_T old_size = qsort_size;
@@ -9694,6 +9703,15 @@ INTERCEPTOR(void, qsort_r, void *base, SIZE_T nmemb, SIZE_T size,
96949703
qsort_r_compar_f compar, void *arg) {
96959704
void *ctx;
96969705
COMMON_INTERCEPTOR_ENTER(ctx, qsort_r, base, nmemb, size, compar, arg);
9706+
// Run the comparator over all array elements to detect any memory issues.
9707+
for (SIZE_T i = 0; i < nmemb; ++i) {
9708+
void *p = (void *)((char *)base + i * size);
9709+
COMMON_INTERCEPTOR_UNPOISON_PARAM(3);
9710+
// Compare each element with itself to trigger an equality check, which
9711+
// typically requires the comparator to look as many of the object fields as
9712+
// possible.
9713+
compar(p, p, arg);
9714+
}
96979715
qsort_r_compar_f old_compar = qsort_r_compar;
96989716
qsort_r_compar = compar;
96999717
SIZE_T old_size = qsort_r_size;

compiler-rt/test/msan/qsort.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clangxx_msan -O0 -g %s -o %t && %run %t
2+
// RUN: %clangxx_msan -DPOISON -O0 -g %s -o %t && not %run %t 2>&1 | FileCheck %s
23

34
#include <assert.h>
45
#include <errno.h>
@@ -65,6 +66,10 @@ int main(int argc, char *argv[]) {
6566
for (int i = 0; i < kSize1; ++i)
6667
p[i] = i * 2 + (i % 3 - 1) * 3;
6768
poison_stack_and_param();
69+
#ifdef POISON
70+
__msan_poison(p + 1, sizeof(long));
71+
// CHECK: Uninitialized bytes in __msan_check_mem_is_initialized at offset 0 inside [{{.*}}, 8)
72+
#endif
6873
qsort(p, kSize1, sizeof(long), compar1);
6974
__msan_check_mem_is_initialized(p, sizeof(long) * kSize1);
7075
assert(seen2);

0 commit comments

Comments
 (0)