Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

llvm-gsymutil now handles empty linkage names correctly. #68931

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

clayborg
Copy link
Collaborator

Previous to this fix, if we had a DW_TAG_subprogram that had a DW_AT_linkage_name that was empty, it would attempt to use this name which would cause an error to be emitted when saving the gsym file to disk:

error: DWARF conversion failed: : attempted to encode invalid FunctionInfo object

This patch fixes this issue and adds a unit test case.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 12, 2023

@llvm/pr-subscribers-debuginfo

Author: Greg Clayton (clayborg)

Changes

Previous to this fix, if we had a DW_TAG_subprogram that had a DW_AT_linkage_name that was empty, it would attempt to use this name which would cause an error to be emitted when saving the gsym file to disk:

error: DWARF conversion failed: : attempted to encode invalid FunctionInfo object

This patch fixes this issue and adds a unit test case.


Full diff: https://github.com/llvm/llvm-project/pull/68931.diff

2 Files Affected:

  • (modified) llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp (+5-5)
  • (modified) llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp (+152)
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index e38347f15e3ae8b..d720c1e33495515 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -132,11 +132,11 @@ static DWARFDie GetParentDeclContextDIE(DWARFDie &Die) {
 static std::optional<uint32_t>
 getQualifiedNameIndex(DWARFDie &Die, uint64_t Language, GsymCreator &Gsym) {
   // If the dwarf has mangled name, use mangled name
-  if (auto LinkageName =
-          dwarf::toString(Die.findRecursively({dwarf::DW_AT_MIPS_linkage_name,
-                                               dwarf::DW_AT_linkage_name}),
-                          nullptr))
-    return Gsym.insertString(LinkageName, /* Copy */ false);
+  if (auto LinkageName = Die.getLinkageName()) {
+    // We have seen cases were linkage name is actually empty.
+    if (strlen(LinkageName) > 0)
+      return Gsym.insertString(LinkageName, /* Copy */ false);
+  }
 
   StringRef ShortName(Die.getName(DINameKind::ShortName));
   if (ShortName.empty())
diff --git a/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp b/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
index 58bc83997d1a926..ad81a2fcd16441a 100644
--- a/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
+++ b/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
@@ -4005,3 +4005,155 @@ TEST(GSYMTest, TestEmptyRangeWarnings) {
   // Make sure we don't see spurious errors in the output:
   EXPECT_TRUE(errors.find("error:") == std::string::npos);
 }
+
+
+TEST(GSYMTest, TestEmptyLinkageName) {
+  // This example has a single compile unit that has a DW_TAG_subprogram that
+  // has a function that has an empty linkage name and a valid normal name.
+  // Previously this would cause an encoding error:
+  //
+  // DWARF conversion failed: attempted to encode invalid FunctionInfo object
+  //
+  // This was because we would get a valid but empty linkage name and we would
+  // try to use this in the GSYM FunctionInfo and that would cause the error
+  // as the name was empty.
+  //
+  // 0x0000000b: DW_TAG_compile_unit
+  //               DW_AT_name        ("/tmp/main.cpp")
+  //               DW_AT_language    (DW_LANG_C)
+  //               DW_AT_stmt_list   (0x00000000)
+  //
+  // 0x00000015:   DW_TAG_subprogram
+  //                 DW_AT_name      ("foo")
+  //                 DW_AT_linkage_name      ("")
+  //                 DW_AT_low_pc    (0x0000000000001000)
+  //                 DW_AT_high_pc   (0x0000000000001050)
+  //
+  // 0x0000002e:   NULL
+
+
+  StringRef yamldata = R"(
+  debug_str:
+    - ''
+    - '/tmp/main.cpp'
+    - foo
+    - ''
+  debug_abbrev:
+    - ID:              0
+      Table:
+        - Code:            0x1
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_yes
+          Attributes:
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_udata
+            - Attribute:       DW_AT_stmt_list
+              Form:            DW_FORM_sec_offset
+        - Code:            0x2
+          Tag:             DW_TAG_subprogram
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_linkage_name
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_low_pc
+              Form:            DW_FORM_addr
+            - Attribute:       DW_AT_high_pc
+              Form:            DW_FORM_addr
+  debug_info:
+    - Length:          0x2B
+      Version:         4
+      AbbrevTableID:   0
+      AbbrOffset:      0x0
+      AddrSize:        8
+      Entries:
+        - AbbrCode:        0x1
+          Values:
+            - Value:           0x1
+            - Value:           0x2
+            - Value:           0x0
+        - AbbrCode:        0x2
+          Values:
+            - Value:           0xF
+            - Value:           0x13
+            - Value:           0x1000
+            - Value:           0x1050
+        - AbbrCode:        0x0
+  debug_line:
+    - Length:          68
+      Version:         2
+      PrologueLength:  36
+      MinInstLength:   1
+      DefaultIsStmt:   1
+      LineBase:        251
+      LineRange:       14
+      OpcodeBase:      13
+      StandardOpcodeLengths: [ 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 ]
+      IncludeDirs:
+        - '/tmp'
+      Files:
+        - Name:            main.cpp
+          DirIdx:          1
+          ModTime:         0
+          Length:          0
+      Opcodes:
+        - Opcode:          DW_LNS_extended_op
+          ExtLen:          9
+          SubOpcode:       DW_LNE_set_address
+          Data:            4096
+        - Opcode:          DW_LNS_advance_line
+          SData:           9
+          Data:            0
+        - Opcode:          DW_LNS_copy
+          Data:            0
+        - Opcode:          DW_LNS_advance_pc
+          Data:            256
+        - Opcode:          DW_LNS_advance_line
+          SData:           1
+          Data:            0
+        - Opcode:          DW_LNS_copy
+          Data:            0
+        - Opcode:          DW_LNS_advance_pc
+          Data:            256
+        - Opcode:          DW_LNS_extended_op
+          ExtLen:          1
+          SubOpcode:       DW_LNE_end_sequence
+          Data:            0
+  )";
+  auto ErrOrSections = DWARFYAML::emitDebugSections(yamldata);
+  ASSERT_THAT_EXPECTED(ErrOrSections, Succeeded());
+  std::unique_ptr<DWARFContext> DwarfContext =
+      DWARFContext::create(*ErrOrSections, 8);
+  ASSERT_TRUE(DwarfContext.get() != nullptr);
+  std::string errors;
+  raw_string_ostream OS(errors);
+  GsymCreator GC;
+  DwarfTransformer DT(*DwarfContext, GC);
+  const uint32_t ThreadCount = 1;
+  ASSERT_THAT_ERROR(DT.convert(ThreadCount, &OS), Succeeded());
+  ASSERT_THAT_ERROR(GC.finalize(OS), Succeeded());
+  OS.flush();
+  SmallString<512> Str;
+  raw_svector_ostream OutStrm(Str);
+  const auto ByteOrder = llvm::endianness::native;
+  FileWriter FW(OutStrm, ByteOrder);
+  ASSERT_THAT_ERROR(GC.encode(FW), Succeeded());
+  Expected<GsymReader> GR = GsymReader::copyBuffer(OutStrm.str());
+  ASSERT_THAT_EXPECTED(GR, Succeeded());
+  // There should be one function in our GSYM.
+  EXPECT_EQ(GR->getNumAddresses(), 1u);
+  // Verify "foo" is present and has a line table and no inline info.
+  auto ExpFI = GR->getFunctionInfo(0x1000);
+  ASSERT_THAT_EXPECTED(ExpFI, Succeeded());
+  ASSERT_EQ(ExpFI->Range, AddressRange(0x1000, 0x1050));
+  EXPECT_TRUE(ExpFI->OptLineTable.has_value());
+  EXPECT_FALSE(ExpFI->Inline.has_value());
+  StringRef FuncName = GR->getString(ExpFI->Name);
+  EXPECT_EQ(FuncName, "foo");
+
+  // Make sure we don't see spurious errors in the output:
+  EXPECT_TRUE(errors.find("error:") == std::string::npos);
+}

@github-actions
Copy link

github-actions bot commented Oct 12, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 910a4bf5b70ae14e7262677a8880ee98056e44e1 5c2dfa575e60e141d4e09f9d1d66e3d0238a1df0 -- llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
View the diff from clang-format here.
diff --git a/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp b/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
index ad81a2fcd..35c36f314 100644
--- a/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
+++ b/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
@@ -4006,7 +4006,6 @@ TEST(GSYMTest, TestEmptyRangeWarnings) {
   EXPECT_TRUE(errors.find("error:") == std::string::npos);
 }
 
-
 TEST(GSYMTest, TestEmptyLinkageName) {
   // This example has a single compile unit that has a DW_TAG_subprogram that
   // has a function that has an empty linkage name and a valid normal name.
@@ -4031,7 +4030,6 @@ TEST(GSYMTest, TestEmptyLinkageName) {
   //
   // 0x0000002e:   NULL
 
-
   StringRef yamldata = R"(
   debug_str:
     - ''

Previous to this fix, if we had a DW_TAG_subprogram that had a DW_AT_linkage_name that was empty, it would attempt to use this name which would cause an error to be emitted when saving the gsym file to disk:

  error: DWARF conversion failed: : attempted to encode invalid FunctionInfo object

This patch fixes this issue and adds a unit test case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants