[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