[compiler-rt] Add SourceLocations for float_cast_overflow data.

[filcab]

Summary

Compiler-rt part of http://reviews.llvm.org/D11757
I ended up making UBSan work with both the old version and the new
version of the float_cast_overflow data (instead of just erroring with
the previous version). The old version will try to symbolize its caller.

Now we compile the float_cast_overflow tests without -g, and make sure
we have the source file+line+column.

If you think I'm trying too hard to make sure we can still use both
versions, let me know.

[filcab]

Remove undefined behavior (unless I misread the standard) :-)

We could use __builtin_memcpy to avoid the call and get a simple "mov" instruction (on x86), but I'm not sure if that's available for all compilers we support.
Opinions?

[samsonov]

lib/ubsan/ubsan_handlers.h

101 ↗

(On Diff #31523)

with older compilers.

114 ↗

(On Diff #31523)

just use void* Data (or u8* Data) here, and reinterpret_cast it in the function? That would help in looksLikeFloatCastOverflowDataV1

test/ubsan/TestCases/Float/cast-overflow.cpp

89 ↗

(On Diff #31523)

use [[@LINE+1]] here and below to reference line number.

[filcab]

Addressed Alexey's comments.

[filcab]

Addressed Alexey's comments. Had to cheat on the "__int128 not supported" case, but I think the current test is not bad.
Also simplified the return expression on looksLikeFloatCastOverflowDataV1.

That would help in looksLikeFloatCastOverflowDataV1

I'm not sure what you mean here.

[samsonov]

LGTM. Thanks for doing this!

lib/ubsan/ubsan_handlers.cc

297 ↗

(On Diff #31574)

Why not reinterpret_cast<u8*>(Data)?

lib/ubsan/ubsan_handlers.h

114 ↗

(On Diff #31574)

Please add a comment that "void *Data" should be either FloatCastOverflowData* or FloatCastOverflowDataV2*.

[filcab]

Will update on the repo. Thanks for the review.

lib/ubsan/ubsan_handlers.cc

297 ↗

(On Diff #31574)

It would actually be: *reinterpret_cast<u8**>(Data), which would dereferrence a pointer to char* OR a pointer to TypeDescriptor, which AFAICT, would technically be UB in one of those cases, due to dereferencing a pointer to A as a pointer to B (where A is TypeDescriptor, and B is a char *). (the first version of the patch had this, and this shouldn't be a problem, but still... technically UB as far as I can tell).

[samsonov]

lib/ubsan/ubsan_handlers.cc

297 ↗

(On Diff #31574)

I see, thanks for the explanation!

compiler-rt/trunk/lib/ubsan/ubsan_handlers.h

 struct VLABoundData {
   SourceLocation Loc;
   const TypeDescriptor &Type;
 };
 
 /// \brief Handle a VLA with a non-positive bound.
 RECOVERABLE(vla_bound_not_positive, VLABoundData *Data, ValueHandle Bound)
 
+// Keeping this around for binary compatibility with (sanitized) programs
+// compiled with older compilers.
 struct FloatCastOverflowData {
-  // FIXME: SourceLocation Loc;
   const TypeDescriptor &FromType;
   const TypeDescriptor &ToType;
 };
 
-/// \brief Handle overflow in a conversion to or from a floating-point type.
-RECOVERABLE(float_cast_overflow, FloatCastOverflowData *Data, ValueHandle From)
+struct FloatCastOverflowDataV2 {
+  SourceLocation Loc;
+  const TypeDescriptor &FromType;
+  const TypeDescriptor &ToType;
+};
+
+/// Handle overflow in a conversion to or from a floating-point type.
+/// void *Data is one of FloatCastOverflowData* or FloatCastOverflowDataV2*
+RECOVERABLE(float_cast_overflow, void *Data, ValueHandle From)
 
 struct InvalidValueData {
   SourceLocation Loc;
   const TypeDescriptor &Type;
 };
 
 /// \brief Handle a load of an invalid value for the type.
 RECOVERABLE(load_invalid_value, InvalidValueData *Data, ValueHandle Val)

compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc

 }
 void __ubsan::__ubsan_handle_vla_bound_not_positive_abort(VLABoundData *Data,
                                                           ValueHandle Bound) {
   GET_REPORT_OPTIONS(true);
   handleVLABoundNotPositive(Data, Bound, Opts);
   Die();
 }
 
-static void handleFloatCastOverflow(FloatCastOverflowData *Data,
-                                    ValueHandle From, ReportOptions Opts) {
-  // TODO: Add deduplication once a SourceLocation is generated for this check.
-  SymbolizedStackHolder CallerLoc(getCallerLocation(Opts.pc));
-  Location Loc = CallerLoc;
+static bool looksLikeFloatCastOverflowDataV1(void *Data) {
+  // First field is either a pointer to filename or a pointer to a
+  // TypeDescriptor.
+  u8 *FilenameOrTypeDescriptor;
+  internal_memcpy(&FilenameOrTypeDescriptor, Data,
+                  sizeof(FilenameOrTypeDescriptor));
+
+  // Heuristic: For float_cast_overflow, the TypeKind will be either TK_Integer
+  // (0x0) or TK_Float (0x1). Adding both bytes will be 0 or 1 (for BE or LE).
+  // If it were a filename, adding two printable characters will not yield such
+  // a value.
+  u16 MaybeFromTypeKind =
+      FilenameOrTypeDescriptor[0] + FilenameOrTypeDescriptor[1];
+  return MaybeFromTypeKind < 2;
+}
+
+static void handleFloatCastOverflow(void *DataPtr, ValueHandle From,
+                                    ReportOptions Opts) {
+  SymbolizedStackHolder CallerLoc;
+  Location Loc;
+  const TypeDescriptor *FromType, *ToType;
+
+  if (looksLikeFloatCastOverflowDataV1(DataPtr)) {
+    auto Data = reinterpret_cast<FloatCastOverflowData *>(DataPtr);
+    CallerLoc.reset(getCallerLocation(Opts.pc));
+    Loc = CallerLoc;
+    FromType = &Data->FromType;
+    ToType = &Data->ToType;
+  } else {
+    auto Data = reinterpret_cast<FloatCastOverflowDataV2 *>(DataPtr);
+    SourceLocation SLoc = Data->Loc.acquire();
+    if (ignoreReport(SLoc, Opts))
+      return;
+    Loc = SLoc;
+    FromType = &Data->FromType;
+    ToType = &Data->ToType;
+  }
+
   ScopedReport R(Opts, Loc);
 
   Diag(Loc, DL_Error,
        "value %0 is outside the range of representable values of type %2")
-      << Value(Data->FromType, From) << Data->FromType << Data->ToType;
+      << Value(*FromType, From) << *FromType << *ToType;
 }
 
-void __ubsan::__ubsan_handle_float_cast_overflow(FloatCastOverflowData *Data,
-                                                 ValueHandle From) {
+void __ubsan::__ubsan_handle_float_cast_overflow(void *Data, ValueHandle From) {
   GET_REPORT_OPTIONS(false);
   handleFloatCastOverflow(Data, From, Opts);
 }
-void
-__ubsan::__ubsan_handle_float_cast_overflow_abort(FloatCastOverflowData *Data,
+void __ubsan::__ubsan_handle_float_cast_overflow_abort(void *Data,
                                                        ValueHandle From) {
   GET_REPORT_OPTIONS(true);
   handleFloatCastOverflow(Data, From, Opts);
   Die();
 }
 
 static void handleLoadInvalidValue(InvalidValueData *Data, ValueHandle Val,
                                    ReportOptions Opts) {
   SourceLocation Loc = Data->Loc.acquire();

compiler-rt/trunk/test/ubsan/TestCases/Float/cast-overflow.cpp

-// RUN: %clangxx -fsanitize=float-cast-overflow -g %s -o %t
+// RUN: %clangxx -fsanitize=float-cast-overflow %s -o %t
 // RUN: %run %t _
 // RUN: env UBSAN_OPTIONS=print_summary=1 %run %t 0 2>&1 | FileCheck %s --check-prefix=CHECK-0
 // RUN: %run %t 1 2>&1 | FileCheck %s --check-prefix=CHECK-1
 // RUN: %run %t 2 2>&1 | FileCheck %s --check-prefix=CHECK-2
 // RUN: %run %t 3 2>&1 | FileCheck %s --check-prefix=CHECK-3
 // RUN: %run %t 4 2>&1 | FileCheck %s --check-prefix=CHECK-4
 // RUN: %run %t 5 2>&1 | FileCheck %s --check-prefix=CHECK-5
 // RUN: %run %t 6 2>&1 | FileCheck %s --check-prefix=CHECK-6
#endif
 
   switch (argv[1][0]) {
     // FIXME: Produce a source location for these checks and test for it here.
 
     // Floating point -> integer overflow.
   case '0': {
     // Note that values between 0x7ffffe00 and 0x80000000 may or may not
     // successfully round-trip, depending on the rounding mode.
-    // CHECK-0: runtime error: value 2.14748{{.*}} is outside the range of representable values of type 'int'
+    // CHECK-0: {{.*}}cast-overflow.cpp:[[@LINE+1]]:27: runtime error: value 2.14748{{.*}} is outside the range of representable values of type 'int'
     static int test_int = MaxFloatRepresentableAsInt + 0x80;
     // CHECK-0: SUMMARY: {{.*}}Sanitizer: undefined-behavior {{.*}}cast-overflow.cpp:[[@LINE-1]]
     return 0;
     }
   case '1': {
-    // CHECK-1: runtime error: value -2.14748{{.*}} is outside the range of representable values of type 'int'
+    // CHECK-1: {{.*}}cast-overflow.cpp:[[@LINE+1]]:27: runtime error: value -2.14748{{.*}} is outside the range of representable values of type 'int'
     static int test_int = MinFloatRepresentableAsInt - 0x100;
     return 0;
   }
   case '2': {
-    // CHECK-2: runtime error: value -1 is outside the range of representable values of type 'unsigned int'
+    // CHECK-2: {{.*}}cast-overflow.cpp:[[@LINE+2]]:37: runtime error: value -1 is outside the range of representable values of type 'unsigned int'
     volatile float f = -1.0;
     volatile unsigned u = (unsigned)f;
     return 0;
   }
   case '3': {
-    // CHECK-3: runtime error: value 4.2949{{.*}} is outside the range of representable values of type 'unsigned int'
+    // CHECK-3: {{.*}}cast-overflow.cpp:[[@LINE+1]]:37: runtime error: value 4.2949{{.*}} is outside the range of representable values of type 'unsigned int'
     static int test_int = (unsigned)(MaxFloatRepresentableAsUInt + 0x100);
     return 0;
   }
 
   case '4': {
-    // CHECK-4: runtime error: value {{.*}} is outside the range of representable values of type 'int'
+    // CHECK-4: {{.*}}cast-overflow.cpp:[[@LINE+1]]:27: runtime error: value {{.*}} is outside the range of representable values of type 'int'
     static int test_int = Inf;
     return 0;
   }
   case '5': {
-    // CHECK-5: runtime error: value {{.*}} is outside the range of representable values of type 'int'
+    // CHECK-5: {{.*}}cast-overflow.cpp:[[@LINE+1]]:27: runtime error: value {{.*}} is outside the range of representable values of type 'int'
     static int test_int = NaN;
     return 0;
   }
 
     // Integer -> floating point overflow.
   case '6': {
-    // CHECK-6: {{runtime error: value 0xffffff00000000000000000000000001 is outside the range of representable values of type 'float'|__int128 not supported}}
+    // CHECK-6: cast-overflow.cpp:[[@LINE+2]]:{{34: runtime error: value 0xffffff00000000000000000000000001 is outside the range of representable values of type 'float'| __int128 not supported}}
 #if defined(__SIZEOF_INT128__) && !defined(_WIN32)
     static int test_int = (float)(FloatMaxAsUInt128 + 1);
     return 0;
 #else
-    puts("__int128 not supported");
+    // Print the same line as the check above. That way the test is robust to
+    // line changes around it
+    printf("%s:%d: __int128 not supported", __FILE__, __LINE__ - 5);
     return 0;
 #endif
   }
   // FIXME: The backend cannot lower __fp16 operations on x86 yet.
   //case '7':
   //  (__fp16)65504; // ok
   //  // CHECK-7: runtime error: value 65505 is outside the range of representable values of type '__fp16'
   //  return (__fp16)65505;
 
     // Floating point -> floating point overflow.
   case '8':
-    // CHECK-8: runtime error: value 1e+39 is outside the range of representable values of type 'float'
+    // CHECK-8: {{.*}}cast-overflow.cpp:[[@LINE+1]]:19: runtime error: value 1e+39 is outside the range of representable values of type 'float'
     return (float)1e39;
   case '9':
     volatile long double ld = 300.0;
-    // CHECK-9: runtime error: value 300 is outside the range of representable values of type 'char'
+    // CHECK-9: {{.*}}cast-overflow.cpp:[[@LINE+1]]:14: runtime error: value 300 is outside the range of representable values of type 'char'
     char c = ld;
     return c;
   }
 }