[llvm] [XCOFF] make related SD symbols as isFunction (PR #69553)

Chen Zheng via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 12 21:09:11 PST 2023


https://github.com/chenzheng1030 updated https://github.com/llvm/llvm-project/pull/69553

>From 821e60eb87e39de522803d25233af17332b5aadb Mon Sep 17 00:00:00 2001
From: Chen Zheng <czhengsz at cn.ibm.com>
Date: Mon, 30 Oct 2023 01:42:43 -0400
Subject: [PATCH 1/4] ad XTY_SD type function symbol.

---
 llvm/lib/Object/XCOFFObjectFile.cpp           | 73 +++++++++++++++++--
 llvm/test/CodeGen/PowerPC/aix-text.ll         | 12 +--
 .../CodeGen/PowerPC/aix-xcoff-funcsect.ll     | 12 +--
 .../tools/llvm-symbolizer/xcoff-sd-symbol.ll  |  4 +-
 4 files changed, 82 insertions(+), 19 deletions(-)

diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp
index 4c192aa37a7ecc7..cef897373c82240 100644
--- a/llvm/lib/Object/XCOFFObjectFile.cpp
+++ b/llvm/lib/Object/XCOFFObjectFile.cpp
@@ -1242,15 +1242,78 @@ bool XCOFFSymbolRef::isFunction() const {
 
   const XCOFFCsectAuxRef CsectAuxRef = ExpCsectAuxEnt.get();
 
-  // A function definition should be a label definition.
-  // FIXME: This is not necessarily the case when -ffunction-sections is
-  // enabled.
-  if (!CsectAuxRef.isLabel())
+  if (CsectAuxRef.getStorageMappingClass() != XCOFF::XMC_PR)
     return false;
 
-  if (CsectAuxRef.getStorageMappingClass() != XCOFF::XMC_PR)
+  // A function definition should not be a common type symbol or a external
+  // symbol.
+  if (CsectAuxRef.getSymbolType() == XCOFF::XTY_CM ||
+      CsectAuxRef.getSymbolType() == XCOFF::XTY_ER)
     return false;
 
+  // If the next symbol is a XTY_LD type symbol with same address, this XTY_SD
+  // symbol is not a function. Otherwise this is a function symbol for
+  // -ffunction-sections.
+  if (CsectAuxRef.getSymbolType() == XCOFF::XTY_SD) {
+    // If this is a csect with size 0, it won't be a function definition.
+    // This is used to hack situation that llvm always generates below symbol
+    // for -ffunction-sections:
+    // m   0x00000000     .text     1  unamex                    **No Symbol**
+    // a4  0x00000000       0    0     SD       PR    0    0
+    if (getSize() == 0)
+      return false;
+
+    Expected<uint64_t> SymbolAddressOrErr = getAddress();
+    if (!SymbolAddressOrErr) {
+      // If there is no address for this symbol, won't be a function.
+      consumeError(SymbolAddressOrErr.takeError());
+      return false;
+    }
+
+    uint8_t NumberOfAuxEntries = getNumberOfAuxEntries();
+
+    // If this is the last main symbol table entry, there won't be XTY_LD type
+    // symbol below.
+    if (getEntryAddress() == getObject()->getSymbolEntryAddressByIndex(
+                                 getObject()->getNumberOfSymbolTableEntries() -
+                                 NumberOfAuxEntries - 1))
+      return true;
+
+    DataRefImpl Ref;
+    Ref.p = XCOFFObjectFile::getAdvancedSymbolEntryAddress(
+        getEntryAddress(), NumberOfAuxEntries + 1);
+    XCOFFSymbolRef NextSym = getObject()->toSymbolRef(Ref);
+    if (!NextSym.isCsectSymbol())
+      return true;
+
+    Expected<uint64_t> NextSymbolAddressOrErr = NextSym.getAddress();
+    if (!NextSymbolAddressOrErr) {
+      // If there is no address for next symbol, won't be same with the XTY_SD
+      // symbol's address.
+      consumeError(NextSymbolAddressOrErr.takeError());
+      return true;
+    }
+
+    if (SymbolAddressOrErr.get() != NextSymbolAddressOrErr.get())
+      return true;
+
+    // Check next symbol is XTY_LD. If so, this symbol is not a function.
+    Expected<XCOFFCsectAuxRef> NextCsectAuxEnt = NextSym.getXCOFFCsectAuxRef();
+    if (!NextCsectAuxEnt) {
+      // If the next symbol has no aux entries, won't be a XTY_LD symbol.
+      consumeError(NextCsectAuxEnt.takeError());
+      return true;
+    }
+
+    if (NextCsectAuxEnt.get().getSymbolType() == XCOFF::XTY_LD)
+      return false;
+
+    return true;
+  }
+
+  if (CsectAuxRef.getSymbolType() == XCOFF::XTY_LD)
+    return true;
+
   const int16_t SectNum = getSectionNumber();
   Expected<DataRefImpl> SI = getObject()->getSectionByNum(SectNum);
   if (!SI) {
diff --git a/llvm/test/CodeGen/PowerPC/aix-text.ll b/llvm/test/CodeGen/PowerPC/aix-text.ll
index a0d1d0e38d50225..7f7c4e95fe8394d 100644
--- a/llvm/test/CodeGen/PowerPC/aix-text.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-text.ll
@@ -17,13 +17,13 @@ entry:
   ret i32 2
 }
 
-; CHECKFS32: 00000000 l       .text  00000000 (idx: {{[[:digit:]]*}}) [PR]
-; CHECKFS32-NEXT: 00000000 g       .text  {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) .text[PR]
-; CHECKFS32-NEXT: {{([[:xdigit:]]{8})}} g       .text  {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) .text2[PR]
+; CHECKFS32: 00000000 l        .text  00000000 (idx: {{[[:digit:]]*}}) [PR]
+; CHECKFS32-NEXT: 00000000 g      F .text  {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) .text[PR]
+; CHECKFS32-NEXT: {{([[:xdigit:]]{8})}} g      F .text  {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) .text2[PR]
 
-; CHECKFS64: 0000000000000000 l       .text  0000000000000000 
-; CHECKFS64-NEXT: 0000000000000000 g       .text  {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) .text[PR]
-; CHECKFS64-NEXT: {{([[:xdigit:]]{16})}} g       .text  {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) .text2[PR]
+; CHECKFS64: 0000000000000000 l      .text  0000000000000000
+; CHECKFS64-NEXT: 0000000000000000 g      F .text  {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) .text[PR]
+; CHECKFS64-NEXT: {{([[:xdigit:]]{16})}} g      F .text  {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) .text2[PR]
 
 ; CHECK32: 00000000 l       .text  {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) [PR]
 ; CHECK32-NEXT: {{([[:xdigit:]]{8})}} g     F .text (csect: (idx: {{[[:digit:]]*}}) [PR])   00000000 (idx: {{[[:digit:]]*}}) .text
diff --git a/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll b/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll
index 09c517c73dff296..2600fac01425da8 100644
--- a/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll
@@ -117,9 +117,9 @@ entry:
 ; XCOFF32-NEXT: 00000000 l       .text	00000000 (idx: 5) [PR]
 ; XCOFF32-NEXT: 00000000 g       .text	00000019 (idx: 7) .foo[PR]
 ; XCOFF32-NEXT: 00000000 g     F .text (csect: (idx: 7) .foo[PR]) 	00000000 (idx: 9) .alias_foo
-; XCOFF32-NEXT: 00000020 g       .text	00000020 .hidden (idx: 11) .hidden_foo[PR]
-; XCOFF32-NEXT: 00000040 g       .text	00000059 (idx: 13) .bar[PR]
-; XCOFF32-NEXT: 000000c0 l       .text	0000002a (idx: 15) .static_overalign_foo[PR]
+; XCOFF32-NEXT: 00000020 g     F .text	00000020 .hidden (idx: 11) .hidden_foo[PR]
+; XCOFF32-NEXT: 00000040 g     F .text	00000059 (idx: 13) .bar[PR]
+; XCOFF32-NEXT: 000000c0 l     F .text	0000002a (idx: 15) .static_overalign_foo[PR]
 ; XCOFF32-NEXT: 000000ec g     O .data	0000000c (idx: 17) foo[DS]
 ; XCOFF32-NEXT: 000000ec g     O .data (csect: (idx: 17) foo[DS]) 	00000000 (idx: 19) alias_foo
 ; XCOFF32-NEXT: 000000f8 g     O .data	0000000c .hidden (idx: 21) hidden_foo[DS]
@@ -152,9 +152,9 @@ entry:
 ; XCOFF64-NEXT: 0000000000000000 l       .text	0000000000000000 (idx: 5) [PR]
 ; XCOFF64-NEXT: 0000000000000000 g       .text	0000000000000019 (idx: 7) .foo[PR]
 ; XCOFF64-NEXT: 0000000000000000 g     F .text (csect: (idx: 7) .foo[PR]) 	0000000000000000 (idx: 9) .alias_foo
-; XCOFF64-NEXT: 0000000000000020 g       .text	0000000000000020 .hidden (idx: 11) .hidden_foo[PR]
-; XCOFF64-NEXT: 0000000000000040 g       .text	0000000000000059 (idx: 13) .bar[PR]
-; XCOFF64-NEXT: 00000000000000c0 l       .text	000000000000002a (idx: 15) .static_overalign_foo[PR]
+; XCOFF64-NEXT: 0000000000000020 g     F .text	0000000000000020 .hidden (idx: 11) .hidden_foo[PR]
+; XCOFF64-NEXT: 0000000000000040 g     F .text	0000000000000059 (idx: 13) .bar[PR]
+; XCOFF64-NEXT: 00000000000000c0 l     F .text	000000000000002a (idx: 15) .static_overalign_foo[PR]
 ; XCOFF64-NEXT: 00000000000000f0 g     O .data	0000000000000018 (idx: 17) foo[DS]
 ; XCOFF64-NEXT: 00000000000000f0 g     O .data (csect: (idx: 17) foo[DS]) 	0000000000000000 (idx: 19) alias_foo
 ; XCOFF64-NEXT: 0000000000000108 g     O .data	0000000000000018 .hidden (idx: 21) hidden_foo[DS]
diff --git a/llvm/test/tools/llvm-symbolizer/xcoff-sd-symbol.ll b/llvm/test/tools/llvm-symbolizer/xcoff-sd-symbol.ll
index 781ac72933a1523..aedceb0227b8962 100644
--- a/llvm/test/tools/llvm-symbolizer/xcoff-sd-symbol.ll
+++ b/llvm/test/tools/llvm-symbolizer/xcoff-sd-symbol.ll
@@ -16,10 +16,10 @@ entry:
   ret void
 }
 
-; CHECK: ??
+; CHECK: .foo
 ; CHECK: ??:0:0
 ; CHECK-EMPTY:
 
-; CHECK: ??
+; CHECK: .foo1
 ; CHECK: ??:0:0
 ; CHECK-EMPTY:

>From 8cee9cca418d78175c9eb533e0433d499da12489 Mon Sep 17 00:00:00 2001
From: Chen Zheng <czhengsz at cn.ibm.com>
Date: Mon, 30 Oct 2023 21:15:57 -0400
Subject: [PATCH 2/4] address comments

---
 llvm/lib/Object/XCOFFObjectFile.cpp | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp
index cef897373c82240..a7e4cd9c993d894 100644
--- a/llvm/lib/Object/XCOFFObjectFile.cpp
+++ b/llvm/lib/Object/XCOFFObjectFile.cpp
@@ -1242,7 +1242,8 @@ bool XCOFFSymbolRef::isFunction() const {
 
   const XCOFFCsectAuxRef CsectAuxRef = ExpCsectAuxEnt.get();
 
-  if (CsectAuxRef.getStorageMappingClass() != XCOFF::XMC_PR)
+  if (CsectAuxRef.getStorageMappingClass() != XCOFF::XMC_PR &&
+      CsectAuxRef.getStorageMappingClass() != XCOFF::XMC_GL)
     return false;
 
   // A function definition should not be a common type symbol or a external
@@ -1263,13 +1264,6 @@ bool XCOFFSymbolRef::isFunction() const {
     if (getSize() == 0)
       return false;
 
-    Expected<uint64_t> SymbolAddressOrErr = getAddress();
-    if (!SymbolAddressOrErr) {
-      // If there is no address for this symbol, won't be a function.
-      consumeError(SymbolAddressOrErr.takeError());
-      return false;
-    }
-
     uint8_t NumberOfAuxEntries = getNumberOfAuxEntries();
 
     // If this is the last main symbol table entry, there won't be XTY_LD type
@@ -1286,13 +1280,13 @@ bool XCOFFSymbolRef::isFunction() const {
     if (!NextSym.isCsectSymbol())
       return true;
 
+    Expected<uint64_t> SymbolAddressOrErr = getAddress();
+    if (!SymbolAddressOrErr)
+      return false;
+
     Expected<uint64_t> NextSymbolAddressOrErr = NextSym.getAddress();
-    if (!NextSymbolAddressOrErr) {
-      // If there is no address for next symbol, won't be same with the XTY_SD
-      // symbol's address.
-      consumeError(NextSymbolAddressOrErr.takeError());
+    if (!NextSymbolAddressOrErr)
       return true;
-    }
 
     if (SymbolAddressOrErr.get() != NextSymbolAddressOrErr.get())
       return true;

>From fd9e55bbc57108ed5f149a21d123fb25bbddcdae Mon Sep 17 00:00:00 2001
From: Chen Zheng <czhengsz at cn.ibm.com>
Date: Sun, 5 Nov 2023 22:04:49 -0500
Subject: [PATCH 3/4] address comments

---
 llvm/lib/Object/XCOFFObjectFile.cpp | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp
index a7e4cd9c993d894..b5e31ef05fee676 100644
--- a/llvm/lib/Object/XCOFFObjectFile.cpp
+++ b/llvm/lib/Object/XCOFFObjectFile.cpp
@@ -1280,15 +1280,7 @@ bool XCOFFSymbolRef::isFunction() const {
     if (!NextSym.isCsectSymbol())
       return true;
 
-    Expected<uint64_t> SymbolAddressOrErr = getAddress();
-    if (!SymbolAddressOrErr)
-      return false;
-
-    Expected<uint64_t> NextSymbolAddressOrErr = NextSym.getAddress();
-    if (!NextSymbolAddressOrErr)
-      return true;
-
-    if (SymbolAddressOrErr.get() != NextSymbolAddressOrErr.get())
+    if (cantFail(getAddress()) != cantFail(NextSym.getAddress()))
       return true;
 
     // Check next symbol is XTY_LD. If so, this symbol is not a function.

>From 1659f652915735ab1256ea2e2ca12f4147ba74d3 Mon Sep 17 00:00:00 2001
From: Chen Zheng <czhengsz at cn.ibm.com>
Date: Fri, 10 Nov 2023 03:19:52 -0500
Subject: [PATCH 4/4] address comments

---
 llvm/include/llvm/Object/XCOFFObjectFile.h |  3 +++
 llvm/lib/Object/XCOFFObjectFile.cpp        | 29 ++++------------------
 llvm/tools/clang                           |  1 +
 3 files changed, 9 insertions(+), 24 deletions(-)
 create mode 120000 llvm/tools/clang

diff --git a/llvm/include/llvm/Object/XCOFFObjectFile.h b/llvm/include/llvm/Object/XCOFFObjectFile.h
index 63064abb4d3c322..aa7d586482a51bb 100644
--- a/llvm/include/llvm/Object/XCOFFObjectFile.h
+++ b/llvm/include/llvm/Object/XCOFFObjectFile.h
@@ -853,6 +853,9 @@ class xcoff_symbol_iterator : public symbol_iterator {
   xcoff_symbol_iterator(const basic_symbol_iterator &B)
       : symbol_iterator(B) {}
 
+  xcoff_symbol_iterator(const XCOFFSymbolRef *Symbol)
+      : symbol_iterator(*Symbol) {}
+
   const XCOFFSymbolRef *operator->() const {
     return static_cast<const XCOFFSymbolRef *>(symbol_iterator::operator->());
   }
diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp
index b5e31ef05fee676..1321c8e5e545115 100644
--- a/llvm/lib/Object/XCOFFObjectFile.cpp
+++ b/llvm/lib/Object/XCOFFObjectFile.cpp
@@ -1264,27 +1264,17 @@ bool XCOFFSymbolRef::isFunction() const {
     if (getSize() == 0)
       return false;
 
-    uint8_t NumberOfAuxEntries = getNumberOfAuxEntries();
-
+    xcoff_symbol_iterator NextIt(this);
     // If this is the last main symbol table entry, there won't be XTY_LD type
     // symbol below.
-    if (getEntryAddress() == getObject()->getSymbolEntryAddressByIndex(
-                                 getObject()->getNumberOfSymbolTableEntries() -
-                                 NumberOfAuxEntries - 1))
-      return true;
-
-    DataRefImpl Ref;
-    Ref.p = XCOFFObjectFile::getAdvancedSymbolEntryAddress(
-        getEntryAddress(), NumberOfAuxEntries + 1);
-    XCOFFSymbolRef NextSym = getObject()->toSymbolRef(Ref);
-    if (!NextSym.isCsectSymbol())
+    if (++NextIt == getObject()->symbol_end())
       return true;
 
-    if (cantFail(getAddress()) != cantFail(NextSym.getAddress()))
+    if (cantFail(getAddress()) != cantFail(NextIt->getAddress()))
       return true;
 
     // Check next symbol is XTY_LD. If so, this symbol is not a function.
-    Expected<XCOFFCsectAuxRef> NextCsectAuxEnt = NextSym.getXCOFFCsectAuxRef();
+    Expected<XCOFFCsectAuxRef> NextCsectAuxEnt = NextIt->getXCOFFCsectAuxRef();
     if (!NextCsectAuxEnt) {
       // If the next symbol has no aux entries, won't be a XTY_LD symbol.
       consumeError(NextCsectAuxEnt.takeError());
@@ -1300,16 +1290,7 @@ bool XCOFFSymbolRef::isFunction() const {
   if (CsectAuxRef.getSymbolType() == XCOFF::XTY_LD)
     return true;
 
-  const int16_t SectNum = getSectionNumber();
-  Expected<DataRefImpl> SI = getObject()->getSectionByNum(SectNum);
-  if (!SI) {
-    // If we could not get the section, then this symbol should not be
-    // a function. So consume the error and return `false` to move on.
-    consumeError(SI.takeError());
-    return false;
-  }
-
-  return (getObject()->getSectionFlags(SI.get()) & XCOFF::STYP_TEXT);
+  return false;
 }
 
 bool XCOFFSymbolRef::isCsectSymbol() const {
diff --git a/llvm/tools/clang b/llvm/tools/clang
new file mode 120000
index 000000000000000..7700edcd10231dd
--- /dev/null
+++ b/llvm/tools/clang
@@ -0,0 +1 @@
+../../clang
\ No newline at end of file



More information about the llvm-commits mailing list