Merge pull request #73528 from Snowy1803/6.0-rdar127348128

[6.0] [DebugInfo] Salvage stores + Fix verifier crash for complex switch

Author: adrian-prantl

include/swift/SIL/SILBuilder.h

@@ -262,7 +262,11 @@ class SILBuilder {
   /// scopes. To avoid a verification error later in the pipeline, drop all
   /// variables without a proper source location.
   bool shouldDropVariable(SILDebugVariable Var, SILLocation Loc) {
-    return !Var.ArgNo && Loc.isSynthesizedAST();
+    if (Var.ArgNo)
+      return false;
+    if (Var.Loc)
+      return Var.Loc->isSynthesizedAST();
+    return Loc.isSynthesizedAST();
   }
 
 
@@ -419,6 +423,9 @@ class SILBuilder {
 #else
     (void)skipVarDeclAssert;
 #endif
+    // Don't apply location overrides on variables.
+    if (Var && !Var->Loc)
+      Var->Loc = Loc;
     return insert(AllocStackInst::create(
         getSILDebugLocation(Loc, true), elementType, getFunction(),
         substituteAnonymousArgs(Name, Var, Loc), dynamic, isLexical,

include/swift/SIL/SILInstruction.h

@@ -2083,6 +2083,13 @@ class AllocStackInst final
   /// VarDecl.
   void setIsFromVarDecl() { sharedUInt8().AllocStackInst.fromVarDecl = true; }
 
+  /// Return the SILLocation for the debug variable.
+  SILLocation getVarLoc() const {
+    if (hasAuxDebugLocation())
+      return *getTrailingObjects<SILLocation>();
+    return getLoc();
+  }
+
   /// Return the debug variable information attached to this instruction.
   std::optional<SILDebugVariable> getVarInfo() const {
     // If we used to have debug info attached but our debug info is now
@@ -5340,6 +5347,13 @@ class DebugValueInst final
   /// or null if we don't have one.
   VarDecl *getDecl() const;
 
+  /// Return the SILLocation for the debug variable.
+  SILLocation getVarLoc() const {
+    if (hasAuxDebugLocation())
+      return *getTrailingObjects<SILLocation>();
+    return getLoc();
+  }
+
   /// Return the debug variable information attached to this instruction.
   std::optional<SILDebugVariable> getVarInfo() const {
     std::optional<SILType> AuxVarType;

include/swift/SIL/SILLocation.h

@@ -391,6 +391,16 @@ class SILLocation {
   /// Check is this location is part of a function's implicit prologue.
   bool isInPrologue() const { return kindAndFlags.fields.inPrologue; }
 
+  /// Returns this location with the auto-generated and prologue bits stripped.
+  /// These bits only make sense for instructions, and should be stripped for
+  /// variables.
+  SILLocation strippedForDebugVariable() const {
+    SILLocation loc = *this;
+    loc.kindAndFlags.fields.autoGenerated = false;
+    loc.kindAndFlags.fields.inPrologue = false;
+    return loc;
+  }
+
   /// Check if the corresponding source code location definitely points
   ///  to the end of the AST node.
   bool pointsToEnd() const;

include/swift/SILOptimizer/Utils/DebugOptUtils.h

@@ -48,6 +48,14 @@ inline void deleteAllDebugUses(SILInstruction *inst,
 /// new `debug_value` instruction before \p I is deleted.
 void salvageDebugInfo(SILInstruction *I);
 
+/// Transfer debug info associated with the store-like instruction \p SI to a
+/// new `debug_value` instruction before \p SI is deleted.
+/// \param SI The store instruction being deleted
+/// \param SrcVal The old source, where the debuginfo should be propagated to
+/// \param DestVal The old destination, where the debuginfo was
+void salvageStoreDebugInfo(SILInstruction *SI,
+                           SILValue SrcVal, SILValue DestVal);
+
 /// Transfer debug information associated with the result of \p load to the
 /// load's address operand.
 ///

lib/IRGen/IRGenDebugInfo.cpp

@@ -3186,10 +3186,14 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
   // Create the descriptor for the variable.
   unsigned DVarLine = DInstLine;
   uint16_t DVarCol = DInstLoc.Column;
-  if (VarInfo.Loc) {
-    auto DVarLoc = getStartLocation(VarInfo.Loc);
-    DVarLine = DVarLoc.Line;
-    DVarCol = DVarLoc.Column;
+  auto VarInfoLoc = VarInfo.Loc ? VarInfo.Loc : DbgInstLoc;
+  if (VarInfoLoc) {
+    auto VarLoc = VarInfoLoc->strippedForDebugVariable();
+    if (VarLoc != DbgInstLoc) {
+      auto DVarLoc = getStartLocation(VarLoc);
+      DVarLine = DVarLoc.Line;
+      DVarCol = DVarLoc.Column;
+    }
   }
   llvm::DIScope *VarScope = Scope;
   if (ArgNo == 0 && VarInfo.Scope) {

lib/SIL/IR/SILInstructions.cpp

@@ -266,6 +266,15 @@ AllocStackInst *AllocStackInst::create(SILDebugLocation Loc,
                                        IsLexical_t isLexical,
                                        IsFromVarDecl_t isFromVarDecl,
                                        UsesMoveableValueDebugInfo_t wasMoved) {
+  // Don't store the same information twice.
+  if (Var) {
+    if (Var->Loc == Loc.getLocation())
+      Var->Loc = {};
+    if (Var->Scope == Loc.getScope())
+      Var->Scope = nullptr;
+    if (Var->Type == elementType)
+      Var->Type = {};
+  }
   SmallVector<SILValue, 8> TypeDependentOperands;
   collectTypeDependentOperands(TypeDependentOperands, F,
                                elementType.getASTType());
@@ -277,6 +286,9 @@ AllocStackInst *AllocStackInst::create(SILDebugLocation Loc,
 }
 
 VarDecl *AllocationInst::getDecl() const {
+  if (auto ASI = dyn_cast<AllocStackInst>(this)) {
+    return ASI->getVarLoc().getAsASTNode<VarDecl>();
+  }
   return getLoc().getAsASTNode<VarDecl>();
 }
 
@@ -460,6 +472,13 @@ DebugValueInst *DebugValueInst::create(SILDebugLocation DebugLoc,
                                        SILDebugVariable Var, bool poisonRefs,
                                        UsesMoveableValueDebugInfo_t wasMoved,
                                        bool trace) {
+  // Don't store the same information twice.
+  if (Var.Loc == DebugLoc.getLocation())
+    Var.Loc = {};
+  if (Var.Scope == DebugLoc.getScope())
+    Var.Scope = nullptr;
+  if (Var.Type == Operand->getType().getObjectType())
+    Var.Type = {};
   void *buf = allocateDebugVarCarryingInst<DebugValueInst>(M, Var);
   return ::new (buf)
     DebugValueInst(DebugLoc, Operand, Var, poisonRefs, wasMoved, trace);
@@ -474,9 +493,8 @@ DebugValueInst::createAddr(SILDebugLocation DebugLoc, SILValue Operand,
   if (!isa<AllocStackInst>(Operand))
     Var.DIExpr.prependElements(
       {SILDIExprElement::createOperator(SILDIExprOperator::Dereference)});
-  void *buf = allocateDebugVarCarryingInst<DebugValueInst>(M, Var);
-  return ::new (buf) DebugValueInst(DebugLoc, Operand, Var,
-                                    /*poisonRefs=*/false, wasMoved, trace);
+  return DebugValueInst::create(DebugLoc, Operand, M, Var,
+                                /*poisonRefs=*/false, wasMoved, trace);
 }
 
 bool DebugValueInst::exprStartsWithDeref() const {
@@ -490,7 +508,7 @@ bool DebugValueInst::exprStartsWithDeref() const {
 }
 
 VarDecl *DebugValueInst::getDecl() const {
-  return getLoc().getAsASTNode<VarDecl>();
+  return getVarLoc().getAsASTNode<VarDecl>();
 }
 
 VarDecl *SILDebugVariable::getDecl() const {

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -749,8 +749,7 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
         // an alloc_stack), which don't have any `op_deref` in its
         // di-expression, because that memory doesn't need to be initialized
         // when `debug_value` is referencing it.
-        if (cast<DebugValueInst>(&I)->hasAddrVal() &&
-            cast<DebugValueInst>(&I)->exprStartsWithDeref())
+        if (!DebugValueInst::hasAddrVal(&I))
           requireBitsSet(bits, I.getOperand(0), &I);
         break;
       case SILInstructionKind::UncheckedTakeEnumDataAddrInst: {

lib/SILOptimizer/Transforms/CopyForwarding.cpp

@@ -69,6 +69,7 @@
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/CFGOptUtils.h"
+#include "swift/SILOptimizer/Utils/DebugOptUtils.h"
 #include "swift/SILOptimizer/Utils/ValueLifetime.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/Statistic.h"
@@ -698,6 +699,8 @@ propagateCopy(CopyAddrInst *CopyInst) {
       SILBuilderWithScope(CurrentCopy)
           .createDestroyAddr(CurrentCopy->getLoc(), CurrentCopy->getDest());
     }
+    swift::salvageStoreDebugInfo(CurrentCopy, CurrentCopy->getSrc(),
+                                 CurrentCopy->getDest());
     CurrentCopy->eraseFromParent();
     HasChanged = true;
     ++NumCopyForward;
@@ -706,6 +709,8 @@ propagateCopy(CopyAddrInst *CopyInst) {
   // Forward propagation failed. Attempt to backward propagate.
   if (CurrentCopy->isInitializationOfDest() && backwardPropagateCopy()) {
     LLVM_DEBUG(llvm::dbgs() << "  Reversing Copy:" << *CurrentCopy);
+    swift::salvageStoreDebugInfo(CurrentCopy, CurrentCopy->getDest(),
+                                 CurrentCopy->getSrc());
     CurrentCopy->eraseFromParent();
     HasChanged = true;
     ++NumCopyBackward;
@@ -817,6 +822,9 @@ forwardDeadTempCopy(CopyAddrInst *destCopy) {
       .createDestroyAddr(srcCopy->getLoc(), srcCopy->getDest());
   }
 
+  // Salvage debug values before deleting them.
+  swift::salvageStoreDebugInfo(srcCopy, srcCopy->getSrc(), srcCopy->getDest());
+
   // Delete all dead debug_value instructions
   for (auto *deadDebugUser : debugInstsToDelete) {
     deadDebugUser->eraseFromParent();
@@ -1190,9 +1198,13 @@ bool CopyForwarding::backwardPropagateCopy() {
   // Now that an init was found, it is safe to substitute all recorded uses
   // with the copy's dest.
   for (auto *Oper : ValueUses) {
-    Oper->set(CurrentCopy->getDest());
-    if (isa<CopyAddrInst>(Oper->getUser()))
+    if (auto *SI = dyn_cast<CopyAddrInst>(Oper->getUser())) {
       HasForwardedToCopy = true;
+      // This instruction gets "replaced", so we need to salvage its previous
+      // debug info.
+      swift::salvageStoreDebugInfo(SI, SI->getSrc(), SI->getDest());
+    }
+    Oper->set(CurrentCopy->getDest());
   }
   return true;
 }

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

@@ -301,68 +301,6 @@ replaceDestroy(DestroyAddrInst *dai, SILValue newValue, SILBuilderContext &ctx,
   prepareForDeletion(dai, instructionsToDelete);
 }
 
-/// Whether the specified debug_value's operand names the address at the
-/// indicated alloc_stack.
-///
-/// If it's a guaranteed alloc_stack (i.e. a store_borrow location), that
-/// includes the values produced by any store_borrows whose destinations are the
-/// alloc_stack since those values amount to aliases for the alloc_stack's
-/// storage.
-static bool isDebugValueOfAllocStack(DebugValueInst *dvi, AllocStackInst *asi) {
-  auto value = dvi->getOperand();
-  if (value == asi)
-    return true;
-  auto *sbi = dyn_cast<StoreBorrowInst>(value);
-  if (!sbi)
-    return false;
-  return sbi->getDest() == asi;
-}
-
-/// Promote a DebugValue w/ address value to a DebugValue of non-address value.
-static void promoteDebugValueAddr(DebugValueInst *dvai, SILValue value,
-                                  SILBuilderContext &ctx,
-                                  InstructionDeleter &deleter) {
-  assert(dvai->getOperand()->getType().isLoadable(*dvai->getFunction()) &&
-         "Unexpected promotion of address-only type!");
-  assert(value && "Expected valid value");
-
-  // Avoid inserting the same debug_value twice.
-  //
-  // We remove the di expression when comparing since:
-  //
-  // 1. dvai is on will always have the deref diexpr since it is on addresses.
-  //
-  // 2. We are only trying to delete debug_var that are on values... values will
-  //    never have an op_deref meaning that the comparison will always fail and
-  //    not serve out purpose here.
-  auto dvaiWithoutDIExpr = dvai->getVarInfo()->withoutDIExpr();
-  for (auto *use : value->getUses()) {
-    if (auto *dvi = dyn_cast<DebugValueInst>(use->getUser())) {
-      if (!dvi->hasAddrVal() && *dvi->getVarInfo() == dvaiWithoutDIExpr) {
-        deleter.forceDelete(dvai);
-        return;
-      }
-    }
-  }
-
-  // Drop op_deref if dvai is actually a debug_value instruction
-  auto varInfo = *dvai->getVarInfo();
-  if (isa<DebugValueInst>(dvai)) {
-    auto &diExpr = varInfo.DIExpr;
-    // FIXME: There should always be a DIExpr starting with an op_deref here
-    // The debug_value is attached to a pointer type, and those don't exist
-    // in Swift, so they should always be dereferenced.
-    // However, this rule is broken in a lot of spaces, so we have to leave
-    // this check to recover from wrong info
-    if (diExpr && diExpr.startsWithDeref())
-      diExpr.eraseElement(diExpr.element_begin());
-  }
-
-  SILBuilderWithScope b(dvai, ctx);
-  b.createDebugValue(dvai->getLoc(), value, std::move(varInfo));
-  deleter.forceDelete(dvai);
-}
-
 /// Returns true if \p I is a load which loads from \p ASI.
 static bool isLoadFromStack(SILInstruction *i, AllocStackInst *asi) {
   if (!isa<LoadInst>(i) && !isa<LoadBorrowInst>(i))
@@ -1172,14 +1110,8 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
       continue;
     }
 
-    // Replace debug_value w/ address value with debug_value of
-    // the promoted value.
-    // if we have a valid value to use at this point. Otherwise we'll
-    // promote this when we deal with hooking up phis.
+    // Debug values will automatically be salvaged, we can ignore them.
     if (auto *dvi = DebugValueInst::hasAddrVal(inst)) {
-      if (isDebugValueOfAllocStack(dvi, asi) && runningVals)
-        promoteDebugValueAddr(dvi, runningVals->value.replacement(asi, dvi),
-                              ctx, deleter);
       continue;
     }
 
@@ -1432,12 +1364,8 @@ void StackAllocationPromoter::fixBranchesAndUses(
     // on.
     SILBasicBlock *userBlock = user->getParent();
 
+    // Debug values will automatically be salvaged, we can ignore them.
     if (auto *dvi = DebugValueInst::hasAddrVal(user)) {
-      // Replace debug_value w/ address-type value with
-      // a new debug_value w/ promoted value.
-      auto def = getEffectiveLiveInValues(phiBlocks, userBlock);
-      promoteDebugValueAddr(dvi, def.replacement(asi, dvi), ctx, deleter);
-      ++NumInstRemoved;
       continue;
     }
 
@@ -2036,20 +1964,8 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
       continue;
     }
 
-    // Replace debug_value w/ address value with debug_value of
-    // the promoted value.
+    // Debug values will automatically be salvaged, we can ignore them.
     if (auto *dvi = DebugValueInst::hasAddrVal(inst)) {
-      if (isDebugValueOfAllocStack(dvi, asi)) {
-        if (runningVals) {
-          promoteDebugValueAddr(dvi, runningVals->value.replacement(asi, dvi),
-                                ctx, deleter);
-        } else {
-          // Drop debug_value of uninitialized void values.
-          assert(asi->getElementType().isVoid() &&
-                 "Expected initialization of non-void type!");
-          deleter.forceDelete(dvi);
-        }
-      }
       continue;
     }
 

lib/SILOptimizer/Utils/GenericCloner.cpp

@@ -68,6 +68,8 @@ void GenericCloner::populateCloned() {
   SILBasicBlock *ClonedEntryBB = Cloned->createBasicBlock();
   getBuilder().setInsertionPoint(ClonedEntryBB);
 
+  RemappedScopeCache.insert({Original.getDebugScope(), Cloned->getDebugScope()});
+
   // Create the entry basic block with the function arguments.
   auto origConv = Original.getConventions();
   unsigned ArgIdx = 0;
@@ -138,26 +140,6 @@ void GenericCloner::populateCloned() {
               mappedType, OrigArg->getDecl());
           NewArg->copyFlags(cast<SILFunctionArgument>(OrigArg));
 
-          // Try to create a new debug_value from an existing debug_value w/
-          // address value for the argument. We do this before storing to
-          // ensure that when we are cloning code in ossa the argument has
-          // not been consumed by the store below.
-          for (Operand *ArgUse : OrigArg->getUses()) {
-            if (auto *DVI = DebugValueInst::hasAddrVal(ArgUse->getUser())) {
-              auto *oldScope = getBuilder().getCurrentDebugScope();
-              getBuilder().setCurrentDebugScope(
-                  remapScope(DVI->getDebugScope()));
-              auto VarInfo = DVI->getVarInfo();
-              assert(VarInfo && VarInfo->DIExpr &&
-                     "No DebugVarInfo or no DIExpr operand?");
-              // Drop the op_deref
-              VarInfo->DIExpr.eraseElement(VarInfo->DIExpr.element_begin());
-              getBuilder().createDebugValue(DVI->getLoc(), NewArg, *VarInfo);
-              getBuilder().setCurrentDebugScope(oldScope);
-              break;
-            }
-          }
-
           // Store the new direct parameter to an alloc_stack.
           createAllocStack();
           SILValue addr;