[PATCH] D23354: [compiler-rt] Suport dynamic shadow address instrumentation

[Etienne Bergeron via llvm-commits]

etienneb added a comment.

FYI: This should be landed with the patch I'm completing (compiler-rt changes to expose the shadow address).


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1770
@@ +1769,3 @@
+  IRBuilder<> IRB(&F.front().front());
+  LocalDynamicShadow = IRB.CreateAlloca(IntptrTy);
+  assert(LocalDynamicShadow->isStaticAlloca());
----------------
rnk wrote:
> Why create a local alloca and then load from it? This might end up being really slow in -O0 if we reload the shadow base from the stack before every user memory access. If we know we're in the entry block, we should be able to skip the alloca and change this to:
>   Value *GlobalDynamicAddress = F.getParent()->getOrInsertGlobal(
>       kAsanShadowMemoryDynamicAddress, IntptrTy);
>   LocalDynamicShadow = IRB.CreateLoad(GlobalDynamicAddress);
> 
> We can then skip the extra load in memToShadow and use LocalDynamicShadow directly.
Turns out that it's more efficient to put it on the stack for -O1 and -O2.
The alloca-slot is promoted to register for the scope of the function.
This is more efficient than having a load from memory for every instrumented load/store.

Even for /O0, the code should be better.
For every instrumented load/store, the code is slighty better (loading from the stack instead of global address).
The only added cost is the required instructions to copy the shadow address on the stack.


https://reviews.llvm.org/D23354