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

stephan.yichao.zhao via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 25 09:40:51 PDT 2021


stephan.yichao.zhao added inline comments.


================
Comment at: compiler-rt/lib/dfsan/dfsan.cpp:169
+    // TODO(browneee): Removed this after testing and not hit.
+    CHECK(MEM_IS_SHADOW(s));
+    // if (!MEM_IS_SHADOW(s)) {
----------------
Based on the recent issue about using a replaced memmove.
It seems safe to always keep this CHECK and update the comments to reflect this.



================
Comment at: compiler-rt/lib/dfsan/dfsan.cpp:170
+    CHECK(MEM_IS_SHADOW(s));
+    // if (!MEM_IS_SHADOW(s)) {
+    //   // The current DFSan memory layout is not always correct. For example,
----------------
Please remove the commented code.


================
Comment at: compiler-rt/lib/dfsan/dfsan.cpp:177
+
+    if (*s) {
+      uptr aligned_addr = OriginAlignDown(SHADOW_TO_ORIGIN(s));
----------------
This branch seems redundant with the one below.


================
Comment at: compiler-rt/lib/dfsan/dfsan.cpp:332
                        StackTrace *stack) {
-  if (!has_valid_shadow_addr(dst) ||
-      !has_valid_shadow_addr((void *)((uptr)dst + size)) ||
-      !has_valid_shadow_addr(src) ||
-      !has_valid_shadow_addr((void *)((uptr)src + size))) {
+  // TODO(browneee): Removed this after testing and not hit.
+  if (!MEM_IS_SHADOW(shadow_for(dst)) ||
----------------
Please update the comments to be consistent with other MEM_IS_SHADOW checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104896



More information about the cfe-commits mailing list