[PATCH] [compiler-rt] Add ASan debugging API to get malloc/free stack traces and shadow memory mapping info

[Alexander Potapenko]

Looks mostly good.

================
Comment at: lib/asan/asan_debugging.cc:60
@@ +59,3 @@
+uptr __asan_get_alloc_stack(uptr addr, uptr *trace, uptr size, u32 *thread_id) {
+  return AsanGetStack(addr, trace, size, thread_id, true);
+}
----------------
Can you please write parameter names before constants, e.g.:

  AsanGetStack(addr, trace, size, thread_id, /*alloc_stack*/ true);

?

================
Comment at: lib/asan/asan_debugging.cc:65
@@ +64,3 @@
+uptr __asan_get_free_stack(uptr addr, uptr *trace, uptr size, u32 *thread_id) {
+  return AsanGetStack(addr, trace, size, thread_id, false);
+}
----------------
ditto

================
Comment at: lib/asan/tests/asan_debugging_noinst_test.cc:14
@@ +13,3 @@
+//===----------------------------------------------------------------------===//
+#include "asan_test_utils.h"
+#include "asan_mapping.h"
----------------
Please sort the includes alphabetically.

================
Comment at: lib/asan/tests/asan_debugging_noinst_test.cc:20
@@ +19,3 @@
+  size_t scale, offset;
+  __asan_get_shadow_mapping(&scale, &offset);
+
----------------
Please move the test for __asan_get_shadow_mapping() to a separate lit test.
The _noinst tests are intended to test only the ASan internals, not the public interfaces, and your test may end up checking something completely different.

================
Comment at: test/asan/TestCases/debug_stacks.cc:5
@@ +4,3 @@
+
+#include <stdlib.h>
+#include <stdio.h>
----------------
Please keep the includes sorted.

================
Comment at: test/asan/TestCases/debug_stacks.cc:28
@@ +27,3 @@
+  fprintf(stderr, "alloc stack retval %s\n", (num_frames > 0 && num_frames < 10)
+          ? "ok" : ""); // CHECK: alloc stack retval ok
+  fprintf(stderr, "thread id = %d\n", thread_id); // CHECK: thread id = 0
----------------
Please add a second space before "//" here and below.

================
Comment at: test/asan/TestCases/debug_stacks.cc:38
@@ +37,3 @@
+          ? "ok" : ""); // CHECK: free stack retval ok
+  fprintf(stderr, "free stack retval = 1\n"); // CHECK: free stack retval = 1
+  fprintf(stderr, "thread id = %d\n", thread_id); // CHECK: thread id = 0
----------------
What's the point in checking for a constant string you have just printed?

http://reviews.llvm.org/D4466