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
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
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
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
! 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
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.
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,
! 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
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?
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
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
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,
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
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
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
ping
http://reviews.llvm.org/D4527
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
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,
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
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
19 matches
Mail list logo
