[llvm-commits] [ASan] Replace AsanProcMaps::GetObjectNameAndOffset with a simpler AsanProcMaps::DescribeAddress (issue 5643047)
timurrrr at google.com
timurrrr at google.com
Tue Feb 7 09:41:15 PST 2012
Reviewers: kcc1, Evgeniy Stepanov, glider,
Message:
Hi,
Can you please review this patch?
Rationale: on Windows, it's much easier to get function name, source
file and source line than obj+offset we use on POSIX (where we have to
use an exterenal symbolizer script).
I think the old API goes a little bit too much into the details on how
things work, so I'm suggesting a slightly more generic API.
There's one question inline, see comments.
Thanks,
Timur
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, ...);
This was wrong in the first place
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;
this code is identical to that on Mac, see comment below
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)) {
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?
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
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 review this at http://codereview.appspot.com/5643047/
Affected files:
M lib/asan/asan_internal.h
M lib/asan/asan_linux.cc
M lib/asan/asan_mac.cc
M lib/asan/asan_procmaps.h
M lib/asan/asan_stack.cc
Index: lib/asan/asan_internal.h
diff --git lib/asan/asan_internal.h lib/asan/asan_internal.h
index
041fa3dce803be895a325f1601891bc98bffad92..31842013b64419263a9ba67f8dca30d08cca4fab
100644
--- lib/asan/asan_internal.h
+++ lib/asan/asan_internal.h
@@ -142,7 +142,7 @@ size_t ReadFileToBuffer(const char *file_name, char
**buff,
// asan_printf.cc
void RawWrite(const char *buffer);
-int SNPrint(char *buffer, size_t length, const char *format, ...);
+int SNPrintf(char *buffer, size_t length, const char *format, ...);
void Printf(const char *format, ...);
int SScanf(const char *str, const char *format, ...);
void Report(const char *format, ...);
Index: lib/asan/asan_linux.cc
diff --git lib/asan/asan_linux.cc lib/asan/asan_linux.cc
index
01a019eac850c2de05d8204bc068f19e9b25a52a..d530415cc69c459496c6f50a561aebac204f7623
100644
--- lib/asan/asan_linux.cc
+++ lib/asan/asan_linux.cc
@@ -210,11 +210,16 @@ bool AsanProcMaps::Next(uintptr_t *start, uintptr_t
*end,
#ifdef __arm__
-// Gets the object name and the offset by walking AsanProcMaps.
-bool AsanProcMaps::GetObjectNameAndOffset(uintptr_t addr, uintptr_t
*offset,
- char filename[],
- size_t filename_size) {
- return IterateForObjectNameAndOffset(addr, offset, filename,
filename_size);
+void AsanProcMaps::DescribeAddress(uintptr_t addr,
+ char out_buffer[],
+ size_t buffer_size) {
+ uintptr_t offset;
+ char objname[4096];
+ if (IterateForObjectNameAndOffset(addr, &offset, objname,
sizeof(objname))) {
+ SNPrintf(out_buffer, buffer_size, " (%s+0x%lx)", objname, offset);
+ } else {
+ out_buffer[0] = '\0';
+ }
}
#else // __arm__
@@ -248,19 +253,21 @@ static int dl_iterate_phdr_callback(struct
dl_phdr_info *info,
}
// Gets the object name and the offset using dl_iterate_phdr.
-bool AsanProcMaps::GetObjectNameAndOffset(uintptr_t addr, uintptr_t
*offset,
- char filename[],
- size_t filename_size) {
+void AsanProcMaps::DescribeAddress(uintptr_t addr,
+ char out_buffer[],
+ size_t buffer_size) {
+ char filename[4096];
DlIterateData data;
data.count = 0;
data.addr = addr;
data.filename = filename;
- data.filename_size = filename_size;
+ data.filename_size = sizeof(filename);
if (dl_iterate_phdr(dl_iterate_phdr_callback, &data)) {
- *offset = data.offset;
- return true;
+ SNPrintf(out_buffer, buffer_size,
+ " (%s+0x%lx)", data.filename, data.offset);
+ } else {
+ out_buffer[0] = '\0';
}
- return false;
}
#endif // __arm__
Index: lib/asan/asan_mac.cc
diff --git lib/asan/asan_mac.cc lib/asan/asan_mac.cc
index
112aebf8d93b52a9c8c75e516ccd7907afadac44..7832ce8a17722192f87f96cce90733ea6fadc4fa
100644
--- lib/asan/asan_mac.cc
+++ lib/asan/asan_mac.cc
@@ -271,10 +271,16 @@ bool AsanProcMaps::Next(uintptr_t *start, uintptr_t
*end,
return false;
}
-bool AsanProcMaps::GetObjectNameAndOffset(uintptr_t addr, uintptr_t
*offset,
- char filename[],
- size_t filename_size) {
- return IterateForObjectNameAndOffset(addr, offset, filename,
filename_size);
+void AsanProcMaps::DescribeAddress(uintptr_t addr,
+ char out_buffer[],
+ size_t buffer_size) {
+ uintptr_t offset;
+ char objname[4096];
+ if (IterateForObjectNameAndOffset(addr, &offset, objname,
sizeof(objname))) {
+ SNPrintf(out_buffer, buffer_size, " (%s+0x%lx)", objname, offset);
+ } else {
+ out_buffer[0] = '\0';
+ }
}
void AsanThread::SetThreadStackTopAndBottom() {
Index: lib/asan/asan_procmaps.h
diff --git lib/asan/asan_procmaps.h lib/asan/asan_procmaps.h
index
6dd42f9f653610588dca45535371380792516f89..bc5f67b2de033acf4f205101f9a69082ec6bdce3
100644
--- lib/asan/asan_procmaps.h
+++ lib/asan/asan_procmaps.h
@@ -24,10 +24,13 @@ class AsanProcMaps {
bool Next(uintptr_t *start, uintptr_t *end, uintptr_t *offset,
char filename[], size_t filename_size);
void Reset();
- // Gets the object file name and the offset in that object for a given
- // address 'addr'. Returns true on success.
- bool GetObjectNameAndOffset(uintptr_t addr, uintptr_t *offset,
- char filename[], size_t filename_size);
+
+ // Gets all the information possible for a given address 'addr'
+ // (object file name, offset, source file name, source line, etc.)
+ // and puts it into the given buffer with a leading space
+ // or puts an empty string "" if no information is available.
+ void DescribeAddress(uintptr_t addr, char out_buffer[], size_t
buffer_size);
+
~AsanProcMaps();
private:
// Default implementation of GetObjectNameAndOffset.
Index: lib/asan/asan_stack.cc
diff --git lib/asan/asan_stack.cc lib/asan/asan_stack.cc
index
9f7469749085b576d8359b927da2c58623773e15..fe83534e8ff96546c09029705e2d94a1af039c00
100644
--- lib/asan/asan_stack.cc
+++ lib/asan/asan_stack.cc
@@ -42,14 +42,9 @@ void AsanStackTrace::PrintStack(uintptr_t *addr, size_t
size) {
for (size_t i = 0; i < size && addr[i]; i++) {
proc_maps.Reset();
uintptr_t pc = addr[i];
- uintptr_t offset;
- char filename[4096];
- if (proc_maps.GetObjectNameAndOffset(pc, &offset,
- filename, sizeof(filename))) {
- Printf(" #%ld 0x%lx (%s+0x%lx)\n", i, pc, filename, offset);
- } else {
- Printf(" #%ld 0x%lx\n", i, pc);
- }
+ char descr[4096];
+ proc_maps.DescribeAddress(pc, descr, sizeof(descr));
+ Printf(" #%ld 0x%lx%s\n", i, pc, descr);
}
}
#endif // ASAN_USE_EXTERNAL_SYMBOLIZER
More information about the llvm-commits
mailing list