[llvm] 8f3d47c - [DWARF] Do not pass Version to DWARFExpression. NFCI.

Igor Kudrin via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 27 04:23:48 PST 2020


Author: Igor Kudrin
Date: 2020-01-27T19:08:46+07:00
New Revision: 8f3d47c54ac21f99b25d8ad00598b7f5be00d6d8

URL: https://github.com/llvm/llvm-project/commit/8f3d47c54ac21f99b25d8ad00598b7f5be00d6d8
DIFF: https://github.com/llvm/llvm-project/commit/8f3d47c54ac21f99b25d8ad00598b7f5be00d6d8.diff

LOG: [DWARF] Do not pass Version to DWARFExpression. NFCI.

The Version was used only to determine the size of an operand of
DW_OP_call_ref. The size was 4 for all versions apart from 2, but
the DW_OP_call_ref operation was introduced only in DWARF3. Thus,
the code may be simplified and using of Version may be eliminated.

Differential Revision: https://reviews.llvm.org/D73264

Added: 
    llvm/test/DebugInfo/X86/DW_OP_call_ref_ver2.s

Modified: 
    llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
    llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
    llvm/lib/DWARFLinker/DWARFLinker.cpp
    llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
    llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
    llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
    llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
    llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
    llvm/tools/llvm-dwarfdump/Statistics.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
index 456d9df957ad..c4dc53337c07 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
@@ -87,8 +87,7 @@ class DWARFExpression {
     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, uint16_t Version, uint8_t AddressSize,
-                 uint64_t Offset);
+    bool extract(DataExtractor Data, uint8_t AddressSize, uint64_t Offset);
     bool isError() { return Error; }
     bool print(raw_ostream &OS, const DWARFExpression *Expr,
                const MCRegisterInfo *RegInfo, DWARFUnit *U, bool isEH);
@@ -107,7 +106,7 @@ class DWARFExpression {
         : Expr(Expr), Offset(Offset) {
       Op.Error =
           Offset >= Expr->Data.getData().size() ||
-          !Op.extract(Expr->Data, Expr->Version, Expr->AddressSize, Offset);
+          !Op.extract(Expr->Data, Expr->AddressSize, Offset);
     }
 
   public:
@@ -115,7 +114,7 @@ class DWARFExpression {
       Offset = Op.isError() ? Expr->Data.getData().size() : Op.EndOffset;
       Op.Error =
           Offset >= Expr->Data.getData().size() ||
-          !Op.extract(Expr->Data, Expr->Version, Expr->AddressSize, Offset);
+          !Op.extract(Expr->Data, Expr->AddressSize, Offset);
       return Op;
     }
 
@@ -127,8 +126,8 @@ class DWARFExpression {
     friend bool operator==(const iterator &, const iterator &);
   };
 
-  DWARFExpression(DataExtractor Data, uint16_t Version, uint8_t AddressSize)
-      : Data(Data), Version(Version), AddressSize(AddressSize) {
+  DWARFExpression(DataExtractor Data, uint8_t AddressSize)
+      : Data(Data), AddressSize(AddressSize) {
     assert(AddressSize == 8 || AddressSize == 4 || AddressSize == 2);
   }
 
@@ -142,7 +141,6 @@ class DWARFExpression {
 
 private:
   DataExtractor Data;
-  uint16_t Version;
   uint8_t AddressSize;
 };
 

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 27d73cfea4ec..03511d4347ad 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -2210,7 +2210,7 @@ void DwarfDebug::emitDebugLocEntry(ByteStreamer &Streamer,
   DWARFDataExtractor Data(StringRef(DebugLocs.getBytes(Entry).data(),
                                     DebugLocs.getBytes(Entry).size()),
                           Asm->getDataLayout().isLittleEndian(), PtrSize);
-  DWARFExpression Expr(Data, getDwarfVersion(), PtrSize);
+  DWARFExpression Expr(Data, PtrSize);
 
   using Encoding = DWARFExpression::Operation::Encoding;
   uint64_t Offset = 0;

diff  --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index a9fe0564c68d..92e785d33c67 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -953,8 +953,7 @@ unsigned DWARFLinker::DIECloner::cloneBlockAttribute(
     DWARFUnit &OrigUnit = Unit.getOrigUnit();
     DataExtractor Data(StringRef((const char *)Bytes.data(), Bytes.size()),
                        IsLittleEndian, OrigUnit.getAddressByteSize());
-    DWARFExpression Expr(Data, OrigUnit.getVersion(),
-                         OrigUnit.getAddressByteSize());
+    DWARFExpression Expr(Data, OrigUnit.getAddressByteSize());
     cloneExpression(Data, Expr, OF, Unit, Buffer);
     Bytes = Buffer;
   }
@@ -2059,8 +2058,7 @@ void DWARFLinker::DIECloner::cloneAllCompileUnits(DWARFContext &DwarfContext,
         DataExtractor Data(Bytes, IsLittleEndian,
                            OrigUnit.getAddressByteSize());
         cloneExpression(Data,
-                        DWARFExpression(Data, OrigUnit.getVersion(),
-                                        OrigUnit.getAddressByteSize()),
+                        DWARFExpression(Data, OrigUnit.getAddressByteSize()),
                         OF, *CurrentUnit, Buffer);
       };
       Emitter->emitLocationsForUnit(*CurrentUnit, DwarfContext, ProcessExpr);

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index 81b00f65741b..675c844726a3 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -130,8 +130,8 @@ Error CFIProgram::parse(DWARFDataExtractor Data, uint64_t *Offset,
           DataExtractor Extractor(
               Data.getData().slice(*Offset, *Offset + ExprLength),
               Data.isLittleEndian(), Data.getAddressSize());
-          Instructions.back().Expression = DWARFExpression(
-              Extractor, Data.getAddressSize(), dwarf::DWARF_VERSION);
+          Instructions.back().Expression =
+              DWARFExpression(Extractor, Data.getAddressSize());
           *Offset += ExprLength;
           break;
         }
@@ -143,8 +143,8 @@ Error CFIProgram::parse(DWARFDataExtractor Data, uint64_t *Offset,
           DataExtractor Extractor(
               Data.getData().slice(*Offset, *Offset + BlockLength),
               Data.isLittleEndian(), Data.getAddressSize());
-          Instructions.back().Expression = DWARFExpression(
-              Extractor, Data.getAddressSize(), dwarf::DWARF_VERSION);
+          Instructions.back().Expression =
+              DWARFExpression(Extractor, Data.getAddressSize());
           *Offset += BlockLength;
           break;
         }

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
index 0c5f9a9c54ec..131017e749d6 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
@@ -106,16 +106,11 @@ DWARFLocationInterpreter::Interpret(const DWARFLocationEntry &E) {
   }
 }
 
-// When directly dumping the .debug_loc without a compile unit, we have to guess
-// at the DWARF version. This only affects DW_OP_call_ref, which is a rare
-// expression that LLVM doesn't produce. Guessing the wrong version means we
-// won't be able to pretty print expressions in DWARF2 binaries produced by
-// non-LLVM tools.
 static void dumpExpression(raw_ostream &OS, ArrayRef<uint8_t> Data,
                            bool IsLittleEndian, unsigned AddressSize,
                            const MCRegisterInfo *MRI, DWARFUnit *U) {
   DWARFDataExtractor Extractor(toStringRef(Data), IsLittleEndian, AddressSize);
-  DWARFExpression(Extractor, dwarf::DWARF_VERSION, AddressSize).print(OS, MRI, U);
+  DWARFExpression(Extractor, AddressSize).print(OS, MRI, U);
 }
 
 bool DWARFLocationTable::dumpLocationList(uint64_t *Offset, raw_ostream &OS,

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
index c1dc3b68c6ab..2c44e85c3863 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -79,8 +79,7 @@ static void dumpLocation(raw_ostream &OS, DWARFFormValue &FormValue,
     ArrayRef<uint8_t> Expr = *FormValue.getAsBlock();
     DataExtractor Data(StringRef((const char *)Expr.data(), Expr.size()),
                        Ctx.isLittleEndian(), 0);
-    DWARFExpression(Data, U->getVersion(), U->getAddressByteSize())
-        .print(OS, MRI, U);
+    DWARFExpression(Data, U->getAddressByteSize()).print(OS, MRI, U);
     return;
   }
 

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
index 6f28aa46328c..384c34e07562 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
@@ -116,11 +116,7 @@ static DWARFExpression::Operation::Description getOpDesc(unsigned OpCode) {
   return Descriptions[OpCode];
 }
 
-static uint8_t getRefAddrSize(uint8_t AddrSize, uint16_t Version) {
-  return (Version == 2) ? AddrSize : 4;
-}
-
-bool DWARFExpression::Operation::extract(DataExtractor Data, uint16_t Version,
+bool DWARFExpression::Operation::extract(DataExtractor Data,
                                          uint8_t AddressSize, uint64_t Offset) {
   Opcode = Data.getU8(&Offset);
 
@@ -160,14 +156,8 @@ bool DWARFExpression::Operation::extract(DataExtractor Data, uint16_t Version,
       Operands[Operand] = Data.getUnsigned(&Offset, AddressSize);
       break;
     case Operation::SizeRefAddr:
-      if (getRefAddrSize(AddressSize, Version) == 8) {
-        Operands[Operand] = Data.getU64(&Offset);
-      } else if (getRefAddrSize(AddressSize, Version) == 4) {
-        Operands[Operand] = Data.getU32(&Offset);
-      } else {
-        assert(getRefAddrSize(AddressSize, Version) == 2);
-        Operands[Operand] = Data.getU16(&Offset);
-      }
+      // TODO: Add support for 64-bit DWARF format.
+      Operands[Operand] = Data.getU32(&Offset);
       break;
     case Operation::SizeLEB:
       if (Signed)

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 1fd6c1d7d282..68bacbfda467 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -481,8 +481,7 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
       DWARFUnit *U = Die.getDwarfUnit();
       for (const auto &Entry : *Loc) {
         DataExtractor Data(toStringRef(Entry.Expr), DCtx.isLittleEndian(), 0);
-        DWARFExpression Expression(Data, U->getVersion(),
-                                   U->getAddressByteSize());
+        DWARFExpression Expression(Data, U->getAddressByteSize());
         bool Error = any_of(Expression, [](DWARFExpression::Operation &Op) {
           return Op.isError();
         });
@@ -1290,7 +1289,7 @@ static bool isVariableIndexable(const DWARFDie &Die, DWARFContext &DCtx) {
   for (const auto &Entry : *Loc) {
     DataExtractor Data(toStringRef(Entry.Expr), DCtx.isLittleEndian(),
                        U->getAddressByteSize());
-    DWARFExpression Expression(Data, U->getVersion(), U->getAddressByteSize());
+    DWARFExpression Expression(Data, U->getAddressByteSize());
     bool IsInteresting = any_of(Expression, [](DWARFExpression::Operation &Op) {
       return !Op.isError() && (Op.getCode() == DW_OP_addr ||
                                Op.getCode() == DW_OP_form_tls_address ||

diff  --git a/llvm/test/DebugInfo/X86/DW_OP_call_ref_ver2.s b/llvm/test/DebugInfo/X86/DW_OP_call_ref_ver2.s
new file mode 100644
index 000000000000..f14316b731ba
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/DW_OP_call_ref_ver2.s
@@ -0,0 +1,42 @@
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj | \
+# RUN:   llvm-dwarfdump - | \
+# RUN:   FileCheck %s
+
+# This checks that the operand of DW_OP_call_ref is always parsed corresponding
+# to the DWARF format of CU. Our code used to have an exception for verson == 2,
+# where it treated the operand like it had the size of address, but since
+# DW_OP_call_ref was introduced only in DWARF3, the code could be simplified.
+
+# CHECK: DW_TAG_variable
+# CHECK-NEXT: DW_AT_location (DW_OP_call_ref 0x11223344)
+
+	.section	.debug_abbrev,"", at progbits
+	.byte	1                       # Abbreviation Code
+	.byte	17                      # DW_TAG_compile_unit
+	.byte	1                       # DW_CHILDREN_yes
+	.byte	0                       # EOM(1)
+	.byte	0                       # EOM(2)
+	.byte	5                       # Abbreviation Code
+	.byte	52                      # DW_TAG_variable
+	.byte	0                       # DW_CHILDREN_no
+	.byte	2                       # DW_AT_location
+	.byte	10                      # DW_FORM_block1
+	.byte	0                       # EOM(1)
+	.byte	0                       # EOM(2)
+	.byte	0                       # EOM(3)
+
+	.section	.debug_info,"", at progbits
+	.long	.Lcu_end-.Lcu_start     # Length of Unit
+.Lcu_start:
+	.short	2                       # DWARF version number
+	.long	.debug_abbrev           # Offset Into Abbrev. Section
+	.byte	8                       # Address Size (in bytes)
+	.byte	1                       # Abbrev [1] DW_TAG_compile_unit
+	.byte	5                       # Abbrev [5] DW_TAG_variable
+	.byte	.Lloc_end-.Lloc_begin   # DW_AT_location
+.Lloc_begin:
+	.byte	154                     # DW_OP_call_ref
+	.long	0x11223344              # Offset
+.Lloc_end:
+	.byte	0                       # End Of Children Mark
+.Lcu_end:

diff  --git a/llvm/tools/llvm-dwarfdump/Statistics.cpp b/llvm/tools/llvm-dwarfdump/Statistics.cpp
index 5bef4d5148ca..a183501afef7 100644
--- a/llvm/tools/llvm-dwarfdump/Statistics.cpp
+++ b/llvm/tools/llvm-dwarfdump/Statistics.cpp
@@ -206,7 +206,7 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
     DWARFUnit *U = Die.getDwarfUnit();
     DataExtractor Data(toStringRef(D),
                        Die.getDwarfUnit()->getContext().isLittleEndian(), 0);
-    DWARFExpression Expression(Data, U->getVersion(), U->getAddressByteSize());
+    DWARFExpression Expression(Data, U->getAddressByteSize());
     // Consider the expression containing the DW_OP_entry_value as
     // an entry value.
     return llvm::any_of(Expression, [](DWARFExpression::Operation &Op) {


        


More information about the llvm-commits mailing list