[compiler-rt] r174373 - [asan] Fix nonsensical reports of partial right OOB.
Evgeniy Stepanov
eugeni.stepanov at gmail.com
Tue Feb 5 07:04:32 PST 2013
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
>
>
More information about the llvm-commits
mailing list