[compiler-rt] r174373 - [asan] Fix nonsensical reports of partial right OOB.
Alexander Potapenko
glider at google.com
Thu Feb 7 07:31:33 PST 2013
BTW WDYT about adding a flag that asks ASan to describe both
allocations to the left and to the right of the memory address?
If ASan occasionally prints the adjacent allocation with a completely
different allocation size and stack (like it just happened to me with
large_func_test on Darwin), the user may be puzzled.
On Tue, Feb 5, 2013 at 7:04 PM, Evgeniy Stepanov
<eugeni.stepanov at gmail.com> wrote:
> Fixed, I hope.
>
> On Tue, Feb 5, 2013 at 6:40 PM, Kostya Serebryany <kcc at google.com> wrote:
>>
>>
>>
>> On Tue, Feb 5, 2013 at 6:32 PM, Evgeniy Stepanov <eugeni.stepanov at gmail.com>
>> wrote:
>>>
>>> Author: eugenis
>>> Date: Tue Feb 5 08:32:03 2013
>>> New Revision: 174373
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=174373&view=rev
>>> Log:
>>> [asan] Fix nonsensical reports of partial right OOB.
>>>
>>> In case of partial right OOB, ASan was reporting
>>> X is located 0 bytes to the right of [A, B)
>>> where X was actually inside [A, B).
>>>
>>> With this change, ASan will report B as the error address in such case.
>>>
>>> Added:
>>> compiler-rt/trunk/lib/asan/lit_tests/partial_right.cc (with props)
>>> Modified:
>>> compiler-rt/trunk/lib/asan/asan_allocator.cc
>>> compiler-rt/trunk/lib/asan/asan_allocator.h
>>> compiler-rt/trunk/lib/asan/asan_allocator2.cc
>>> compiler-rt/trunk/lib/asan/asan_globals.cc
>>> compiler-rt/trunk/lib/asan/asan_interceptors.cc
>>> compiler-rt/trunk/lib/asan/asan_report.cc
>>> compiler-rt/trunk/lib/asan/asan_report.h
>>> compiler-rt/trunk/lib/asan/lit_tests/strncpy-overflow.cc
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_allocator.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_allocator.cc?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_allocator.cc (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_allocator.cc Tue Feb 5 08:32:03 2013
>>> @@ -369,7 +369,7 @@ class MallocInfo {
>>> left_chunk->chunk_state != CHUNK_AVAILABLE)
>>> return left_chunk;
>>> // Choose based on offset.
>>> - uptr l_offset = 0, r_offset = 0;
>>> + sptr l_offset = 0, r_offset = 0;
>>> CHECK(AsanChunkView(left_chunk).AddrIsAtRight(addr, 1, &l_offset));
>>> CHECK(AsanChunkView(right_chunk).AddrIsAtLeft(addr, 1, &r_offset));
>>> if (l_offset < r_offset)
>>> @@ -389,7 +389,7 @@ class MallocInfo {
>>> CHECK(m->chunk_state == CHUNK_ALLOCATED ||
>>> m->chunk_state == CHUNK_AVAILABLE ||
>>> m->chunk_state == CHUNK_QUARANTINE);
>>> - uptr offset = 0;
>>> + lptr offset = 0;
>>
>>
>>
>> lptr?
>> Does this even compile?
>>
>>>
>>> AsanChunkView m_view(m);
>>> if (m_view.AddrIsInside(addr, 1, &offset))
>>> return m;
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_allocator.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_allocator.h?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_allocator.h (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_allocator.h Tue Feb 5 08:32:03 2013
>>> @@ -55,14 +55,14 @@ class AsanChunkView {
>>> uptr FreeTid();
>>> void GetAllocStack(StackTrace *stack);
>>> void GetFreeStack(StackTrace *stack);
>>> - bool AddrIsInside(uptr addr, uptr access_size, uptr *offset) {
>>> + bool AddrIsInside(uptr addr, uptr access_size, sptr *offset) {
>>> if (addr >= Beg() && (addr + access_size) <= End()) {
>>> *offset = addr - Beg();
>>> return true;
>>> }
>>> return false;
>>> }
>>> - bool AddrIsAtLeft(uptr addr, uptr access_size, uptr *offset) {
>>> + bool AddrIsAtLeft(uptr addr, uptr access_size, sptr *offset) {
>>> (void)access_size;
>>> if (addr < Beg()) {
>>> *offset = Beg() - addr;
>>> @@ -70,12 +70,9 @@ class AsanChunkView {
>>> }
>>> return false;
>>> }
>>> - bool AddrIsAtRight(uptr addr, uptr access_size, uptr *offset) {
>>> + bool AddrIsAtRight(uptr addr, uptr access_size, sptr *offset) {
>>> if (addr + access_size >= End()) {
>>> - if (addr <= End())
>>> - *offset = 0;
>>> - else
>>> - *offset = addr - End();
>>> + *offset = addr - End();
>>> return true;
>>> }
>>> return false;
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_allocator2.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_allocator2.cc?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_allocator2.cc (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_allocator2.cc Tue Feb 5 08:32:03 2013
>>> @@ -547,7 +547,7 @@ AsanChunk *ChooseChunk(uptr addr,
>>> return right_chunk;
>>> }
>>> // Same chunk_state: choose based on offset.
>>> - uptr l_offset = 0, r_offset = 0;
>>> + sptr l_offset = 0, r_offset = 0;
>>> CHECK(AsanChunkView(left_chunk).AddrIsAtRight(addr, 1, &l_offset));
>>> CHECK(AsanChunkView(right_chunk).AddrIsAtLeft(addr, 1, &r_offset));
>>> if (l_offset < r_offset)
>>> @@ -558,7 +558,7 @@ AsanChunk *ChooseChunk(uptr addr,
>>> AsanChunkView FindHeapChunkByAddress(uptr addr) {
>>> AsanChunk *m1 = GetAsanChunkByAddr(addr);
>>> if (!m1) return AsanChunkView(m1);
>>> - uptr offset = 0;
>>> + sptr offset = 0;
>>> if (AsanChunkView(m1).AddrIsAtLeft(addr, 1, &offset)) {
>>> // The address is in the chunk's left redzone, so maybe it is
>>> actually
>>> // a right buffer overflow from the other chunk to the left.
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_globals.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_globals.cc?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_globals.cc (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_globals.cc Tue Feb 5 08:32:03 2013
>>> @@ -48,7 +48,7 @@ void PoisonRedZones(const Global &g) {
>>> }
>>> }
>>>
>>> -bool DescribeAddressIfGlobal(uptr addr) {
>>> +bool DescribeAddressIfGlobal(uptr addr, uptr size) {
>>> if (!flags()->report_globals) return false;
>>> BlockingMutexLock lock(&mu_for_globals);
>>> bool res = false;
>>> @@ -57,7 +57,7 @@ bool DescribeAddressIfGlobal(uptr addr)
>>> if (flags()->report_globals >= 2)
>>> Report("Search Global: beg=%p size=%zu name=%s\n",
>>> (void*)g.beg, g.size, (char*)g.name);
>>> - res |= DescribeAddressRelativeToGlobal(addr, g);
>>> + res |= DescribeAddressRelativeToGlobal(addr, size, g);
>>> }
>>> return res;
>>> }
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_interceptors.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_interceptors.cc?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_interceptors.cc (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_interceptors.cc Tue Feb 5 08:32:03
>>> 2013
>>> @@ -31,12 +31,14 @@ namespace __asan {
>>> // that no extra frames are created, and stack trace contains
>>> // relevant information only.
>>> // We check all shadow bytes.
>>> -#define ACCESS_MEMORY_RANGE(offset, size, isWrite) do {
>>> \
>>> - if (uptr __ptr = __asan_region_is_poisoned((uptr)(offset), size)) {
>>> \
>>> - GET_CURRENT_PC_BP_SP;
>>> \
>>> - __asan_report_error(pc, bp, sp, __ptr, isWrite, /* access_size */1);
>>> \
>>> - }
>>> \
>>> -} while (0)
>>> +#define ACCESS_MEMORY_RANGE(offset, size, isWrite) do { \
>>> + uptr __offset = (uptr)(offset); \
>>> + uptr __size = (uptr)(size); \
>>> + if (__asan_region_is_poisoned(__offset, __size)) { \
>>> + GET_CURRENT_PC_BP_SP; \
>>> + __asan_report_error(pc, bp, sp, __offset, isWrite, __size); \
>>> + } \
>>> + } while (0)
>>>
>>> #define ASAN_READ_RANGE(offset, size) ACCESS_MEMORY_RANGE(offset, size,
>>> false)
>>> #define ASAN_WRITE_RANGE(offset, size) ACCESS_MEMORY_RANGE(offset, size,
>>> true);
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_report.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_report.cc?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_report.cc (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_report.cc Tue Feb 5 08:32:03 2013
>>> @@ -190,19 +190,23 @@ static void PrintGlobalNameIfASCII(const
>>> Printf(" '%s' is ascii string '%s'\n", g.name, (char*)g.beg);
>>> }
>>>
>>> -bool DescribeAddressRelativeToGlobal(uptr addr, const __asan_global &g) {
>>> +bool DescribeAddressRelativeToGlobal(uptr addr, uptr size,
>>> + const __asan_global &g) {
>>> static const uptr kMinimalDistanceFromAnotherGlobal = 64;
>>> if (addr <= g.beg - kMinimalDistanceFromAnotherGlobal) return false;
>>> if (addr >= g.beg + g.size_with_redzone) return false;
>>> Decorator d;
>>> Printf("%s", d.Location());
>>> - Printf("%p is located ", (void*)addr);
>>> if (addr < g.beg) {
>>> - Printf("%zd bytes to the left", g.beg - addr);
>>> - } else if (addr >= g.beg + g.size) {
>>> - Printf("%zd bytes to the right", addr - (g.beg + g.size));
>>> + Printf("%p is located %zd bytes to the left", (void*)addr, g.beg -
>>> addr);
>>> + } else if (addr + size > g.beg + g.size) {
>>> + if (addr < g.beg + g.size)
>>> + addr = g.beg + g.size;
>>> + Printf("%p is located %zd bytes to the right", (void*)addr,
>>> + addr - (g.beg + g.size));
>>> } else {
>>> - Printf("%zd bytes inside", addr - g.beg); // Can it happen?
>>> + // Can it happen?
>>> + Printf("%p is located %zd bytes inside", (void*)addr, addr - g.beg);
>>> }
>>> Printf(" of global variable '%s' (0x%zx) of size %zu\n",
>>> g.name, g.beg, g.size);
>>> @@ -288,18 +292,22 @@ bool DescribeAddressIfStack(uptr addr, u
>>>
>>> static void DescribeAccessToHeapChunk(AsanChunkView chunk, uptr addr,
>>> uptr access_size) {
>>> - uptr offset;
>>> + sptr offset;
>>> Decorator d;
>>> Printf("%s", d.Location());
>>> - Printf("%p is located ", (void*)addr);
>>> - if (chunk.AddrIsInside(addr, access_size, &offset)) {
>>> - Printf("%zu bytes inside of", offset);
>>> - } else if (chunk.AddrIsAtLeft(addr, access_size, &offset)) {
>>> - Printf("%zu bytes to the left of", offset);
>>> + if (chunk.AddrIsAtLeft(addr, access_size, &offset)) {
>>> + Printf("%p is located %zd bytes to the left of", (void*)addr,
>>> offset);
>>> } else if (chunk.AddrIsAtRight(addr, access_size, &offset)) {
>>> - Printf("%zu bytes to the right of", offset);
>>> + if (offset < 0) {
>>> + addr -= offset;
>>> + offset = 0;
>>> + }
>>> + Printf("%p is located %zd bytes to the right of", (void*)addr,
>>> offset);
>>> + } else if (chunk.AddrIsInside(addr, access_size, &offset)) {
>>> + Printf("%p is located %zd bytes inside of", (void*)addr, offset);
>>> } else {
>>> - Printf(" somewhere around (this is AddressSanitizer bug!)");
>>> + Printf("%p is located somewhere around (this is AddressSanitizer
>>> bug!)",
>>> + (void*)addr);
>>> }
>>> Printf(" %zu-byte region [%p,%p)\n", chunk.UsedSize(),
>>> (void*)(chunk.Beg()), (void*)(chunk.End()));
>>> @@ -372,7 +380,7 @@ void DescribeAddress(uptr addr, uptr acc
>>> if (DescribeAddressIfShadow(addr))
>>> return;
>>> CHECK(AddrIsInMem(addr));
>>> - if (DescribeAddressIfGlobal(addr))
>>> + if (DescribeAddressIfGlobal(addr, access_size))
>>> return;
>>> if (DescribeAddressIfStack(addr, access_size))
>>> return;
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_report.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_report.h?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_report.h (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_report.h Tue Feb 5 08:32:03 2013
>>> @@ -21,8 +21,9 @@ namespace __asan {
>>> // The following functions prints address description depending
>>> // on the memory type (shadow/heap/stack/global).
>>> void DescribeHeapAddress(uptr addr, uptr access_size);
>>> -bool DescribeAddressIfGlobal(uptr addr);
>>> -bool DescribeAddressRelativeToGlobal(uptr addr, const __asan_global &g);
>>> +bool DescribeAddressIfGlobal(uptr addr, uptr access_size);
>>> +bool DescribeAddressRelativeToGlobal(uptr addr, uptr access_size,
>>> + const __asan_global &g);
>>> bool DescribeAddressIfShadow(uptr addr);
>>> bool DescribeAddressIfStack(uptr addr, uptr access_size);
>>> // Determines memory type on its own.
>>>
>>> Added: compiler-rt/trunk/lib/asan/lit_tests/partial_right.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/lit_tests/partial_right.cc?rev=174373&view=auto
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/lit_tests/partial_right.cc (added)
>>> +++ compiler-rt/trunk/lib/asan/lit_tests/partial_right.cc Tue Feb 5
>>> 08:32:03 2013
>>> @@ -0,0 +1,17 @@
>>> +// RUN: %clangxx_asan -m64 -O0 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +// RUN: %clangxx_asan -m64 -O1 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +// RUN: %clangxx_asan -m64 -O2 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +// RUN: %clangxx_asan -m64 -O3 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +// RUN: %clangxx_asan -m32 -O0 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +// RUN: %clangxx_asan -m32 -O1 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +// RUN: %clangxx_asan -m32 -O2 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +// RUN: %clangxx_asan -m32 -O3 %s -o %t && %t 2>&1 | %symbolize |
>>> FileCheck %s
>>> +
>>> +#include <stdlib.h>
>>> +int main(int argc, char **argv) {
>>> + volatile int *x = (int*)malloc(2*sizeof(int) + 2);
>>> + int res = x[2]; // BOOOM
>>> + // CHECK: {{READ of size 4 at 0x.* thread T0}}
>>> + // CHECK: [[ADDR:0x[01-9a-fa-f]+]] is located 0 bytes to the right of
>>> {{.*}}-byte region [{{.*}},{{.*}}[[ADDR]])
>>> + return res;
>>> +}
>>>
>>> Propchange: compiler-rt/trunk/lib/asan/lit_tests/partial_right.cc
>>>
>>> ------------------------------------------------------------------------------
>>> svn:eol-style = LF
>>>
>>> Modified: compiler-rt/trunk/lib/asan/lit_tests/strncpy-overflow.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/lit_tests/strncpy-overflow.cc?rev=174373&r1=174372&r2=174373&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/lit_tests/strncpy-overflow.cc (original)
>>> +++ compiler-rt/trunk/lib/asan/lit_tests/strncpy-overflow.cc Tue Feb 5
>>> 08:32:03 2013
>>> @@ -22,7 +22,7 @@ int main(int argc, char **argv) {
>>> strcpy(hello, "hello");
>>> char *short_buffer = (char*)malloc(9);
>>> strncpy(short_buffer, hello, 10); // BOOM
>>> - // CHECK: {{WRITE of size 1 at 0x.* thread T0}}
>>> + // CHECK: {{WRITE of size 10 at 0x.* thread T0}}
>>> // CHECK-Linux: {{ #0 0x.* in .*strncpy}}
>>> // CHECK-Darwin: {{ #0 0x.* in _?wrap_strncpy}}
>>> // CHECK: {{ #1 0x.* in _?main .*strncpy-overflow.cc:}}[[@LINE-4]]
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
--
Alexander Potapenko
Software Engineer
Google Moscow
More information about the llvm-commits
mailing list