[PATCH] D77283: scudo: Add support for diagnosing memory errors when memory tagging is enabled.
Kostya Kortchinsky via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 3 09:07:54 PDT 2020
cryptoad added a comment.
Some initial comments
================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:35
+#if SCUDO_ANDROID && __ANDROID_API__ == 10000
+// This function is not part of the NDK so it does not appear in any public
----------------
I am not sure how `__ANDROID_API__` works, should it be `>=`? (same for all the others)
================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:237
+ enum { kStackSize = 64 };
+ uptr Stack[kStackSize];
+ uptr Size = android_unsafe_frame_pointer_chase(Stack, kStackSize);
----------------
Style wise, the whole code base avoids the `kConstant` naming.
================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:388
+ // the chunk data.
+ memset(TaggedPtr, 0, 16);
+ }
----------------
Should the `16` be a constant?
================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:751
+
+ auto GetGranule = [&](uptr Addr, const char **Data, uint8_t *Tag) -> bool {
+ if (Addr < MemoryAddr || Addr >= MemoryAddr + MemorySize)
----------------
Not sure if the lambda should abide by the usual style, eg `getGranule`, up to you.
================
Comment at: compiler-rt/lib/scudo/standalone/stack_depot.h:12
+
+#include "mutex.h"
+
----------------
Style on that file went the g3 way rather than the LLVM way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77283/new/
https://reviews.llvm.org/D77283
More information about the llvm-commits
mailing list