[PATCH] D39469: [asan] Add CMake hook to override shadow scale in compiler_rt

[Walter Lee via Phabricator via llvm-commits]

waltl marked an inline comment as done.
waltl added inline comments.


================
Comment at: compiler-rt/CMakeLists.txt:51
+  # Check that the shadow scale value is valid.
+  if (NOT (COMPILER_RT_ASAN_SHADOW_SCALE GREATER -1 AND
+           COMPILER_RT_ASAN_SHADOW_SCALE LESS 8))
----------------
vitalybuka wrote:
>   if (COMPILER_RT_ASAN_SHADOW_SCALE GREATER 8 OR
>            COMPILER_RT_ASAN_SHADOW_SCALE LESS 0)
Note, however, that this will also accept non-numbers like "foo".



================
Comment at: compiler-rt/CMakeLists.txt:58
+  set(COMPILER_RT_ASAN_SHADOW_SCALE_LLVM_FLAG
+      -mllvm -asan-mapping-scale -mllvm "${COMPILER_RT_ASAN_SHADOW_SCALE}")
+  set(COMPILER_RT_ASAN_SHADOW_SCALE_DEFINITION
----------------
vitalybuka wrote:
> I'd expect following should work: 
> -mllvm -asan-mapping-scale=${COMPILER_RT_ASAN_SHADOW_SCALE}
You are right.  I'll make the change.


================
Comment at: compiler-rt/lib/asan/asan_mapping.h:135
+#if defined(ASAN_SHADOW_SCALE)
+static const u64 kDefaultShadowScale = ASAN_SHADOW_SCALE;
+#else
----------------
vitalybuka wrote:
> I assume that default for your platform will be changed in separate CL?
Yes -- down the road when I upstream my port, which doesn't exist yet.



================
Comment at: compiler-rt/test/lit.common.configured.in:28
 set_default("emulator", "@COMPILER_RT_EMULATOR@")
+set_default("asan_shadow_scale", "@COMPILER_RT_ASAN_SHADOW_SCALE@")
 set_default("ios", False)
----------------
vitalybuka wrote:
> can you just just use target_cflags and COMPILER_RT_TEST_COMPILER_CFLAGS
Maybe?  Currently COMPILER_RT_TEST_COMPILER_CFLAGS doesn't get propagated to target_cflags because individual sanitizers have their own definitions of target_cflags that supercede it.  Is this intended?  If the right thing to do is to append the target_cflags together I can do that and it'd better serve our purpose here.



https://reviews.llvm.org/D39469