[PATCH] D104896: [DFSan] Change shadow and origin memory layouts to match MSan.

[stephan.yichao.zhao via Phabricator via llvm-commits]

stephan.yichao.zhao added a comment.

Thank you for making this work!



================
Comment at: compiler-rt/lib/dfsan/dfsan.cpp:871
+
+static void CheckMemoryLayoutSanity() {
+  uptr prev_end = 0;
----------------
Please add a comment about that these CheckMemoryLayoutSanity, ... and InitShadow are possible to be shared with MSan (like the TODO in dfsan_platform.h), by moving them and those platform mapping definitions/macros compiler-rt/lib/msan/msan.h to sanitizer_common because it is highly likely that MSan and DFSan always have the same layouts...
Since the change does not branch from MSan's code, w/o comments, it is not easy for others to know they are similar.


================
Comment at: compiler-rt/lib/dfsan/dfsan.cpp:902
+
+static bool CheckMemoryRangeAvailability(uptr beg, uptr size) {
+  if (size > 0) {
----------------
I suggest moving CheckMemoryRangeAvailability and ProtectMemoryRange to sanitizer_common (if this does not break any sanitizer convention).
They are checking some general things and also some corner cases like "protect address 0...".
If shared, it is more likely that others can help DFSan to improve them.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:257
+// x86_64 Linux
+static const MemoryMapParams Linux_X86_64_MemoryMapParams = {
+    0,              // AndMask (not used)
----------------
Does this suggest LinuxX8664MemoryMapParams? Not sure if there is a workaround to suppress this warning.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:460
 
+  /// Memory map parameters used in application-to-shadow calculation.
+  const MemoryMapParams *MapParams;
----------------
in mapping application to shadow and origin....


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104896