[PATCH] D71740: [msan] Intercept qsort, qsort_r.

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:9659
+  COMMON_INTERCEPTOR_UNPOISON_PARAM(2);
+  COMMON_INTERCEPTOR_INITIALIZE_RANGE(a, qsort_size);
+  COMMON_INTERCEPTOR_INITIALIZE_RANGE(b, qsort_size);
----------------
do we need  COMMON_INTERCEPTOR_READ_RANGE(ctx, src, size);  for a and b?
or we going to assume that compar is instrumented?


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:9661
+  COMMON_INTERCEPTOR_INITIALIZE_RANGE(b, qsort_size);
+  return qsort_compar(a, b);
+}
----------------
Problem here that if array has uninitialized fields they are going to be unpoisoned by compar
maybe before calling actual qsort we can call
compar(base[0], base[1])
compar(base[1], base[2])
compar(base[3], base[4])
...
just to trigger report of a bug is there?


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:9686
+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);
----------------
eugenis wrote:
> vitalybuka wrote:
> > you can wrap <qsort_r_size, qsort_r_compar, arg> and pass as arg
> Yes, but I can not do the same in qsort(). I kind of prefer the two interceptors to do the same thing to avoid introducing new, unique bugs in qsort_r, but I don't have a strong feeling about it.
> 
> I could not use qsort_r to implement the interceptor for qsort, because it does not exist on the same set of platforms.
this way you can't call qsort from qsort_compar recursively :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71740/new/

https://reviews.llvm.org/D71740