Clang changes behaviour of MemIntrinsic functions before we instrument their arguments
Closed this issue · 13 comments
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
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
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
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
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
>> 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
Done:
http://code.google.com/p/address-sanitizer/wiki/KnownBugs
Original comment by dvyu...@google.com on 10 Aug 2011 at 11:04
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
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
> 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
> 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
perhaps because it does not treat memset as an intrinsic
Original comment by konstant...@gmail.com on 10 Aug 2011 at 1:02
Yeah, but it should treat memcpy as a, well, memcpy.
Original comment by dvyu...@google.com on 10 Aug 2011 at 1:14
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