[PATCH] [msan] MSanDR: a simple DynamoRio-based tool that handles uninstrumented libraries and dynamically generated code

Kostya Serebryany kcc at google.com
Wed Feb 20 00:40:10 PST 2013


  I'd ask rnk@ to also review this.
  First question: do we want any STL here? Will STL conflict with DR's native exec?


================
Comment at: lib/msandr/msandr.cc:12
@@ +11,3 @@
+//
+// DynamoRio client for MemorySanitizer.
+//===----------------------------------------------------------------------===//
----------------
This deserves a detailed comment (and/or a link) describing why we needs this.

================
Comment at: lib/msandr/msandr.cc:28
@@ +27,3 @@
+
+using std::string;
+
----------------
Can we avoid using STL?
It may give us lots of trouble with DR's native exec. 

================
Comment at: lib/msandr/msandr.cc:43
@@ +42,3 @@
+
+#if defined(VERBOSE_VERBOSE) && !defined(VERBOSE)
+#define VERBOSE
----------------
Can this be VERBOSITY_LEVEL, which is one of 0, 1, 2 so that you use if () instead of #ifdef  ?

================
Comment at: lib/msandr/msandr.cc:59
@@ +58,3 @@
+  // Full path to the module.
+  string path_;
+  module_handle_t handle_;
----------------
fixed sized array, please. No STL

================
Comment at: lib/msandr/msandr.cc:73
@@ +72,3 @@
+// is faster and the bb event occurs far more than the module load event.
+std::vector<ModuleData> g_module_list;
+
----------------
fixed size array, no STL, please

================
Comment at: lib/msandr/msandr.cc:116
@@ +115,3 @@
+
+#define MEM_TO_SHADOW(mem) ((mem) & ~0x400000000000ULL)
+
----------------
any reason not to make this an inline function? 
(we use macros in similar cases if we need to call them in global initializers)

================
Comment at: lib/msandr/msandr.cc:207
@@ +206,3 @@
+  };
+  std::set<reg_id_t> unused_registers(GPR_TO_USE_FOR_R2, GPR_TO_USE_FOR_R2 + 4);
+  unused_registers.erase(R1);
----------------
grrr. std::set is not that easy to replace with non-STL. 
What's the maximal number of elements here? 

================
Comment at: lib/msandr/msandr.cc:362
@@ +361,3 @@
+
+// TODO(rnk): Make sure we instrument after __asan_init.
+dr_emit_flags_t
----------------
__asan ? 


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



More information about the llvm-commits mailing list