call __asan_load_cxx_array_cookie when loading array cookie in asan mode.

[kcc]

Summary

The current implementation of asan cookie is incorrect:
we add nosanitize metadata to the cookie load, but the metadata may be lost
and we will instrument the load from poisoned memory.
This change replaces the load with a call to __asan_load_cxx_array_cookie (r216692)

[rsmith]

lib/CodeGen/ItaniumCXXABI.cpp

1513–1514

Please extend this comment to point out why we need to do this.

1516

I don't think we can support address spaces other than 0 like this (the ASan runtime function will only accept address space 0 pointers). How does ASan deal with non-zero address spaces?

[kcc]

Extend the comment and check that address space is 0 (otherwise asan doesn't want to handle the pointer)

PTAL

[rsmith]

LGTM

lib/CodeGen/ItaniumCXXABI.cpp

if (!CookieOffset.isZero())
     CookiePtr = CGF.Builder.CreateConstInBoundsGEP1_64(CookiePtr,
                                                  CookieOffset.getQuantity());
 
   // Write the number of elements into the appropriate slot.
   llvm::Type *NumElementsTy = CGF.ConvertType(SizeTy)->getPointerTo(AS);
   llvm::Value *NumElementsPtr =
       CGF.Builder.CreateBitCast(CookiePtr, NumElementsTy);
   llvm::Instruction *SI = CGF.Builder.CreateStore(NumElements, NumElementsPtr);
-  if (CGM.getLangOpts().Sanitize.Address &&
+  if (CGM.getLangOpts().Sanitize.Address && AS == 0 &&
       expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+    // The store to the CookiePtr does not need to be instrumented.
     CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
     llvm::FunctionType *FTy =
         llvm::FunctionType::get(CGM.VoidTy, NumElementsTy, false);
     llvm::Constant *F =
         CGM.CreateRuntimeFunction(FTy, "__asan_poison_cxx_array_cookie");
     CGF.Builder.CreateCall(F, NumElementsPtr);
   }
 
llvm::Value *ItaniumCXXABI::readArrayCookieImpl(CodeGenFunction &CGF,
   if (!numElementsOffset.isZero())
     numElementsPtr =
       CGF.Builder.CreateConstInBoundsGEP1_64(numElementsPtr,
                                              numElementsOffset.getQuantity());
 
   unsigned AS = allocPtr->getType()->getPointerAddressSpace();
   numElementsPtr = 
     CGF.Builder.CreateBitCast(numElementsPtr, CGF.SizeTy->getPointerTo(AS));
-  llvm::Instruction *LI = CGF.Builder.CreateLoad(numElementsPtr);
-  if (CGM.getLangOpts().Sanitize.Address)
-    CGM.getSanitizerMetadata()->disableSanitizerForInstruction(LI);
-  return LI;
+  if (!CGM.getLangOpts().Sanitize.Address || AS != 0)
+    return CGF.Builder.CreateLoad(numElementsPtr);
+  // In asan mode emit a function call instead of a regular load and let the
+  // run-time deal with it: if the shadow is properly poisoned return the
+  // cookie, otherwise return 0 to avoid an infinite loop calling DTORs.
+  // We can't simply ignore this load using nosanitize metadata because
+  // the metadata may be lost.
+  llvm::FunctionType *FTy =
+      llvm::FunctionType::get(CGF.SizeTy, CGF.SizeTy->getPointerTo(0), false);
+  llvm::Constant *F =
+      CGM.CreateRuntimeFunction(FTy, "__asan_load_cxx_array_cookie");
+  return CGF.Builder.CreateCall(F, numElementsPtr);
 }
 
 CharUnits ARMCXXABI::getArrayCookieSizeImpl(QualType elementType) {
   // ARM says that the cookie is always:
   //   struct array_cookie {
   //     std::size_t element_size; // element_size != 0
   //     std::size_t element_count;
   //   };

test/CodeGen/address-sanitizer-and-array-cookie.cpp

 
 void CallDelete(C *c) {
   delete [] c;
 }
 
 // PLAIN-LABEL: CallDelete
 // PLAIN-NOT: nosanitize
 // ASAN-LABEL: CallDelete
-// ASAN: load{{.*}}!nosanitize
+// ASAN-NOT: nosanitize
+// ASAN: call i64 @__asan_load_cxx_array_cookie
 // ASAN-NOT: nosanitize
 
 char Buffer[20];
 C *CallPlacementNew() {
   return new (Buffer) C[20];
 }
 // ASAN-LABEL: CallPlacementNew
 // ASAN-NOT: __asan_poison_cxx_array_cookie