[PATCH] [clang/asan] call __asan_poison_cxx_array_cookie after operator new[]

[Kostya Serebryany]

>>! In D4774#11, @samsonov wrote:
> I thought of one more test case we should add:
> 
>   __attribute__((no_sanitize_address))
>   int *createArray(int n) {
>     return new int[n];
>   }
> 
>   int bad_access() {  
>     int *array = createArray(4);
>     return array[-1];
>   }
>   
> We certainly want to print an error in this case, even though we have attribute on createArray() function. I believe current code *would* handle this correctly, but let's test this behavior.

I'd prefer to add such test to compiler-rt, not to clang, as it is a run-time test. 


>>! In D4774#10, @rsmith wrote:
> Is this change correct? Suppose I do this:
> 
>     char Buffer[32];
>     // ...
>     new (Buffer) int[4];
>     // ...
>     new (Buffer) int(0);
> 
> Won't we get a false positive on the last line?

Not sure  I understand this test. 
First, with arrays of PODs you don't have  cookies at all. 
Second, do we have the cookie with placement new at all? 
Pardon my ignorance here; this is my test case:
  #include <stdio.h>
  #include <new>
  struct C {
    int x;
    ~C() {fprintf(stderr, "ZZZZZZZZ\n");}
  };
  char Buffer[100];
  C *c;
  int main(int argc, char **argv) { 
    c = new (Buffer) C[argc];
  } 
 
clang++ -O -fsanitize=address ~/tmp/new_ar_placement.cc -S -o - -emit-llvm

define i32 @main(i32 %argc, i8** nocapture readnone %argv) #0 {
entry:
  store %struct.C* bitcast ({ [100 x i8], [60 x i8] }* @Buffer to %struct.C*), %struct.C** getelementptr inbounds ({ %struct.C*, [56 x i8] }* @c, i32 0, i32 0), align 8, !tbaa !5
  ret i32 0
}

// No cookie

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1450
@@ -1449,3 +1449,3 @@
 
 llvm::Value *ItaniumCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
                                                   llvm::Value *NewPtr,
----------------
samsonov wrote:
> Hm, I think we'd need to ask Timur to implement similar logic for Microsoft ABI?
yes, but not in this round

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1482
@@ +1481,3 @@
+    CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
+    llvm::FunctionType *fty =
+        llvm::FunctionType::get(CGM.VoidTy, NumElementsTy, false);
----------------
samsonov wrote:
> Please start local variable names with capitals
done

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1483
@@ +1482,3 @@
+    llvm::FunctionType *fty =
+        llvm::FunctionType::get(CGM.VoidTy, NumElementsTy, false);
+    llvm::Constant *f =
----------------
samsonov wrote:
> Why don't you use fixed type for __asan_poison_cxx_array_cookie (like IntptrTy)?
What's wrong with i64* ?
Having I64 here will just require one more cast

http://reviews.llvm.org/D4774