Clang changes behaviour of MemIntrinsic functions before we instrument their arguments

[GoogleCodeExporter]

char * s = (char*)malloc(4096);
memset(s+4095, 0, 2);

Here Clang changes "memset 2 bytes" to a single "store i16" instruction, and we 
don't instrument (and fail on) access to the memory outside allocated array.

Original issue reported on code.google.com by samso...@google.com on 10 Aug 2011 at 10:08

[GoogleCodeExporter]

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

Original comment by konstant...@gmail.com on 10 Aug 2011 at 10:16

[GoogleCodeExporter]

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).

Original comment by dvyu...@google.com on 10 Aug 2011 at 10:32

[GoogleCodeExporter]

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... 

Original comment by konstant...@gmail.com on 10 Aug 2011 at 10:36

[GoogleCodeExporter]

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

Original comment by dvyu...@google.com on 10 Aug 2011 at 10:41

[GoogleCodeExporter]

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

Original comment by konstant...@gmail.com on 10 Aug 2011 at 10:53

[GoogleCodeExporter]

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

Original comment by dvyu...@google.com on 10 Aug 2011 at 11:04

[GoogleCodeExporter]

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?

Original comment by samso...@google.com on 10 Aug 2011 at 11:19

[GoogleCodeExporter]

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)

Original comment by konstant...@gmail.com on 10 Aug 2011 at 11:31

[GoogleCodeExporter]

> 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.

Original comment by dvyu...@google.com on 10 Aug 2011 at 11:58

[GoogleCodeExporter]

> 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!

Original comment by dvyu...@google.com on 10 Aug 2011 at 1:01

[GoogleCodeExporter]

perhaps because it does not treat memset as an intrinsic

Original comment by konstant...@gmail.com on 10 Aug 2011 at 1:02

[GoogleCodeExporter]

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

Original comment by dvyu...@google.com on 10 Aug 2011 at 1:14

[GoogleCodeExporter]

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. 

Original comment by konstant...@gmail.com on 14 May 2014 at 1:44

• Changed state: Fixed