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

[Alexey Samsonov]

================
Comment at: lib/asan/asan_debugging.cc:43
@@ +42,3 @@
+  }
+}
+
----------------
what if you haven't found a stack variable? Should you still set descr->region_kind in this case?

================
Comment at: lib/asan/asan_debugging.cc:58
@@ +57,3 @@
+void AsanLocateAddress(uptr addr, AddressDescription *descr) {
+  // Check if addr is in shadow memory.
+  if (DescribeAddressIfShadow(addr, descr, /* print */ false)) {
----------------
Minor nit: comments seems to be redundant in this function. Also, you can reduce the amount of vertical whitespace here.

================
Comment at: lib/asan/asan_debugging.cc:73
@@ +72,3 @@
+  if (thread) {
+    AsanGetStackVarInfo(addr, descr, thread);
+    return;
----------------
Rename this function to GetInfoForStackVar to make this consistent with GetInfoForAddressIfGlobal and GetInfoForHeapAddress.

================
Comment at: lib/asan/asan_report.cc:282
@@ -270,2 +281,3 @@
     return false;
   static const char kAddrInShadowReport[] =
+      "Address %p is located in the %s area.\n";
----------------
Get rid of this variable - just put this string inside a single Printf call.

================
Comment at: lib/asan/asan_report.cc:286
@@ +285,3 @@
+  if (AddrIsInShadowGap(addr)) area_type = "shadow gap";
+  if (AddrIsInHighShadow(addr)) area_type = "high shadow";
+  if (AddrIsInLowShadow(addr)) area_type = "low shadow";
----------------
else if

================
Comment at: lib/asan/asan_report.cc:287
@@ +286,3 @@
+  if (AddrIsInHighShadow(addr)) area_type = "high shadow";
+  if (AddrIsInLowShadow(addr)) area_type = "low shadow";
+  if (area_type != nullptr) {
----------------
else if

================
Comment at: lib/asan/asan_report.cc:956
@@ +955,3 @@
+
+  report_happened = true;
+  report_data = { pc, sp, bp, addr, (bool)is_write, access_size, bug_descr };
----------------
Note that report_happened might be set to true even though report_data is not yet initialized. Have you considered setting this flag inside "ScopedInErrorReport constructor? Yes, you will have no report data for other kinds of error reports, but it still seems like the right place for it.

You can also pass ReportData into ScopedInErrorReport constructor, to make sure you initialize global "report_data" exactly once, under a lock.

http://reviews.llvm.org/D4527