[PATCH] D83247: [compiler-rt][asan][hwasan] Refactor shadow setup into sanitizer_common (NFCI)

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 13 20:59:58 PDT 2020


tejohnson marked 2 inline comments as done.
tejohnson added a comment.

I have addressed the other comments but not yet moved to any new files (e.g. sanitizer_common/sanitizer_shadow_* as suggested). Will do that as a follow up change so it is easier to see the actual code change diffs between this version and the last. Did you want me to just move the stuff from sanitizer_common.[h,cpp] to new sanitzer_shadow_common.* files, or also out of e.g. sanitizer_win.cpp to sanitizer_shadow_win.cpp, etc?

Also, I did manually force the new functions in sanitizer_win.cpp and sanitizer_mac.cpp to compile to ensure that I fixed all the references, but that's the extent of what I was able to test there so far.



================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp:1092
+    RestrictMemoryToMaxAddress(new_max_vm);
+    kHighMemEnd = new_max_vm - 1;
+    space_size = kHighShadowEnd + left_padding;
----------------
vitalybuka wrote:
> kHighShadowEnd and kHighMemEnd are undefined here
Where it was using kHighShadowEnd we can just recompute from the new high mem (since at this point the offset was 0 so it is just doing the shift). For kHighMemEnd, it is more difficult since this code is updating the value. I didn't find a way around this besides passing in a reference and updating it if needed.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_win.cpp:357
+  uptr left_padding = granularity;
+  uptr space_size = kHighShadowEnd + left_padding;
+  uptr shadow_start = FindAvailableMemoryRange(space_size, alignment,
----------------
vitalybuka wrote:
> kHighShadowEnd is asan constant
> probably space_size should be calculated from shadow_size_bytes
> same for mac version
Yep, fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83247





More information about the llvm-commits mailing list