[compiler-rt] Fix alloca_instruments_all_paddings.cc test to work under higher -O levels

[kubamracek]

Summary

Based on the failure from http://reviews.llvm.org/D7741. In alloca_instruments_all_paddings.cc we’re using a dynamic alloca, which is being instrumented, but under higher -O levels all the other allocas are being optimized out. So we end up with only a single dynamic alloca to instrument, and zero static allocas. In FunctionStackPoisoner::poisonStack we then have:

assert(AllocaVec.size() > 0 || DynamicAllocaVec.size() > 0);

if (ClInstrumentAllocas)
  // Handle dynamic allocas.
  for (auto &AllocaCall : DynamicAllocaVec)
    handleDynamicAllocaCall(AllocaCall);

if (AllocaVec.size() == 0) return;
…rest of the function

So we skip the “rest of the function” when there are no static allocas (even when we do have dynamic allocas). However, the “rest of the function” is important, as it ensures that the stack is un-poisoned before return. At the very end of the function we have:

if (ClInstrumentAllocas)
  // Unpoison dynamic allocas.
  for (auto &AllocaCall : DynamicAllocaVec)
    unpoisonDynamicAlloca(AllocaCall);

which doesn’t get executed in this case, but it should. This fix just includes this code when we return early from the AllocaVec.size() check in the beginning of the function.

[samsonov]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

1650–1652

Is it wrong to just call

unpoisonDynamicAlloca(AllocaCall);

here, and leave the rest of the function the same?

[kubamracek]

Updating patch.

[samsonov]

LGTM

lib/Transforms/Instrumentation/AddressSanitizer.cpp

1647

Please add {} for this if as well.

[kubamracek]

Landed in r230316 (llvm) and r230317 (compiler-rt).

lib/Transforms/Instrumentation/AddressSanitizer.cpp

Value *FunctionStackPoisoner::createAllocaForLayout(
   size_t FrameAlignment = std::max(L.FrameAlignment, (size_t)ClRealignStack);
   Alloca->setAlignment(FrameAlignment);
   return IRB.CreatePointerCast(Alloca, IntptrTy);
 }
 
 void FunctionStackPoisoner::poisonStack() {
   assert(AllocaVec.size() > 0 || DynamicAllocaVec.size() > 0);
 
   if (ClInstrumentAllocas)
     // Handle dynamic allocas.
-    for (auto &AllocaCall : DynamicAllocaVec)
+    for (auto &AllocaCall : DynamicAllocaVec) {
       handleDynamicAllocaCall(AllocaCall);
+      unpoisonDynamicAlloca(AllocaCall);
+    }
 
   if (AllocaVec.size() == 0) return;
 
   int StackMallocIdx = -1;
   DebugLoc EntryDebugLocation = getFunctionEntryDebugLocation(F);
 
   Instruction *InsBefore = AllocaVec[0];
   IRBuilder<> IRB(InsBefore);
if (DoStackMalloc) {
       // If we poisoned some allocas in llvm.lifetime analysis,
       // unpoison whole stack frame now.
       poisonAlloca(LocalStackBase, LocalStackSize, IRBRet, false);
     } else {
       poisonRedZones(L.ShadowBytes, IRBRet, ShadowBase, false);
     }
   }
 
-  if (ClInstrumentAllocas)
-    // Unpoison dynamic allocas.
-    for (auto &AllocaCall : DynamicAllocaVec)
-      unpoisonDynamicAlloca(AllocaCall);
-
   // We are done. Remove the old unused alloca instructions.
   for (auto AI : AllocaVec)
     AI->eraseFromParent();
 }
 
 void FunctionStackPoisoner::poisonAlloca(Value *V, uint64_t Size,
                                          IRBuilder<> &IRB, bool DoPoison) {
   // For now just insert the call to ASan runtime.

projects/compiler-rt/test/asan/TestCases/alloca_instruments_all_paddings.cc

 // RUN: %clangxx_asan -O0 -mllvm -asan-instrument-allocas %s -o %t
+// RUN: %clangxx_asan -O3 -mllvm -asan-instrument-allocas %s -o %t
 // RUN: %run %t 2>&1
 //
 
 #include "sanitizer/asan_interface.h"
 #include <assert.h>
 
 __attribute__((noinline)) void foo(int index, int len) {
   volatile char str[len] __attribute__((aligned(32)));