[Patch] UBsan: Type-based blacklisting

[byoungyoung]

Summary

Added Type-based blacklisting for sanitizers (SanitizerBlacklist::isBlacklistedType). On top of this, also added type-based blacklisting for UBsan's vptr.

[rsmith]

The code change looks fine to me, but needs a testcase.

[byoungyoung]

Thanks for taking a look, and let me add a testcase (soon).

[byoungyoung]

Added a testcase for ubsan's vptr blacklisting.

[samsonov]

lib/CodeGen/CGExpr.cpp

539

Nit: please upload the diffs with more context. This can be done with e.g. "arc" commandline tool (see http://llvm.org/docs/Phabricator.html). This change is probably fine, though, as it's local.

544

Now you shouldn't have generic "isIn(StringRef)" machinery in SanitizerBlacklist. Instead, it would be better to extend SanitizerBlacklist (which is now moved to lib/CodeGen/SanitizerBlacklist.h) with function like "isBlacklistedType(StringRef MangledTypeName)" and use it here.

[samsonov]

That is, LLVM part of your patch shouldn't be required now.

[byoungyoung]

Modified to be compatible with r212643, and thus dropping the dependency with http://reviews.llvm.org/D4406.

[byoungyoung]

Thanks Alex for the comments!

[samsonov]

LGTM modulo nit below. Let me know if I should commit this for you. Test in Clang is fine, but I would also add an "end-to-end" test case in compiler-rt ubsan lit test-suite. This can of course be done separately. Thanks!

test/CodeGen/ubsan-type-blacklist.cpp

20 ↗

(On Diff #11274)

I'd use less restrictive pattern for TYPE-NOT, smth. like

TYPE-NOT: @__ubsan_handle_dynamic_type_cache_miss

[byoungyoung]

Thanks Alex! Yes, please commit this if it is possible.

[samsonov]

Landed with minor modifications as r212770. Thanks!

lib/CodeGen/CGExpr.cpp

     llvm::raw_svector_ostream Out(MangledName);
     CGM.getCXXABI().getMangleContext().mangleCXXRTTI(Ty.getUnqualifiedType(),
                                                      Out);
-    llvm::hash_code TypeHash = hash_value(Out.str());
 
-    // Load the vptr, and compute hash_16_bytes(TypeHash, vptr).
-    llvm::Value *Low = llvm::ConstantInt::get(Int64Ty, TypeHash);
-    llvm::Type *VPtrTy = llvm::PointerType::get(IntPtrTy, 0);
-    llvm::Value *VPtrAddr = Builder.CreateBitCast(Address, VPtrTy);
-    llvm::Value *VPtrVal = Builder.CreateLoad(VPtrAddr);
-    llvm::Value *High = Builder.CreateZExt(VPtrVal, Int64Ty);
+    // Blacklist based on the mangled types
+    if (!CGM.getSanitizerBlacklist().isIn(Out.str())) {
+      llvm::hash_code TypeHash = hash_value(Out.str());
 
-    llvm::Value *Hash = emitHash16Bytes(Builder, Low, High);
-    Hash = Builder.CreateTrunc(Hash, IntPtrTy);
+      // Load the vptr, and compute hash_16_bytes(TypeHash, vptr).
+      llvm::Value *Low = llvm::ConstantInt::get(Int64Ty, TypeHash);
+      llvm::Type *VPtrTy = llvm::PointerType::get(IntPtrTy, 0);
+      llvm::Value *VPtrAddr = Builder.CreateBitCast(Address, VPtrTy);
+      llvm::Value *VPtrVal = Builder.CreateLoad(VPtrAddr);
+      llvm::Value *High = Builder.CreateZExt(VPtrVal, Int64Ty);
 
-    // Look the hash up in our cache.
-    const int CacheSize = 128;
-    llvm::Type *HashTable = llvm::ArrayType::get(IntPtrTy, CacheSize);
-    llvm::Value *Cache = CGM.CreateRuntimeVariable(HashTable,
-                                                   "__ubsan_vptr_type_cache");
-    llvm::Value *Slot = Builder.CreateAnd(Hash,
-                                          llvm::ConstantInt::get(IntPtrTy,
-                                                                 CacheSize-1));
-    llvm::Value *Indices[] = { Builder.getInt32(0), Slot };
-    llvm::Value *CacheVal =
-      Builder.CreateLoad(Builder.CreateInBoundsGEP(Cache, Indices));
+      llvm::Value *Hash = emitHash16Bytes(Builder, Low, High);
+      Hash = Builder.CreateTrunc(Hash, IntPtrTy);
 
-    // If the hash isn't in the cache, call a runtime handler to perform the
-    // hard work of checking whether the vptr is for an object of the right
-    // type. This will either fill in the cache and return, or produce a
-    // diagnostic.
-    llvm::Constant *StaticData[] = {
-      EmitCheckSourceLocation(Loc),
-      EmitCheckTypeDescriptor(Ty),
-      CGM.GetAddrOfRTTIDescriptor(Ty.getUnqualifiedType()),
-      llvm::ConstantInt::get(Int8Ty, TCK)
-    };
-    llvm::Value *DynamicData[] = { Address, Hash };
-    EmitCheck(Builder.CreateICmpEQ(CacheVal, Hash),
-              "dynamic_type_cache_miss", StaticData, DynamicData,
-              CRK_AlwaysRecoverable);
+      // Look the hash up in our cache.
+      const int CacheSize = 128;
+      llvm::Type *HashTable = llvm::ArrayType::get(IntPtrTy, CacheSize);
+      llvm::Value *Cache = CGM.CreateRuntimeVariable(HashTable,
+                                                     "__ubsan_vptr_type_cache");
+      llvm::Value *Slot =
+        Builder.CreateAnd(Hash,llvm::ConstantInt::get(IntPtrTy, CacheSize-1));
+      llvm::Value *Indices[] = { Builder.getInt32(0), Slot };
+      llvm::Value *CacheVal =
+        Builder.CreateLoad(Builder.CreateInBoundsGEP(Cache, Indices));
+
+      // If the hash isn't in the cache, call a runtime handler to perform the
+      // hard work of checking whether the vptr is for an object of the right
+      // type. This will either fill in the cache and return, or produce a
+      // diagnostic.
+      llvm::Constant *StaticData[] = {
+        EmitCheckSourceLocation(Loc),
+        EmitCheckTypeDescriptor(Ty),
+        CGM.GetAddrOfRTTIDescriptor(Ty.getUnqualifiedType()),
+        llvm::ConstantInt::get(Int8Ty, TCK)
+      };
+      llvm::Value *DynamicData[] = { Address, Hash };
+      EmitCheck(Builder.CreateICmpEQ(CacheVal, Hash),
+                "dynamic_type_cache_miss", StaticData, DynamicData,
+                CRK_AlwaysRecoverable);
+    }
   }
 
   if (Done) {