[HWASan] remove replacement of DbgVariableIntrinsics.

[fmayer]

Summary

This code was dead because we AI->replaceUsesWithIf above. I verified
this doesn't actually get run by applying
https://gist.github.com/fmayer/aea7cbb4700cfe2c9d932591ae1073c3 to the
Android toolchain and building AOSP, without any crash.

[fmayer]

Oh hold on, maybe what we actually want (to fix this) is to replace NewPtr (the BitCast from alignAndPadAlloca) with Info.AI. Sorry for jumping the gun here, the change as is is correct in that it keeps current behaviour, but that is possibly not what we want.

[fmayer]

In D119802#3321598, @fmayer wrote:

Oh hold on, maybe what we actually want (to fix this) is to replace NewPtr (the BitCast from alignAndPadAlloca) with Info.AI. Sorry for jumping the gun here, the change as is is correct in that it keeps current behaviour, but that is possibly not what we want.

Nevermind, no. We actually want the cast, because otherwise we cast to the (wrong) type of the padded alloca; or at least that's what the test in asserting. Correct me if that's wrong.

[eugenis]

In D119802#3321609, @fmayer wrote:

In D119802#3321598, @fmayer wrote:

Oh hold on, maybe what we actually want (to fix this) is to replace NewPtr (the BitCast from alignAndPadAlloca) with Info.AI. Sorry for jumping the gun here, the change as is is correct in that it keeps current behaviour, but that is possibly not what we want.

Nevermind, no. We actually want the cast, because otherwise we cast to the (wrong) type of the padded alloca; or at least that's what the test in asserting. Correct me if that's wrong.

Which test?

[fmayer]

In D119802#3323886, @eugenis wrote:

In D119802#3321609, @fmayer wrote:

In D119802#3321598, @fmayer wrote:

Oh hold on, maybe what we actually want (to fix this) is to replace NewPtr (the BitCast from alignAndPadAlloca) with Info.AI. Sorry for jumping the gun here, the change as is is correct in that it keeps current behaviour, but that is possibly not what we want.

Nevermind, no. We actually want the cast, because otherwise we cast to the (wrong) type of the padded alloca; or at least that's what the test in asserting. Correct me if that's wrong.

Which test?

llvm/test/Instrumentation/HWAddressSanitizer/dbg-value-tag-offset.ll

[eugenis]

LGTM

[fmayer]

Correcting the commit message (sorry): This never got run, because in alignAndPadAlloca we call replaceAllUsesWith, which includes all metadata uses. This means we would never find a LocationOp with AI as value. This was also true before the refactoring to use a shared alignAndPadAlloca, because the loop only got run if !AllocaToPaddedAllocaMap.empty(), which only got populated in the padding code.

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

if (ShouldDetectUseAfterScope && StandardLifetime) {
         TagEnd(RI);
       if (!StandardLifetime) {
         for (auto &II : Info.LifetimeStart)
           II->eraseFromParent();
         for (auto &II : Info.LifetimeEnd)
           II->eraseFromParent();
       }
     }
-    if (memtag::alignAndPadAlloca(Info, Align(Mapping.getObjectAlignment()))) {
-      for (auto DVI : Info.DbgVariableIntrinsics) {
-        SmallDenseSet<Value *> LocationOps(DVI->location_ops().begin(),
-                                           DVI->location_ops().end());
-        for (Value *V : LocationOps) {
-          if (auto *AI = dyn_cast_or_null<AllocaInst>(V)) {
-            if (V == AI)
-              DVI->replaceVariableLocationOp(V, Info.AI);
-          }
-        }
-      }
+    if (memtag::alignAndPadAlloca(Info, Align(Mapping.getObjectAlignment())))
       AI->eraseFromParent();
-    }
-  }
   for (auto &I : SInfo.UnrecognizedLifetimes)
     I->eraseFromParent();
   return true;
 }
 
 bool HWAddressSanitizer::isInterestingAlloca(const AllocaInst &AI) {
   return (AI.getAllocatedType()->isSized() &&
           // FIXME: instrument dynamic allocas, too