[DebugInfo] LowerDbgDeclare: Add derefs when handling CallInst users

[vsk]

Summary

LowerDbgDeclare inserts a dbg.value before each use of an address
described by a dbg.declare. When inserting a dbg.value before a CallInst
use, however, it fails to append DW_OP_deref to the DIExpression.

The DW_OP_deref is needed to reflect the fact that a dbg.value describes
a source variable directly (as opposed to a dbg.declare, which relies on
pointer indirection).

This patch adds in the DW_OP_deref where needed. This results in the
correct values being shown during a debug session for a program compiled
with ASan and optimizations (see https://reviews.llvm.org/D49520). Note
that ConvertDebugDeclareToDebugValue is already correct -- no changes
there were needed.

One complication is that SelectionDAG is unable to distinguish between
direct and indirect frame-index (FRAMEIX) SDDbgValues. This patch also
fixes this long-standing issue in order to not regress integration tests
relying on the incorrect assumption that all frame-index SDDbgValues are
indirect. This is a necessary fix: the newly-added DW_OP_derefs cannot
be lowered properly otherwise. Basically the fix prevents a direct
SDDbgValue with DIExpression(DW_OP_deref) from being dereferenced twice
by a debugger. There were a handful of tests relying on this incorrect
"FRAMEIX => indirect" assumption which actually had incorrect
DW_AT_locations: these are all fixed up in this patch.

Testing:

• check-llvm, and an end-to-end test using lldb to debug an optimized program.
• Existing unit tests for DIExpression::appendToStack fully cover the new DIExpression::append utility.
• check-debuginfo (the debug info integration tests)

[aprantl]

This looks like the right change as it makes the IR more regular.
Note that in the past this worked because allocas were implicitly indirect since they caused AsmPrinter to emit a what DWARF calls a "memory location". It's possible that that code path no longer exists or isn't triggered any more.

Have you tested the clang debuginfo-tests with this change, particularly the asan and array testcases?

[vsk]

In D49454#1166103, @aprantl wrote:

This looks like the right change as it makes the IR more regular.
Note that in the past this worked because allocas were implicitly indirect since they caused AsmPrinter to emit a what DWARF calls a "memory location". It's possible that that code path no longer exists or isn't triggered any more.

Have you tested the clang debuginfo-tests with this change, particularly the asan and array testcases?

Thanks for suggesting this.

As-is, this patch breaks debuginfo-tests/stack-var.c. Without this patch we would emit a dbg.value with a concrete location:

  call void @llvm.dbg.value(metadata i32* %test, metadata !25, metadata !DIExpression()), !dbg !27
...
!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!25 = !DILocalVariable(name: "test", scope: !21, file: !1, line: 6, type: !12)

When the backend sees this, it knows to emit a DBG_VALUE which refines the concrete location:

DBG_VALUE %rbp, 0, !"test", !DIExpression(DW_OP_constu, 4, DW_OP_minus); line no:6 indirect

With this patch applied, we would instead emit:

call void @llvm.dbg.value(metadata i32* %test, metadata !25, metadata !DIExpression(DW_OP_deref)), !dbg !27

The backend sees the extra DW_OP_deref and emits a DBG_VALUE which tells the debugger to dereference a value it pulls out of the stack:

DBG_VALUE debug-use $rbp, 0, !"test", !DIExpression(DW_OP_constu, 4, DW_OP_minus, DW_OP_deref), debug-location !27; line no:6 indirect

The problem with this patch is that it adds a DW_OP_deref without creating an implicit value to prevent that extra, incorrect dereference. The fix to this seemed simple: just add DW_OP_stack_value. That should tell the debugger not to dereference what it finds in the stack. But, that's just a more expensive way to encode the exact same behavior as a {pointer, empty DIExpression} dbg.value. Critically, it also doesn't fix my ASan test case.

Using a single DW_OP_deref (without appending DW_OP_stack_value) gives the correct location for a ASan-ified program appending to a std::deque:

(lldb) p test
(log_t) $0 = size=1 {
  [0] = (a = "toto")
}
...
Variable: id = {0x00009f25}, name = "test", type = "log_t", location =  [r15+0], decl = r376.cc:12

Note that we get the correct result (1 dereference) even though this is the *wrong* DIExpression to use, because (as an indirect value) it says to do 2 dereferences.

Adding a DW_OP_stack_value -- which fixed the stack-var.c test case -- does not fix the ASan test case. I'd expect it to cause the debugger to do 1 dereference, but instead it does 2:

(lldb) p test
error: Couldn't materialize: couldn't get the value of variable test: extracting data from value failed
...
     Variable: id = {0x00009f25}, name = "test", type = "log_t", location =  [r15+0] DW_OP_deref , decl = r376.cc:12

I think I know why adding DW_OP_stack_value fixes one test case but breaks another. My theory is that we're treating a pointer to a std::deque as a direct value (aka "implicit location"?) instead of as an indirect value (aka "concrete location"?). Here's some confirmation:

DBG_VALUE debug-use $r15, debug-use $noreg, !"test"

IIUC the fact that the second DBG_VALUE operand is $noreg means that the value is direct (implicit). The LangRef is vague about what should happen in this case, but it doesn't seem like we should get here to begin with.

[rnk]

I haven't read this carefully, but this seems like the right direction. Thanks for working on it. When I last touched this, I wanted to spend time making our dbg.values of pointers unambiguous as to whether the value is the pointer or the memory the pointer refers to.

[vsk]

This update fixes the long-standing issue where SelectionDAG was unable to distinguish between direct and indirect frame index debug values. Tackling this issue not only fixed the stack-var.c -O2 integration test from debuginfo-tests, but caught a slew of LLVM test cases which were checking for bogus DW_AT_locations.

[rnk]

Thanks so much for seeing this through! This will finally allow us to solve int x = 0; int *px = &x;, which has been my favorite optimized debug info test case for a while now. If I filed a bug about it, I can't find it, just https://bugs.llvm.org/show_bug.cgi?id=34136, which has too much discussion on it to really be actionable at this point.

lib/CodeGen/SelectionDAG/SDNodeDbgValue.h

77 ↗

(On Diff #156915)

SDDbgValue creation is kind of an implementation detail of SelectionDAG. I think it might be better to collapse the frame index and virtual register constructor overloads and add a SDDbgValue::DbgValueKind parameter that SelectionDAG::getFrameIndexDbgValue and getVRegDbgValue fill in appropriately.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

4965–4971 ↗

(On Diff #156915)

So, this should only be called in the case where the value of the variable is the address of some stack location, i.e. int x = 0; int *px = &x;, after optimization, we'll have this direct frame index dbg.value for px. I think that source fragment in a comment would help clarify why IsIndirect is false.

5183 ↗

(On Diff #156915)

Yikes, this was easier to read before rL314363. INT_MAX is better. =P

[vsk]

Unify the SDDbgValue constructor for vregs and frame-indices; add a more descriptive comment in ::getDbgValue about why a FrameIndexDbgValue must be direct.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

4965–4971 ↗

(On Diff #156915)

Isn't there one more case, where the value of the variable lives inside of some stack location?

To build on your example, I think the case you described looks like this:

dbg.value(i32* %px, !"int *px", !DIExpression())

And the one I'm pointing out looks like this:

dbg.value(i32* %px, !"int x", !DIExpression(DW_OP_deref))

[shafik]

include/llvm/CodeGen/SelectionDAG.h

1253 ↗

(On Diff #156942)

Instead of using a bool for IsIndirect perhaps using an enum with two enumerator Direct and Indirect would be more readable and maintainable?

I make this comment since I noticed you were commenting your usage below, which indicated lack of clarity.

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

700 ↗

(On Diff #156942)

Does using auto gain us much here?

lib/IR/DebugInfoMetadata.cpp

855 ↗

(On Diff #156942)

Are we expecting to only do this once and that is why we don't clear NewOps after the appendToVector below?

[vsk]

include/llvm/CodeGen/SelectionDAG.h

1253 ↗

(On Diff #156942)

I think commented use of a bool is reasonably clear. Refactoring the code to use enums would be fine as a follow-up change, but doesn't seem necessary for this patch.

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

700 ↗

(On Diff #156942)

Yes, it's the convention in the backends to feign ignorance as to what BuildMI returns:

$ gg -E "auto [^=]+= BuildMI" | wc -l
      52

It's a bit distracting to see 'MachineInstrBuilder' here, c.f unordered_map iterators.

lib/IR/DebugInfoMetadata.cpp

855 ↗

(On Diff #156942)

Right. I'll try to make that clearer in the final comment in this function.

[rnk]

lgtm

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

4965–4971 ↗

(On Diff #156915)

Right, yes, which is the original motivating example for inserting dbg.values before calls.

debuginfo-tests/trunk/asan-deque.cpp

+// RUN: %clangxx -arch x86_64 %target_itanium_abi_host_triple -O1 -g %s -o %t.out -fsanitize=address
+// RUN: %test_debuginfo %s %t.out
+// REQUIRES: not_asan
+//           Zorg configures the ASAN stage2 bots to not build the asan
+//           compiler-rt. Only run this test on non-asanified configurations.
+
+#include <deque>
+
+struct A {
+  int a;
+  A(int a) : a(a) {}
+};
+
+using log_t = std::deque<A>;
+
+static void __attribute__((noinline, optnone)) escape(log_t &log) {
+  static volatile log_t *sink;
+  sink = &log;
+}
+
+int main() {
+  log_t log;
+  log.emplace_back(1234);
+  log.emplace_back(56789);
+  escape(log);
+  // DEBUGGER: break 25
+  while (!log.empty()) {
+    auto record = log.front();
+    log.pop_front();
+    escape(log);
+    // DEBUGGER: break 30
+  }
+}
+
+// DEBUGGER: r
+
+// (at line 25)
+// DEBUGGER: p log
+// CHECK: 1234
+// CHECK: 56789
+
+// DEBUGGER: c
+
+// (at line 30)
+// DEBUGGER: p log
+// CHECK: 56789

llvm/trunk/include/llvm/CodeGen/SelectionDAG.h

#endif
 
   /// Creates a constant SDDbgValue node.
   SDDbgValue *getConstantDbgValue(DIVariable *Var, DIExpression *Expr,
                                   const Value *C, const DebugLoc &DL,
                                   unsigned O);
 
   /// Creates a FrameIndex SDDbgValue node.
   SDDbgValue *getFrameIndexDbgValue(DIVariable *Var, DIExpression *Expr,
-                                    unsigned FI, const DebugLoc &DL,
-                                    unsigned O);
+                                    unsigned FI, bool IsIndirect,
+                                    const DebugLoc &DL, unsigned O);
 
   /// Creates a VReg SDDbgValue node.
   SDDbgValue *getVRegDbgValue(DIVariable *Var, DIExpression *Expr,
                               unsigned VReg, bool IsIndirect,
                               const DebugLoc &DL, unsigned O);
 
   /// Creates a SDDbgLabel node.
   SDDbgLabel *getDbgLabel(DILabel *Label, const DebugLoc &DL, unsigned O);

llvm/trunk/include/llvm/IR/DebugInfoMetadata.h

static DIExpression *prepend(const DIExpression *Expr, bool DerefBefore,
                                bool StackValue = false);
 
   /// Prepend \p DIExpr with the given opcodes and optionally turn it into a
   /// stack value.
   static DIExpression *prependOpcodes(const DIExpression *Expr,
                                       SmallVectorImpl<uint64_t> &Ops,
                                       bool StackValue = false);
 
+  /// Append the opcodes \p Ops to \p DIExpr. Unlike \ref appendToStack, the
+  /// returned expression is a stack value only if \p DIExpr is a stack value.
+  /// If \p DIExpr describes a fragment, the returned expression will describe
+  /// the same fragment.
+  static DIExpression *append(const DIExpression *Expr, ArrayRef<uint64_t> Ops);
+
   /// Convert \p DIExpr into a stack value if it isn't one already by appending
   /// DW_OP_deref if needed, and appending \p Ops to the resulting expression.
   /// If \p DIExpr describes a fragment, the returned expression will describe
   /// the same fragment.
   static DIExpression *appendToStack(const DIExpression *Expr,
                                      ArrayRef<uint64_t> Ops);
 
   /// Create a DIExpression to describe one part of an aggregate variable that

llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

InstrEmitter::EmitDbgValue(SDDbgValue *SD,
   MDNode *Expr = SD->getExpression();
   DebugLoc DL = SD->getDebugLoc();
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
 
   if (SD->getKind() == SDDbgValue::FRAMEIX) {
     // Stack address; this needs to be lowered in target-dependent fashion.
     // EmitTargetCodeForFrameDebugValue is responsible for allocation.
-    return BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE))
-        .addFrameIndex(SD->getFrameIx())
-        .addImm(0)
-        .addMetadata(Var)
-        .addMetadata(Expr);
+    auto FrameMI = BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE))
+                       .addFrameIndex(SD->getFrameIx());
+    if (SD->isIndirect())
+      // Push [fi + 0] onto the DIExpression stack.
+      FrameMI.addImm(0);
+    else
+      // Push fi onto the DIExpression stack.
+      FrameMI.addReg(0);
+    return FrameMI.addMetadata(Var).addMetadata(Expr);
   }
   // Otherwise, we're going to create an instruction here.
   const MCInstrDesc &II = TII->get(TargetOpcode::DBG_VALUE);
   MachineInstrBuilder MIB = BuildMI(*MF, DL, II);
   if (SD->getKind() == SDDbgValue::SDNODE) {
     SDNode *Node = SD->getSDNode();
     SDValue Op = SDValue(Node, SD->getResNo());
     // It's possible we replaced this SDNode with other(s) and therefore

llvm/trunk/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h

public:
   /// Constructor for constants.
   SDDbgValue(DIVariable *Var, DIExpression *Expr, const Value *C, DebugLoc dl,
              unsigned O)
       : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(false) {
     kind = CONST;
     u.Const = C;
   }
 
-  /// Constructor for frame indices.
-  SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned FI, DebugLoc dl,
-             unsigned O)
-      : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(false) {
-    kind = FRAMEIX;
-    u.FrameIx = FI;
-  }
-
-  /// Constructor for virtual registers.
-  SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned VReg, bool indir,
-             DebugLoc dl, unsigned O)
-      : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(indir) {
-    kind = VREG;
-    u.VReg = VReg;
+  /// Constructor for virtual registers and frame indices.
+  SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned VRegOrFrameIdx,
+             bool IsIndirect, DebugLoc DL, unsigned Order,
+             enum DbgValueKind Kind)
+      : Var(Var), Expr(Expr), DL(DL), Order(Order), IsIndirect(IsIndirect) {
+    assert((Kind == VREG || Kind == FRAMEIX) &&
+           "Invalid SDDbgValue constructor");
+    kind = Kind;
+    if (kind == VREG)
+      u.VReg = VRegOrFrameIdx;
+    else
+      u.FrameIx = VRegOrFrameIdx;
   }
 
   /// Returns the kind.
   DbgValueKind getKind() const { return kind; }
 
   /// Returns the DIVariable pointer for the variable.
   DIVariable *getVariable() const { return Var; }
 

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

SDDbgValue *SelectionDAG::getConstantDbgValue(DIVariable *Var,
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
   return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, C, DL, O);
 }
 
 /// FrameIndex
 SDDbgValue *SelectionDAG::getFrameIndexDbgValue(DIVariable *Var,
                                                 DIExpression *Expr, unsigned FI,
+                                                bool IsIndirect,
                                                 const DebugLoc &DL,
                                                 unsigned O) {
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
-  return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, FI, DL, O);
+  return new (DbgInfo->getAlloc())
+      SDDbgValue(Var, Expr, FI, IsIndirect, DL, O, SDDbgValue::FRAMEIX);
 }
 
 /// VReg
 SDDbgValue *SelectionDAG::getVRegDbgValue(DIVariable *Var,
                                           DIExpression *Expr,
                                           unsigned VReg, bool IsIndirect,
                                           const DebugLoc &DL, unsigned O) {
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
-  return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, VReg, IsIndirect, DL,
-                                              O);
+  return new (DbgInfo->getAlloc())
+      SDDbgValue(Var, Expr, VReg, IsIndirect, DL, O, SDDbgValue::VREG);
 }
 
 void SelectionDAG::transferDbgValues(SDValue From, SDValue To,
                                      unsigned OffsetInBits, unsigned SizeInBits,
                                      bool InvalidateDbg) {
   SDNode *FromNode = From.getNode();
   SDNode *ToNode = To.getNode();
   assert(FromNode && ToNode && "Can't modify dbg values");

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

 /// Return the appropriate SDDbgValue based on N.
 SDDbgValue *SelectionDAGBuilder::getDbgValue(SDValue N,
                                              DILocalVariable *Variable,
                                              DIExpression *Expr,
                                              const DebugLoc &dl,
                                              unsigned DbgSDNodeOrder) {
   if (auto *FISDN = dyn_cast<FrameIndexSDNode>(N.getNode())) {
     // Construct a FrameIndexDbgValue for FrameIndexSDNodes so we can describe
-    // stack slot locations as such instead of as indirectly addressed
-    // locations.
-    return DAG.getFrameIndexDbgValue(Variable, Expr, FISDN->getIndex(), dl,
-                                     DbgSDNodeOrder);
+    // stack slot locations. 
+    //
+    // Consider "int x = 0; int *px = &x;". There are two kinds of interesting
+    // debug values here after optimization:
+    //
+    //   dbg.value(i32* %px, !"int *px", !DIExpression()), and
+    //   dbg.value(i32* %px, !"int x", !DIExpression(DW_OP_deref))
+    //
+    // Both describe the direct values of their associated variables.
+    return DAG.getFrameIndexDbgValue(Variable, Expr, FISDN->getIndex(),
+                                     /*IsIndirect*/ false, dl, DbgSDNodeOrder);
   }
-  return DAG.getDbgValue(Variable, Expr, N.getNode(), N.getResNo(), false, dl,
-                         DbgSDNodeOrder);
+  return DAG.getDbgValue(Variable, Expr, N.getNode(), N.getResNo(),
+                         /*IsIndirect*/ false, dl, DbgSDNodeOrder);
 }
 
 // VisualStudio defines setjmp as _setjmp
 #if defined(_MSC_VER) && defined(setjmp) && \
                          !defined(setjmp_undefined_for_msvc)
 #  pragma push_macro("setjmp")
 #  undef setjmp
 #  define setjmp_undefined_for_msvc
if (const auto *AI =
       FI = FuncInfo.getArgumentFrameIndex(Arg);
     }
 
     // llvm.dbg.addr is control dependent and always generates indirect
     // DBG_VALUE instructions. llvm.dbg.declare is handled as a frame index in
     // the MachineFunction variable table.
     if (FI != std::numeric_limits<int>::max()) {
       if (Intrinsic == Intrinsic::dbg_addr) {
-         SDDbgValue *SDV = DAG.getFrameIndexDbgValue(Variable, Expression,
-                                                     FI, dl, SDNodeOrder);
-         DAG.AddDbgValue(SDV, getRoot().getNode(), isParameter);
+        SDDbgValue *SDV = DAG.getFrameIndexDbgValue(
+            Variable, Expression, FI, /*IsIndirect*/ true, dl, SDNodeOrder);
       }
       return nullptr;
     }
 
     SDValue &N = NodeMap[Address];
     if (!N.getNode() && isa<Argument>(Address))
       // Check unused arguments map.
       N = UnusedArgNodeMap[Address];
     SDDbgValue *SDV;
     if (N.getNode()) {
       if (const BitCastInst *BCI = dyn_cast<BitCastInst>(Address))
         Address = BCI->getOperand(0);
       // Parameters are handled specially.
       auto FINode = dyn_cast<FrameIndexSDNode>(N.getNode());
       if (isParameter && FINode) {
         // Byval parameter. We have a frame index at this point.
-        SDV = DAG.getFrameIndexDbgValue(Variable, Expression,
-                                        FINode->getIndex(), dl, SDNodeOrder);
+        SDV =
+            DAG.getFrameIndexDbgValue(Variable, Expression, FINode->getIndex(),
+                                      /*IsIndirect*/ true, dl, SDNodeOrder);
       } else if (isa<Argument>(Address)) {
         // Address is an argument, so try to emit its dbg value using
         // virtual register info from the FuncInfo.ValueMap.
         EmitFuncArgumentDbgValue(Address, Variable, Expression, dl, true, N);
         return nullptr;
       } else {
         SDV = DAG.getDbgValue(Variable, Expression, N.getNode(), N.getResNo(),
                               true, dl, SDNodeOrder);

llvm/trunk/lib/IR/DebugInfoMetadata.cpp

for (auto Op : Expr->expr_ops()) {
     }
     Op.appendToVector(Ops);
   }
   if (StackValue)
     Ops.push_back(dwarf::DW_OP_stack_value);
   return DIExpression::get(Expr->getContext(), Ops);
 }
 
+DIExpression *DIExpression::append(const DIExpression *Expr,
+                                   ArrayRef<uint64_t> Ops) {
+  assert(Expr && !Ops.empty() && "Can't append ops to this expression");
+
+  // Copy Expr's current op list.
+  SmallVector<uint64_t, 16> NewOps;
+  for (auto Op : Expr->expr_ops()) {
+    // Append new opcodes before DW_OP_{stack_value, LLVM_fragment}.
+    if (Op.getOp() == dwarf::DW_OP_stack_value ||
+        Op.getOp() == dwarf::DW_OP_LLVM_fragment) {
+      NewOps.append(Ops.begin(), Ops.end());
+
+      // Ensure that the new opcodes are only appended once.
+      Ops = None;
+    }
+    Op.appendToVector(NewOps);
+  }
+
+  NewOps.append(Ops.begin(), Ops.end());
+  return DIExpression::get(Expr->getContext(), NewOps);
+}
+
 DIExpression *DIExpression::appendToStack(const DIExpression *Expr,
                                           ArrayRef<uint64_t> Ops) {
   assert(Expr && !Ops.empty() && "Can't append ops to this expression");
+  assert(none_of(Ops,
+                 [](uint64_t Op) {
+                   return Op == dwarf::DW_OP_stack_value ||
+                          Op == dwarf::DW_OP_LLVM_fragment;
+                 }) &&
+         "Can't append this op");
 
   // Append a DW_OP_deref after Expr's current op list if it's non-empty and
   // has no DW_OP_stack_value.
   //
   // Match .* DW_OP_stack_value (DW_OP_LLVM_fragment A B)?.
   Optional<FragmentInfo> FI = Expr->getFragmentInfo();
   unsigned DropUntilStackValue = FI.hasValue() ? 3 : 0;
-  bool NeedsDeref =
-      (Expr->getNumElements() > DropUntilStackValue) &&
-      (Expr->getElements().drop_back(DropUntilStackValue).back() !=
-       dwarf::DW_OP_stack_value);
+  ArrayRef<uint64_t> ExprOpsBeforeFragment =
+      Expr->getElements().drop_back(DropUntilStackValue);
+  bool NeedsDeref = (Expr->getNumElements() > DropUntilStackValue) &&
+                    (ExprOpsBeforeFragment.back() != dwarf::DW_OP_stack_value);
+  bool NeedsStackValue = NeedsDeref || ExprOpsBeforeFragment.empty();
 
-  // Copy Expr's current op list, add a DW_OP_deref if needed, and ensure that
-  // a DW_OP_stack_value is present.
+  // Append a DW_OP_deref after Expr's current op list if needed, then append
+  // the new ops, and finally ensure that a single DW_OP_stack_value is present.
   SmallVector<uint64_t, 16> NewOps;
-  for (auto Op : Expr->expr_ops()) {
-    if (Op.getOp() == dwarf::DW_OP_stack_value ||
-        Op.getOp() == dwarf::DW_OP_LLVM_fragment)
-      break;
-    Op.appendToVector(NewOps);
-  }
   if (NeedsDeref)
     NewOps.push_back(dwarf::DW_OP_deref);
   NewOps.append(Ops.begin(), Ops.end());
+  if (NeedsStackValue)
     NewOps.push_back(dwarf::DW_OP_stack_value);
-
-  // If Expr is a fragment, make the new expression a fragment as well.
-  if (FI)
-    NewOps.append(
-        {dwarf::DW_OP_LLVM_fragment, FI->OffsetInBits, FI->SizeInBits});
-  return DIExpression::get(Expr->getContext(), NewOps);
+  return DIExpression::append(Expr, NewOps);
 }
 
 Optional<DIExpression *> DIExpression::createFragmentExpression(
     const DIExpression *Expr, unsigned OffsetInBits, unsigned SizeInBits) {
   SmallVector<uint64_t, 8> Ops;
   // Copy over the expression, but leave off any trailing DW_OP_LLVM_fragment.
   if (Expr) {
     for (auto Op : Expr->expr_ops()) {

llvm/trunk/lib/Transforms/Utils/Local.cpp

for (auto &I : Dbgs) {
     for (auto &AIUse : AI->uses()) {
       User *U = AIUse.getUser();
       if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
         if (AIUse.getOperandNo() == 1)
           ConvertDebugDeclareToDebugValue(DDI, SI, DIB);
       } else if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
         ConvertDebugDeclareToDebugValue(DDI, LI, DIB);
       } else if (CallInst *CI = dyn_cast<CallInst>(U)) {
-        // This is a call by-value or some other instruction that
-        // takes a pointer to the variable. Insert a *value*
-        // intrinsic that describes the alloca.
-        DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(),
-                                    DDI->getExpression(), DDI->getDebugLoc(),
-                                    CI);
+        // This is a call by-value or some other instruction that takes a
+        // pointer to the variable. Insert a *value* intrinsic that describes
+        // the variable by dereferencing the alloca.
+        auto *DerefExpr =
+            DIExpression::append(DDI->getExpression(), dwarf::DW_OP_deref);
+        DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr,
+                                    DDI->getDebugLoc(), CI);
       }
     }
     DDI->eraseFromParent();
   }
   return true;
 }
 
 /// Propagate dbg.value intrinsics through the newly inserted PHIs.

llvm/trunk/test/DebugInfo/AArch64/coalescing.ll

 
 ; Function Attrs: nounwind optsize
 define void @_Z5startv() #0 !dbg !4 {
 entry:
   %size = alloca i32, align 4
   %0 = bitcast i32* %size to i8*, !dbg !15
   %call = call i8* @_Z3fooPv(i8* %0) #3, !dbg !15
   call void @llvm.dbg.value(metadata i32* %size, metadata !10, metadata !16), !dbg !17
+
+  ; The *location* of the variable should be $sp+12. This tells the debugger to
+  ; look up its value in [$sp+12].
   ; CHECK: .debug_info contents:
   ; CHECK: DW_TAG_variable
   ; CHECK-NEXT: DW_AT_location
-  ; CHECK-NEXT: DW_OP_breg31 WSP+12, DW_OP_deref
+  ; CHECK-NEXT: DW_OP_breg31 WSP+12
   ; CHECK-NEXT: DW_AT_name {{.*}}"size"
   ret void, !dbg !18
 }
 
 ; Function Attrs: optsize
 declare i8* @_Z3fooPv(i8*) #1
 
 ; Function Attrs: nounwind readnone

llvm/trunk/test/DebugInfo/Mips/dsr-fixed-objects.ll

 ;   return x;
 ; }
 
 declare void @llvm.lifetime.start(i64, i8* nocapture)
 declare void @llvm.lifetime.end(i64, i8* nocapture)
 
 declare void @foo(i32*)
 
+; CHECK: DW_AT_name {{.*}}"f0"
 ; CHECK: DW_AT_name {{.*}}"e"
 ; CHECK: DW_TAG_variable
 ; CHECK-NEXT: DW_AT_location [DW_FORM_sec_offset] (
 ; CHECK-NEXT:   [0x00000028, 0x0000002c): DW_OP_reg1 AT_64
-; CHECK-NEXT:   [0x0000002c, 0x00000048): DW_OP_breg29 SP_64+16, DW_OP_deref)
+; CHECK-NEXT:   [0x0000002c, 0x00000048): DW_OP_breg29 SP_64+16)
 ; CHECK-NEXT: DW_AT_name [DW_FORM_strp]     ( .debug_str[0x0000006b] = "x")
 
 define i32 @f0(i32 signext %a, i32 signext %b, i32 signext %c, i32 signext %d, i32 signext %e) !dbg !4 {
 entry:
   %x = alloca i32, align 4
   tail call void @llvm.dbg.value(metadata i32 %a, metadata !9,  metadata !DIExpression()), !dbg !27
   tail call void @llvm.dbg.value(metadata i32 %b, metadata !10, metadata !DIExpression()), !dbg !28
   tail call void @llvm.dbg.value(metadata i32 %c, metadata !11, metadata !DIExpression()), !dbg !29
entry:
   call void @llvm.lifetime.end(i64 4, i8* %0) #4, !dbg !44
   ret i32 %1, !dbg !45
 }
 
 
 ; CHECK: DW_TAG_variable
 ; CHECK-NEXT: DW_AT_location [DW_FORM_sec_offset]  (
 ; CHECK-NEXT:   [0x00000080, 0x00000084): DW_OP_reg1 AT_64
-; CHECK-NEXT:   [0x00000084, 0x00000098): DW_OP_breg29 SP_64+16, DW_OP_deref)
+; CHECK-NEXT:   [0x00000084, 0x00000098): DW_OP_breg29 SP_64+16)
 ; CHECK-NEXT: DW_AT_name [DW_FORM_strp]     ( .debug_str[0x0000006b] = "x")
 
 define i32 @f1(i32 signext %a, i32 signext %b, i32 signext %c, i32 signext %d, i32 signext %e) !dbg !15 {
 entry:
   %x = alloca i32, align 16
   tail call void @llvm.dbg.value(metadata i32 %a, metadata !17, metadata !DIExpression()), !dbg !46
   tail call void @llvm.dbg.value(metadata i32 %b, metadata !18, metadata !DIExpression()), !dbg !47
   tail call void @llvm.dbg.value(metadata i32 %c, metadata !19, metadata !DIExpression()), !dbg !48

llvm/trunk/test/DebugInfo/X86/bbjoin.ll

 ; RUN: llc -mtriple=x86_64-apple-macosx10.9.0 %s -stop-after=livedebugvars \
 ; RUN:   -o - | FileCheck %s
 ; Generated from:
 ; void g(int *);
 ; int f() {
 ;   int x = 23;
 ;   g(&x);
 ;   if (x == 42)
 ;     ++x;
 ;   return x; // check that x is not a constant here.
 ; }
 ; CHECK: ![[X:.*]] = !DILocalVariable(name: "x",
 ; CHECK: bb.0.entry:
 ; CHECK:   DBG_VALUE 23, debug-use $noreg, ![[X]],
-; CHECK:   DBG_VALUE debug-use $rsp, 0, ![[X]], !DIExpression(DW_OP_plus_uconst, 4, DW_OP_deref),
+; CHECK:   DBG_VALUE debug-use $rsp, debug-use $noreg, ![[X]], !DIExpression(DW_OP_plus_uconst, 4, DW_OP_deref),
 ; CHECK: bb.1.if.then:
 ; CHECK:   DBG_VALUE 43, debug-use $noreg, ![[X]],
 ; CHECK: bb.2.if.end:
 ; CHECK-NOT:  DBG_VALUE 23, debug-use $noreg, ![[X]],
 ; CHECK:   RETQ $eax
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"

llvm/trunk/test/DebugInfo/X86/dbg-value-const-byref.ll

 ;
 ; CHECK: .debug_info contents:
 ; CHECK: DW_TAG_variable
 ; CHECK-NOT: DW_TAG
 ; CHECK:     DW_AT_location {{.*}}({{.*}}
 ; CHECK-NEXT:  [0x{{0*.*}}, 0x[[C1:.*]]): DW_OP_consts +3
 ; CHECK-NEXT:      [0x[[C1]], 0x[[C2:.*]]): DW_OP_consts +7
 ; CHECK-NEXT:      [0x[[C2]], 0x[[R1:.*]]): DW_OP_reg0 RAX
-; CHECK-NEXT:      [0x[[R1]], 0x[[R2:.*]]): DW_OP_breg7 RSP+4, DW_OP_deref)
+; CHECK-NEXT:      [0x[[R1]], 0x[[R2:.*]]): DW_OP_breg7 RSP+4)
 ; CHECK-NOT: DW_TAG
 ; CHECK: DW_AT_name{{.*}}"i"
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9.0"
 
 ; Function Attrs: nounwind ssp uwtable
 define i32 @foo() #0 !dbg !4 {

llvm/trunk/test/DebugInfo/X86/dbg-value-frame-index.ll

 while.body:
   br i1 0, label %while.end, label %while.cond
 
 while.end:
   ret i1 true
 }
 
 ; CHECK-LABEL: test
-; CHECK:       #DEBUG_VALUE: test:w <- [DW_OP_plus_uconst 8] [$rsp+0]
+; To get the value of the variable, we need to do [$rsp+8], i.e:
+; CHECK:       #DEBUG_VALUE: test:w <- [DW_OP_plus_uconst 8, DW_OP_deref] $rsp
 ; DWARF:  DW_AT_location [DW_FORM_sec_offset] (
 ; DWARF-NEXT:   [{{.*}}, {{.*}}): DW_OP_breg7 RSP+8)
 
+; Note: A previous version of this test checked for `[DW_OP_plus_uconst 8] [$rsp+0]`,
+; which is incorrect, because it adds the stack offset after dereferencing the stack pointer.
+
 declare i1 @fn(i64*, i64*, i64*, i8*, i64, i64*, i32*, i8*)
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!2,!3}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 4.0.0", emissionKind: FullDebug)
 !1 = !DIFile(filename: "test.c", directory: "/")
 !2 = !{i32 2, !"Dwarf Version", i32 4}
 !3 = !{i32 2, !"Debug Info Version", i32 3}
 !4 = distinct !DISubprogram(name: "test", type: !10, unit: !0)
 !5 = !DILocalVariable(name: "w", scope: !4, type: !9)
-!6 = !DIExpression()
+!6 = !DIExpression(DW_OP_deref)
 !7 = !DILocation(line: 210, column: 12, scope: !4)
 !8 = !{!9}
 !9 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
 !10 = !DISubroutineType(types: !8)

llvm/trunk/test/DebugInfo/X86/debug-loc-frame.ll

 ; }
 ;
 ; CHECK:      .debug_info contents
 ; CHECK:      DW_TAG_subprogram
 ; CHECK-NOT:  NULL
 ; CHECK:      DW_TAG_variable
 ; CHECK:      DW_AT_location [DW_FORM_sec_offset] ({{.*}}
 ; CHECK-NEXT:   [{{0x.*}}, {{0x.*}}): DW_OP_reg0 RAX
-; CHECK-NEXT:   [{{0x.*}}, {{0x.*}}): DW_OP_breg7 RSP+4, DW_OP_deref)
+;
+; Note: This is a location, so we don't want an extra DW_OP_deref at the end.
+; LLDB gets the location right without it:
+;     Variable:  ... name = "val", type = "int", location =  [rsp+4], decl = frame.c:10
+; Adding the deref actually creates an invalid location:
+;     ... [rsp+4] DW_OP_deref
+;
+; CHECK-NEXT:   [{{0x.*}}, {{0x.*}}): DW_OP_breg7 RSP+4)
 ; CHECK-NEXT: DW_AT_name {{.*}}"val"
 
 ; ModuleID = 'frame.c'
 source_filename = "frame.c"
 
 @data = global i32 17, align 4, !dbg !0
 @sum = local_unnamed_addr global i32 0, align 4, !dbg !6
 @zero = local_unnamed_addr global i32 0, align 4, !dbg !9

llvm/trunk/test/Transforms/InstCombine/lower-dbg-declare.ll

+; RUN: opt -instcombine < %s -S | FileCheck %s
+
+; This tests dbg.declare lowering for CallInst users of an alloca. The
+; resulting dbg.value expressions should add a deref to the declare's expression.
+
+; Hand-reduced from this example (-g -Og -fsanitize=address):
+
+;   static volatile int sink;
+;   struct OneElementVector {
+;     int Element;
+;     OneElementVector(int Element) : Element(Element) { sink = Element; }
+;     bool empty() const { return false; }
+;   };
+;   using container = OneElementVector;
+;   static void escape(container &c) { sink = c.Element; }
+;   int main() {
+;     container d1 = {42};
+;     while (!d1.empty())
+;       escape(d1);
+;     return 0;
+;   }
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.13.0"
+
+%struct.OneElementVector = type { i32 }
+
+define i1 @escape(%struct.OneElementVector* %d1) {
+  ret i1 false
+}
+
+; CHECK-LABEL: @main
+define i32 @main() !dbg !15 {
+entry:
+  %d1 = alloca %struct.OneElementVector, align 4
+  %0 = bitcast %struct.OneElementVector* %d1 to i8*, !dbg !34
+
+; CHECK: dbg.value(metadata %struct.OneElementVector* [[var:%.*]], metadata !DIExpression(DW_OP_deref))
+; CHECK-NEXT: call i1 @escape
+  call void @llvm.dbg.declare(metadata %struct.OneElementVector* %d1, metadata !19, metadata !DIExpression()), !dbg !35
+  call i1 @escape(%struct.OneElementVector* %d1)
+  br label %while.cond, !dbg !37
+
+while.cond:                                       ; preds = %while.body, %entry
+; CHECK: dbg.value(metadata %struct.OneElementVector* [[var]], metadata !DIExpression(DW_OP_deref))
+; CHECK-NEXT: call i1 @escape
+  %call = call i1 @escape(%struct.OneElementVector* %d1), !dbg !38
+  %lnot = xor i1 %call, true, !dbg !39
+  br i1 %lnot, label %while.body, label %while.end, !dbg !37
+
+while.body:                                       ; preds = %while.cond
+; CHECK: dbg.value(metadata %struct.OneElementVector* [[var]], metadata !DIExpression(DW_OP_deref))
+; CHECK-NEXT: call i1 @escape
+  call i1 @escape(%struct.OneElementVector* %d1)
+  br label %while.cond, !dbg !37, !llvm.loop !42
+
+while.end:                                        ; preds = %while.cond
+  ret i32 0, !dbg !45
+}
+
+; CHECK-LABEL: @main2
+define i32 @main2() {
+entry:
+  %d1 = alloca %struct.OneElementVector, align 4
+  %0 = bitcast %struct.OneElementVector* %d1 to i8*, !dbg !34
+
+; CHECK: dbg.value(metadata %struct.OneElementVector* [[var:%.*]], metadata !DIExpression(DW_OP_lit0, DW_OP_mul, DW_OP_deref))
+; CHECK-NEXT: call i1 @escape
+  call void @llvm.dbg.declare(metadata %struct.OneElementVector* %d1, metadata !19, metadata !DIExpression(DW_OP_lit0, DW_OP_mul)), !dbg !35
+  call i1 @escape(%struct.OneElementVector* %d1)
+  br label %while.cond, !dbg !37
+
+while.cond:                                       ; preds = %while.body, %entry
+; CHECK: dbg.value(metadata %struct.OneElementVector* [[var]], metadata !DIExpression(DW_OP_lit0, DW_OP_mul, DW_OP_deref))
+; CHECK-NEXT: call i1 @escape
+  %call = call i1 @escape(%struct.OneElementVector* %d1), !dbg !38
+  %lnot = xor i1 %call, true, !dbg !39
+  br i1 %lnot, label %while.body, label %while.end, !dbg !37
+
+while.body:                                       ; preds = %while.cond
+; CHECK: dbg.value(metadata %struct.OneElementVector* [[var]], metadata !DIExpression(DW_OP_lit0, DW_OP_mul, DW_OP_deref))
+; CHECK-NEXT: call i1 @escape
+  call i1 @escape(%struct.OneElementVector* %d1)
+  br label %while.cond, !dbg !37, !llvm.loop !42
+
+while.end:                                        ; preds = %while.cond
+  ret i32 0, !dbg !45
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!2}
+!llvm.asan.globals = !{!8}
+!llvm.module.flags = !{!10, !11, !12, !13}
+!llvm.ident = !{!14}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "sink", linkageName: "_ZL4sink", scope: !2, file: !3, line: 1, type: !6, isLocal: true, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 7.0.0 (trunk 337207) (llvm/trunk 337204)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "test.cc", directory: "/Users/vsk/src/builds/llvm.org-master-RA")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !7)
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{}
+!9 = !{!"test.cc", i32 1, i32 21}
+!10 = !{i32 2, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{i32 1, !"wchar_size", i32 4}
+!13 = !{i32 7, !"PIC Level", i32 2}
+!14 = !{!"clang version 7.0.0 (trunk 337207) (llvm/trunk 337204)"}
+!15 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 18, type: !16, isLocal: false, isDefinition: true, scopeLine: 18, flags: DIFlagPrototyped, isOptimized: true, unit: !2, retainedNodes: !18)
+!16 = !DISubroutineType(types: !17)
+!17 = !{!7}
+!18 = !{!19}
+!19 = !DILocalVariable(name: "d1", scope: !15, file: !3, line: 21, type: !20)
+!20 = !DIDerivedType(tag: DW_TAG_typedef, name: "container", file: !3, line: 12, baseType: !21)
+!21 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "OneElementVector", file: !3, line: 3, size: 32, flags: DIFlagTypePassByValue, elements: !22, identifier: "_ZTS16OneElementVector")
+!22 = !{!23, !24, !28}
+!23 = !DIDerivedType(tag: DW_TAG_member, name: "Element", scope: !21, file: !3, line: 4, baseType: !7, size: 32)
+!24 = !DISubprogram(name: "OneElementVector", scope: !21, file: !3, line: 6, type: !25, isLocal: false, isDefinition: false, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: true)
+!25 = !DISubroutineType(types: !26)
+!26 = !{null, !27, !7}
+!27 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !21, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!28 = !DISubprogram(name: "empty", linkageName: "_ZNK16OneElementVector5emptyEv", scope: !21, file: !3, line: 8, type: !29, isLocal: false, isDefinition: false, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true)
+!29 = !DISubroutineType(types: !30)
+!30 = !{!31, !32}
+!31 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
+!32 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !33, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!33 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !21)
+!34 = !DILocation(line: 21, column: 3, scope: !15)
+!35 = !DILocation(line: 21, column: 13, scope: !15)
+!36 = !DILocation(line: 21, column: 18, scope: !15)
+!37 = !DILocation(line: 22, column: 3, scope: !15)
+!38 = !DILocation(line: 22, column: 14, scope: !15)
+!39 = !DILocation(line: 22, column: 10, scope: !15)
+!40 = !DILocation(line: 23, column: 5, scope: !41)
+!41 = distinct !DILexicalBlock(scope: !15, file: !3, line: 22, column: 23)
+!42 = distinct !{!42, !37, !43}
+!43 = !DILocation(line: 24, column: 3, scope: !15)
+!44 = !DILocation(line: 26, column: 1, scope: !15)
+!45 = !DILocation(line: 25, column: 3, scope: !15)
+!46 = distinct !DISubprogram(name: "OneElementVector", linkageName: "_ZN16OneElementVectorC1Ei", scope: !21, file: !3, line: 6, type: !25, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: true, unit: !2, declaration: !24, retainedNodes: !47)
+!47 = !{!48, !50}
+!48 = !DILocalVariable(name: "this", arg: 1, scope: !46, type: !49, flags: DIFlagArtificial | DIFlagObjectPointer)
+!49 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !21, size: 64)
+!50 = !DILocalVariable(name: "Element", arg: 2, scope: !46, file: !3, line: 6, type: !7)
+!51 = !DILocation(line: 0, scope: !46)
+!52 = !DILocation(line: 6, column: 24, scope: !46)
+!53 = !DILocation(line: 6, column: 52, scope: !46)
+!54 = !DILocation(line: 6, column: 70, scope: !46)
+!55 = distinct !DISubprogram(name: "empty", linkageName: "_ZNK16OneElementVector5emptyEv", scope: !21, file: !3, line: 8, type: !29, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, unit: !2, declaration: !28, retainedNodes: !56)
+!56 = !{!57}
+!57 = !DILocalVariable(name: "this", arg: 1, scope: !55, type: !58, flags: DIFlagArtificial | DIFlagObjectPointer)
+!58 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !33, size: 64)
+!59 = !DILocation(line: 0, scope: !55)
+!60 = !DILocation(line: 8, column: 24, scope: !55)
+!61 = distinct !DISubprogram(name: "escape", linkageName: "_ZL6escapeR16OneElementVector", scope: !3, file: !3, line: 14, type: !62, isLocal: true, isDefinition: true, scopeLine: 14, flags: DIFlagPrototyped, isOptimized: true, unit: !2, retainedNodes: !65)
+!62 = !DISubroutineType(types: !63)
+!63 = !{null, !64}
+!64 = !DIDerivedType(tag: DW_TAG_reference_type, baseType: !20, size: 64)
+!65 = !{!66}
+!66 = !DILocalVariable(name: "c", arg: 1, scope: !61, file: !3, line: 14, type: !64)
+!67 = !DILocation(line: 14, column: 31, scope: !61)
+!68 = !DILocation(line: 15, column: 12, scope: !61)
+!69 = !{!70, !71, i64 0}
+!70 = !{!"_ZTS16OneElementVector", !71, i64 0}
+!71 = !{!"int", !72, i64 0}
+!72 = !{!"omnipotent char", !73, i64 0}
+!73 = !{!"Simple C++ TBAA"}
+!74 = !DILocation(line: 15, column: 8, scope: !61)
+!75 = !{!71, !71, i64 0}
+!76 = !DILocation(line: 16, column: 1, scope: !61)
+!77 = distinct !DISubprogram(name: "OneElementVector", linkageName: "_ZN16OneElementVectorC2Ei", scope: !21, file: !3, line: 6, type: !25, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: true, unit: !2, declaration: !24, retainedNodes: !78)
+!78 = !{!79, !80}
+!79 = !DILocalVariable(name: "this", arg: 1, scope: !77, type: !49, flags: DIFlagArtificial | DIFlagObjectPointer)
+!80 = !DILocalVariable(name: "Element", arg: 2, scope: !77, file: !3, line: 6, type: !7)
+!81 = !DILocation(line: 0, scope: !77)
+!82 = !DILocation(line: 6, column: 24, scope: !77)
+!83 = !DILocation(line: 6, column: 35, scope: !77)
+!84 = !DILocation(line: 6, column: 59, scope: !85)
+!85 = distinct !DILexicalBlock(scope: !77, file: !3, line: 6, column: 52)
+!86 = !DILocation(line: 6, column: 70, scope: !77)

llvm/trunk/unittests/IR/MetadataTest.cpp

TEST_F(DIExpressionTest, get) {
   EXPECT_EQ(0u, N->getElement(4));
 
   TempDIExpression Temp = N->clone();
   EXPECT_EQ(N, MDNode::replaceWithUniqued(std::move(Temp)));
 
   // Test DIExpression::prepend().
   uint64_t Elts0[] = {dwarf::DW_OP_LLVM_fragment, 0, 32};
   auto *N0 = DIExpression::get(Context, Elts0);
-  N0 = DIExpression::prepend(N0, true, 64, true, true);
+  auto *N0WithPrependedOps = DIExpression::prepend(N0, true, 64, true, true);
   uint64_t Elts1[] = {dwarf::DW_OP_deref,
                       dwarf::DW_OP_plus_uconst, 64,
                       dwarf::DW_OP_deref,
                       dwarf::DW_OP_stack_value,
                       dwarf::DW_OP_LLVM_fragment, 0, 32};
   auto *N1 = DIExpression::get(Context, Elts1);
-  EXPECT_EQ(N0, N1);
+  EXPECT_EQ(N0WithPrependedOps, N1);
+
+  // Test DIExpression::append().
+  uint64_t Elts2[] = {dwarf::DW_OP_deref, dwarf::DW_OP_plus_uconst, 64,
+                      dwarf::DW_OP_deref, dwarf::DW_OP_stack_value};
+  auto *N2 = DIExpression::append(N0, Elts2);
+  EXPECT_EQ(N0WithPrependedOps, N2);
 }
 
 TEST_F(DIExpressionTest, isValid) {
 #define EXPECT_VALID(...)                                                      \
   do {                                                                         \
     uint64_t Elements[] = {__VA_ARGS__};                                       \
     EXPECT_TRUE(DIExpression::get(Context, Elements)->isValid());              \
   } while (false)