[msan] Change the way origin ids are built.

Previously, all origin ids were "chained" origins, i.e values of
ChainedOriginDepot. This added a level of indirection for simple
stack and heap allocation, which were represented as chains of
length 1. This costs both RAM and CPU, but provides a joined 2**29
origin id space. It also made function (any instrumented function)
entry non-async-signal-safe, but that does not really matter because
memory stores in track-origins=2 mode are not async-signal-safe anyway.

With this change, the type of the origin is encoded in origin id.
See comment in msan_origin.h for more details. This reduces chained and stack
origin id range to 2**28 each, but leaves extra 2**31 for heap origins.

This change should not have any user-visible effects.

llvm-svn: 223233

compiler-rt/lib/msan/msan.cc

@@ -205,10 +205,10 @@ void PrintWarningWithOrigin(uptr pc, uptr bp, u32 origin) {
   GET_FATAL_STACK_TRACE_PC_BP(pc, bp);
 
   u32 report_origin =
-    (__msan_get_track_origins() && Origin(origin).isValid()) ? origin : 0;
+    (__msan_get_track_origins() && Origin::isValidId(origin)) ? origin : 0;
   ReportUMR(&stack, report_origin);
 
-  if (__msan_get_track_origins() && !Origin(origin).isValid()) {
+  if (__msan_get_track_origins() && !Origin::isValidId(origin)) {
     Printf(
         "  ORIGIN: invalid (%x). Might be a bug in MemorySanitizer origin "
         "tracking.\n    This could still be a bug in your code, too!\n",
@@ -258,32 +258,9 @@ u32 ChainOrigin(u32 id, StackTrace *stack) {
   if (t && t->InSignalHandler())
     return id;
 
-  Origin o(id);
-  int depth = o.depth();
-  // 0 means unlimited depth.
-  if (flags()->origin_history_size > 0 && depth > 0) {
-    if (depth >= flags()->origin_history_size) {
-      return id;
-    } else {
-      ++depth;
-    }
-  }
-
-  StackDepotHandle h = StackDepotPut_WithHandle(*stack);
-  if (!h.valid()) return id;
-
-  if (flags()->origin_history_per_stack_limit > 0) {
-    int use_count = h.use_count();
-    if (use_count > flags()->origin_history_per_stack_limit) return id;
-  }
-
-  u32 chained_id;
-  bool inserted = ChainedOriginDepotPut(h.id(), o.id(), &chained_id);
-
-  if (inserted && flags()->origin_history_per_stack_limit > 0)
-    h.inc_use_count_unsafe();
-
-  return Origin(chained_id, depth).raw_id();
+  Origin o = Origin::FromRawId(id);
+  Origin chained = Origin::CreateChainedOrigin(o, stack);
+  return chained.raw_id();
 }
 
 }  // namespace __msan
@@ -542,14 +519,14 @@ void __msan_set_alloca_origin4(void *a, uptr size, char *descr, uptr pc) {
     CHECK_LT(idx, kNumStackOriginDescrs);
     StackOriginDescr[idx] = descr + 4;
     StackOriginPC[idx] = pc;
-    ChainedOriginDepotPut(idx, Origin::kStackRoot, &id);
+    id = Origin::CreateStackOrigin(idx).raw_id();
     *id_ptr = id;
     if (print)
       Printf("First time: idx=%d id=%d %s %p \n", idx, id, descr + 4, pc);
   }
   if (print)
     Printf("__msan_set_alloca_origin: descr=%s id=%x\n", descr + 4, id);
-  __msan_set_origin(a, size, Origin(id, 1).raw_id());
+  __msan_set_origin(a, size, id);
 }
 
 u32 __msan_chain_origin(u32 id) {

compiler-rt/lib/msan/msan_allocator.cc

@@ -14,10 +14,8 @@
 
 #include "sanitizer_common/sanitizer_allocator.h"
 #include "sanitizer_common/sanitizer_allocator_interface.h"
-#include "sanitizer_common/sanitizer_stackdepot.h"
 #include "msan.h"
 #include "msan_allocator.h"
-#include "msan_chained_origin_depot.h"
 #include "msan_origin.h"
 #include "msan_thread.h"
 
@@ -114,11 +112,8 @@ static void *MsanAllocate(StackTrace *stack, uptr size, uptr alignment,
   } else if (flags()->poison_in_malloc) {
     __msan_poison(allocated, size);
     if (__msan_get_track_origins()) {
-      u32 stack_id = StackDepotPut(*stack);
-      CHECK(stack_id);
-      u32 id;
-      ChainedOriginDepotPut(stack_id, Origin::kHeapRoot, &id);
-      __msan_set_origin(allocated, size, Origin(id, 1).raw_id());
+      Origin o = Origin::CreateHeapOrigin(stack);
+      __msan_set_origin(allocated, size, o.raw_id());
     }
   }
   MSAN_MALLOC_HOOK(allocated, size);
@@ -137,11 +132,8 @@ void MsanDeallocate(StackTrace *stack, void *p) {
   if (flags()->poison_in_free) {
     __msan_poison(p, size);
     if (__msan_get_track_origins()) {
-      u32 stack_id = StackDepotPut(*stack);
-      CHECK(stack_id);
-      u32 id;
-      ChainedOriginDepotPut(stack_id, Origin::kHeapRoot, &id);
-      __msan_set_origin(p, size, Origin(id, 1).raw_id());
+      Origin o = Origin::CreateHeapOrigin(stack);
+      __msan_set_origin(p, size, o.raw_id());
     }
   }
   MsanThread *t = GetCurrentThread();

compiler-rt/lib/msan/msan_chained_origin_depot.cc

@@ -94,8 +94,7 @@ struct ChainedOriginDepotNode {
   typedef Handle handle_type;
 };
 
-// kTabSizeLog = 22 => 32Mb static storage for bucket pointers.
-static StackDepotBase<ChainedOriginDepotNode, 3, 20> chainedOriginDepot;
+static StackDepotBase<ChainedOriginDepotNode, 4, 20> chainedOriginDepot;
 
 StackDepotStats *ChainedOriginDepotGetStats() {
   return chainedOriginDepot.GetStats();

compiler-rt/lib/msan/msan_interceptors.cc

@@ -956,10 +956,8 @@ void __msan_allocated_memory(const void* data, uptr size) {
   if (flags()->poison_in_malloc)
     __msan_poison(data, size);
   if (__msan_get_track_origins()) {
-    u32 stack_id = StackDepotPut(stack);
-    u32 id;
-    ChainedOriginDepotPut(stack_id, Origin::kHeapRoot, &id);
-    __msan_set_origin(data, size, Origin(id, 1).raw_id());
+    Origin o = Origin::CreateHeapOrigin(&stack);
+    __msan_set_origin(data, size, o.raw_id());
   }
 }
 

compiler-rt/lib/msan/msan_origin.h

@@ -12,6 +12,9 @@
 #ifndef MSAN_ORIGIN_H
 #define MSAN_ORIGIN_H
 
+#include "sanitizer_common/sanitizer_stackdepot.h"
+#include "msan_chained_origin_depot.h"
+
 namespace __msan {
 
 // Origin handling.
@@ -20,9 +23,22 @@ namespace __msan {
 // the program and describes, more or less exactly, how this memory came to be
 // uninitialized.
 //
-// Origin ids are values of ChainedOriginDepot, which is a mapping of (stack_id,
-// prev_id) -> id, where
-//  * stack_id describes an event in the program, usually a memory store.
+// There are 3 kinds of origin ids:
+// 1xxx xxxx xxxx xxxx   heap origin id
+// 0000 xxxx xxxx xxxx   stack origin id
+// 0zzz xxxx xxxx xxxx   chained origin id
+//
+// Heap origin id describes a heap memory allocation and contains (in the xxx
+// part) a value of StackDepot.
+//
+// Stack origin id describes a stack memory allocation and contains (in the xxx
+// part) an index into StackOriginDescr and StackOriginPC. We don't store a
+// stack trace for such origins for performance reasons.
+//
+// Chained origin id describes an event of storing an uninitialized value to
+// memory. The xxx part is a value of ChainedOriginDepot, which is a mapping of
+// (stack_id, prev_id) -> id, where
+//  * stack_id describes the event.
 //    StackDepot keeps a mapping between those and corresponding stack traces.
 //  * prev_id is another origin id that describes the earlier part of the
 //    uninitialized value history.
@@ -33,43 +49,119 @@ namespace __msan {
 // points in value history marked with origin ids, and edges are events that are
 // marked with stack_id.
 //
-// There are 2 special root origin ids:
-// * kHeapRoot - an origin with prev_id == kHeapRoot describes an event of
-//   allocating memory from heap.
-// * kStackRoot - an origin with prev_id == kStackRoot describes an event of
-//   allocating memory from stack (i.e. on function entry).
-// Note that ChainedOriginDepot does not store any node for kHeapRoot or
-// kStackRoot. These are just special id values.
-//
-// Three highest bits of origin id are used to store the length (or depth) of
-// the origin chain. Special depth value of 0 means unlimited.
+// The "zzz" bits of chained origin id are used to store the length (or depth)
+// of the origin chain.
 
 class Origin {
  public:
-  static const int kDepthBits = 3;
-  static const int kDepthShift = 32 - kDepthBits;
-  static const u32 kIdMask = ((u32)-1) >> (32 - kDepthShift);
-  static const u32 kDepthMask = ~kIdMask;
+  static bool isValidId(u32 id) { return id != 0 && id != (u32)-1; }
 
-  static const int kMaxDepth = (1 << kDepthBits) - 1;
-
-  static const u32 kHeapRoot = (u32)-1;
-  static const u32 kStackRoot = (u32)-2;
-
-  explicit Origin(u32 raw_id) : raw_id_(raw_id) {}
-  Origin(u32 id, u32 depth) : raw_id_((depth << kDepthShift) | id) {
-    CHECK_EQ(this->depth(), depth);
-    CHECK_EQ(this->id(), id);
-  }
-  int depth() const { return raw_id_ >> kDepthShift; }
-  u32 id() const { return raw_id_ & kIdMask; }
   u32 raw_id() const { return raw_id_; }
-  bool isStackRoot() const { return raw_id_ == kStackRoot; }
-  bool isHeapRoot() const { return raw_id_ == kHeapRoot; }
-  bool isValid() const { return raw_id_ != 0 && raw_id_ != (u32)-1; }
+  bool isHeapOrigin() const {
+    // 1xxx xxxx xxxx xxxx
+    return raw_id_ >> kHeapShift == 0;
+  }
+  bool isStackOrigin() const {
+    // 1000 xxxx xxxx xxxx
+    return (raw_id_ >> kDepthShift) == (1 << kDepthBits);
+  }
+  bool isChainedOrigin() const {
+    // 1zzz xxxx xxxx xxxx, zzz != 000
+    return (raw_id_ >> kDepthShift) > (1 << kDepthBits);
+  }
+  u32 getChainedId() const {
+    CHECK(isChainedOrigin());
+    return raw_id_ & kChainedIdMask;
+  }
+  u32 getStackId() const {
+    CHECK(isStackOrigin());
+    return raw_id_ & kChainedIdMask;
+  }
+  u32 getHeapId() const {
+    CHECK(isHeapOrigin());
+    return raw_id_ & kHeapIdMask;
+  }
+
+  // Returns the next origin in the chain and the current stack trace.
+  Origin getNextChainedOrigin(StackTrace *stack) const {
+    CHECK(isChainedOrigin());
+    u32 prev_id;
+    u32 stack_id = ChainedOriginDepotGet(getChainedId(), &prev_id);
+    *stack = StackDepotGet(stack_id);
+    return Origin(prev_id);
+  }
+
+  StackTrace getStackTraceForHeapOrigin() const {
+    return StackDepotGet(getHeapId());
+  }
+
+  static Origin CreateStackOrigin(u32 id) {
+    CHECK((id & kStackIdMask) == id);
+    return Origin((1 << kHeapShift) | id);
+  }
+
+  static Origin CreateHeapOrigin(StackTrace *stack) {
+    u32 stack_id = StackDepotPut(*stack);
+    CHECK(stack_id);
+    CHECK((stack_id & kHeapIdMask) == stack_id);
+    return Origin(stack_id);
+  }
+
+  static Origin CreateChainedOrigin(Origin prev, StackTrace *stack) {
+    int depth = prev.isChainedOrigin() ? prev.depth() : 0;
+    // depth is the length of the chain minus 1.
+    // origin_history_size of 0 means unlimited depth.
+    if (flags()->origin_history_size > 0) {
+      if (depth + 1 >= flags()->origin_history_size) {
+        return prev;
+      } else {
+        ++depth;
+        CHECK(depth < (1 << kDepthBits));
+      }
+    }
+
+    StackDepotHandle h = StackDepotPut_WithHandle(*stack);
+    if (!h.valid()) return prev;
+
+    if (flags()->origin_history_per_stack_limit > 0) {
+      int use_count = h.use_count();
+      if (use_count > flags()->origin_history_per_stack_limit) return prev;
+    }
+
+    u32 chained_id;
+    bool inserted = ChainedOriginDepotPut(h.id(), prev.raw_id(), &chained_id);
+    CHECK((chained_id & kChainedIdMask) == chained_id);
+
+    if (inserted && flags()->origin_history_per_stack_limit > 0)
+      h.inc_use_count_unsafe();
+
+    return Origin((1 << kHeapShift) | (depth << kDepthShift) | chained_id);
+  }
+
+  static Origin FromRawId(u32 id) {
+    return Origin(id);
+  }
 
  private:
+  static const int kDepthBits = 3;
+  static const int kDepthShift = 32 - kDepthBits - 1;
+
+  static const int kHeapShift = 31;
+  static const u32 kChainedIdMask = ((u32)-1) >> (32 - kDepthShift);
+  static const u32 kStackIdMask = ((u32)-1) >> (32 - kDepthShift);
+  static const u32 kHeapIdMask = ((u32)-1) >> (32 - kHeapShift);
+
   u32 raw_id_;
+
+  explicit Origin(u32 raw_id) : raw_id_(raw_id) {}
+
+  int depth() const {
+    CHECK(isChainedOrigin());
+    return (raw_id_ >> kDepthShift) & ((1 << kDepthBits) - 1);
+  }
+
+ public:
+  static const int kMaxDepth = (1 << kDepthBits) - 1;
 };
 
 }  // namespace __msan

compiler-rt/lib/msan/msan_report.cc

@@ -61,34 +61,23 @@ static void DescribeStackOrigin(const char *so, uptr pc) {
 static void DescribeOrigin(u32 id) {
   VPrintf(1, "  raw origin id: %d\n", id);
   Decorator d;
-  while (true) {
-    Origin o(id);
-    if (!o.isValid()) {
-      Printf("  %sinvalid origin id(%d)%s\n", d.Warning(), id, d.End());
-      break;
-    }
-    u32 prev_id;
-    u32 stack_id = ChainedOriginDepotGet(o.id(), &prev_id);
-    Origin prev_o(prev_id);
-
-    if (prev_o.isStackRoot()) {
-      uptr pc;
-      const char *so = GetStackOriginDescr(stack_id, &pc);
-      DescribeStackOrigin(so, pc);
-      break;
-    } else if (prev_o.isHeapRoot()) {
-      Printf("  %sUninitialized value was created by a heap allocation%s\n",
-             d.Origin(), d.End());
-      StackDepotGet(stack_id).Print();
-      break;
-    } else {
-      // chained origin
-      // FIXME: copied? modified? passed through? observed?
+  Origin o = Origin::FromRawId(id);
+  while (o.isChainedOrigin()) {
+    StackTrace stack;
+    o = o.getNextChainedOrigin(&stack);
     Printf("  %sUninitialized value was stored to memory at%s\n", d.Origin(),
         d.End());
-      StackDepotGet(stack_id).Print();
-      id = prev_id;
+    stack.Print();
   }
+  if (o.isStackOrigin()) {
+    uptr pc;
+    const char *so = GetStackOriginDescr(o.getStackId(), &pc);
+    DescribeStackOrigin(so, pc);
+  } else {
+    StackTrace stack = o.getStackTraceForHeapOrigin();
+    Printf("  %sUninitialized value was created by a heap allocation%s\n",
+           d.Origin(), d.End());
+    stack.Print();
   }
 }