[PATCH] Interceptors etc for standalone LSan.

Sergey Matveev earthdok at google.com
Tue Apr 30 06:21:11 PDT 2013



================
Comment at: lib/lsan/lsan_allocator.h:25
@@ +24,3 @@
+struct ChunkMetadata {
+  bool allocated : 8; // Must be first.
+  uptr tag : 2;
----------------
Kostya Serebryany wrote:
> bool is always 8 bits. maybe make it u8? 
> (also, make sure to run lint)
ok.

I did run lint, is there an issue here?

================
Comment at: lib/lsan/lsan_allocator.cc:57
@@ +56,3 @@
+  m->requested_size = size;
+  atomic_store((atomic_uint8_t*)m, 1, memory_order_relaxed);
+}
----------------
Kostya Serebryany wrote:
> Why not &m->allocated ? 
> (Then m->allocated does not *have* to be first)
Makes sense. I adopted this idiom from ASan, where chunk_state is a 8-bit u32 bit field member which is accessed in this fashion. Is there a reason why it's done that way there?

================
Comment at: lib/lsan/lsan_interceptors.cc:231
@@ +230,3 @@
+
+// TODO(smatveev): find a home for this
+void Init() {
----------------
Kostya Serebryany wrote:
> resolve TODO (lsan.cc ?)
Currently lsan.cc is where the shared functionality resides. I think eventually it'll move into sanitizer_common, and this can go into lsan.cc. But I was planning to do that in another CL.

================
Comment at: lib/lsan/lsan_stack.cc:46
@@ +45,3 @@
+void PrintStackTrace(const uptr *trace, uptr size, bool symbolize) {
+  static char current_path[256];
+  static bool getcwd_attempted;
----------------
Kostya Serebryany wrote:
> static??? Please don't
Why not? It doesn't change between calls to this function. Is there really a need to call getcwd for every stack trace?

================
Comment at: lib/lsan/lsan_allocator.h:20
@@ +19,3 @@
+#include "sanitizer_common/sanitizer_internal_defs.h"
+#include "lsan.h"
+
----------------
Kostya Serebryany wrote:
> this is not committed yet, right? 
No. In my defense, there's no significant dependency here. We only depend on it for two reasons:
- because we need to declare some templated ForEachChunk versions here (I could move that into the next CL).
- because we read some options like stack_trace_size from lsan's flags. All of those options should be factored out into some kind of common sanitizer flags. (We have discussed this before but there was no definite plan.)


http://llvm-reviews.chandlerc.com/D702



More information about the llvm-commits mailing list