[DebugInfo] Fix handling DW_OP_call_ref in DWARF64 units.

[ikudrin]

Summary

DW_OP_call_ref is the only operation that has an operand which depends on the DWARF format. The patch fixes handling that operation in DWARF64 units.

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

135

This comment is correct now, but without verification, this is bound to break eventually. Would it be possible to just pass the correct value here, or, alternatively, make the format optional and pass an llvm::None value here and then produce an error in DWARFExpression if no format was specified when one is needed?

[ikudrin]

• The DWARF format was made optional in DWARFExpression.
• Added a test to check that DW_OP_call_ref is not supported in location lists. That checks the code path when the DWARF format for DWARFExpression is not specified.

@aprantl, how do you find this variant?

[aprantl]

Looks good!

llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h

 //===--- DWARFExpression.h - DWARF Expression handling ----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_DWARFEXPRESSION_H
 #define LLVM_DEBUGINFO_DWARFEXPRESSION_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/Support/DataExtractor.h"
 
 namespace llvm {
 class DWARFUnit;
 class MCRegisterInfo;
 class raw_ostream;
 
 class DWARFExpression {
private:
     uint64_t OperandEndOffsets[2];
 
   public:
     Description &getDescription() { return Desc; }
     uint8_t getCode() { return Opcode; }
     uint64_t getRawOperand(unsigned Idx) { return Operands[Idx]; }
     uint64_t getOperandEndOffset(unsigned Idx) { return OperandEndOffsets[Idx]; }
     uint64_t getEndOffset() { return EndOffset; }
-    bool extract(DataExtractor Data, uint8_t AddressSize, uint64_t Offset);
+    bool extract(DataExtractor Data, uint8_t AddressSize, uint64_t Offset,
+                 Optional<dwarf::DwarfFormat> Format);
     bool isError() { return Error; }
     bool print(raw_ostream &OS, const DWARFExpression *Expr,
                const MCRegisterInfo *RegInfo, DWARFUnit *U, bool isEH);
     bool verify(DWARFUnit *U);
   };
 
   /// An iterator to go through the expression operations.
   class iterator
       : public iterator_facade_base<iterator, std::forward_iterator_tag,
                                     Operation> {
     friend class DWARFExpression;
     const DWARFExpression *Expr;
     uint64_t Offset;
     Operation Op;
     iterator(const DWARFExpression *Expr, uint64_t Offset)
         : Expr(Expr), Offset(Offset) {
       Op.Error =
           Offset >= Expr->Data.getData().size() ||
-          !Op.extract(Expr->Data, Expr->AddressSize, Offset);
+          !Op.extract(Expr->Data, Expr->AddressSize, Offset, Expr->Format);
     }
 
   public:
     class Operation &operator++() {
       Offset = Op.isError() ? Expr->Data.getData().size() : Op.EndOffset;
       Op.Error =
           Offset >= Expr->Data.getData().size() ||
-          !Op.extract(Expr->Data, Expr->AddressSize, Offset);
+          !Op.extract(Expr->Data, Expr->AddressSize, Offset, Expr->Format);
       return Op;
     }
 
     class Operation &operator*() {
       return Op;
     }
 
     // Comparison operators are provided out of line.
     friend bool operator==(const iterator &, const iterator &);
   };
 
-  DWARFExpression(DataExtractor Data, uint8_t AddressSize)
-      : Data(Data), AddressSize(AddressSize) {
+  DWARFExpression(DataExtractor Data, uint8_t AddressSize,
+                  Optional<dwarf::DwarfFormat> Format = None)
+      : Data(Data), AddressSize(AddressSize), Format(Format) {
     assert(AddressSize == 8 || AddressSize == 4 || AddressSize == 2);
   }
 
   iterator begin() const { return iterator(this, 0); }
   iterator end() const { return iterator(this, Data.getData().size()); }
 
   void print(raw_ostream &OS, const MCRegisterInfo *RegInfo, DWARFUnit *U,
              bool IsEH = false) const;
 
   bool verify(DWARFUnit *U);
 
 private:
   DataExtractor Data;
   uint8_t AddressSize;
+  Optional<dwarf::DwarfFormat> Format;
 };
 
 inline bool operator==(const DWARFExpression::iterator &LHS,
                        const DWARFExpression::iterator &RHS) {
   return LHS.Expr == RHS.Expr && LHS.Offset == RHS.Offset;
 }
 
 inline bool operator!=(const DWARFExpression::iterator &LHS,
                        const DWARFExpression::iterator &RHS) {
   return !(LHS == RHS);
 }
 }
 #endif

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

void DwarfDebug::emitDebugLocEntry(ByteStreamer &Streamer,
   // DwarfExpression::addExpression) so for those ops (e.g. DW_OP_convert) that
   // need to reference a base_type DIE the offset of that DIE is not yet known.
   // To deal with this we instead insert a placeholder early and then extract
   // it here and replace it with the real reference.
   unsigned PtrSize = Asm->MAI->getCodePointerSize();
   DWARFDataExtractor Data(StringRef(DebugLocs.getBytes(Entry).data(),
                                     DebugLocs.getBytes(Entry).size()),
                           Asm->getDataLayout().isLittleEndian(), PtrSize);
-  DWARFExpression Expr(Data, PtrSize);
+  DWARFExpression Expr(Data, PtrSize, Asm->OutContext.getDwarfFormat());
 
   using Encoding = DWARFExpression::Operation::Encoding;
   uint64_t Offset = 0;
   for (auto &Op : Expr) {
     assert(Op.getCode() != dwarf::DW_OP_const_type &&
            "3 operand ops not yet supported");
     Streamer.EmitInt8(Op.getCode(), Comment != End ? *(Comment++) : "");
     Offset++;

llvm/lib/DWARFLinker/DWARFLinker.cpp

unsigned DWARFLinker::DIECloner::cloneBlockAttribute(
   SmallVector<uint8_t, 32> Buffer;
   ArrayRef<uint8_t> Bytes = *Val.getAsBlock();
   if (DWARFAttribute::mayHaveLocationDescription(AttrSpec.Attr) &&
       (Val.isFormClass(DWARFFormValue::FC_Block) ||
        Val.isFormClass(DWARFFormValue::FC_Exprloc))) {
     DWARFUnit &OrigUnit = Unit.getOrigUnit();
     DataExtractor Data(StringRef((const char *)Bytes.data(), Bytes.size()),
                        IsLittleEndian, OrigUnit.getAddressByteSize());
-    DWARFExpression Expr(Data, OrigUnit.getAddressByteSize());
+    DWARFExpression Expr(Data, OrigUnit.getAddressByteSize(),
+                         OrigUnit.getFormParams().Format);
     cloneExpression(Data, Expr, File, Unit, Buffer);
     Bytes = Buffer;
   }
   for (auto Byte : Bytes)
     Attr->addValue(DIEAlloc, static_cast<dwarf::Attribute>(0),
                    dwarf::DW_FORM_data1, DIEInteger(Byte));
 
   // FIXME: If DIEBlock and DIELoc just reuses the Size field of
if (!Linker.Options.NoOutput) {
 
       Linker.patchRangesForUnit(*CurrentUnit, DwarfContext, File);
       auto ProcessExpr = [&](StringRef Bytes,
                              SmallVectorImpl<uint8_t> &Buffer) {
         DWARFUnit &OrigUnit = CurrentUnit->getOrigUnit();
         DataExtractor Data(Bytes, IsLittleEndian,
                            OrigUnit.getAddressByteSize());
         cloneExpression(Data,
-                        DWARFExpression(Data, OrigUnit.getAddressByteSize()),
+                        DWARFExpression(Data, OrigUnit.getAddressByteSize(),
+                                        OrigUnit.getFormParams().Format),
                         File, *CurrentUnit, Buffer);
       };
       Emitter->emitLocationsForUnit(*CurrentUnit, DwarfContext, ProcessExpr);
     }
   }
 
   if (!Linker.Options.NoOutput) {
     assert(Emitter);

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

if (Primary) {
           break;
         }
         case DW_CFA_def_cfa_expression: {
           uint32_t ExprLength = Data.getULEB128(Offset);
           addInstruction(Opcode, 0);
           DataExtractor Extractor(
               Data.getData().slice(*Offset, *Offset + ExprLength),
               Data.isLittleEndian(), Data.getAddressSize());
+          // Note. We do not pass the DWARF format to DWARFExpression, because
+          // DW_OP_call_ref, the only operation which depends on the format, is
+          // prohibited in call frame instructions, see sec. 6.4.2 in DWARFv5.
           Instructions.back().Expression =
               DWARFExpression(Extractor, Data.getAddressSize());
           *Offset += ExprLength;
           break;
         }
         case DW_CFA_expression:
         case DW_CFA_val_expression: {
           auto RegNum = Data.getULEB128(Offset);
           auto BlockLength = Data.getULEB128(Offset);
           addInstruction(Opcode, RegNum, 0);
           DataExtractor Extractor(
               Data.getData().slice(*Offset, *Offset + BlockLength),
               Data.isLittleEndian(), Data.getAddressSize());
+          // Note. We do not pass the DWARF format to DWARFExpression, because
+          // DW_OP_call_ref, the only operation which depends on the format, is
+          // prohibited in call frame instructions, see sec. 6.4.2 in DWARFv5.
           Instructions.back().Expression =
               DWARFExpression(Extractor, Data.getAddressSize());
           *Offset += BlockLength;
           break;
         }
       }
     }
   }

llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp

default:
     llvm_unreachable("unreachable locations list kind");
   }
 }
 
 static void dumpExpression(raw_ostream &OS, ArrayRef<uint8_t> Data,
                            bool IsLittleEndian, unsigned AddressSize,
                            const MCRegisterInfo *MRI, DWARFUnit *U) {
   DWARFDataExtractor Extractor(Data, IsLittleEndian, AddressSize);
+  // Note. We do not pass any format to DWARFExpression, even if the
+  // corresponding unit is known. For now, there is only one operation,
+  // DW_OP_call_ref, which depends on the format; it is rarely used, and
+  // is unexpected in location tables.
   DWARFExpression(Extractor, AddressSize).print(OS, MRI, U);
 }
 
 bool DWARFLocationTable::dumpLocationList(uint64_t *Offset, raw_ostream &OS,
                                           Optional<SectionedAddress> BaseAddr,
                                           const MCRegisterInfo *MRI,
                                           const DWARFObject &Obj, DWARFUnit *U,
                                           DIDumpOptions DumpOpts,

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

static void dumpLocation(raw_ostream &OS, DWARFFormValue &FormValue,
                          DIDumpOptions DumpOpts) {
   DWARFContext &Ctx = U->getContext();
   const MCRegisterInfo *MRI = Ctx.getRegisterInfo();
   if (FormValue.isFormClass(DWARFFormValue::FC_Block) ||
       FormValue.isFormClass(DWARFFormValue::FC_Exprloc)) {
     ArrayRef<uint8_t> Expr = *FormValue.getAsBlock();
     DataExtractor Data(StringRef((const char *)Expr.data(), Expr.size()),
                        Ctx.isLittleEndian(), 0);
-    DWARFExpression(Data, U->getAddressByteSize()).print(OS, MRI, U);
+    DWARFExpression(Data, U->getAddressByteSize(), U->getFormParams().Format)
+        .print(OS, MRI, U);
     return;
   }
 
   if (FormValue.isFormClass(DWARFFormValue::FC_SectionOffset)) {
     uint64_t Offset = *FormValue.getAsSectionOffset();
 
     if (FormValue.getForm() == DW_FORM_loclistx) {
       FormValue.dump(OS, DumpOpts);

llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp

 //===-- DWARFExpression.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/DWARF/DWARFExpression.h"
 #include "llvm/DebugInfo/DWARF/DWARFUnit.h"
-#include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/Format.h"
 #include <cassert>
 #include <cstdint>
 #include <vector>
 
 using namespace llvm;
 using namespace dwarf;
static DWARFExpression::Operation::Description getOpDesc(unsigned OpCode) {
   static DescVector Descriptions = getDescriptions();
   // Handle possible corrupted or unsupported operation.
   if (OpCode >= Descriptions.size())
     return {};
   return Descriptions[OpCode];
 }
 
 bool DWARFExpression::Operation::extract(DataExtractor Data,
-                                         uint8_t AddressSize, uint64_t Offset) {
+                                         uint8_t AddressSize, uint64_t Offset,
+                                         Optional<DwarfFormat> Format) {
   Opcode = Data.getU8(&Offset);
 
   Desc = getOpDesc(Opcode);
   if (Desc.Version == Operation::DwarfNA) {
     EndOffset = Offset;
     return false;
   }
 
case Operation::Size4:
       break;
     case Operation::Size8:
       Operands[Operand] = Data.getU64(&Offset);
       break;
     case Operation::SizeAddr:
       Operands[Operand] = Data.getUnsigned(&Offset, AddressSize);
       break;
     case Operation::SizeRefAddr:
-      // TODO: Add support for 64-bit DWARF format.
-      Operands[Operand] = Data.getU32(&Offset);
+      if (!Format)
+        return false;
+      Operands[Operand] =
+          Data.getUnsigned(&Offset, dwarf::getDwarfOffsetByteSize(*Format));
       break;
     case Operation::SizeLEB:
       if (Signed)
         Operands[Operand] = Data.getSLEB128(&Offset);
       else
         Operands[Operand] = Data.getULEB128(&Offset);
       break;
     case Operation::BaseTypeRef:

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

case DW_AT_stmt_list:
     ReportError("DIE has invalid DW_AT_stmt_list encoding:");
     break;
   case DW_AT_location: {
     if (Expected<std::vector<DWARFLocationExpression>> Loc =
             Die.getLocations(DW_AT_location)) {
       DWARFUnit *U = Die.getDwarfUnit();
       for (const auto &Entry : *Loc) {
         DataExtractor Data(toStringRef(Entry.Expr), DCtx.isLittleEndian(), 0);
-        DWARFExpression Expression(Data, U->getAddressByteSize());
+        DWARFExpression Expression(Data, U->getAddressByteSize(),
+                                   U->getFormParams().Format);
         bool Error = any_of(Expression, [](DWARFExpression::Operation &Op) {
           return Op.isError();
         });
         if (Error || !Expression.verify(U))
           ReportError("DIE contains invalid DWARF expression:");
       }
     } else
       ReportError(toString(Loc.takeError()));
static bool isVariableIndexable(const DWARFDie &Die, DWARFContext &DCtx) {
   if (!Loc) {
     consumeError(Loc.takeError());
     return false;
   }
   DWARFUnit *U = Die.getDwarfUnit();
   for (const auto &Entry : *Loc) {
     DataExtractor Data(toStringRef(Entry.Expr), DCtx.isLittleEndian(),
                        U->getAddressByteSize());
-    DWARFExpression Expression(Data, U->getAddressByteSize());
+    DWARFExpression Expression(Data, U->getAddressByteSize(),
+                               U->getFormParams().Format);
     bool IsInteresting = any_of(Expression, [](DWARFExpression::Operation &Op) {
       return !Op.isError() && (Op.getCode() == DW_OP_addr ||
                                Op.getCode() == DW_OP_form_tls_address ||
                                Op.getCode() == DW_OP_GNU_push_tls_address);
     });
     if (IsInteresting)
       return true;
   }

llvm/test/DebugInfo/X86/DW_OP_call_ref_dwarf64.s

+# RUN: llvm-mc -triple x86_64 %s -filetype=obj -o %t.o
+# RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s
+# RUN: llvm-dwarfdump -verify %t.o | FileCheck %s --check-prefix=VERIFY
+
+# CHECK: DW_TAG_variable
+# CHECK-NEXT: DW_AT_location (DW_OP_call_ref 0x1100223344)
+
+# VERIFY-NOT: error: DIE contains invalid DWARF expression:
+# VERIFY: No errors.
+
+    .section .debug_abbrev,"",@progbits
+    .uleb128 1                      # Abbreviation Code
+    .uleb128 17                     # DW_TAG_compile_unit
+    .byte 1                         # DW_CHILDREN_yes
+    .byte 0                         # EOM(1)
+    .byte 0                         # EOM(2)
+    .uleb128 5                      # Abbreviation Code
+    .uleb128 52                     # DW_TAG_variable
+    .byte 0                         # DW_CHILDREN_no
+    .uleb128 2                      # DW_AT_location
+    .uleb128 24                     # DW_FORM_exprloc
+    .byte 0                         # EOM(1)
+    .byte 0                         # EOM(2)
+    .byte 0                         # EOM(3)
+
+    .section .debug_info,"",@progbits
+    .long 0xffffffff                # DWARF64 mark
+    .quad .Lcu_end-.Lcu_start       # Length of Unit
+.Lcu_start:
+    .short 5                        # DWARF version number
+    .byte 1                         # DW_UT_compile
+    .byte 8                         # Address Size
+    .quad .debug_abbrev             # Offset Into Abbrev. Section
+    .uleb128 1                      # Abbrev [1] DW_TAG_compile_unit
+    .uleb128 5                      # Abbrev [5] DW_TAG_variable
+    .byte .Lloc_end-.Lloc_begin     # DW_AT_location
+.Lloc_begin:
+    .byte 154                       # DW_OP_call_ref
+    .quad 0x1100223344              # Offset
+.Lloc_end:
+    .byte 0                         # End Of Children Mark
+.Lcu_end:

llvm/test/DebugInfo/X86/DW_OP_call_ref_unexpected.s

+# RUN: llvm-mc -triple x86_64 %s -filetype=obj -o - | \
+# RUN:   llvm-dwarfdump -debug-loc - | \
+# RUN:   FileCheck %s
+
+# This checks that we do not try to interpret DW_OP_call_ref if it is
+# encountered in a location table.
+
+# CHECK: .debug_loc contents:
+# CHECK-NEXT: 0x00000000:
+# CHECK-NEXT:   (0x0000000000000000, 0x0000000000000015): <decoding error> 9a ff 00 00 00
+
+    .section .debug_loc, "", @progbits
+    .quad 0                         # Beginning address offset
+    .quad 0x15                      # Ending address offset
+    .short .LDescrEnd-.LDescrBegin  # Location description length
+.LDescrBegin:
+    .byte 0x9a                      # DW_OP_call_ref
+    .long 0xff
+.LDescrEnd:
+    .quad 0, 0                      # EOL entry
+
+# A dummy CU to provide the parser of .debug_loc with the address size.
+    .section .debug_info,"",@progbits
+    .long .LCUEnd-.LCUBegin         # Length of Unit
+.LCUBegin:
+    .short 4                        # DWARF version number
+    .long 0                         # Offset Into Abbrev. Section
+    .byte 8                         # Address Size
+.LCUEnd:

llvm/tools/llvm-dwarfdump/Statistics.cpp

static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
 
   if (Die.findRecursively(dwarf::DW_AT_type))
     HasType = true;
 
   auto IsEntryValue = [&](ArrayRef<uint8_t> D) -> bool {
     DWARFUnit *U = Die.getDwarfUnit();
     DataExtractor Data(toStringRef(D),
                        Die.getDwarfUnit()->getContext().isLittleEndian(), 0);
-    DWARFExpression Expression(Data, U->getAddressByteSize());
+    DWARFExpression Expression(Data, U->getAddressByteSize(),
+                               U->getFormParams().Format);
     // Consider the expression containing the DW_OP_entry_value as
     // an entry value.
     return llvm::any_of(Expression, [](DWARFExpression::Operation &Op) {
       return Op.getCode() == dwarf::DW_OP_entry_value ||
              Op.getCode() == dwarf::DW_OP_GNU_entry_value;
     });
   };