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

[Teresa Johnson via Phabricator via llvm-commits]

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

A couple comments and a question below. Also, what is the best way to test this for mac and win? I tried to find some directions for doing some cross-compiler builds of compiler-rt at least, but couldn't find something clear and definitive.



================
Comment at: compiler-rt/lib/asan/asan_linux.cpp:122
-  uptr granularity = GetMmapGranularity();
-  uptr alignment = granularity * 8;
-  uptr left_padding = granularity;
----------------
The code in asan is multiplying the mmap granularity by 8, whereas the hwasan version shifts it by kShadowScale. I wasn't sure if the 8 here is supposed to be equivalent to a left shift by the shadow scale (which is typically 3 in asan), or is specifically hardcoded separately not using SHADOW_SCALE since it could be something other than 3 in some cases (e.g. 5 for myriad, or user set via ASAN_SHADOW_SCALE). Depending on what was intended here, I would keep the hardcoding of "3" passed to my refactored MapDynamicShadow, or change that to SHADOW_SCALE.


================
Comment at: compiler-rt/lib/asan/asan_shadow_setup.cpp:102
   if (shadow_start == kDefaultShadowSentinel) {
-    __asan_shadow_memory_dynamic_address = 0;
-    CHECK_EQ(0, kLowShadowBeg);
----------------
See patch description on why I removed this code.


================
Comment at: compiler-rt/lib/hwasan/hwasan_mapping.h:40
 
+// With the zero shadow base we can not actually map pages starting from 0.
+// This constant is somewhat arbitrary.
----------------
Copied from the same values in asan. See the patch description for more info. Would it be better to keep a single copy of these values in sanitizer_common?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83247