[compiler-rt] r174373 - [asan] Fix nonsensical reports of partial right OOB.
Kostya Serebryany
kcc at google.com
Tue Feb 5 06:40:34 PST 2013
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130205/d8fb0d37/attachment.html>
More information about the llvm-commits
mailing list