[PATCH] D58184: [scudo][standalone] Introduce platform specific code & mutexes

Vitaly Buka via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 19 18:21:57 PST 2019


vitalybuka added a comment.

also this patch includes to many unrelated changes



================
Comment at: lib/scudo/standalone/internal_defs.h:126
+
+#define COMPILER_CHECK(Pred) static_assert(Pred, "")
+
----------------
Why not to use static_assert directly
Same for INLINE


================
Comment at: lib/scudo/standalone/linux.cc:41
+
+uptr getPageSize() { return static_cast<uptr>(sysconf(_SC_PAGESIZE)); }
+
----------------
sysconf can be intercepted, e.g. in Google internal code


================
Comment at: lib/scudo/standalone/linux.cc:106
+
+// Blocking is possibly unused if the getrandom block is not compiled in.
+bool getRandom(void *Buffer, uptr Length, UNUSED bool Blocking) {
----------------
I assume this comment is for "#if defined"
could you move it there?


================
Comment at: lib/scudo/standalone/linux.cc:118
+      syscall(SYS_getrandom, Buffer, Length, Blocking ? 0 : GRND_NONBLOCK);
+  if (ReadBytes == static_cast<ssize_t>(Length))
+    return true;
----------------
do you need static_cast here?


================
Comment at: lib/scudo/standalone/linux.h:55
+// store sanitizer thread specific data.
+static const int TLS_SLOT_SANITIZER = 8; // TODO(kostyak): 6 for Q!!
+
----------------
please run clang-format


================
Comment at: lib/scudo/standalone/mutex.h:45
+      if (atomic_load_relaxed(&State) == 0 &&
+          atomic_exchange(&State, 1, memory_order_acquire) == 0)
+        return;
----------------
this if does not look atomic
atomic_compare_exchange? 


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58184/new/

https://reviews.llvm.org/D58184





More information about the llvm-commits mailing list