Clang changes behaviour of MemIntrinsic functions before we instrument their arguments about address-sanitizer HOT 13 CLOSED

So, what is the problem? 
The code is buggy and asan detects the bug. 
No? 

The problem is with mops that "split 2 shadow bytes". Since we don't analyse 
the second byte, we don't catch the error. The minimal test is:

TEST(AddressSanitizer, DISABLED_StrangeMemIntrinsicBehaviorTest2){
  int const size = 4096;
  char* s = (char*)malloc(size);
  EXPECT_DEATH(memcpy(s+size-1, s, 2), TO_THE_RIGHT(0));
  free(s);
}

If 4096 is replaced with 4095, the test passes (that is, the program crashes).

ah! 
This is 
http://code.google.com/p/address-sanitizer/wiki/AddressSanitizerAlgorithm#Unalig
ned_accesses

Not sure if we want to do anything with this right now... 

I think it's worth moving to KnownBugs, because it's where I looked first.

>> I think it's worth moving to KnownBugs,
Agree. Give a link there. 

Done:
http://code.google.com/p/address-sanitizer/wiki/KnownBugs

Can we instrument arguments of memintrinsic functions _before_ these functions 
are modified by compiler and lead to unaligned access? Or we should just leave 
everything as is now?

I don't know for sure (need to investigate when does memset lowering happen), 
but probably not. asan instrumentation should happen at the later stages, when 
the majority of other optimizations already happened. 

I'd leave it as is for now. 
Long term we'll need to implement checking for unaligned accesses (as an option)

> Can we instrument arguments of memintrinsic functions _before_ these 
functions are modified by compiler and lead to unaligned access? 

There should a compiler option that prevents inlining of intrinsic functions.

> There should a compiler option that prevents inlining of intrinsic functions

When I compile the following test with -fno-builtin

TEST(AddressSanitizer, DISABLED_StrangeMemIntrinsicBehaviorTest2){
  char * s = (char*)malloc(4096);
  memcpy(s+4096-1, s, 2);
}

it does not insert any instrumentation at all:

0808fb00 
<AddressSanitizer_DISABLED_StrangeMemIntrinsicBehaviorTest2_Test::TestBody()>:
 808fb00:       55                      push   %ebp
 808fb01:       89 e5                   mov    %esp,%ebp
 808fb03:       83 ec 18                sub    $0x18,%esp
 808fb06:       c7 04 24 00 10 00 00    movl   $0x1000,(%esp)
 808fb0d:       e8 8e ea 09 00          call   812e5a0 <malloc>
 808fb12:       89 44 24 04             mov    %eax,0x4(%esp)
 808fb16:       05 ff 0f 00 00          add    $0xfff,%eax
 808fb1b:       89 04 24                mov    %eax,(%esp)
 808fb1e:       c7 44 24 08 02 00 00    movl   $0x2,0x8(%esp)
 808fb25:       00 
 808fb26:       e8 2d ea fe ff          call   807e558 <memcpy@plt>
 808fb2b:       83 c4 18                add    $0x18,%esp
 808fb2e:       5d                      pop    %ebp
 808fb2f:       c3                      ret    

ouch!

perhaps because it does not treat memset as an intrinsic

Yeah, but it should treat memcpy as a, well, memcpy.

since http://llvm.org/viewvc/llvm-project?rev=206746&view=rev
asan does not instrument memset/memmove/memcpy calls, instead it replaces the 
calls
with calls to __asan_memset/etc. 

I think this allows us to close this bug.