[PATCH] Interceptors etc for standalone LSan.
Kostya Serebryany
kcc at google.com
Tue Apr 30 07:20:36 PDT 2013
This CL has a few quite independent parts. Any reason not to split it?
================
Comment at: lib/lsan/lsan_allocator.h:25
@@ +24,3 @@
+struct ChunkMetadata {
+ bool allocated : 8; // Must be first.
+ uptr tag : 2;
----------------
Sergey Matveev wrote:
> 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?
lint usually doesn't like one space before //
This is what made me suspect that you did not run it
================
Comment at: lib/lsan/lsan_allocator.cc:57
@@ +56,3 @@
+ m->requested_size = size;
+ atomic_store((atomic_uint8_t*)m, 1, memory_order_relaxed);
+}
----------------
Sergey Matveev wrote:
> 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?
chunks state is u32 bitfield is because the next field has to be 24 bytes.
If you need to pack things tightly here -- sure, use it.
But your ChunkMetadata is rather sparse now.
If you want to pack it (you do, probaaly) I'd make it like this:
uptr allocated: 8; // Must be first.
uptr tag : 2;
uptr requested_size : 54;
u32 stack_trace_id;
Then of course use atomic_store((atomic_uint8_t*)m, 1, memory_order_relaxed)
================
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;
----------------
Sergey Matveev wrote:
> 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?
will this be locked externally?
Also, I think we should not strip CWD, instead we should do the same thing as we do in asan (flag)
================
Comment at: lib/lsan/lsan_thread.cc:18
@@ +17,3 @@
+#include <asm/prctl.h>
+#include <stddef.h>
+
----------------
We don't include system headers in system-independent files.
So, please don't
http://llvm-reviews.chandlerc.com/D702
More information about the llvm-commits
mailing list