[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

975

should this get the same fix too?

[aralisza]

change order of frexp too

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

975

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

977

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

977

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

977

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

977

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

977

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

977

COMMON_INTERCEPTOR_INITIALIZE_RANGE

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

enum {
     CIMT_FILE
   } type;
   union {
     FileMetadata file;
   };
 };
 
 #if SI_POSIX
 typedef AddrHashMap<CommonInterceptorMetadata, 31051> MetadataHashMap;
 
 static MetadataHashMap *interceptor_metadata_map;
 
 UNUSED static void SetInterceptorMetadata(__sanitizer_FILE *addr,
                                           const FileMetadata &file) {
   MetadataHashMap::Handle h(interceptor_metadata_map, (uptr)addr);
   CHECK(h.created());
   h->type = CommonInterceptorMetadata::CIMT_FILE;
   h->file = file;
 }
 
 UNUSED static const FileMetadata *GetInterceptorMetadata(
     __sanitizer_FILE *addr) {

 
 UNUSED static void DeleteInterceptorMetadata(void *addr) {
   MetadataHashMap::Handle h(interceptor_metadata_map, (uptr)addr, true);
   CHECK(h.exists());
 }
 #endif  // SI_POSIX
 
 #if SANITIZER_INTERCEPT_STRLEN
 INTERCEPTOR(SIZE_T, strlen, const char *s) {
   // Sometimes strlen is called prior to InitializeCommonInterceptors,
   // in which case the REAL(strlen) typically used in
   // COMMON_INTERCEPTOR_ENTER will fail.  We use internal_strlen here
   // to handle that.
   if (COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED)
     return internal_strlen(s);
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, strlen, s);
   SIZE_T result = REAL(strlen)(s);
   if (common_flags()->intercept_strlen)
     COMMON_INTERCEPTOR_READ_RANGE(ctx, s, result + 1);
   return result;
 }
 #define INIT_STRLEN COMMON_INTERCEPT_FUNCTION(strlen)
 #else
 #define INIT_STRLEN
 #endif
 
 #if SANITIZER_INTERCEPT_STRNLEN
 INTERCEPTOR(SIZE_T, strnlen, const char *s, SIZE_T maxlen) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, strnlen, s, maxlen);
   SIZE_T length = REAL(strnlen)(s, maxlen);
   if (common_flags()->intercept_strlen)
     COMMON_INTERCEPTOR_READ_RANGE(ctx, s, Min(length + 1, maxlen));
   return length;
 }
 #define INIT_STRNLEN COMMON_INTERCEPT_FUNCTION(strnlen)
 #else
 #define INIT_STRNLEN
 #endif
 
 #if SANITIZER_INTERCEPT_STRNDUP
 INTERCEPTOR(char*, strndup, const char *s, uptr size) {
   void *ctx;
   COMMON_INTERCEPTOR_STRNDUP_IMPL(ctx, s, size);
 }
 #define INIT_STRNDUP COMMON_INTERCEPT_FUNCTION(strndup)
 #else
 #define INIT_STRNDUP
 #endif // SANITIZER_INTERCEPT_STRNDUP
 

 
 #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));
   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);
   return res;
 }
 
 #define INIT_FREXPF_FREXPL           \
   COMMON_INTERCEPT_FUNCTION(frexpf); \
   COMMON_INTERCEPT_FUNCTION_LDBL(frexpl)
 #else
 #define INIT_FREXPF_FREXPL