[DebugInfo] Fix fragment offset emission order for symbol locations

[dstenb]

Summary

When emitting the location for a global variable with fragmented debug
expressions, make sure that the offset pieces, which represent
optimized-out parts of the variable, are emitted before their succeeding
fragments' expressions. Previously, if the succeeding fragment's
location was a symbol, the offset piece was emitted after, rather than
before, that symbol's expression. This effectively meant that the symbols
were associated with the wrong parts of the variable.

This fixes PR36085.

[dstenb]

Added reviewers.

[JDevlieghere]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

232 ↗

(On Diff #131414)

Just from looking at the code, as neither Expr nor DwarfExpr are used in the if-clause above, can we move up this part too?

[dstenb]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

232 ↗

(On Diff #131414)

The current order needs to be kept for the case where Expr is a DW_OP_LLVM_fragment expression. If we are dealing with a fragment expression, and we emit a symbol location in the if (Global) clause, then the piece resulting from the fragment expression must be emitted after that. This situation can be seen for the fragments corresponding to the bar and qux fields in the attached reproducer.

I don't if there are other cases where the emission order of the symbol location and Expr matters.

[aprantl]

Good catch!

[uabelho]

I submitted this for David.

[JDevlieghere]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

232 ↗

(On Diff #131414)

Thanks for the explanation!

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (!Global && (!Expr || !Expr->isConstant()))
       continue;
 
     if (!Loc) {
       addToAccelTable = true;
       Loc = new (DIEValueAllocator) DIELoc;
       DwarfExpr = llvm::make_unique<DIEDwarfExpression>(*Asm, *this, *Loc);
     }
 
+    if (Expr)
+      DwarfExpr->addFragmentOffset(Expr);
+
     if (Global) {
       const MCSymbol *Sym = Asm->getSymbol(Global);
       if (Global->isThreadLocal()) {
         if (Asm->TM.Options.EmulatedTLS) {
           // TODO: add debug info for emulated thread local mode.
         } else {
           // FIXME: Make this work with -gsplit-dwarf.
           unsigned PointerSize = Asm->getDataLayout().getPointerSize();
if (Global) {
                   DD->useGNUTLSOpcode() ? dwarf::DW_OP_GNU_push_tls_address
                                         : dwarf::DW_OP_form_tls_address);
         }
       } else {
         DD->addArangeLabel(SymbolCU(this, Sym));
         addOpAddress(*Loc, Sym);
       }
     }
-    if (Expr) {
-      DwarfExpr->addFragmentOffset(Expr);
+    if (Expr)
       DwarfExpr->addExpression(Expr);
-    }
-  }
   if (Loc)
     addBlock(*VariableDIE, dwarf::DW_AT_location, DwarfExpr->finalize());
 
   if (DD->useAllLinkageNames())
     addLinkageName(*VariableDIE, GV->getLinkageName());
 
   if (addToAccelTable) {
     DD->addAccelName(GV->getName(), *VariableDIE);

llvm/trunk/test/DebugInfo/X86/fragment-offset-order.ll

+; RUN: llc -dwarf-version=4 -filetype=obj -O0 \
+; RUN:     -mtriple=x86_64-unknown-linux-gnu < %s \
+; RUN:     | llvm-dwarfdump -v - | FileCheck %s
+
+; Verify that the empty DW_OP_piece operations that are created for the
+; optimized-out struct fields `foo' and `baz' are emitted before their
+; succeeding fields' symbol location expressions.
+
+; Test file based on foo.c:
+;
+; struct {
+;   long foo;
+;   void *bar;
+;   long baz;
+;   void *qux;
+; } static var;
+; 
+; void *ptr;
+; 
+; int main() {
+;   if (var.foo == 0)
+;     var.bar = var.qux = ptr;
+;   return 0;
+; }
+;
+; which was built using:
+;
+; clang -O0 -g2 -S -emit-llvm foo.c -o foo.ll
+; opt -S -globalopt foo.ll -o foo.opt.ll
+
+; CHECK: DW_AT_name {{.*}}"var"
+; CHECK: DW_AT_location [DW_FORM_exprloc] (DW_OP_piece 0x8, DW_OP_addr 0x0, DW_OP_piece 0x8, DW_OP_piece 0x8, DW_OP_addr 0x0, DW_OP_piece 0x8)
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@var.1 = internal unnamed_addr global i8* null, align 8, !dbg !0
+@var.3 = internal unnamed_addr global i8* null, align 8, !dbg !15
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!16, !17}
+!llvm.ident = !{!18}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_LLVM_fragment, 64, 64))
+!1 = distinct !DIGlobalVariable(name: "var", scope: !2, file: !3, line: 15, type: !7, isLocal: true, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 7.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "foo.c", directory: "/")
+!4 = !{}
+!5 = !{!6}
+!6 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!7 = distinct !DICompositeType(tag: DW_TAG_structure_type, file: !3, line: 10, size: 256, elements: !8)
+!8 = !{!9, !11, !13, !14}
+!9 = !DIDerivedType(tag: DW_TAG_member, name: "foo", scope: !7, file: !3, line: 11, baseType: !10, size: 64)
+!10 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+!11 = !DIDerivedType(tag: DW_TAG_member, name: "bar", scope: !7, file: !3, line: 12, baseType: !12, size: 64, offset: 64)
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
+!13 = !DIDerivedType(tag: DW_TAG_member, name: "baz", scope: !7, file: !3, line: 13, baseType: !10, size: 64, offset: 128)
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "qux", scope: !7, file: !3, line: 14, baseType: !12, size: 64, offset: 192)
+!15 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_LLVM_fragment, 192, 64))
+!16 = !{i32 2, !"Dwarf Version", i32 4}
+!17 = !{i32 2, !"Debug Info Version", i32 3}
+!18 = !{!"clang version 7.0.0"}