Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Reverted in r218501 due to test failure on the sanitizer-x86_64-linux buildbot. Committed again in r218538 with a fix, intptr_t - uintptr_t, and an extra free(heap_ptr) to fix the LeakSanitizer report. Both changes are in the test file only (debug_locate.cc). http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Landed in r218481, thanks for review! http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Addressing review comments. http://reviews.llvm.org/D4527 Files: include/sanitizer/asan_interface.h lib/asan/asan_debugging.cc lib/asan/asan_globals.cc lib/asan/asan_interface_internal.h lib/asan/asan_report.cc lib/asan/asan_report.h test/asan/TestCases/debug_locate.cc test/asan/TestCases/debug_report.cc Index: include/sanitizer/asan_interface.h === --- include/sanitizer/asan_interface.h +++ include/sanitizer/asan_interface.h @@ -62,6 +62,32 @@ // Print the description of addr (useful when debugging in gdb). void __asan_describe_address(void *addr); + // Useful for calling from a debugger to get information about an ASan error. + // Returns 1 if an error has been (or is being) reported, otherwise returns 0. + int __asan_report_present(); + + // Useful for calling from a debugger to get information about an ASan error. + // If an error has been (or is being) reported, the following functions return + // the pc, bp, sp, address, access type (0 = read, 1 = write), access size and + // bug description (e.g. heap-use-after-free). Otherwise they return 0. + void *__asan_get_report_pc(); + void *__asan_get_report_bp(); + void *__asan_get_report_sp(); + void *__asan_get_report_address(); + int __asan_get_report_access_type(); + size_t __asan_get_report_access_size(); + const char *__asan_get_report_description(); + + // Useful for calling from the debugger to get information about a pointer. + // Returns the category of the given pointer as a constant string. + // Possible return values are global, stack, stack-fake, heap, + // heap-invalid, shadow-low, shadow-gap, shadow-high, unknown. + // If global or stack, tries to also return the variable name, address and + // size. If heap, tries to return the chunk address and size. 'name' should + // point to an allocated buffer of size 'name_size'. + const char *__asan_locate_address(void *addr, char *name, size_t name_size, +void **region_address, size_t *region_size); + // Useful for calling from the debugger to get the allocation stack trace // and thread ID for a heap address. Stores up to 'size' frames into 'trace', // returns the number of stored frames or 0 on error. Index: lib/asan/asan_debugging.cc === --- lib/asan/asan_debugging.cc +++ lib/asan/asan_debugging.cc @@ -17,10 +17,63 @@ #include asan_flags.h #include asan_internal.h #include asan_mapping.h +#include asan_report.h #include asan_thread.h namespace __asan { +void GetInfoForStackVar(uptr addr, AddressDescription *descr, AsanThread *t) { + uptr offset = 0; + uptr frame_pc = 0; + const char *frame_descr = t-GetFrameNameByAddr(addr, offset, frame_pc); + InternalMmapVectorStackVarDescr vars(16); + ParseFrameDescription(frame_descr, vars); + + descr-name[0] = 0; + descr-region_address = 0; + descr-region_size = 0; + descr-region_kind = stack; + + for (uptr i = 0; i vars.size(); i++) { +if (offset = vars[i].beg + vars[i].size) { + internal_strncat(descr-name, vars[i].name_pos, + Min(descr-name_size, vars[i].name_len)); + descr-region_address = addr - (offset - vars[i].beg); + descr-region_size = vars[i].size; + return; +} + } +} + +void GetInfoForHeapAddress(uptr addr, AddressDescription *descr) { + AsanChunkView chunk = FindHeapChunkByAddress(addr); + if (!chunk.IsValid()) { +descr-region_kind = heap-invalid; +return; + } + + descr-region_address = chunk.Beg(); + descr-region_size = chunk.UsedSize(); + descr-region_kind = heap; +} + +void AsanLocateAddress(uptr addr, AddressDescription *descr) { + if (DescribeAddressIfShadow(addr, descr, /* print */ false)) { +return; + } + if (GetInfoForAddressIfGlobal(addr, descr)) { +return; + } + asanThreadRegistry().Lock(); + AsanThread *thread = FindThreadByStackAddress(addr); + asanThreadRegistry().Unlock(); + if (thread) { +GetInfoForStackVar(addr, descr, thread); +return; + } + GetInfoForHeapAddress(addr, descr); +} + uptr AsanGetStack(uptr addr, uptr *trace, uptr size, u32 *thread_id, bool alloc_stack) { AsanChunkView chunk = FindHeapChunkByAddress(addr); @@ -56,6 +109,16 @@ using namespace __asan; SANITIZER_INTERFACE_ATTRIBUTE +const char *__asan_locate_address(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size) { + AddressDescription descr = { name, name_size, 0, 0, 0 }; + AsanLocateAddress(addr, descr); + if (region_address) *region_address = descr.region_address; + if (region_size) *region_size = descr.region_size; + return descr.region_kind; +} + +SANITIZER_INTERFACE_ATTRIBUTE uptr __asan_get_alloc_stack(uptr addr, uptr *trace, uptr size, u32 *thread_id) { return AsanGetStack(addr, trace, size, thread_id, /* alloc_stack */ true); }
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Uploading patch with more context. http://reviews.llvm.org/D4527 Files: include/sanitizer/asan_interface.h lib/asan/asan_debugging.cc lib/asan/asan_globals.cc lib/asan/asan_interface_internal.h lib/asan/asan_report.cc lib/asan/asan_report.h test/asan/TestCases/debug_locate.cc test/asan/TestCases/debug_report.cc Index: include/sanitizer/asan_interface.h === --- include/sanitizer/asan_interface.h +++ include/sanitizer/asan_interface.h @@ -62,6 +62,32 @@ // Print the description of addr (useful when debugging in gdb). void __asan_describe_address(void *addr); + // Useful for calling from a debugger to get information about an ASan error. + // Returns 1 if an error has been (or is being) reported, otherwise returns 0. + int __asan_report_present(); + + // Useful for calling from a debugger to get information about an ASan error. + // If an error has been (or is being) reported, the following functions return + // the pc, bp, sp, address, access type (0 = read, 1 = write), access size and + // bug description (e.g. heap-use-after-free). Otherwise they return 0. + void *__asan_get_report_pc(); + void *__asan_get_report_bp(); + void *__asan_get_report_sp(); + void *__asan_get_report_address(); + int __asan_get_report_access_type(); + size_t __asan_get_report_access_size(); + const char *__asan_get_report_description(); + + // Useful for calling from the debugger to get information about a pointer. + // Returns the category of the given pointer as a constant string. + // Possible return values are global, stack, stack-fake, heap, + // heap-invalid, shadow-low, shadow-gap, shadow-high, unknown. + // If global or stack, tries to also return the variable name, address and + // size. If heap, tries to return the chunk address and size. 'name' should + // point to an allocated buffer of size 'name_size'. + const char *__asan_locate_address(void *addr, char *name, size_t name_size, +void **region_address, size_t *region_size); + // Useful for calling from the debugger to get the allocation stack trace // and thread ID for a heap address. Stores up to 'size' frames into 'trace', // returns the number of stored frames or 0 on error. Index: lib/asan/asan_debugging.cc === --- lib/asan/asan_debugging.cc +++ lib/asan/asan_debugging.cc @@ -17,10 +17,63 @@ #include asan_flags.h #include asan_internal.h #include asan_mapping.h +#include asan_report.h #include asan_thread.h namespace __asan { +void GetInfoForStackVar(uptr addr, AddressDescription *descr, AsanThread *t) { + uptr offset = 0; + uptr frame_pc = 0; + const char *frame_descr = t-GetFrameNameByAddr(addr, offset, frame_pc); + InternalMmapVectorStackVarDescr vars(16); + ParseFrameDescription(frame_descr, vars); + + descr-name[0] = 0; + descr-region_address = 0; + descr-region_size = 0; + descr-region_kind = stack; + + for (uptr i = 0; i vars.size(); i++) { +if (offset = vars[i].beg + vars[i].size) { + internal_strncat(descr-name, vars[i].name_pos, + Min(descr-name_size, vars[i].name_len)); + descr-region_address = addr - (offset - vars[i].beg); + descr-region_size = vars[i].size; + return; +} + } +} + +void GetInfoForHeapAddress(uptr addr, AddressDescription *descr) { + AsanChunkView chunk = FindHeapChunkByAddress(addr); + if (!chunk.IsValid()) { +descr-region_kind = heap-invalid; +return; + } + + descr-region_address = chunk.Beg(); + descr-region_size = chunk.UsedSize(); + descr-region_kind = heap; +} + +void AsanLocateAddress(uptr addr, AddressDescription *descr) { + if (DescribeAddressIfShadow(addr, descr, /* print */ false)) { +return; + } + if (GetInfoForAddressIfGlobal(addr, descr)) { +return; + } + asanThreadRegistry().Lock(); + AsanThread *thread = FindThreadByStackAddress(addr); + asanThreadRegistry().Unlock(); + if (thread) { +GetInfoForStackVar(addr, descr, thread); +return; + } + GetInfoForHeapAddress(addr, descr); +} + uptr AsanGetStack(uptr addr, uptr *trace, uptr size, u32 *thread_id, bool alloc_stack) { AsanChunkView chunk = FindHeapChunkByAddress(addr); @@ -56,6 +109,16 @@ using namespace __asan; SANITIZER_INTERFACE_ATTRIBUTE +const char *__asan_locate_address(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size) { + AddressDescription descr = { name, name_size, 0, 0, 0 }; + AsanLocateAddress(addr, descr); + if (region_address) *region_address = descr.region_address; + if (region_size) *region_size = descr.region_size; + return descr.region_kind; +} + +SANITIZER_INTERFACE_ATTRIBUTE uptr __asan_get_alloc_stack(uptr addr, uptr *trace, uptr size, u32 *thread_id) { return AsanGetStack(addr, trace, size, thread_id, /* alloc_stack */
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
! In D4527#33, @samsonov wrote: Why not internal_strncpy(descr-name, vars[i].name_pos, Min(descr-name_size - 1, vars[i].name_len)) ? Because that would not write \0 after the string (we're copying just a part of the string pointed to by vars[i].name_pos). So I would have to memset the whole buffer to zero before. Should I do that, instead of the strncat? (The strncat is also used in asan_report.cc:436). Right, let's keep strncat here then. Ok. So, are you fine with how the patch looks like now? http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Updated patch. Consider introducing a struct for address description instead of passing quadruple (name, name_size, region_address, region_size) around. It would be much easier to modify it later. You can also add region_kind (global, stack etc.) string there. Introduced AddressDescription. However, I still kept the external interface not to use this struct, again for LLDB convenience reasons. http://reviews.llvm.org/D4527 Files: include/sanitizer/asan_interface.h lib/asan/asan_debugging.cc lib/asan/asan_globals.cc lib/asan/asan_interface_internal.h lib/asan/asan_report.cc lib/asan/asan_report.h test/asan/TestCases/debug_locate.cc test/asan/TestCases/debug_report.cc Index: include/sanitizer/asan_interface.h === --- include/sanitizer/asan_interface.h +++ include/sanitizer/asan_interface.h @@ -62,6 +62,32 @@ // Print the description of addr (useful when debugging in gdb). void __asan_describe_address(void *addr); + // Useful for calling from a debugger to get information about an ASan error. + // Returns 1 if an error has been (or is being) reported, otherwise returns 0. + int __asan_report_present(); + + // Useful for calling from a debugger to get information about an ASan error. + // If an error has been (or is being) reported, the following functions return + // the pc, bp, sp, address, access type (0 = read, 1 = write), access size and + // bug description (e.g. heap-use-after-free). Otherwise they return 0. + void *__asan_get_report_pc(); + void *__asan_get_report_bp(); + void *__asan_get_report_sp(); + void *__asan_get_report_address(); + int __asan_get_report_access_type(); + size_t __asan_get_report_access_size(); + const char *__asan_get_report_description(); + + // Useful for calling from the debugger to get information about a pointer. + // Returns the category of the given pointer as a constant string. + // Possible return values are global, stack, stack-fake, heap, + // heap-invalid, shadow-low, shadow-gap, shadow-high, unknown. + // If global or stack, tries to also return the variable name, address and + // size. If heap, tries to return the chunk address and size. 'name' should + // point to an allocated buffer of size 'name_size'. + const char *__asan_locate_address(void *addr, char *name, size_t name_size, +void **region_address, size_t *region_size); + // Useful for calling from the debugger to get the allocation stack trace // and thread ID for a heap address. Stores up to 'size' frames into 'trace', // returns the number of stored frames or 0 on error. Index: lib/asan/asan_debugging.cc === --- lib/asan/asan_debugging.cc +++ lib/asan/asan_debugging.cc @@ -17,10 +17,66 @@ #include asan_flags.h #include asan_internal.h #include asan_mapping.h +#include asan_report.h #include asan_thread.h namespace __asan { +void AsanGetStackVarInfo(uptr addr, AddressDescription *descr, AsanThread *t) { + uptr offset = 0; + uptr frame_pc = 0; + const char *frame_descr = t-GetFrameNameByAddr(addr, offset, frame_pc); + InternalMmapVectorStackVarDescr vars(16); + ParseFrameDescription(frame_descr, vars); + + for (uptr i = 0; i vars.size(); i++) { +if (offset = vars[i].beg + vars[i].size) { + descr-name[0] = 0; + internal_strncat(descr-name, vars[i].name_pos, + Min(descr-name_size, vars[i].name_len)); + descr-region_address = addr - (offset - vars[i].beg); + descr-region_size = vars[i].size; + descr-region_kind = stack; + break; +} + } +} + +void AsanLocateAddress(uptr addr, AddressDescription *descr) { + // Check if addr is in shadow memory. + const char *stack_address_type = nullptr; + if (DescribeAddressIfShadow(addr, stack_address_type, /* print */ false)) { +descr-region_kind = stack_address_type; +return; + } + + // Check if addr is in global memory. + if (GetInfoForAddressIfGlobal(addr, descr)) { +descr-region_kind = global; +return; + } + + // Check if addr is a pointer into a stack. + asanThreadRegistry().Lock(); + AsanThread *thread = FindThreadByStackAddress(addr); + asanThreadRegistry().Unlock(); + if (thread) { +AsanGetStackVarInfo(addr, descr, thread); +return; + } + + // Let's assume addr is a heap address. + AsanChunkView chunk = FindHeapChunkByAddress(addr); + if (!chunk.IsValid()) { +descr-region_kind = heap-invalid; +return; + } + + descr-region_address = chunk.Beg(); + descr-region_size = chunk.UsedSize(); + descr-region_kind = heap; +} + uptr AsanGetStack(uptr addr, uptr *trace, uptr size, u32 *thread_id, bool alloc_stack) { AsanChunkView chunk = FindHeapChunkByAddress(addr); @@ -56,6 +112,16 @@ using namespace __asan; SANITIZER_INTERFACE_ATTRIBUTE +const char *__asan_locate_address(uptr addr, char *name, uptr
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Getting closer. Comment at: lib/asan/asan_debugging.cc:35 @@ +34,3 @@ + descr-name[0] = 0; + internal_strncat(descr-name, vars[i].name_pos, + Min(descr-name_size, vars[i].name_len)); Why not internal_strncpy(descr-name, vars[i].name_pos, Min(descr-name_size - 1, vars[i].name_len)) ? Comment at: lib/asan/asan_debugging.cc:40 @@ +39,3 @@ + descr-region_kind = stack; + break; +} s/break/return Comment at: lib/asan/asan_debugging.cc:48 @@ +47,3 @@ + const char *stack_address_type = nullptr; + if (DescribeAddressIfShadow(addr, stack_address_type, /* print */ false)) { +descr-region_kind = stack_address_type; See another comment about passing AddressDescription into DescribeAddressIfShadow() Comment at: lib/asan/asan_debugging.cc:55 @@ +54,3 @@ + if (GetInfoForAddressIfGlobal(addr, descr)) { +descr-region_kind = global; +return; This should be filled by GtInfoForAddressIfGlobal Comment at: lib/asan/asan_debugging.cc:69 @@ +68,3 @@ + // Let's assume addr is a heap address. + AsanChunkView chunk = FindHeapChunkByAddress(addr); + if (!chunk.IsValid()) { Consider factoring out filling heap address description into a separate routine. AsanLocateAddress would then be really straightforward. Comment at: lib/asan/asan_globals.cc:105 @@ +104,3 @@ +} else { + CHECK(output_global); + if (IsAddressNearGlobal(addr, g)) { put CHECK() under if() Comment at: lib/asan/asan_globals.cc:126 @@ +125,3 @@ +descr-region_size = g.size; +return true; + } You can (and probably should) fill descr-region_type here. Comment at: lib/asan/asan_report.h:44 @@ +43,3 @@ +bool GetInfoForAddressIfGlobal(uptr addr, AddressDescription *descr); +bool DescribeAddressIfShadow(uptr addr, const char **shadow_type = 0, + bool print = true); Why not pass AddressDescription here as well? http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
! In D4527#31, @kubabrecka wrote: Getting closer. Thanks for the patience :) Why not internal_strncpy(descr-name, vars[i].name_pos, Min(descr-name_size - 1, vars[i].name_len)) ? Because that would not write \0 after the string (we're copying just a part of the string pointed to by vars[i].name_pos). So I would have to memset the whole buffer to zero before. Should I do that, instead of the strncat? (The strncat is also used in asan_report.cc:436). Right, let's keep strncat here then. http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Comment at: lib/asan/asan_debugging.cc:38 @@ +37,3 @@ +uptr var_name_len = vars[i].name_len; +if (vars[i].name_len name_size - 1) var_name_len = name_size - 1; +memcpy(name, vars[i].name_pos, var_name_len); internal_strncpy? Comment at: lib/asan/asan_debugging.cc:121 @@ +120,3 @@ + uptr *region_address, uptr *region_size) { + return AsanLocateAddress(addr, name, name_size, region_address, region_size); +} Consider introducing a struct for address description instead of passing quadruple (name, name_size, region_address, region_size) around. It would be much easier to modify it later. You can also add region_kind (global, stack etc.) string there. Comment at: lib/asan/asan_globals.cc:85 @@ -84,2 +84,3 @@ -bool DescribeAddressIfGlobal(uptr addr, uptr size) { +bool DescribeOrGetInfoIfGlobal(uptr addr, uptr size, bool print, + Global *output_global) { This function can be static. Comment at: lib/asan/asan_globals.cc:98 @@ +97,3 @@ + if (IsAddressNearGlobal(addr, g)) { +if (output_global) *output_global = g; +return true; I believe output_global should always be nonnull in this case. Remove the if() and optionally add a CHECK. Comment at: lib/asan/asan_interface_internal.h:97 @@ +96,3 @@ + + // The following functions extract various information from a report (if a + // report has already been printed out or is currently being generated). Don't duplicate the comments here. Comment at: lib/asan/asan_report.cc:253 @@ -241,1 +252,3 @@ + +bool IsAddressNearGlobal(uptr addr, const __asan_global g) { if (addr = g.beg - kMinimalDistanceFromAnotherGlobal) return false; This clearly belongs to asan_globals.cc Comment at: lib/asan/asan_report.cc:298 @@ -283,1 +297,3 @@ +if (print) Printf(kAddrInShadowReport, addr, area_type); +if (output_type) *output_type = area_type; return true; I guess that if print() is false, output_type should be nonnull. Comment at: lib/asan/asan_report.cc:961 @@ +960,3 @@ + report_happened = true; + report_data.description = bug_descr; + report_data.pc = pc; Can you use aggregate assignment here? report_data = { ... }; http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Addressing comments from review. http://reviews.llvm.org/D4527 Files: include/sanitizer/asan_interface.h lib/asan/asan_debugging.cc lib/asan/asan_globals.cc lib/asan/asan_interface_internal.h lib/asan/asan_report.cc lib/asan/asan_report.h test/asan/TestCases/debug_locate.cc test/asan/TestCases/debug_report.cc Index: include/sanitizer/asan_interface.h === --- include/sanitizer/asan_interface.h +++ include/sanitizer/asan_interface.h @@ -62,6 +62,32 @@ // Print the description of addr (useful when debugging in gdb). void __asan_describe_address(void *addr); + // Useful for calling from a debugger to get information about an ASan error. + // Returns 1 if an error has been (or is being) reported, otherwise returns 0. + int __asan_report_present(); + + // Useful for calling from a debugger to get information about an ASan error. + // If an error has been (or is being) reported, the following functions return + // the pc, bp, sp, address, access type (0 = read, 1 = write), access size and + // bug description (e.g. heap-use-after-free). Otherwise they return 0. + void *__asan_get_report_pc(); + void *__asan_get_report_bp(); + void *__asan_get_report_sp(); + void *__asan_get_report_address(); + int __asan_get_report_access_type(); + size_t __asan_get_report_access_size(); + const char *__asan_get_report_description(); + + // Useful for calling from the debugger to get information about a pointer. + // Returns the category of the given pointer as a constant string. + // Possible return values are global, stack, stack-fake, heap, + // heap-invalid, shadow-low, shadow-gap, shadow-high, unknown. + // If global or stack, tries to also return the variable name, address and + // size. If heap, tries to return the chunk address and size. 'name' should + // point to an allocated buffer of size 'name_size'. + const char *__asan_locate_address(void *addr, char *name, size_t name_size, +void **region_address, size_t *region_size); + // Useful for calling from the debugger to get the allocation stack trace // and thread ID for a heap address. Stores up to 'size' frames into 'trace', // returns the number of stored frames or 0 on error. Index: lib/asan/asan_debugging.cc === --- lib/asan/asan_debugging.cc +++ lib/asan/asan_debugging.cc @@ -17,10 +17,70 @@ #include asan_flags.h #include asan_internal.h #include asan_mapping.h +#include asan_report.h #include asan_thread.h namespace __asan { +void AsanGetStackVarInfo(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size, + AsanThread *t) { + uptr offset = 0; + uptr frame_pc = 0; + const char *frame_descr = t-GetFrameNameByAddr(addr, offset, frame_pc); + InternalMmapVectorStackVarDescr vars(16); + ParseFrameDescription(frame_descr, vars); + + for (uptr i = 0; i vars.size(); i++) { +if (offset = vars[i].beg + vars[i].size) { + if (name) { +uptr var_name_len = vars[i].name_len; +if (vars[i].name_len name_size - 1) var_name_len = name_size - 1; +memcpy(name, vars[i].name_pos, var_name_len); +name[var_name_len] = '\0'; + } + if (region_address) *region_address = addr - (offset - vars[i].beg); + if (region_size) *region_size = vars[i].size; + break; +} + } +} + +const char *AsanLocateAddress(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size) { + // Check if addr is in shadow memory. + const char *stack_address_type = 0; + if (DescribeAddressIfShadow(addr, stack_address_type, /* print */ false)) { +return stack_address_type; + } + + // Check if addr is in global memory. + if (GetInfoForAddressIfGlobal(addr, name, name_size, region_address, +region_size)) { +return global; + } + + // Check if addr is a pointer into a stack. + asanThreadRegistry().Lock(); + AsanThread *thread = FindThreadByStackAddress(addr); + asanThreadRegistry().Unlock(); + if (thread) { +AsanGetStackVarInfo(addr, name, name_size, region_address, region_size, +thread); +return stack; + } + + // Let's assume addr is a heap address. + AsanChunkView chunk = FindHeapChunkByAddress(addr); + if (!chunk.IsValid()) +return heap-invalid; + + if (region_address) *region_address = chunk.Beg(); + if (region_size) *region_size = chunk.UsedSize(); + + return heap; +} + uptr AsanGetStack(uptr addr, uptr *trace, uptr size, u32 *thread_id, bool alloc_stack) { AsanChunkView chunk = FindHeapChunkByAddress(addr); @@ -56,6 +116,12 @@ using namespace __asan; SANITIZER_INTERFACE_ATTRIBUTE +const char *__asan_locate_address(uptr addr, char *name, uptr name_size, +
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Addressing the review comments. Tried to minimize any code duplication. Added full context. http://reviews.llvm.org/D4527 Files: include/sanitizer/asan_interface.h lib/asan/asan_debugging.cc lib/asan/asan_globals.cc lib/asan/asan_interface_internal.h lib/asan/asan_report.cc lib/asan/asan_report.h test/asan/TestCases/debug_locate.cc test/asan/TestCases/debug_report.cc Index: include/sanitizer/asan_interface.h === --- include/sanitizer/asan_interface.h +++ include/sanitizer/asan_interface.h @@ -62,6 +62,32 @@ // Print the description of addr (useful when debugging in gdb). void __asan_describe_address(void *addr); + // Useful for calling from a debugger to get information about an ASan error. + // Returns 1 if an error has been (or is being) reported, otherwise returns 0. + int __asan_report_present(); + + // Useful for calling from a debugger to get information about an ASan error. + // If an error has been (or is being) reported, the following functions return + // the pc, bp, sp, address, access type (0 = read, 1 = write), access size and + // bug description (e.g. heap-use-after-free). Otherwise they return 0. + void *__asan_get_report_pc(); + void *__asan_get_report_bp(); + void *__asan_get_report_sp(); + void *__asan_get_report_address(); + int __asan_get_report_access_type(); + size_t __asan_get_report_access_size(); + const char *__asan_get_report_description(); + + // Useful for calling from the debugger to get information about a pointer. + // Returns the category of the given pointer as a constant string. + // Possible return values are global, stack, stack-fake, heap, + // heap-invalid, shadow-low, shadow-gap, shadow-high, unknown. + // If global or stack, tries to also return the variable name, address and + // size. If heap, tries to return the chunk address and size. 'name' should + // point to an allocated buffer of size 'name_size'. + const char *__asan_locate_address(void *addr, char *name, size_t name_size, +void **region_address, size_t *region_size); + // Useful for calling from the debugger to get the allocation stack trace // and thread ID for a heap address. Stores up to 'size' frames into 'trace', // returns the number of stored frames or 0 on error. Index: lib/asan/asan_debugging.cc === --- lib/asan/asan_debugging.cc +++ lib/asan/asan_debugging.cc @@ -17,10 +17,70 @@ #include asan_flags.h #include asan_internal.h #include asan_mapping.h +#include asan_report.h #include asan_thread.h namespace __asan { +void AsanGetStackVarInfo(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size, + AsanThread *t) { + uptr offset = 0; + uptr frame_pc = 0; + const char *frame_descr = t-GetFrameNameByAddr(addr, offset, frame_pc); + InternalMmapVectorStackVarDescr vars(16); + ParseFrameDescription(frame_descr, vars); + + for (uptr i = 0; i vars.size(); i++) { +if (offset = vars[i].beg + vars[i].size) { + if (name) { +uptr var_name_len = vars[i].name_len; +if (vars[i].name_len name_size - 1) var_name_len = name_size - 1; +memcpy(name, vars[i].name_pos, var_name_len); +name[var_name_len] = '\0'; + } + if (region_address) *region_address = addr - (offset - vars[i].beg); + if (region_size) *region_size = vars[i].size; + break; +} + } +} + +const char *AsanLocateAddress(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size) { + // Check if addr is in shadow memory. + const char *stack_address_type = 0; + if (DescribeAddressIfShadow(addr, stack_address_type, /* print */ false)) { +return stack_address_type; + } + + // Check if addr is in global memory. + if (GetInfoForAddressIfGlobal(addr, name, name_size, region_address, +region_size)) { +return global; + } + + // Check if addr is a pointer into a stack. + asanThreadRegistry().Lock(); + AsanThread *thread = FindThreadByStackAddress(addr); + asanThreadRegistry().Unlock(); + if (thread) { +AsanGetStackVarInfo(addr, name, name_size, region_address, region_size, +thread); +return stack; + } + + // Let's assume addr is a heap address. + AsanChunkView chunk = FindHeapChunkByAddress(addr); + if (!chunk.IsValid()) +return heap-invalid; + + if (region_address) *region_address = chunk.Beg(); + if (region_size) *region_size = chunk.UsedSize(); + + return heap; +} + uptr AsanGetStack(uptr addr, uptr *trace, uptr size, u32 *thread_id, bool alloc_stack) { AsanChunkView chunk = FindHeapChunkByAddress(addr); @@ -56,6 +116,12 @@ using namespace __asan; SANITIZER_INTERFACE_ATTRIBUTE +const char *__asan_locate_address(uptr
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Comment at: lib/asan/asan_globals.cc:85 @@ -84,2 +84,3 @@ -bool DescribeAddressIfGlobal(uptr addr, uptr size) { +bool DescribeOrGetInfoIfGlobal(uptr addr, uptr size, bool only_get_info, + char *name, uptr name_size, uptr *region_address, Can you make this function do one of the following: (1) print the address description (what it does now) if print input parameter is true (2) set __asan_global output parameter to the structure describing the global. Then you won't need GetInfoForAddressRelativeToGlobal at all. Comment at: lib/asan/asan_globals.cc:108 @@ +107,3 @@ +bool DescribeAddressIfGlobal(uptr addr, uptr size) { + return DescribeOrGetInfoIfGlobal(addr, size, /* only_get_info */ false, 0, 0, + 0, 0); See above: passing a pack of fake parameters is ugly. Instead you can call this function in two modes: DescrineOrGetInfoForGlobal(addr, size, /*print*/true, /*global*/nullptr); or Global g; DescribeOrGetInfoForGlobal(addr, 1, /*print*/false, g); Comment at: lib/asan/asan_report.cc:294 @@ +293,3 @@ +if (name_len name_size - 1) name_len = name_size - 1; +memcpy(name, g.name, name_len); +name[name_len] = '\0'; Consider using internal_strncpy here. Comment at: lib/asan/asan_report.cc:308 @@ -273,2 +307,3 @@ if (AddrIsInShadowGap(addr)) { -Printf(kAddrInShadowReport, addr, shadow gap area); +if (shadow_type) *shadow_type = shadow-gap; +if (print) Printf(kAddrInShadowReport, addr, shadow gap area); Can you use the identical strings in the output and in shadow_type variable? That is, make shadow_type equal to shadow gap, or high shadow, or low shadow. Comment at: lib/asan/asan_report.cc:990 @@ +989,3 @@ + + ScopedInErrorReport in_report; + Why have you moved ScopedInErrorReport here? Comment at: lib/asan/asan_report.cc:1058 @@ +1057,3 @@ +uptr __asan_get_report_access_size() { + return report_data.access_size; +} Any specific reason to not expose report_data structure layout in the interface header? This pack of functions doesn't look nice. http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Any specific reason to not expose report_data structure layout in the interface header? This pack of functions doesn't look nice. It's to make it easier to use from LLDB. When we add a new function, it's easier to check whether that symbol exists than to version this struct. See the original suggestion at http://reviews.llvm.org/D4527?id=11470#inline-37585 Why have you moved ScopedInErrorReport here? Because ScopedInErrorReport's constructor does things where I'd like the report data to already be available. Namely, it calls __asan_on_error, which seems like the only sensible place for a breakpoint *before* the actual report is printed (I'm using it in the debug_report.cc test case). I know we have to be careful about this, but it seems to me this move should be safe (the code in between doesn't do any locks, etc.). Or is there any subtle issue due to this move? I agree with the other comments, will submit a new patch. Kuba http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Looks mostly good. Comment at: lib/asan/asan_debugging.cc:51 @@ +50,3 @@ + uptr *region_address, uptr *region_size) { + // check for shadow + if (!AddrIsInMem(addr)) { Single-line comments must start with a capital letter and end with a period. Please fix here and below. Comment at: lib/asan/asan_interface_internal.h:94 @@ -93,1 +93,3 @@ SANITIZER_INTERFACE_ATTRIBUTE + int __asan_report_present(); + Please add a comment for this family of functions. Comment at: lib/asan/asan_report.cc:35 @@ +34,3 @@ +struct ReportData { + bool already_happened; + uptr pc; IIUC for a non-empty ReportData the value of |already_happened| is always true. I suggest to extract it into a separate flag variable to avoid copying this meaningless value when passing ReportData around. Comment at: test/asan/TestCases/debug_report.cc:6 @@ +5,3 @@ +#include sanitizer/asan_interface.h +#include stdlib.h +#include stdio.h Please mind the order of includes. Comment at: test/asan/TestCases/debug_report.cc:16 @@ +15,3 @@ + // CHECK: no report + + heap_ptr[0] = 'A'; // BOOM nit: too much vertical whitespace in this function. http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Sorry for the delay, will review tomorrow (MSK time) http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
ping http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Comment at: lib/asan/asan_report.cc:947 @@ +946,3 @@ + + ScopedInErrorReport in_report; + Kostya Serebryany wrote: why did you move this here? Because ScopedInErrorReport's constructor does things where I'd like the report data to already be available. Namely, it calls __asan_on_error, which seems like the only sensible place for a breakpoint *before* the actual report is printed (I'm using it in the debug_report.cc test case). http://reviews.llvm.org/D4527 ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
On 2014-Jul-16, at 14:54, Kuba Brecka kuba.bre...@gmail.com wrote: Comment at: lib/asan/asan_report.cc:35 @@ +34,3 @@ +struct ReportData { + bool already_happened = 0; + uptr pc = 0; Alexey Samsonov wrote: I believe this wouldn't compile on Windows :-/ I don't have a Windows box nearby, would it be because of the member initialization or because of assignment of zero into a bool? http://reviews.llvm.org/D4527 I don't think member initialization is supported on MSVC. In general, you can check the coding standards [1] for supported C++11 features [2]. [1]: http://llvm.org/docs/CodingStandards.html [2]: http://llvm.org/docs/CodingStandards.html#supported-c-11-language-and-library-features ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses
Update addressing review comments. http://reviews.llvm.org/D4527 Files: include/sanitizer/asan_interface.h lib/asan/asan_debugging.cc lib/asan/asan_globals.cc lib/asan/asan_interface_internal.h lib/asan/asan_report.cc lib/asan/asan_report.h test/asan/TestCases/debug_locate.cc test/asan/TestCases/debug_report.cc Index: include/sanitizer/asan_interface.h === --- include/sanitizer/asan_interface.h +++ include/sanitizer/asan_interface.h @@ -62,6 +62,32 @@ // Print the description of addr (useful when debugging in gdb). void __asan_describe_address(void *addr); + // Useful for calling from a debugger to get information about an ASan error. + // Returns 1 if an error has been (or is being) reported, otherwise returns 0. + int __asan_report_present(); + + // Useful for calling from a debugger to get information about an ASan error. + // If an error has been (or is being) reported, the following functions return + // the pc, bp, sp, address, access type (0 = read, 1 = write), access type and + // bug description (e.g. heap-use-after-free). Otherwrite they return 0. + void *__asan_get_report_pc(); + void *__asan_get_report_bp(); + void *__asan_get_report_sp(); + void *__asan_get_report_address(); + int __asan_get_report_access_type(); + size_t __asan_get_report_access_size(); + const char *__asan_get_report_description(); + + // Useful for calling from the debugger to get information about a pointer. + // Returns the category of the given pointer as a constant string. + // Possible return values are global, stack, stack-fake, heap, + // heap-invalid, shadow-low, shadow-gap, shadow-high, unknown. + // If global or stack, tries to also return the variable name, address and + // size. If heap, tries to return the chunk address and size. 'name' should + // point to an allocated buffer of size 'name_size'. + const char *__asan_locate_address(void *addr, char *name, size_t name_size, +void **region_address, size_t *region_size); + // Useful for calling from the debugger to get the allocation stack trace // and thread ID for a heap address. Stores up to 'size' frames into 'trace', // returns the number of stored frames or 0 on error. Index: lib/asan/asan_debugging.cc === --- lib/asan/asan_debugging.cc +++ lib/asan/asan_debugging.cc @@ -17,10 +17,75 @@ #include asan_flags.h #include asan_internal.h #include asan_mapping.h +#include asan_report.h #include asan_thread.h namespace __asan { +void AsanGetStackVarInfo(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size, + AsanThread *t) { + uptr offset = 0; + uptr frame_pc = 0; + const char *frame_descr = t-GetFrameNameByAddr(addr, offset, frame_pc); + InternalMmapVectorStackVarDescr vars(16); + ParseFrameDescription(frame_descr, vars); + + for (uptr i = 0; i vars.size(); i++) { +if (offset = vars[i].beg + vars[i].size) { + if (name) { +uptr var_name_len = vars[i].name_len; +if (vars[i].name_len name_size - 1) var_name_len = name_size - 1; +memcpy(name, vars[i].name_pos, var_name_len); +name[var_name_len] = '\0'; + } + if (region_address) *region_address = addr - (offset - vars[i].beg); + if (region_size) *region_size = vars[i].size; + break; +} + } +} + +const char *AsanLocateAddress(uptr addr, char *name, uptr name_size, + uptr *region_address, uptr *region_size) { + // check for shadow + if (!AddrIsInMem(addr)) { +if (AddrIsInShadowGap(addr)) + return shadow-gap; +if (AddrIsInHighShadow(addr)) + return shadow-high; +if (AddrIsInLowShadow(addr)) + return shadow-low; +return unknown; + } + + // check for global + if (GetInfoForAddressIfGlobal(addr, name, name_size, region_address, +region_size)) { +return global; + } + + // check for stack + asanThreadRegistry().Lock(); + AsanThread *thread = FindThreadByStackAddress(addr); + asanThreadRegistry().Unlock(); + if (thread) { +AsanGetStackVarInfo(addr, name, name_size, region_address, region_size, +thread); +return stack; + } + + // assume it is a heap address + AsanChunkView chunk = FindHeapChunkByAddress(addr); + if (!chunk.IsValid()) +return heap-invalid; + + if (region_address) *region_address = chunk.Beg(); + if (region_size) *region_size = chunk.UsedSize(); + + return heap; +} + uptr AsanGetStack(uptr addr, uptr *trace, uptr size, u32 *thread_id, bool alloc_stack) { AsanChunkView chunk = FindHeapChunkByAddress(addr); @@ -56,6 +121,12 @@ using namespace __asan; SANITIZER_INTERFACE_ATTRIBUTE +const char *__asan_locate_address(uptr addr, char *name, uptr name_size, +