[compiler-rt] r174373 - [asan] Fix nonsensical reports of partial right OOB.
Kostya Serebryany
kcc at google.com
Thu Feb 7 08:38:05 PST 2013
That's possible, but
1. option which is off by default either rots or costs extra effort to keep
it working
2. turning such option on by default will confuse users even more.
So, I'd prefer to just recommend rerunning the test with increased redzone
--kcc
On Thu, Feb 7, 2013 at 7:31 PM, Alexander Potapenko <glider at google.com>wrote:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130207/6c3904e6/attachment.html>
More information about the llvm-commits
mailing list