[llvm-commits] [ASan] Replace AsanProcMaps::GetObjectNameAndOffset with a simpler AsanProcMaps::DescribeAddress (issue 5643047)

[glider at google.com]

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_internal.h
File lib/asan/asan_internal.h (left):

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_internal.h#oldcode145
lib/asan/asan_internal.h:145: int SNPrint(char *buffer, size_t length,
const char *format, ...);
On 2012/02/07 17:41:15, timurrrr_at_google_com wrote:
> This was wrong in the first place

I think it's generally better to send separate CLs for cleanup. I'm ok
with this particular change however.

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_linux.cc
File lib/asan/asan_linux.cc (right):

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_linux.cc#newcode216
lib/asan/asan_linux.cc:216: uintptr_t offset;
On 2012/02/07 17:41:15, timurrrr_at_google_com wrote:
> this code is identical to that on Mac, see comment below

You'd better put the default implementation into asan_procmaps.h instead
of IterateForObjectNameAndOffset.

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_linux.cc#newcode221
lib/asan/asan_linux.cc:221: out_buffer[0] = '\0';
Please check that out_buffer!=NULL

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_linux.cc#newcode265
lib/asan/asan_linux.cc:265: if
(dl_iterate_phdr(dl_iterate_phdr_callback, &data)) {
On 2012/02/07 17:41:15, timurrrr_at_google_com wrote:
> Alternative suggestion:
> I can extract this code to IterateForObjectNameAndOffset
> and move the common AsanProcMaps::DescribeAddress code to
asan_posix.cc
> (we have the same symbolization capabilities on Linux and Mac, right?)

> WDYT?

We do not have dl_iterate_phdr on Mac.

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_linux.cc#newcode266
lib/asan/asan_linux.cc:266: SNPrintf(out_buffer, buffer_size,
Please mind the return value of snprintf.

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_procmaps.h
File lib/asan/asan_procmaps.h (right):

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_procmaps.h#newcode30
lib/asan/asan_procmaps.h:30: // and puts it into the given buffer with a
leading space
On 2012/02/07 17:41:15, timurrrr_at_google_com wrote:
> I know this may sound a bit confusing, but look at how easy it is to
use this
> function now (asan_stack.cc, which is the only place this is used)

Please no leading spaces.

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_stack.cc
File lib/asan/asan_stack.cc (right):

http://codereview.appspot.com/5643047/diff/1/lib/asan/asan_stack.cc#newcode47
lib/asan/asan_stack.cc:47: Printf("    #%ld 0x%lx%s\n", i, pc, descr);
Please keep in mind this should be %p (not %lx) on Windows

See also http://mail.python.org/pipermail/patches/2000-June/000871.html

http://codereview.appspot.com/5643047/