[llvm] c1059dc - [AppleAccelTable] Keep track of the size of each hash data entry

Felipe de Azevedo Piovezan via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 8 11:21:08 PDT 2023


Author: Felipe de Azevedo Piovezan
Date: 2023-06-08T14:20:54-04:00
New Revision: c1059dcb1c88405bff4d3ceaa58af22c2f8c536e

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

LOG: [AppleAccelTable] Keep track of the size of each hash data entry

In a future patch, it will be desirable to skip over all hash data entries for a
particular string. In order to do so, we must know how many bytes each of those
entries have.

In its full specification, Apple tables allow for variable length entries, which
would make the above impossible without reading the data of each entry. However,
this is largely unsupported today (as a FIXME in the code indicates, there is a
bug with hash collisions exactly because we don't know how to skip over data),
and the documentation[1] states that:

> For the current implementations of the “.apple_names” (all functions +
> globals), the “.apple_types” (names of all types that are defined), and the
> “.apple_namespaces” (all namespaces), we currently set the Atom array to be:
> [...]
> This defines the contents to be the DIE offset (eAtomTypeDIEOffset) that is
> encoded as a 32 bit value (DW_FORM_data4).

In other words, we only produce fixed sized entries.

A few tests containing invalid dwarf had to be updated, as the error is now
detected earlier (when the accelerator table is being parsed, instead of inside
the explicit call to "verify").

[1]: https://llvm.org/docs/SourceLevelDebugging.html#fixed-lookup

Depends on D152156

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

Added: 
    

Modified: 
    llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
    llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
    llvm/test/tools/dsymutil/X86/basic-lto-dw4-linking-x86.test
    llvm/test/tools/dsymutil/X86/basic-lto-linking-x86.test
    llvm/test/tools/dsymutil/X86/objc.test
    llvm/test/tools/llvm-dwarfdump/X86/apple_names_verify_form.s

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index a1f50110a7d57..c85fca4f50b5e 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -106,6 +106,7 @@ class AppleAcceleratorTable : public DWARFAcceleratorTable {
   Header Hdr;
   HeaderData HdrData;
   dwarf::FormParams FormParams;
+  uint32_t HashDataEntryLength;
   bool IsValid = false;
 
   /// Returns true if we should continue scanning for entries or false if we've
@@ -261,6 +262,9 @@ class AppleAcceleratorTable : public DWARFAcceleratorTable {
   uint32_t getSizeHdr() const;
   uint32_t getHeaderDataLength() const;
 
+  /// Returns the size of one HashData entry.
+  uint32_t getHashDataEntryLength() const { return HashDataEntryLength; }
+
   /// Return the Atom description, which can be used to interpret the raw values
   /// of the Accelerator Entries in this table.
   ArrayRef<std::pair<HeaderData::AtomType, HeaderData::Form>> getAtomsDesc();

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 89b4cb9cfa201..a4aa50db11f88 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -66,10 +66,23 @@ Error AppleAcceleratorTable::extract() {
   HdrData.DIEOffsetBase = AccelSection.getU32(&Offset);
   uint32_t NumAtoms = AccelSection.getU32(&Offset);
 
+  HashDataEntryLength = 0;
+  auto MakeUnsupportedFormError = [](dwarf::Form Form) {
+    return createStringError(errc::not_supported,
+                             "Unsupported form:" +
+                                 dwarf::FormEncodingString(Form));
+  };
+
   for (unsigned i = 0; i < NumAtoms; ++i) {
     uint16_t AtomType = AccelSection.getU16(&Offset);
     auto AtomForm = static_cast<dwarf::Form>(AccelSection.getU16(&Offset));
     HdrData.Atoms.push_back(std::make_pair(AtomType, AtomForm));
+
+    std::optional<uint8_t> FormSize =
+        dwarf::getFixedFormByteSize(AtomForm, FormParams);
+    if (!FormSize)
+      return MakeUnsupportedFormError(AtomForm);
+    HashDataEntryLength += *FormSize;
   }
 
   IsValid = true;
@@ -207,6 +220,7 @@ LLVM_DUMP_METHOD void AppleAcceleratorTable::dump(raw_ostream &OS) const {
 
   W.printNumber("DIE offset base", HdrData.DIEOffsetBase);
   W.printNumber("Number of atoms", uint64_t(HdrData.Atoms.size()));
+  W.printNumber("Size of each hash data entry", getHashDataEntryLength());
   SmallVector<DWARFFormValue, 3> AtomForms;
   {
     ListScope AtomsScope(W, "Atoms");

diff  --git a/llvm/test/tools/dsymutil/X86/basic-lto-dw4-linking-x86.test b/llvm/test/tools/dsymutil/X86/basic-lto-dw4-linking-x86.test
index 867d822a9d9b8..15c3d87830b00 100644
--- a/llvm/test/tools/dsymutil/X86/basic-lto-dw4-linking-x86.test
+++ b/llvm/test/tools/dsymutil/X86/basic-lto-dw4-linking-x86.test
@@ -193,6 +193,7 @@ CHECK-NEXT:   HeaderData length: 12
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 1
+CHECK-NEXT: Size of each hash data entry: 4
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset
@@ -286,6 +287,7 @@ CHECK-NEXT:   HeaderData length: 24
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 4
+CHECK-NEXT: Size of each hash data entry: 11
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset
@@ -342,6 +344,7 @@ CHECK-NEXT:   HeaderData length: 12
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 1
+CHECK-NEXT: Size of each hash data entry: 4
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset
@@ -363,6 +366,7 @@ CHECK-NEXT:   HeaderData length: 12
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 1
+CHECK-NEXT: Size of each hash data entry: 4
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset

diff  --git a/llvm/test/tools/dsymutil/X86/basic-lto-linking-x86.test b/llvm/test/tools/dsymutil/X86/basic-lto-linking-x86.test
index 4e5d142315f13..3fdb3bfe1d568 100644
--- a/llvm/test/tools/dsymutil/X86/basic-lto-linking-x86.test
+++ b/llvm/test/tools/dsymutil/X86/basic-lto-linking-x86.test
@@ -192,6 +192,7 @@ CHECK-NEXT:   HeaderData length: 12
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 1
+CHECK-NEXT: Size of each hash data entry: 4
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset
@@ -285,6 +286,7 @@ CHECK-NEXT:   HeaderData length: 24
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 4
+CHECK-NEXT: Size of each hash data entry: 11
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset
@@ -341,6 +343,7 @@ CHECK-NEXT:   HeaderData length: 12
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 1
+CHECK-NEXT: Size of each hash data entry: 4
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset
@@ -362,6 +365,7 @@ CHECK-NEXT:   HeaderData length: 12
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 1
+CHECK-NEXT: Size of each hash data entry: 4
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset

diff  --git a/llvm/test/tools/dsymutil/X86/objc.test b/llvm/test/tools/dsymutil/X86/objc.test
index 47dfef56b24c3..25b02e09338b0 100644
--- a/llvm/test/tools/dsymutil/X86/objc.test
+++ b/llvm/test/tools/dsymutil/X86/objc.test
@@ -21,6 +21,7 @@ CHECK-NEXT:   HeaderData length: 12
 CHECK-NEXT: }
 CHECK-NEXT: DIE offset base: 0
 CHECK-NEXT: Number of atoms: 1
+CHECK-NEXT: Size of each hash data entry: 4
 CHECK-NEXT: Atoms [
 CHECK-NEXT:   Atom 0 {
 CHECK-NEXT:     Type: DW_ATOM_die_offset

diff  --git a/llvm/test/tools/llvm-dwarfdump/X86/apple_names_verify_form.s b/llvm/test/tools/llvm-dwarfdump/X86/apple_names_verify_form.s
index c3cc8719a0825..cf72bdcae1638 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/apple_names_verify_form.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/apple_names_verify_form.s
@@ -3,7 +3,7 @@
 # RUN: | FileCheck %s
 
 # CHECK: Verifying .apple_names...
-# CHECK-NEXT:	error: Unsupported form: failed to read HashData.
+# CHECK-NEXT:	error: Unsupported form:
 
 # This test is meant to verify that the -verify option
 # in llvm-dwarfdump, correctly identifies that Atom[0].form is unsupported.
@@ -34,7 +34,7 @@ Lnames_begin:
 	.long	0                       ## HeaderData Die Offset Base
 	.long	1                       ## HeaderData Atom Count
 	.short	1                       ## DW_ATOM_die_offset
-	.short	400                     ## DW_FORM_data4 -- error: unsupported form; failed to read HashData.
+	.short	400                     ## not a valid form -- error: unsupported form; failed to read HashData.
 	.long	0                       ## Bucket 0
 	.long	1                       ## Bucket 1
 	.long	177678                  ## Hash in Bucket 0


        


More information about the llvm-commits mailing list