[compiler-rt] change write order of frexpl & frexpf so it doesn't corrupt stack ids

[aralisza]

Summary

This was fixed in the past for frexp, but was not made for frexpl & frexpf https://github.com/google/sanitizers/issues/321
This patch copies the fix over to frexpl because it caused frexp_interceptor.cpp test to fail on iPhone and frexpf for consistency.

rdar://79652161

[aralisza]

remove comment

[kubamracek]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

972–973

should this get the same fix too?

[aralisza]

change order of frexp too

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

972–973

done~ I didn't do it originally because I figured there was a reason the original author didn't go ahead an make all these changes, but it probably makes sense to do it now anyway

[delcypher]

This seems reasonable to me. However, we are touching paths used by non-Apple platforms here so I'd like someone outside of Apple to approve too.

[aralisza]

Ping on this, would love to get an additional reviewer.

[delcypher]

@vitalybuka ping

[vitalybuka]

Can you please add trivial tests in /sanitizer_common?

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

976

Even if it does not fail could you also add COMMON_INTERCEPTOR_INITIALIZE_RANGE after the call
to make sure msan shadow is not corrupted by the calls internals

[delcypher]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

976

I would expect COMMON_INTERCEPTOR_WRITE_RANGE(ctx, exp, sizeof(*exp)) before the call to initialize the range. Do we actually suspect that REAL(frexpf)(x, exp) could corrupt MSan's shadow? I'm not sure how that would happen.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

976

e.g. calculate on stack of non-instrumented function and then memcpy into output which will be intercepted and also copy "uninitialized" shadow

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

976

With the current implementation of these function probably unnecessary.
But without assuming implementation details, COMMON_INTERCEPTOR_INITIALIZE_RANGE is right thing to do.

[aralisza]

add tests for frexpl and frexpf and call COMMON_INTERCEPTOR_INITIALIZE_RANGE

[aralisza]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

976

Is this what you are looking for? By "after the call" do you mean after the call to REAL(... or COMMON_INTERCEPTOR_WRITE_RANGE?

[aralisza]

make ordering more consistent

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

976

COMMON_INTERCEPTOR_INITIALIZE_RANGE

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

 
 #if SANITIZER_INTERCEPT_FREXP
 INTERCEPTOR(double, frexp, double x, int *exp) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, frexp, x, exp);
   // Assuming frexp() always writes to |exp|.
   COMMON_INTERCEPTOR_WRITE_RANGE(ctx, exp, sizeof(*exp));
   double res = REAL(frexp)(x, exp);
+  COMMON_INTERCEPTOR_INITIALIZE_RANGE(exp, sizeof(*exp));
   return res;
 }
 
 #define INIT_FREXP COMMON_INTERCEPT_FUNCTION(frexp);
 #else
 #define INIT_FREXP
 #endif  // SANITIZER_INTERCEPT_FREXP
 
 #if SANITIZER_INTERCEPT_FREXPF_FREXPL
 INTERCEPTOR(float, frexpf, float x, int *exp) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, frexpf, x, exp);
-  // FIXME: under ASan the call below may write to freed memory and corrupt
-  // its metadata. See
-  // https://github.com/google/sanitizers/issues/321.
-  float res = REAL(frexpf)(x, exp);
   COMMON_INTERCEPTOR_WRITE_RANGE(ctx, exp, sizeof(*exp));
+  float res = REAL(frexpf)(x, exp);
+  COMMON_INTERCEPTOR_INITIALIZE_RANGE(exp, sizeof(*exp));
   return res;
 }
 
 INTERCEPTOR(long double, frexpl, long double x, int *exp) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, frexpl, x, exp);
-  // FIXME: under ASan the call below may write to freed memory and corrupt
-  // its metadata. See
-  // https://github.com/google/sanitizers/issues/321.
-  long double res = REAL(frexpl)(x, exp);
   COMMON_INTERCEPTOR_WRITE_RANGE(ctx, exp, sizeof(*exp));
+  long double res = REAL(frexpl)(x, exp);
+  COMMON_INTERCEPTOR_INITIALIZE_RANGE(exp, sizeof(*exp));
   return res;
 }
 
 #define INIT_FREXPF_FREXPL           \
   COMMON_INTERCEPT_FUNCTION(frexpf); \
   COMMON_INTERCEPT_FUNCTION_LDBL(frexpl)
 #else
 #define INIT_FREXPF_FREXPL

compiler-rt/test/asan/TestCases/frexpf_interceptor.cpp

+// RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s
+
+// Test the frexpf() interceptor.
+
+#include <math.h>
+#include <stdio.h>
+#include <stdlib.h>
+int main() {
+  float x = 3.14;
+  int *exp = (int *)malloc(sizeof(int));
+  free(exp);
+  double y = frexpf(x, exp);
+  // CHECK: use-after-free
+  // CHECK: SUMMARY
+  return 0;
+}

compiler-rt/test/asan/TestCases/frexpl_interceptor.cpp

+// RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s
+
+// Test the frexpl() interceptor.
+
+#include <math.h>
+#include <stdio.h>
+#include <stdlib.h>
+int main() {
+  long double x = 3.14;
+  int *exp = (int *)malloc(sizeof(int));
+  free(exp);
+  double y = frexpl(x, exp);
+  // CHECK: use-after-free
+  // CHECK: SUMMARY
+  return 0;
+}