[Lldb-commits] [PATCH] [compiler-rt] ASan debugging API for report info extraction and locating addresses

[Kostya Serebryany]

================
Comment at: include/sanitizer/asan_interface.h:69
@@ +68,3 @@
+  // value of the function is 1. If no error occurred yet, returns 0.
+  int __asan_get_report_data(void **pc, void **bp, void **sp, void **addr,
+                             int *is_write, size_t *access_size,
----------------
I don't like the interface. 
You may eventually need to get other kinds of data, so you will have to change this function.
Instead, I suggest to add a set of functions like __asan_get_report_foo each of which returns just one part of report if such is known. 

================
Comment at: include/sanitizer/asan_interface.h:78
@@ +77,3 @@
+    __ADDRESS_TYPE_SHADOW_GAP = 2,
+    __ADDRESS_TYPE_SHADOW_HIGH = 3,
+    __ADDRESS_TYPE_GLOBAL = 4,
----------------
Do you need this to be a enum? 
Can this be just a 'const char *' containing the description, e.g. 
'stack', "heap".
BTW, this is missign at least one kind of memory: fake-stack. 


================
Comment at: lib/asan/asan_debugging.cc:46
@@ +45,3 @@
+  for (uptr i = 0; i < n_objects; i++) {
+    uptr beg  = (uptr)internal_simple_strtoll(p, &p, 10);
+    uptr size = (uptr)internal_simple_strtoll(p, &p, 10);
----------------
here and below, please avoid copy-paste at all costs. 
You've copied this code from DescribeAddressIfStack -- don't do it. 
Instead, refactor the code so that you can call it from other places. 
If possible, please send the refactoring as separate patches. 

================
Comment at: lib/asan/asan_report.cc:34
@@ -33,1 +33,3 @@
 
+static bool report_already_happened = 0;
+static uptr report_pc = 0;
----------------
wrap these into a struct. 

struct ReportData {
  uptr foo;
  uptr bar;
};

static ReportData report_data;

================
Comment at: lib/asan/asan_report.cc:947
@@ +946,3 @@
+
+  ScopedInErrorReport in_report;
+
----------------
why did you move this here?

http://reviews.llvm.org/D4527