[llvm] f5fc8b6 - [DebugInfo][NFC] Move ObjC Selector name handling to lib DebugInfo

Felipe de Azevedo Piovezan via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 7 07:13:25 PDT 2023


Author: Felipe de Azevedo Piovezan
Date: 2023-09-07T10:11:01-04:00
New Revision: f5fc8b6251726c6fe31e11f010271d446956dd54

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

LOG: [DebugInfo][NFC] Move ObjC Selector name handling to lib DebugInfo

The DWARFLinker library has code to identify ObjC selector names, which is used
by the debug linker to generate accelerator table entries. In the future, we
would like the DWARF verifier to also have access to such code, so that it can
identify these names when verifying accelerator tables (e.g. debug_names).

This patch follows the same intent of D155723, where we also moved code
generating simplified template names.

Since this is moving code around and changing the log, we also replace raw
pointer manipulation with the more expressive
StringRef::{drop_front,take_front,...} methods.

We also change a test so that it verifies its output, and that requires having
dsymutil not write to stdout.

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

Added: 
    

Modified: 
    llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
    llvm/lib/DWARFLinker/DWARFLinker.cpp
    llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
    llvm/test/tools/dsymutil/X86/objc.test

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index d8279d35ba3b819..1ba555a061904cc 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -749,6 +749,22 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
 /// E.g.: StripTemplateParameters("foo<int>") = "foo".
 std::optional<StringRef> StripTemplateParameters(StringRef Name);
 
+struct ObjCSelectorNames {
+  /// For "-[A(Category) method:]", this would be "method:"
+  StringRef Selector;
+  /// For "-[A(Category) method:]", this would be "A(category)"
+  StringRef ClassName;
+  /// For "-[A(Category) method:]", this would be "A"
+  std::optional<StringRef> ClassNameNoCategory;
+  /// For "-[A(Category) method:]", this would be "A method:"
+  std::optional<std::string> MethodNameNoCategory;
+};
+
+/// If `Name` is the AT_name of a DIE which refers to an Objective-C selector,
+/// returns an instance of ObjCSelectorNames. The Selector and ClassName fields
+/// are guaranteed to be non-empty in the result.
+std::optional<ObjCSelectorNames> getObjCNamesIfSelector(StringRef Name);
+
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_DWARF_DWARFACCELERATORTABLE_H

diff  --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index ae95a30873d9f89..35e5cefc3877060 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -1587,51 +1587,25 @@ unsigned DWARFLinker::DIECloner::cloneAttribute(
   return 0;
 }
 
-static bool isObjCSelector(StringRef Name) {
-  return Name.size() > 2 && (Name[0] == '-' || Name[0] == '+') &&
-         (Name[1] == '[');
-}
-
 void DWARFLinker::DIECloner::addObjCAccelerator(CompileUnit &Unit,
                                                 const DIE *Die,
                                                 DwarfStringPoolEntryRef Name,
                                                 OffsetsStringPool &StringPool,
                                                 bool SkipPubSection) {
-  assert(isObjCSelector(Name.getString()) && "not an objc selector");
-  // Objective C method or class function.
-  // "- [Class(Category) selector :withArg ...]"
-  StringRef ClassNameStart(Name.getString().drop_front(2));
-  size_t FirstSpace = ClassNameStart.find(' ');
-  if (FirstSpace == StringRef::npos)
-    return;
-
-  StringRef SelectorStart(ClassNameStart.data() + FirstSpace + 1);
-  if (!SelectorStart.size())
+  std::optional<ObjCSelectorNames> Names =
+      getObjCNamesIfSelector(Name.getString());
+  if (!Names)
     return;
-
-  StringRef Selector(SelectorStart.data(), SelectorStart.size() - 1);
-  Unit.addNameAccelerator(Die, StringPool.getEntry(Selector), SkipPubSection);
-
-  // Add an entry for the class name that points to this
-  // method/class function.
-  StringRef ClassName(ClassNameStart.data(), FirstSpace);
-  Unit.addObjCAccelerator(Die, StringPool.getEntry(ClassName), SkipPubSection);
-
-  if (ClassName[ClassName.size() - 1] == ')') {
-    size_t OpenParens = ClassName.find('(');
-    if (OpenParens != StringRef::npos) {
-      StringRef ClassNameNoCategory(ClassName.data(), OpenParens);
-      Unit.addObjCAccelerator(Die, StringPool.getEntry(ClassNameNoCategory),
-                              SkipPubSection);
-
-      std::string MethodNameNoCategory(Name.getString().data(), OpenParens + 2);
-      // FIXME: The missing space here may be a bug, but
-      //        dsymutil-classic also does it this way.
-      MethodNameNoCategory.append(std::string(SelectorStart));
-      Unit.addNameAccelerator(Die, StringPool.getEntry(MethodNameNoCategory),
-                              SkipPubSection);
-    }
-  }
+  Unit.addNameAccelerator(Die, StringPool.getEntry(Names->Selector),
+                          SkipPubSection);
+  Unit.addObjCAccelerator(Die, StringPool.getEntry(Names->ClassName),
+                          SkipPubSection);
+  if (Names->ClassNameNoCategory)
+    Unit.addObjCAccelerator(
+        Die, StringPool.getEntry(*Names->ClassNameNoCategory), SkipPubSection);
+  if (Names->MethodNameNoCategory)
+    Unit.addNameAccelerator(
+        Die, StringPool.getEntry(*Names->MethodNameNoCategory), SkipPubSection);
 }
 
 static bool
@@ -1781,7 +1755,7 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE,
       Unit.addNameAccelerator(Die, AttrInfo.Name,
                               Tag == dwarf::DW_TAG_inlined_subroutine);
     }
-    if (AttrInfo.Name && isObjCSelector(AttrInfo.Name.getString()))
+    if (AttrInfo.Name)
       addObjCAccelerator(Unit, Die, AttrInfo.Name, DebugStrPool,
                          /* SkipPubSection =*/true);
 

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 97a0a438b882aed..7d8289ed420abb9 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -970,6 +970,43 @@ DWARFDebugNames::getCUNameIndex(uint64_t CUOffset) {
   return CUToNameIndex.lookup(CUOffset);
 }
 
+static bool isObjCSelector(StringRef Name) {
+  return Name.size() > 2 && (Name[0] == '-' || Name[0] == '+') &&
+         (Name[1] == '[');
+}
+
+std::optional<ObjCSelectorNames> llvm::getObjCNamesIfSelector(StringRef Name) {
+  if (!isObjCSelector(Name))
+    return std::nullopt;
+  // "-[Atom setMass:]"
+  StringRef ClassNameStart(Name.drop_front(2));
+  size_t FirstSpace = ClassNameStart.find(' ');
+  if (FirstSpace == StringRef::npos)
+    return std::nullopt;
+
+  StringRef SelectorStart = ClassNameStart.drop_front(FirstSpace + 1);
+  if (!SelectorStart.size())
+    return std::nullopt;
+
+  ObjCSelectorNames Ans;
+  Ans.ClassName = ClassNameStart.take_front(FirstSpace);
+  Ans.Selector = SelectorStart.drop_back(); // drop ']';
+
+  // "-[Class(Category) selector :withArg ...]"
+  if (Ans.ClassName.back() == ')') {
+    size_t OpenParens = Ans.ClassName.find('(');
+    if (OpenParens != StringRef::npos) {
+      Ans.ClassNameNoCategory = Ans.ClassName.take_front(OpenParens);
+
+      Ans.MethodNameNoCategory = Name.take_front(OpenParens + 2);
+      // FIXME: The missing space here may be a bug, but dsymutil-classic also
+      // does it this way.
+      append_range(*Ans.MethodNameNoCategory, SelectorStart);
+    }
+  }
+  return Ans;
+}
+
 std::optional<StringRef> llvm::StripTemplateParameters(StringRef Name) {
   // We are looking for template parameters to strip from Name. e.g.
   //

diff  --git a/llvm/test/tools/dsymutil/X86/objc.test b/llvm/test/tools/dsymutil/X86/objc.test
index 25b02e09338b0e1..1dd3ff19f2f5d2a 100644
--- a/llvm/test/tools/dsymutil/X86/objc.test
+++ b/llvm/test/tools/dsymutil/X86/objc.test
@@ -1,5 +1,5 @@
-RUN: dsymutil -f -oso-prepend-path=%p/.. %p/../Inputs/objc.macho.x86_64 -o - \
-RUN:   | llvm-dwarfdump -apple-types -apple-objc - | FileCheck %s
+RUN: dsymutil --verify-dwarf=output -f -oso-prepend-path=%p/.. %p/../Inputs/objc.macho.x86_64 -o %t.d4
+RUN: llvm-dwarfdump -apple-types -apple-objc %t.d4 | FileCheck %s
 
 CHECK: .apple_types contents:
 CHECK: String: 0x00000066 "A"


        


More information about the llvm-commits mailing list