[XCOFF][OBJECT] get symbol size by calling XCOFF interfaces #67304

chenzheng1030 · 2023-09-25T09:31:21Z

Computing the symbol size as the gap between sorted symbols are not right for XCOFF.

For XCOFF, the size info is stored in aux symbol and can be got from existing XCOFF interface getSymbolSize().
This patch changes XCOFFObjectFile to call this API to get sizes for symbols.

llvmbot · 2023-09-25T09:32:27Z

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-binary-utilities

Changes

XCOFF has aux symbol to indicate a symbol size. For example:

[27]	m   0x00000090      .bss     1  unamex                    _ZZ1fvE15function_global
[28]	a4  0x00000004       0    0     CM       BS    0    0
[29]	m   0x00000094      .bss     1  unamex                    _ZL4beta
[30]	a4  0x00000004       0    0     CM       BS    0    0
[31]	m   0x00000098      .bss     1  unamex                    _ZL5alpha
[32]	a4  0x00000004       0    0     CM       BS    0    0

The case is from #67284

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

5 Files Affected:

(modified) llvm/include/llvm/Object/XCOFFObjectFile.h (+31-1)
(modified) llvm/lib/Object/SymbolSize.cpp (+7)
(modified) llvm/lib/Object/XCOFFObjectFile.cpp (+4)
(added) llvm/test/tools/llvm-symbolizer/Inputs/xcoff-dwarf.o ()
(added) llvm/test/tools/llvm-symbolizer/xcoff.test (+39)

diff --git a/llvm/include/llvm/Object/XCOFFObjectFile.h b/llvm/include/llvm/Object/XCOFFObjectFile.h
index 5f51aacfabc0851..f7fd3b38e059f96 100644
--- a/llvm/include/llvm/Object/XCOFFObjectFile.h
+++ b/llvm/include/llvm/Object/XCOFFObjectFile.h
@@ -13,6 +13,7 @@
 #ifndef LLVM_OBJECT_XCOFFOBJECTFILE_H
 #define LLVM_OBJECT_XCOFFOBJECTFILE_H
 
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/BinaryFormat/XCOFF.h"
@@ -23,6 +24,8 @@
 namespace llvm {
 namespace object {
 
+class xcoff_symbol_iterator;
+
 struct XCOFFFileHeader32 {
   support::ubig16_t Magic;
   support::ubig16_t NumberOfSections;
@@ -576,6 +579,10 @@ class XCOFFObjectFile : public ObjectFile {
   Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
   basic_symbol_iterator symbol_begin() const override;
   basic_symbol_iterator symbol_end() const override;
+
+  using xcoff_symbol_iterator_range = iterator_range<xcoff_symbol_iterator>;
+  xcoff_symbol_iterator_range symbols() const;
+
   bool is64Bit() const override;
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
@@ -761,7 +768,7 @@ struct XCOFFSymbolEntry64 {
   uint8_t NumberOfAuxEntries;
 };
 
-class XCOFFSymbolRef {
+class XCOFFSymbolRef : public SymbolRef {
 public:
   enum { NAME_IN_STR_TBL_MAGIC = 0x0 };
 
@@ -787,6 +794,14 @@ class XCOFFSymbolRef {
 
   uint64_t getValue64() const { return Entry64->Value; }
 
+  const XCOFFObjectFile *getObject() const {
+    return cast<XCOFFObjectFile>(BasicSymbolRef::getObject());
+  }
+
+  uint64_t getSize() const {
+    return getObject()->getSymbolSize(getRawDataRefImpl());
+  }
+
 #define GETVALUE(X) Entry32 ? Entry32->X : Entry64->X
 
   int16_t getSectionNumber() const { return GETVALUE(SectionNumber); }
@@ -827,6 +842,21 @@ class XCOFFSymbolRef {
   const XCOFFSymbolEntry64 *Entry64 = nullptr;
 };
 
+class xcoff_symbol_iterator : public symbol_iterator {
+public:
+  xcoff_symbol_iterator(const basic_symbol_iterator &B)
+      : symbol_iterator(SymbolRef(B->getRawDataRefImpl(),
+                                  cast<XCOFFObjectFile>(B->getObject()))) {}
+
+  const XCOFFSymbolRef *operator->() const {
+    return static_cast<const XCOFFSymbolRef *>(symbol_iterator::operator->());
+  }
+
+  const XCOFFSymbolRef &operator*() const {
+    return static_cast<const XCOFFSymbolRef &>(symbol_iterator::operator*());
+  }
+};
+
 class TBVectorExt {
   uint16_t Data;
   SmallString<32> VecParmsInfo;
diff --git a/llvm/lib/Object/SymbolSize.cpp b/llvm/lib/Object/SymbolSize.cpp
index f93a5f7d9bd5442..c4f30b1072d52da 100644
--- a/llvm/lib/Object/SymbolSize.cpp
+++ b/llvm/lib/Object/SymbolSize.cpp
@@ -59,6 +59,13 @@ llvm::object::computeSymbolSizes(const ObjectFile &O) {
     return Ret;
   }
 
+  if (const auto *E = dyn_cast<XCOFFObjectFile>(&O)) {
+    auto Syms = E->symbols();
+    for (XCOFFSymbolRef Sym : Syms)
+      Ret.push_back({Sym, Sym.getSize()});
+    return Ret;
+  }
+
   // Collect sorted symbol addresses. Include dummy addresses for the end
   // of each section.
   std::vector<SymEntry> Addresses;
diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp
index fa4917e354e92b1..7dcf344282e14fd 100644
--- a/llvm/lib/Object/XCOFFObjectFile.cpp
+++ b/llvm/lib/Object/XCOFFObjectFile.cpp
@@ -689,6 +689,10 @@ basic_symbol_iterator XCOFFObjectFile::symbol_end() const {
   return basic_symbol_iterator(SymbolRef(SymDRI, this));
 }
 
+XCOFFObjectFile::xcoff_symbol_iterator_range XCOFFObjectFile::symbols() const {
+  return xcoff_symbol_iterator_range(symbol_begin(), symbol_end());
+}
+
 section_iterator XCOFFObjectFile::section_begin() const {
   DataRefImpl DRI;
   DRI.p = getSectionHeaderTableAddress();
diff --git a/llvm/test/tools/llvm-symbolizer/Inputs/xcoff-dwarf.o b/llvm/test/tools/llvm-symbolizer/Inputs/xcoff-dwarf.o
new file mode 100644
index 000000000000000..04326b6ccedd046
Binary files /dev/null and b/llvm/test/tools/llvm-symbolizer/Inputs/xcoff-dwarf.o differ
diff --git a/llvm/test/tools/llvm-symbolizer/xcoff.test b/llvm/test/tools/llvm-symbolizer/xcoff.test
new file mode 100644
index 000000000000000..35dd72c3ab38508
--- /dev/null
+++ b/llvm/test/tools/llvm-symbolizer/xcoff.test
@@ -0,0 +1,39 @@
+
+RUN: llvm-symbolizer --obj=%p/Inputs/xcoff-dwarf.o 'DATA 0x60' \
+RUN:    'DATA 0x61' 'DATA 0x64' 'DATA 0X68' 'DATA 0x90' 'DATA 0x94' \
+RUN:    'DATA 0X98' | FileCheck %s
+
+CHECK: bss_global
+CHECK-NEXT: 96 4
+CHECK-NEXT: ??:?
+CHECK-EMPTY:
+
+CHECK: bss_global
+CHECK-NEXT: 96 4
+CHECK-NEXT: ??:?
+CHECK-EMPTY:
+
+CHECK: data_global
+CHECK-NEXT: 100 4
+CHECK-NEXT: ??:?
+CHECK-EMPTY:
+
+CHECK: str
+CHECK-NEXT: 104 4
+CHECK-NEXT: ??:?
+CHECK-EMPTY:
+
+CHECK: f()::function_global
+CHECK-NEXT: 144 4
+CHECK-NEXT: ??:?
+CHECK-EMPTY:
+
+CHECK: beta
+CHECK-NEXT: 148 4
+CHECK-NEXT: ??:?
+CHECK-EMPTY:
+
+CHECK: alpha
+CHECK-NEXT: 152 4
+CHECK-NEXT: ??:?
+CHECK-EMPTY:

github-actions · 2023-09-25T09:42:35Z

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

You can test this locally with the following command:

git-clang-format --diff 479057887fbc8bfef17c86694f78496c54550f21 382ee766d2a6a8a948843632f3612379980e3539 -- llvm/include/llvm/Object/XCOFFObjectFile.h llvm/lib/Object/SymbolSize.cpp llvm/lib/Object/XCOFFObjectFile.cpp

View the diff from clang-format here.

diff --git a/llvm/include/llvm/Object/XCOFFObjectFile.h b/llvm/include/llvm/Object/XCOFFObjectFile.h
index be468e888aa5..6c66a45260f5 100644
--- a/llvm/include/llvm/Object/XCOFFObjectFile.h
+++ b/llvm/include/llvm/Object/XCOFFObjectFile.h
@@ -832,9 +832,7 @@ public:
 
 #undef GETVALUE
 
-  uintptr_t getEntryAddress() const {
-    return getRawDataRefImpl().p;
-  }
+  uintptr_t getEntryAddress() const { return getRawDataRefImpl().p; }
 
   Expected<StringRef> getName() const;
   bool isFunction() const;
@@ -849,8 +847,7 @@ private:
 
 class xcoff_symbol_iterator : public symbol_iterator {
 public:
-  xcoff_symbol_iterator(const basic_symbol_iterator &B)
-      : symbol_iterator(B) {}
+  xcoff_symbol_iterator(const basic_symbol_iterator &B) : symbol_iterator(B) {}
 
   const XCOFFSymbolRef *operator->() const {
     return static_cast<const XCOFFSymbolRef *>(symbol_iterator::operator->());

llvm/include/llvm/Object/XCOFFObjectFile.h

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff.test

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff-symbolize-data.ll

llvm/include/llvm/Object/XCOFFObjectFile.h

chenzheng1030

Thanks for comments @diggerlin . Updated.

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff-symbolize-data.ll

jh7370

Basically looks good from my point of view. However, I'd like @diggerlin to give an LGTM, and I'd also like to see it in its final form once #67284 has landed. In other words, once @diggerlin is happy and the other PR has landed, please rebase this PR and force push, making sure that the PR description lines up with your final intended commit message, so that I can review that too.

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff-symbolize-data.ll

chenzheng1030 · 2023-09-27T09:07:50Z

once @diggerlin is happy and the other PR has landed, please rebase this PR and force push, making sure that the PR description lines up with your final intended commit message, so that I can review that too.

Thanks, I changed to use latest case from #67284 and the pr description is also finalized.

llvm/include/llvm/Object/XCOFFObjectFile.h

llvm/lib/Object/SymbolSize.cpp

llvm/include/llvm/Object/XCOFFObjectFile.h

jh7370 · 2023-09-28T07:34:10Z

Some tweaks to the final message required:

Computing the symbol size as the gap between sorted symbols is not right for XCOFF.
For XCOFF, the size info is stored in aux symbol and can be got from the existing XCOFF interface
getSymbolSize(). This patch changes XCOFFObjectFile to call this API to get sizes for symbols.

llvm/include/llvm/Object/XCOFFObjectFile.h

diggerlin

LGTM

jh7370

It looks like the formatting check failed for this patch. You should run clang-format on your new/modified code.

Please could you make a small patch (no PR necessary unless you're uncertain about anything) to address this and my other comments.

llvm/include/llvm/Object/XCOFFObjectFile.h

llvm/lib/Object/SymbolSize.cpp

chenzheng1030 · 2023-10-18T08:49:59Z

Please could you make a small patch (no PR necessary unless you're uncertain about anything) to address this and my other comments.

Thanks, 8c369eb is committed to address your code format comments.

chenzheng1030 requested review from hubert-reinterpretcast, EsmeYi and jh7370 September 25, 2023 09:31

chenzheng1030 self-assigned this Sep 25, 2023

llvmbot added the llvm:binary-utilities label Sep 25, 2023

chenzheng1030 mentioned this pull request Sep 25, 2023

Support big endian in llvm-symbolizer's data location dwarf info parser #67284

Merged

chenzheng1030 requested a review from nemanjai September 25, 2023 09:43

chenzheng1030 force-pushed the symbolize-data-size branch from cd5209b to 23b0ed8 Compare September 26, 2023 01:41

llvmbot added the debuginfo label Sep 26, 2023

jh7370 reviewed Sep 26, 2023

View reviewed changes

llvm/include/llvm/Object/XCOFFObjectFile.h Outdated Show resolved Hide resolved

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff.test Outdated Show resolved Hide resolved

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff.test Outdated Show resolved Hide resolved

jh7370 requested a review from diggerlin September 26, 2023 06:47

chenzheng1030 force-pushed the symbolize-data-size branch from 7d736ef to b9a3a7a Compare September 26, 2023 08:08

diggerlin reviewed Sep 26, 2023

View reviewed changes

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff-symbolize-data.ll Outdated Show resolved Hide resolved

llvm/include/llvm/Object/XCOFFObjectFile.h Outdated Show resolved Hide resolved

chenzheng1030 commented Sep 27, 2023

View reviewed changes

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff-symbolize-data.ll Outdated Show resolved Hide resolved

chenzheng1030 requested review from diggerlin and jh7370 September 27, 2023 06:21

jh7370 reviewed Sep 27, 2023

View reviewed changes

llvm/test/DebugInfo/Symbolize/XCOFF/xcoff-symbolize-data.ll Outdated Show resolved Hide resolved

chenzheng1030 force-pushed the symbolize-data-size branch from 99cd89b to cdf67b8 Compare September 27, 2023 09:03

chenzheng1030 requested a review from jh7370 September 27, 2023 09:06

diggerlin reviewed Sep 27, 2023

View reviewed changes

llvm/include/llvm/Object/XCOFFObjectFile.h Outdated Show resolved Hide resolved

llvm/lib/Object/SymbolSize.cpp Show resolved Hide resolved

diggerlin reviewed Sep 27, 2023

View reviewed changes

llvm/include/llvm/Object/XCOFFObjectFile.h Outdated Show resolved Hide resolved

chenzheng1030 requested a review from diggerlin September 28, 2023 07:57

chenzheng1030 force-pushed the symbolize-data-size branch from 8b0e507 to ca2fcaa Compare October 10, 2023 05:54

Chen Zheng added 3 commits October 10, 2023 01:56

get symbol size by calling xcoff interfaces.

6e2d6b9

address comments

c383e9c

address comments

5b7e39d

chenzheng1030 force-pushed the symbolize-data-size branch from ca2fcaa to 5b7e39d Compare October 10, 2023 05:57

address comment

be84368

diggerlin reviewed Oct 11, 2023

View reviewed changes

llvm/include/llvm/Object/XCOFFObjectFile.h Outdated Show resolved Hide resolved

llvm/include/llvm/Object/XCOFFObjectFile.h Outdated Show resolved Hide resolved

don't use macro

382ee76

chenzheng1030 requested a review from diggerlin October 11, 2023 14:43

diggerlin approved these changes Oct 11, 2023

View reviewed changes

chenzheng1030 merged commit 1379a72 into llvm:main Oct 12, 2023

chenzheng1030 deleted the symbolize-data-size branch October 12, 2023 03:16

jh7370 reviewed Oct 17, 2023

View reviewed changes

llvm/include/llvm/Object/XCOFFObjectFile.h Show resolved Hide resolved

llvm/lib/Object/SymbolSize.cpp Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[XCOFF][OBJECT] get symbol size by calling XCOFF interfaces #67304

[XCOFF][OBJECT] get symbol size by calling XCOFF interfaces #67304

chenzheng1030 commented Sep 25, 2023 •

edited

Loading

llvmbot commented Sep 25, 2023 •

edited

Loading

github-actions bot commented Sep 25, 2023 •

edited

Loading

chenzheng1030 left a comment

jh7370 left a comment

chenzheng1030 commented Sep 27, 2023

jh7370 commented Sep 28, 2023

diggerlin left a comment

jh7370 left a comment

chenzheng1030 commented Oct 18, 2023

[XCOFF][OBJECT] get symbol size by calling XCOFF interfaces #67304

[XCOFF][OBJECT] get symbol size by calling XCOFF interfaces #67304

Conversation

chenzheng1030 commented Sep 25, 2023 • edited Loading

llvmbot commented Sep 25, 2023 • edited Loading

github-actions bot commented Sep 25, 2023 • edited Loading

chenzheng1030 left a comment

Choose a reason for hiding this comment

jh7370 left a comment

Choose a reason for hiding this comment

chenzheng1030 commented Sep 27, 2023

jh7370 commented Sep 28, 2023

diggerlin left a comment

Choose a reason for hiding this comment

jh7370 left a comment

Choose a reason for hiding this comment

chenzheng1030 commented Oct 18, 2023

chenzheng1030 commented Sep 25, 2023 •

edited

Loading

llvmbot commented Sep 25, 2023 •

edited

Loading

github-actions bot commented Sep 25, 2023 •

edited

Loading