[llvm] 85460c4 - [DebugInfo] Do not emit entry values for composite locations
David Stenberg via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 1 01:51:36 PDT 2020
Author: David Stenberg
Date: 2020-07-01T10:50:55+02:00
New Revision: 85460c4ea273784dd45da558ad9a6f13a79b2d91
URL: https://github.com/llvm/llvm-project/commit/85460c4ea273784dd45da558ad9a6f13a79b2d91
DIFF: https://github.com/llvm/llvm-project/commit/85460c4ea273784dd45da558ad9a6f13a79b2d91.diff
LOG: [DebugInfo] Do not emit entry values for composite locations
Summary:
This is a fix for PR45009.
When working on D67492 I made DwarfExpression emit a single
DW_OP_entry_value operation covering the whole composite location
description that is produced if a register does not have a valid DWARF
number, and is instead composed of multiple register pieces. Looking
closer at the standard, this appears to not be valid DWARF. A
DW_OP_entry_value operation's block can only be a DWARF expression or a
register location description, so it appears to not be valid for it to
hold a composite location description like that.
See DWARFv5 sec. 2.5.1.7:
"The DW_OP_entry_value operation pushes the value that the described
location held upon entering the current subprogram. It has two
operands: an unsigned LEB128 length, followed by a block containing a
DWARF expression or a register location description (see Section
2.6.1.1.3 on page 39)."
Here is a dwarf-discuss mail thread regarding this:
http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2020-March/004610.html
There was not a strong consensus reached there, but people seem to lean
towards that operations specified under 2.6 (e.g. DW_OP_piece) may not
be part of a DWARF expression, and thus the DW_OP_entry_value operation
can't contain those.
Perhaps we instead want to emit a entry value operation per each
DW_OP_reg* operation, e.g.:
- DW_OP_entry_value(DW_OP_regx sub_reg0),
DW_OP_stack_value,
DW_OP_piece 8,
- DW_OP_entry_value(DW_OP_regx sub_reg1),
DW_OP_stack_value,
DW_OP_piece 8,
[...]
The question then becomes how the call site should look; should a
composite location description be emitted there, and we then leave it up
to the debugger to match those two composite location descriptions?
Another alternative could be to emit a call site parameter entry for
each sub-register, but firstly I'm unsure if that is even valid DWARF,
and secondly it seems like that would complicate the collection of call
site values quite a bit. As far as I can tell GCC does not emit any
entry values / call sites in these cases, so we do not have something to
compare with, but the former seems like the more reasonable approach.
Currently when trying to emit a call site entry for a parameter composed
of multiple DWARF registers a (DwarfRegs.size() == 1) assert is
triggered in addMachineRegExpression(). Until the call site
representation is figured out, and until there is use for these entry
values in practice, this commit simply stops the invalid DWARF from
being emitted.
Reviewers: djtodoro, vsk, aprantl
Reviewed By: djtodoro, vsk
Subscribers: jyknight, hiraditya, fedor.sergeev, jrtc27, llvm-commits
Tags: #debug-info, #llvm
Differential Revision: https://reviews.llvm.org/D75270
Added:
Modified:
llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
index 7b64c2238bd6..d4762121d105 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
@@ -237,8 +237,17 @@ bool DwarfExpression::addMachineRegExpression(const TargetRegisterInfo &TRI,
// If the register can only be described by a complex expression (i.e.,
// multiple subregisters) it doesn't safely compose with another complex
// expression. For example, it is not possible to apply a DW_OP_deref
- // operation to multiple DW_OP_pieces.
- if (HasComplexExpression && DwarfRegs.size() > 1) {
+ // operation to multiple DW_OP_pieces, since composite location descriptions
+ // do not push anything on the DWARF stack.
+ //
+ // DW_OP_entry_value operations can only hold a DWARF expression or a
+ // register location description, so we can't emit a single entry value
+ // covering a composite location description. In the future we may want to
+ // emit entry value operations for each register location in the composite
+ // location, but until that is supported do not emit anything.
+ if ((HasComplexExpression || IsEmittingEntryValue) && DwarfRegs.size() > 1) {
+ if (IsEmittingEntryValue)
+ cancelEntryValue();
DwarfRegs.clear();
LocationKind = Unknown;
return false;
@@ -349,7 +358,6 @@ void DwarfExpression::beginEntryValueExpression(
assert(Op->getArg(0) == 1 &&
"Can currently only emit entry values covering a single operation");
- emitOp(CU.getDwarf5OrGNULocationAtom(dwarf::DW_OP_entry_value));
IsEmittingEntryValue = true;
enableTemporaryBuffer();
}
@@ -358,6 +366,8 @@ void DwarfExpression::finalizeEntryValue() {
assert(IsEmittingEntryValue && "Entry value not open?");
disableTemporaryBuffer();
+ emitOp(CU.getDwarf5OrGNULocationAtom(dwarf::DW_OP_entry_value));
+
// Emit the entry value's size operand.
unsigned Size = getTemporaryBufferSize();
emitUnsigned(Size);
@@ -368,6 +378,18 @@ void DwarfExpression::finalizeEntryValue() {
IsEmittingEntryValue = false;
}
+void DwarfExpression::cancelEntryValue() {
+ assert(IsEmittingEntryValue && "Entry value not open?");
+ disableTemporaryBuffer();
+
+ // The temporary buffer can't be emptied, so for now just assert that nothing
+ // has been emitted to it.
+ assert(getTemporaryBufferSize() == 0 &&
+ "Began emitting entry value block before cancelling entry value");
+
+ IsEmittingEntryValue = false;
+}
+
unsigned DwarfExpression::getOrCreateBaseType(unsigned BitSize,
dwarf::TypeKind Encoding) {
// Reuse the base_type if we already have one in this CU otherwise we
@@ -401,6 +423,10 @@ static bool isMemoryLocation(DIExpressionCursor ExprCursor) {
void DwarfExpression::addExpression(DIExpressionCursor &&ExprCursor,
unsigned FragmentOffsetInBits) {
+ // Entry values can currently only cover the initial register location,
+ // and not any other parts of the following DWARF expression.
+ assert(!IsEmittingEntryValue && "Can't emit entry value around expression");
+
// If we need to mask out a subregister, do it now, unless the next
// operation would emit an OpPiece anyway.
auto N = ExprCursor.peek();
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
index 42be827cd5a0..757b17511453 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
@@ -276,6 +276,9 @@ class DwarfExpression {
/// DWARF block which has been emitted to the temporary buffer.
void finalizeEntryValue();
+ /// Cancel the emission of an entry value.
+ void cancelEntryValue();
+
~DwarfExpression() = default;
public:
diff --git a/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll b/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
index 0af5619b75d7..096df49110c5 100644
--- a/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
+++ b/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
@@ -1,11 +1,14 @@
; RUN: llc -debug-entry-values -filetype=asm -o - %s | FileCheck %s
-; Verify that the entry value covers both of the DW_OP_regx pieces. Previously
-; the size operand of the entry value would be hardcoded to one.
+; The q0 register does not have a DWARF register number, and is instead emitted
+; as a composite location description with two sub-registers. Previously we
+; emitted a single DW_OP_entry_value wrapping that whole composite location
+; description, but that is not valid DWARF; DW_OP_entry_value operations can
+; only hold DWARF expressions and register location descriptions.
;
-; XXX: Is this really what should be emitted, or should we instead emit one
-; entry value operation per DW_OP_regx? GDB can currently not understand
-; entry values containing complex expressions like this.
+; In the future we may want to emit a composite location description where each
+; DW_OP_regx operation is wrapped in an entry value operation, but for now
+; just verify that no invalid DWARF is emitted.
target datalayout = "E-m:e-i64:64-n32:64-S128"
target triple = "sparc64"
@@ -20,8 +23,11 @@ target triple = "sparc64"
; return 123;
; }
-; CHECK: .byte 243 ! DW_OP_GNU_entry_value
-; CHECK-NEXT: .byte 8 ! 8
+; Verify that we got an entry value in the DIExpression...
+; CHECK: DEBUG_VALUE: foo:p <- [DW_OP_LLVM_entry_value 1] $q0
+
+; ... but that no entry value location was emitted:
+; CHECK: .half 8 ! Loc expr size
; CHECK-NEXT: .byte 144 ! sub-register DW_OP_regx
; CHECK-NEXT: .byte 72 ! 72
; CHECK-NEXT: .byte 147 ! DW_OP_piece
@@ -30,7 +36,8 @@ target triple = "sparc64"
; CHECK-NEXT: .byte 73 ! 73
; CHECK-NEXT: .byte 147 ! DW_OP_piece
; CHECK-NEXT: .byte 8 ! 8
-; CHECK-NEXT: .byte 159 ! DW_OP_stack_value
+; CHECK-NEXT: .xword 0
+; CHECK-NEXT: .xword 0
@global = common global fp128 0xL00000000000000000000000000000000, align 16, !dbg !0
More information about the llvm-commits
mailing list