[PATCH] [sanitizer] Implement TwoLevelByteMap and use it for the internal allocator on 64-bit.

Dmitry Vyukov dvyukov at google.com
Sun Nov 24 22:34:00 PST 2013


  atomics - LGTM


================
Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:838
@@ +837,3 @@
+  m.TestOnlyInit();
+  TestMapUnmapCallback::map_count = 0;
+  TestMapUnmapCallback::unmap_count = 0;
----------------
as far as I see, you have a data race on these counters

================
Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:848
@@ +847,3 @@
+    EXPECT_EQ(
+        0, pthread_create(&t[i], 0, TwoLevelByteMapUserThread, (void *)&p[i]));
+  }
----------------
remove (void*)

================
Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:854
@@ +853,3 @@
+  m.TestOnlyUnmap();
+  EXPECT_EQ((uptr)TestMapUnmapCallback::map_count, m.size1());
+  EXPECT_EQ((uptr)TestMapUnmapCallback::unmap_count, m.size1());
----------------
I would check (map_count == m.size1() && unmap_count == 0) before TestOnlyUnmap()
and (map_count == m.size1() && unmap_count ==  m.size1()) afterwards
it makes it clear that unmaps happen only in tests

================
Comment at: lib/sanitizer_common/sanitizer_allocator.h:615
@@ +614,3 @@
+
+  void set(uptr idx, u8 val) {
+    CHECK_LT(idx, kSize1 * kSize2);
----------------
s/set/Set/

================
Comment at: lib/sanitizer_common/sanitizer_allocator.h:622
@@ +621,3 @@
+
+  u8 operator[] (uptr idx) {
+    CHECK_LT(idx, kSize1 * kSize2);
----------------
this can be const method


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

BRANCH
  svn

ARCANIST PROJECT
  compiler-rt



More information about the llvm-commits mailing list