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

[Alexander Potapenko]

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