Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

stbergmann · stbergmann · commit 5745eccef54d · 2019-05-02T06:40:33.000Z
This follows up after b7692bc "[UBSan] Fix isDerivedFromAtOffset on iOS ARM64" fixed the RTTI comparison in isDerivedFromAtOffset on just one platform and then a25a2c7 "Always compare C++ typeinfo (based on libstdc++ implementation)" extended that fix to more platforms. But there is another RTTI comparison for -fsanitize=function generated in clang's CodeGenFunction::EmitCall as just a pointer comparison. For SANITIZER_NON_UNIQUE_TYPEINFO platforms this needs to be extended to also do string comparison. For that, __ubsan_handle_function_type_mismatch[_abort] takes the two std::type_info pointers as additional parameters now, checks them internally for potential equivalence, and returns without reporting failure if they turn out to be equivalent after all. (NORETURN needed to be dropped from the _abort variant for that.) Also these functions depend on ABI-specific RTTI now, so needed to be moved from plain UBSAN_SOURCES (ubsan_handlers.h/cc) to UBSAN_CXXABI_SOURCES (ubsan_handlers_cxx.h/cc), but as -fsanitize=function is only supported in C++ mode that's not a problem. Differential Revision: https://reviews.llvm.org/D60760 llvm-svn: 359759
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
@@ -4672,7 +4672,8 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
       llvm::Constant *StaticData[] = {EmitCheckSourceLocation(E->getBeginLoc()),
                                       EmitCheckTypeDescriptor(CalleeType)};
       EmitCheck(std::make_pair(CalleeRTTIMatch, SanitizerKind::Function),
-                SanitizerHandler::FunctionTypeMismatch, StaticData, CalleePtr);
+                SanitizerHandler::FunctionTypeMismatch, StaticData,
+                {CalleePtr, CalleeRTTI, FTRTTIConst});
 
       Builder.CreateBr(Cont);
       EmitBlock(Cont);
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cc b/compiler-rt/lib/ubsan/ubsan_handlers.cc
@@ -598,42 +598,6 @@ void __ubsan::__ubsan_handle_invalid_builtin_abort(InvalidBuiltinData *Data) {
   Die();
 }
 
-static void handleFunctionTypeMismatch(FunctionTypeMismatchData *Data,
-                                       ValueHandle Function,
-                                       ReportOptions Opts) {
-  SourceLocation CallLoc = Data->Loc.acquire();
-  ErrorType ET = ErrorType::FunctionTypeMismatch;
-
-  if (ignoreReport(CallLoc, Opts, ET))
-    return;
-
-  ScopedReport R(Opts, CallLoc, ET);
-
-  SymbolizedStackHolder FLoc(getSymbolizedLocation(Function));
-  const char *FName = FLoc.get()->info.function;
-  if (!FName)
-    FName = "(unknown)";
-
-  Diag(CallLoc, DL_Error, ET,
-       "call to function %0 through pointer to incorrect function type %1")
-      << FName << Data->Type;
-  Diag(FLoc, DL_Note, ET, "%0 defined here") << FName;
-}
-
-void
-__ubsan::__ubsan_handle_function_type_mismatch(FunctionTypeMismatchData *Data,
-                                               ValueHandle Function) {
-  GET_REPORT_OPTIONS(false);
-  handleFunctionTypeMismatch(Data, Function, Opts);
-}
-
-void __ubsan::__ubsan_handle_function_type_mismatch_abort(
-    FunctionTypeMismatchData *Data, ValueHandle Function) {
-  GET_REPORT_OPTIONS(true);
-  handleFunctionTypeMismatch(Data, Function, Opts);
-  Die();
-}
-
 static void handleNonNullReturn(NonNullReturnData *Data, SourceLocation *LocPtr,
                                 ReportOptions Opts, bool IsAttr) {
   if (!LocPtr)
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.h b/compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -168,15 +168,6 @@ struct InvalidBuiltinData {
 /// Handle a builtin called in an invalid way.
 RECOVERABLE(invalid_builtin, InvalidBuiltinData *Data)
 
-struct FunctionTypeMismatchData {
-  SourceLocation Loc;
-  const TypeDescriptor &Type;
-};
-
-RECOVERABLE(function_type_mismatch,
-            FunctionTypeMismatchData *Data,
-            ValueHandle Val)
-
 struct NonNullReturnData {
   SourceLocation AttrLoc;
 };
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc b/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
@@ -156,6 +156,51 @@ void __ubsan_handle_cfi_bad_type(CFICheckFailData *Data, ValueHandle Vtable,
     Diag(Loc, DL_Note, ET, "check failed in %0, vtable located in %1")
         << SrcModule << DstModule;
 }
+
+static bool handleFunctionTypeMismatch(FunctionTypeMismatchData *Data,
+                                       ValueHandle Function,
+                                       ValueHandle calleeRTTI,
+                                       ValueHandle fnRTTI, ReportOptions Opts) {
+  if (checkTypeInfoEquality(reinterpret_cast<void *>(calleeRTTI),
+                            reinterpret_cast<void *>(fnRTTI)))
+    return false;
+
+  SourceLocation CallLoc = Data->Loc.acquire();
+  ErrorType ET = ErrorType::FunctionTypeMismatch;
+
+  if (ignoreReport(CallLoc, Opts, ET))
+    return true;
+
+  ScopedReport R(Opts, CallLoc, ET);
+
+  SymbolizedStackHolder FLoc(getSymbolizedLocation(Function));
+  const char *FName = FLoc.get()->info.function;
+  if (!FName)
+    FName = "(unknown)";
+
+  Diag(CallLoc, DL_Error, ET,
+       "call to function %0 through pointer to incorrect function type %1")
+      << FName << Data->Type;
+  Diag(FLoc, DL_Note, ET, "%0 defined here") << FName;
+  return true;
+}
+
+void __ubsan_handle_function_type_mismatch(FunctionTypeMismatchData *Data,
+                                           ValueHandle Function,
+                                           ValueHandle calleeRTTI,
+                                           ValueHandle fnRTTI) {
+  GET_REPORT_OPTIONS(false);
+  handleFunctionTypeMismatch(Data, Function, calleeRTTI, fnRTTI, Opts);
+}
+
+void __ubsan_handle_function_type_mismatch_abort(FunctionTypeMismatchData *Data,
+                                                 ValueHandle Function,
+                                                 ValueHandle calleeRTTI,
+                                                 ValueHandle fnRTTI) {
+  GET_REPORT_OPTIONS(true);
+  if (handleFunctionTypeMismatch(Data, Function, calleeRTTI, fnRTTI, Opts))
+    Die();
+}
 }  // namespace __ubsan
 
 #endif // CAN_SANITIZE_UB
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers_cxx.h b/compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
@@ -33,6 +33,21 @@ void __ubsan_handle_dynamic_type_cache_miss(
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 void __ubsan_handle_dynamic_type_cache_miss_abort(
   DynamicTypeCacheMissData *Data, ValueHandle Pointer, ValueHandle Hash);
+
+struct FunctionTypeMismatchData {
+  SourceLocation Loc;
+  const TypeDescriptor &Type;
+};
+
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE void
+__ubsan_handle_function_type_mismatch(FunctionTypeMismatchData *Data,
+                                      ValueHandle Val, ValueHandle calleeRTTI,
+                                      ValueHandle fnRTTI);
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE void
+__ubsan_handle_function_type_mismatch_abort(FunctionTypeMismatchData *Data,
+                                            ValueHandle Val,
+                                            ValueHandle calleeRTTI,
+                                            ValueHandle fnRTTI);
 }
 
 #endif // UBSAN_HANDLERS_H
diff --git a/compiler-rt/lib/ubsan/ubsan_type_hash.h b/compiler-rt/lib/ubsan/ubsan_type_hash.h
@@ -64,6 +64,10 @@ const int VptrMaxOffsetToTop = 1<<20;
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 HashValue __ubsan_vptr_type_cache[VptrTypeCacheSize];
 
+/// \brief Do whatever is required by the ABI to check for std::type_info
+/// equivalence beyond simple pointer comparison.
+bool checkTypeInfoEquality(const void *TypeInfo1, const void *TypeInfo2);
+
 } // namespace __ubsan
 
 #endif // UBSAN_TYPE_HASH_H
diff --git a/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc b/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc
@@ -117,9 +117,7 @@ static bool isDerivedFromAtOffset(const abi::__class_type_info *Derived,
                                   const abi::__class_type_info *Base,
                                   sptr Offset) {
   if (Derived->__type_name == Base->__type_name ||
-      (SANITIZER_NON_UNIQUE_TYPEINFO &&
-       Derived->__type_name[0] != '*' &&
-       !internal_strcmp(Derived->__type_name, Base->__type_name)))
+      __ubsan::checkTypeInfoEquality(Derived, Base))
     return Offset == 0;
 
   if (const abi::__si_class_type_info *SI =
@@ -258,4 +256,13 @@ __ubsan::getDynamicTypeInfoFromVtable(void *VtablePtr) {
                          ObjectType ? ObjectType->__type_name : "<unknown>");
 }
 
+bool __ubsan::checkTypeInfoEquality(const void *TypeInfo1,
+                                    const void *TypeInfo2) {
+  auto TI1 = static_cast<const std::type_info *>(TypeInfo1);
+  auto TI2 = static_cast<const std::type_info *>(TypeInfo2);
+  return SANITIZER_NON_UNIQUE_TYPEINFO && TI1->__type_name[0] != '*' &&
+         TI2->__type_name[0] != '*' &&
+         !internal_strcmp(TI1->__type_name, TI2->__type_name);
+}
+
 #endif  // CAN_SANITIZE_UB && !SANITIZER_WINDOWS
diff --git a/compiler-rt/lib/ubsan/ubsan_type_hash_win.cc b/compiler-rt/lib/ubsan/ubsan_type_hash_win.cc
@@ -77,4 +77,9 @@ __ubsan::getDynamicTypeInfoFromVtable(void *VtablePtr) {
                          "<unknown>");
 }
 
+bool __ubsan::checkTypeInfoEquality(const std::type_info *,
+                                    const std::type_info *) {
+  return false;
+}
+
 #endif  // CAN_SANITIZE_UB && SANITIZER_WINDOWS

Original file line number	Diff line number	Diff line change
`@@ -77,4 +77,9 @@ __ubsan::getDynamicTypeInfoFromVtable(void *VtablePtr) {`
`77`	`77`	`"<unknown>");`
`78`	`78`	`}`
`79`	`79`
	`80`	`+bool __ubsan::checkTypeInfoEquality(const std::type_info *,`
	`81`	`+ const std::type_info *) {`
	`82`	`+ return false;`
	`83`	`+}`
	`84`	`+`
`80`	`85`	`#endif // CAN_SANITIZE_UB && SANITIZER_WINDOWS`