Allow user to override shadow scale in compiler_rt by passing
-DCOMPILER_RT_ASAN_SHADOW_SCALE=n to CMake. Propagate the override
shadow scale value via a compiler define to compiler-rt and asan
tests. Tests will use the define to partially disable unsupported
tests. Set "-mllvm -asan-mapping-scale=<n>" for compiler_rt tests.
Details
Diff Detail
- Build Status
Buildable 11932 Build 11932: arc lint + arc unit
Event Timeline
See http://lists.llvm.org/pipermail/llvm-dev/2017-October/118679.html for background. The end-goal for me is to port ASan to an embedding platform that has no virtual memory and limited memory, where reserving 12.5% of the memory for shadow memory is too high. Getting this to work for x86 is only a small amount of extra work and gives me a way to test/validate the changes in the community.
I think LLVM_ASAN_SHADOW_SCALE is not best approach.
Seems unusual that compiler which can generate code for multiple platforms needs to be reconfigure and rebuild.
@kcc already suggested using regular compile flag: http://lists.llvm.org/pipermail/llvm-dev/2017-October/118685.html
So clang will be able to generate both versions of instrumented code. Obviously we need to have multiple version of run-time so driver will pick proper version based on -fsanitize-address-granularity value. (or somehow we can init the same runtime correctly, not sure yet if it's possible)
Just talked with @eugenis
Maybe simpler approach is following:
- we have llvm flag in AddressSanitizer.cpp which deppends on platform, 8 or 32
- runtime also will pick 8 or 32 from platfor
- if you'd like to setup build bot on e.g. x86 just to test 32, than you call add override flag like this. but it should be in compiler-rt only: e.g. COMPILER_RT_ASAN_SHADOW_GRANULARITY
This flag can switch granularity of runtime and also add -mllvm -asan-shadow-granularity=32 for tests
That sounds good; I will work on it. One followup question: the current flag is called -asan-shadow-scale. Is that ok or should I change that to -asan-shadow-granularity? I don't know if there are any users who depend on the existing name.
Correction: current flag is called -asan-mapping-scale; there is also an -asan-mapping-offset flag for changing the shadow offset.
I'd keep it as is --asan-mapping-scale. I noticed that you are using this in other changes, but didn't realize that it's already there.
--asan-maping-scale is an -mmlv flag.
Such flags should not be given to users -- they are internal testing only flags.
All this #ifdef stuff really sucks. I don't claim I immediately see a better way to do this, but pleeeeeease, try to avoid such ifdefs when possible.
Users do not need this flag. Platform will control granularity. So this flag is going to be used by bots (llvm and compiler-rt tests), when we enforce granularity unusual for host platform.
compiler-rt/CMakeLists.txt | ||
---|---|---|
51 ↗ | (On Diff #122458) | if (COMPILER_RT_ASAN_SHADOW_SCALE GREATER 8 OR COMPILER_RT_ASAN_SHADOW_SCALE LESS 0) |
58 ↗ | (On Diff #122458) | I'd expect following should work: |
compiler-rt/lib/asan/asan_mapping.h | ||
135 | I assume that default for your platform will be changed in separate CL? | |
compiler-rt/test/lit.common.configured.in | ||
28 ↗ | (On Diff #122458) | can you just just use target_cflags and COMPILER_RT_TEST_COMPILER_CFLAGS |
compiler-rt/CMakeLists.txt | ||
---|---|---|
51 ↗ | (On Diff #122458) | Note, however, that this will also accept non-numbers like "foo". |
58 ↗ | (On Diff #122458) | You are right. I'll make the change. |
compiler-rt/lib/asan/asan_mapping.h | ||
135 | Yes -- down the road when I upstream my port, which doesn't exist yet. | |
compiler-rt/test/lit.common.configured.in | ||
28 ↗ | (On Diff #122458) | 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. |
compiler-rt/test/lit.common.configured.in | ||
---|---|---|
28 ↗ | (On Diff #122458) | They append content of COMPILER_RT_TEST_COMPILER_CFLAGS, it's in get_test_cc_for_arch implementation |
compiler-rt/test/lit.common.configured.in | ||
---|---|---|
28 ↗ | (On Diff #122458) | Sorry if I'm missing something, but it looks to me that in get_test_cc_for_arch, COMPILER_RT_TEST_COMPILER_CFLAGS is only used for ANDROID, arm, and aarch64. For others, the target CFLAGS comes from TARGET_${arch}_CFLAGS which is constructed by test_targets(), but I am not seeing COMPILER_RT_TEST_COMPILER_CFLAGS referenced in there. This is also supported by experimentation. |
I partially addressed this issue by setting config.target_flags in compiler-rt/test/lit.common.cfg. This still requires config.asan_shadow_scale to be set but we need that to set a feature anyways (https://reviews.llvm.org/D39772).
I assume that default for your platform will be changed in separate CL?