[PATCH] D57412: [scudo] Initial standalone skeleton check-in

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: lib/scudo/standalone/atomic_helpers.h:53
+  typedef u64 Type;
+  // On 32-bit platforms u64 is not necessary aligned on 8 bytes.
+  volatile ALIGNED(8) Type ValDoNotUse;
----------------
necessarily*


================
Comment at: lib/scudo/standalone/atomic_helpers.h:63
+template <typename T>
+INLINE typename T::Type atomic_load(const volatile T *A, memory_order MO) {
+  DCHECK(!(reinterpret_cast<uptr>(A) % sizeof(*A)));
----------------
Have we  lost the `memory_order` checking that sanitizer_atomic_clang_x86.h does, or is this done in `__atomic_load`?


================
Comment at: lib/scudo/standalone/atomic_helpers.h:98
+  __atomic_exchange(&A->ValDoNotUse, &V, &R, MO);
+  return R;
+}
----------------
Can we simplify to `return __atomic_exchange(&A->ValDoNotUse, &V, MO)` instead?


================
Comment at: lib/scudo/standalone/list.h:148
+
+  // private, do not use directly.
+  uptr Size;
----------------
Any reason not to put a `private:` here?


================
Comment at: lib/scudo/standalone/tests/CMakeLists.txt:36
+        SOURCES ${TEST_SOURCES} ${COMPILER_RT_GTEST_SOURCE} 
+        # RUNTIME ${SCUDO_TEST_RUNTIME}
+        COMPILE_DEPS ${TEST_HEADERS}
----------------
Will this be uncommented eventually?


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D57412