[XCOFF] make related SD symbols as isFunction #69553
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chenzheng1030
requested review from
hubert-reinterpretcast,
EsmeYi,
diggerlin,
jh7370 and
stephenpeckham
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-llvm-binary-utilities Author: Chen Zheng (chenzheng1030) ChangesThis is based on #69552 This will help tools like llvm-symbolizer recognizes more functions. Full diff: https://github.com/llvm/llvm-project/pull/69553.diff 15 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/Symbolize/SymbolizableObjectFile.h b/llvm/include/llvm/DebugInfo/Symbolize/SymbolizableObjectFile.h
index 075dbe3e0e372ed..8a881acb010ac3c 100644
--- a/llvm/include/llvm/DebugInfo/Symbolize/SymbolizableObjectFile.h
+++ b/llvm/include/llvm/DebugInfo/Symbolize/SymbolizableObjectFile.h
@@ -17,6 +17,7 @@
#include "llvm/DebugInfo/Symbolize/SymbolizableModule.h"
#include "llvm/Support/Error.h"
#include <cstdint>
+#include <map>
#include <memory>
#include <string>
#include <utility>
@@ -73,7 +74,6 @@ class SymbolizableObjectFile : public SymbolizableModule {
bool UntagAddresses;
struct SymbolDesc {
- uint64_t Addr;
// If size is 0, assume that symbol occupies the whole memory range up to
// the following symbol.
uint64_t Size;
@@ -82,12 +82,8 @@ class SymbolizableObjectFile : public SymbolizableModule {
// Non-zero if this is an ELF local symbol. See the comment in
// getNameFromSymbolTable.
uint32_t ELFLocalSymIdx;
-
- bool operator<(const SymbolDesc &RHS) const {
- return Addr != RHS.Addr ? Addr < RHS.Addr : Size < RHS.Size;
- }
};
- std::vector<SymbolDesc> Symbols;
+ std::map<uint64_t, SymbolDesc> Symbols;
// (index, filename) pairs of ELF STT_FILE symbols.
std::vector<std::pair<uint32_t, StringRef>> FileSymbols;
diff --git a/llvm/include/llvm/Object/XCOFFObjectFile.h b/llvm/include/llvm/Object/XCOFFObjectFile.h
index 63064abb4d3c322..0ec2c9ceecc46f0 100644
--- a/llvm/include/llvm/Object/XCOFFObjectFile.h
+++ b/llvm/include/llvm/Object/XCOFFObjectFile.h
@@ -780,6 +780,10 @@ class XCOFFSymbolRef : public SymbolRef {
"Symbol table entry pointer cannot be nullptr!");
}
+ XCOFFSymbolRef(const SymbolRef &B) : SymbolRef(B) {
+ assert(isa<XCOFFObjectFile>(SymbolRef::getObject()));
+ }
+
const XCOFFSymbolEntry32 *getSymbol32() const {
return reinterpret_cast<const XCOFFSymbolEntry32 *>(getRawDataRefImpl().p);
}
diff --git a/llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp b/llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
index 6b8068a531c05fa..398829b46fd7fa7 100644
--- a/llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
+++ b/llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
@@ -16,6 +16,7 @@
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
#include "llvm/Object/COFF.h"
#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/XCOFFObjectFile.h"
#include "llvm/Object/ObjectFile.h"
#include "llvm/Object/SymbolSize.h"
#include "llvm/Support/Casting.h"
@@ -70,20 +71,6 @@ SymbolizableObjectFile::create(const object::ObjectFile *Obj,
return std::move(E);
}
- std::vector<SymbolDesc> &SS = res->Symbols;
- // Sort by (Addr,Size,Name). If several SymbolDescs share the same Addr,
- // pick the one with the largest Size. This helps us avoid symbols with no
- // size information (Size=0).
- llvm::stable_sort(SS);
- auto I = SS.begin(), E = SS.end(), J = SS.begin();
- while (I != E) {
- auto OI = I;
- while (++I != E && OI->Addr == I->Addr) {
- }
- *J++ = I[-1];
- }
- SS.erase(J, SS.end());
-
return std::move(res);
}
@@ -134,7 +121,10 @@ Error SymbolizableObjectFile::addCoffExportSymbols(
uint32_t NextOffset = I != E ? I->Offset : Export.Offset + 1;
uint64_t SymbolStart = ImageBase + Export.Offset;
uint64_t SymbolSize = NextOffset - Export.Offset;
- Symbols.push_back({SymbolStart, SymbolSize, Export.Name, 0});
+ // If the SymbolStart exists, use the one with the large Size.
+ // This helps us avoid symbols with no size information (Size = 0).
+ if (!Symbols.count(SymbolStart) || SymbolSize >= Symbols[SymbolStart].Size)
+ Symbols[SymbolStart] = {SymbolSize, Export.Name, 0};
}
return Error::success();
}
@@ -215,7 +205,38 @@ Error SymbolizableObjectFile::addSymbol(const SymbolRef &Symbol,
if (Obj.isELF() && ELFSymbolRef(Symbol).getBinding() != ELF::STB_LOCAL)
ELFSymIdx = 0;
- Symbols.push_back({SymbolAddress, SymbolSize, SymbolName, ELFSymIdx});
+
+ // If the SymbolAddress exists, use the one with the large Size.
+ // This helps us avoid symbols with no size information (Size = 0).
+ if (!Symbols.count(SymbolAddress) ||
+ (!Obj.isXCOFF() && SymbolSize >= Symbols[SymbolAddress].Size)) {
+ Symbols[SymbolAddress] = {SymbolSize, SymbolName, ELFSymIdx};
+ } else if (Obj.isXCOFF()) {
+ // If the existing entry has no name, it must be a artificial symbol,
+ // replace it.
+ if (Symbols[SymbolAddress].Name.empty()) {
+ Symbols[SymbolAddress] = {SymbolSize, SymbolName, ELFSymIdx};
+ return Error::success();
+ }
+
+ // For XCOFF, XTY_SD symbol and its first XTY_LD symbol have same addresses,
+ // the symbol size of XTY_SD symbol is the size of the section while symbol
+ // size of the XTY_LD symbol is 0, but we need the XTY_LD symbol instead of
+ // the XTY_SD symbol.
+ XCOFFSymbolRef XCOFFSymbol = XCOFFSymbolRef(Symbol);
+ if (XCOFFSymbol.isCsectSymbol()) {
+ Expected<XCOFFCsectAuxRef> ExpCsectAuxEnt =
+ XCOFFSymbolRef(Symbol).getXCOFFCsectAuxRef();
+ // If there is no aux entry for, just ignore this symbol entry.
+ if (!ExpCsectAuxEnt)
+ consumeError(ExpCsectAuxEnt.takeError());
+ else {
+ if (ExpCsectAuxEnt.get().getSymbolType() == XCOFF::XTY_LD)
+ Symbols[SymbolAddress] = {SymbolSize, SymbolName, ELFSymIdx};
+ }
+ }
+ }
+
return Error::success();
}
@@ -234,26 +255,25 @@ uint64_t SymbolizableObjectFile::getModulePreferredBase() const {
bool SymbolizableObjectFile::getNameFromSymbolTable(
uint64_t Address, std::string &Name, uint64_t &Addr, uint64_t &Size,
std::string &FileName) const {
- SymbolDesc SD{Address, UINT64_C(-1), StringRef(), 0};
- auto SymbolIterator = llvm::upper_bound(Symbols, SD);
+ auto SymbolIterator = Symbols.upper_bound(Address);
if (SymbolIterator == Symbols.begin())
return false;
--SymbolIterator;
- if (SymbolIterator->Size != 0 &&
- SymbolIterator->Addr + SymbolIterator->Size <= Address)
+
+ auto &SD = SymbolIterator->second;
+ if (SD.Size != 0 && SymbolIterator->first + SD.Size <= Address)
return false;
- Name = SymbolIterator->Name.str();
- Addr = SymbolIterator->Addr;
- Size = SymbolIterator->Size;
+ Name = SD.Name.str();
+ Addr = SymbolIterator->first;
+ Size = SD.Size;
- if (SymbolIterator->ELFLocalSymIdx != 0) {
+ if (SD.ELFLocalSymIdx != 0) {
// If this is an ELF local symbol, find the STT_FILE symbol preceding
// SymbolIterator to get the filename. The ELF spec requires the STT_FILE
// symbol (if present) precedes the other STB_LOCAL symbols for the file.
assert(Module->isELF());
- auto It = llvm::upper_bound(
- FileSymbols,
- std::make_pair(SymbolIterator->ELFLocalSymIdx, StringRef()));
+ auto It = llvm::upper_bound(FileSymbols,
+ std::make_pair(SD.ELFLocalSymIdx, StringRef()));
if (It != FileSymbols.begin())
FileName = It[-1].second.str();
}
diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp
index 4c192aa37a7ecc7..a090d4c842d5f15 100644
--- a/llvm/lib/Object/XCOFFObjectFile.cpp
+++ b/llvm/lib/Object/XCOFFObjectFile.cpp
@@ -1242,11 +1242,9 @@ 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())
- return false;
+ // A function definition should not be a common type symbol.
+ if (CsectAuxRef.getSymbolType() == XCOFF::XTY_CM)
+ return false;
if (CsectAuxRef.getStorageMappingClass() != XCOFF::XMC_PR)
return false;
diff --git a/llvm/test/CodeGen/PowerPC/aix-alias-alignment-2.ll b/llvm/test/CodeGen/PowerPC/aix-alias-alignment-2.ll
index 209f0e9c38385c7..fab134d73a3eab7 100644
--- a/llvm/test/CodeGen/PowerPC/aix-alias-alignment-2.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-alias-alignment-2.ll
@@ -59,7 +59,7 @@ define void @foo3(%struct.B %a1) {
; SYM: SYMBOL TABLE:
; SYM-NEXT: 00000000 df *DEBUG* 00000000 <stdin>
-; SYM-NEXT: 00000000 l .text 0000008a
+; SYM-NEXT: 00000000 l F .text 0000008a
; SYM-NEXT: 00000000 g F .text (csect: ) 00000000 .foo1
; SYM-NEXT: 00000030 g F .text (csect: ) 00000000 .foo2
; SYM-NEXT: 00000060 g F .text (csect: ) 00000000 .foo3
diff --git a/llvm/test/CodeGen/PowerPC/aix-alias-alignment.ll b/llvm/test/CodeGen/PowerPC/aix-alias-alignment.ll
index a1ad80481adf160..0f08ff4ef6aefca 100644
--- a/llvm/test/CodeGen/PowerPC/aix-alias-alignment.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-alias-alignment.ll
@@ -62,7 +62,7 @@ define void @foo(i32 %a1, i32 %a2, i32 %a3) {
; SYM: SYMBOL TABLE:
; SYM-NEXT: 00000000 df *DEBUG* 00000000 <stdin>
-; SYM-NEXT: 00000000 l .text 00000029
+; SYM-NEXT: 00000000 l F .text 00000029
; SYM-NEXT: 00000000 g F .text (csect: ) 00000000 .foo
; SYM-NEXT: 0000002c l .data 00000008 .data
; SYM-NEXT: 0000002c g O .data (csect: .data) 00000000 _MergedGlobals
diff --git a/llvm/test/CodeGen/PowerPC/aix-text.ll b/llvm/test/CodeGen/PowerPC/aix-text.ll
index a0d1d0e38d50225..50dc723d42cc271 100644
--- a/llvm/test/CodeGen/PowerPC/aix-text.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-text.ll
@@ -17,18 +17,18 @@ 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 F .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 F .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: 00000000 l F .text {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) [PR]
; CHECK32-NEXT: {{([[:xdigit:]]{8})}} g F .text (csect: (idx: {{[[:digit:]]*}}) [PR]) 00000000 (idx: {{[[:digit:]]*}}) .text
; CHECK32-NEXT: {{([[:xdigit:]]{8})}} g F .text (csect: (idx: {{[[:digit:]]*}}) [PR]) 00000000 (idx: {{[[:digit:]]*}}) .text2
-; CHECK64: 0000000000000000 l .text {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) [PR]
+; CHECK64: 0000000000000000 l F .text {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) [PR]
; CHECK64-NEXT: {{([[:xdigit:]]{16})}} g F .text (csect: (idx: {{[[:digit:]]*}}) [PR]) 0000000000000000 (idx: {{[[:digit:]]*}}) .text
; CHECK64-NEXT: {{([[:xdigit:]]{8})}} g F .text (csect: (idx: {{[[:digit:]]*}}) [PR]) 0000000000000000 (idx: {{[[:digit:]]*}}) .text2
diff --git a/llvm/test/CodeGen/PowerPC/aix-xcoff-cold.ll b/llvm/test/CodeGen/PowerPC/aix-xcoff-cold.ll
index db6071653b6bcca..ea236c7c582a397 100644
--- a/llvm/test/CodeGen/PowerPC/aix-xcoff-cold.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-xcoff-cold.ll
@@ -3,7 +3,7 @@
; CHECK: SYMBOL TABLE:
; CHECK-NEXT: 0000000000000000 df *DEBUG* 0000000000000000 <stdin>
-; CHECK-NEXT: 0000000000000000 l .text 000000000000001e
+; CHECK-NEXT: 0000000000000000 l F .text 000000000000001e
; CHECK-NEXT: 0000000000000000 g F .text (csect: ) 0000000000000000 .cold_fun
; CHECK-NEXT: 0000000000000020 g O .data 0000000000000018 cold_fun
diff --git a/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll b/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll
index 09c517c73dff296..bbfd808a140f378 100644
--- a/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-xcoff-funcsect.ll
@@ -114,12 +114,12 @@ entry:
; XCOFF32-NEXT: 00000000 df *DEBUG* 00000000 (idx: 0) <stdin>
; XCOFF32-NEXT: 00000000 *UND* 00000000 (idx: 1) .extern_foo[PR]
; XCOFF32-NEXT: 00000000 *UND* 00000000 (idx: 3) extern_foo[DS]
-; XCOFF32-NEXT: 00000000 l .text 00000000 (idx: 5) [PR]
-; XCOFF32-NEXT: 00000000 g .text 00000019 (idx: 7) .foo[PR]
+; XCOFF32-NEXT: 00000000 l F .text 00000000 (idx: 5) [PR]
+; XCOFF32-NEXT: 00000000 g F .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]
@@ -149,12 +149,12 @@ entry:
; XCOFF64-NEXT: 0000000000000000 df *DEBUG* 0000000000000000 (idx: 0) <stdin>
; XCOFF64-NEXT: 0000000000000000 *UND* 0000000000000000 (idx: 1) .extern_foo[PR]
; XCOFF64-NEXT: 0000000000000000 *UND* 0000000000000000 (idx: 3) extern_foo[DS]
-; XCOFF64-NEXT: 0000000000000000 l .text 0000000000000000 (idx: 5) [PR]
-; XCOFF64-NEXT: 0000000000000000 g .text 0000000000000019 (idx: 7) .foo[PR]
+; XCOFF64-NEXT: 0000000000000000 l F .text 0000000000000000 (idx: 5) [PR]
+; XCOFF64-NEXT: 0000000000000000 g F .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/CodeGen/PowerPC/pgo-ref-directive.ll b/llvm/test/CodeGen/PowerPC/pgo-ref-directive.ll
index 480b44caaded7a5..7dd4d31fca3589f 100644
--- a/llvm/test/CodeGen/PowerPC/pgo-ref-directive.ll
+++ b/llvm/test/CodeGen/PowerPC/pgo-ref-directive.ll
@@ -122,8 +122,8 @@ entry:
; WITHVNDS-OBJ: SYMBOL TABLE:
; WITHVNDS-OBJ-NEXT: 00000000 df *DEBUG* 00000000 <stdin>
-; WITHVNDS-OBJ-NEXT: 00000000 l .text 00000008
-; WITHVNDS-OBJ-NEXT: 00000000 g F .text (csect: ) 00000000 .main
+; WITHVNDS-OBJ-NEXT: 00000000 l F .text 00000008
+; WITHVNDS-OBJ-NEXT: 00000000 g F .text (csect: ) 00000000 .main
; WITHVNDS-OBJ-NEXT: 00000008 l .text 00000006 __llvm_prf_names
; WITHVNDS-OBJ-NEXT: 00000010 l O .data 00000008 __llvm_prf_cnts
; WITHVNDS-OBJ-NEXT: 00000018 l O .data 00000008 __llvm_prf_data
diff --git a/llvm/test/MC/PowerPC/aix-file-symbols-empty.s b/llvm/test/MC/PowerPC/aix-file-symbols-empty.s
index e166eef7e3fd9f6..af69781d4932c95 100644
--- a/llvm/test/MC/PowerPC/aix-file-symbols-empty.s
+++ b/llvm/test/MC/PowerPC/aix-file-symbols-empty.s
@@ -9,6 +9,6 @@
# CHECK: SYMBOL TABLE:
# CHECK-NEXT: 00000000 df *DEBUG* 00000000 .file
-# CHECK-NEXT: 00000000 l .text 00000000
+# CHECK-NEXT: 00000000 l F .text 00000000
# CHECK-NEXT: 00000000 g F .text (csect: ) 00000000 .var1
# CHECK-NEXT: 00000000 g F .text (csect: ) 00000000 .var2
diff --git a/llvm/test/MC/PowerPC/aix-file-symbols.s b/llvm/test/MC/PowerPC/aix-file-symbols.s
index 7ab0244f3c7141e..6f9d6341f345055 100644
--- a/llvm/test/MC/PowerPC/aix-file-symbols.s
+++ b/llvm/test/MC/PowerPC/aix-file-symbols.s
@@ -14,6 +14,6 @@
# CHECK-NEXT: 00000000 df *DEBUG* 00000000 1.c
# CHECK-NEXT: 00000000 df *DEBUG* 00000000 2.c
# CHECK-NEXT: 00000000 df *DEBUG* 00000000 3.c
-# CHECK-NEXT: 00000000 l .text 00000000
+# CHECK-NEXT: 00000000 l F .text 00000000
# CHECK-NEXT: 00000000 g F .text (csect: ) 00000000 .var1
# CHECK-NEXT: 00000000 g F .text (csect: ) 00000000 .var2
diff --git a/llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-priority.ll b/llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-priority.ll
index 6db8451ea6a13b8..72c82fdc8b006f0 100644
--- a/llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-priority.ll
+++ b/llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-priority.ll
@@ -4,7 +4,10 @@
; CHECK: Disassembly of section .text:
; CHECK: 00000000 <.foo3>:
; CHECK: 00000020 <.foo4>:
-; CHECK: 00000040 <.foo>:
+
+;; FIXME: should show the function name instead of the section name.
+; CHECK: 00000040 <explicit_sec>:
+
; CHECK: 00000060 <.foo2>:
define dso_local signext i32 @foo(i32 noundef signext %a) #0 section "explicit_sec" {
diff --git a/llvm/test/tools/llvm-objdump/XCOFF/symbol-table.test b/llvm/test/tools/llvm-objdump/XCOFF/symbol-table.test
index f8fc114f05a6206..109718e8552ae2d 100644
--- a/llvm/test/tools/llvm-objdump/XCOFF/symbol-table.test
+++ b/llvm/test/tools/llvm-objdump/XCOFF/symbol-table.test
@@ -68,7 +68,7 @@ entry:
; SYM: SYMBOL TABLE:
; SYM-NEXT: 00000000 df *DEBUG* 00000000 <stdin>
; SYM-NEXT: 00000000 *UND* 00000000 ei
-; SYM-NEXT: 00000000 l .text 00000091
+; SYM-NEXT: 00000000 l F .text 00000091
; SYM-NEXT: 00000000 g F .text (csect: ) 00000000 .bar
; SYM-NEXT: 00000050 g F .text (csect: ) 00000000 .foo
; SYM-NEXT: 00000094 l .text 00000013 L...str
@@ -90,7 +90,7 @@ entry:
; SYM-DES: SYMBOL TABLE:
; SYM-DES-NEXT: 00000000 df *DEBUG* 00000000 (idx: 0) <stdin>
; SYM-DES-NEXT: 00000000 *UND* 00000000 (idx: 1) ei[UA]
-; SYM-DES-NEXT: 00000000 l .text 00000091 (idx: 3) [PR]
+; SYM-DES-NEXT: 00000000 l F .text 00000091 (idx: 3) [PR]
; SYM-DES-NEXT: 00000000 g F .text (csect: (idx: 3) [PR]) 00000000 (idx: 5) .bar
; SYM-DES-NEXT: 00000050 g F .text (csect: (idx: 3) [PR]) 00000000 (idx: 7) .foo
; SYM-DES-NEXT: 00000094 l .text 00000013 (idx: 9) L...str[RO]
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:
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
chenzheng1030
force-pushed
the
sd_symbol
branch
from
a88c5ea
to
e2c187e
Compare
|
@chenzheng1030, the prerequisite change is still under review. Can you make this a draft PR or something? |
diggerlin
reviewed
Oct 24, 2023
chenzheng1030
force-pushed
the
sd_symbol
branch
from
e2c187e
to
0b7bb24
Compare
|
Thanks for review @diggerlin . I've made a new solution. Could you please help to review again? FYI @jh7370. This is the new solution based on your comments in #69552 |
jh7370
reviewed
Oct 26, 2023
chenzheng1030
force-pushed
the
sd_symbol
branch
3 times, most recently
from
bea3649
to
415b078
Compare
chenzheng1030
commented
Oct 27, 2023
llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-priority.ll
Outdated
Show resolved
Hide resolved
jh7370
reviewed
Oct 27, 2023
diggerlin
reviewed
Oct 27, 2023
|
From your comments, I can't tell what you're trying to accomplish, and I'm not sure of all the things that llvm-symbolizer does. But here are some things to consider (some of them are already done correctly in the code):
If you use the AIX dump command, you see output similar to: [1] m 0x00000000 .text 1 unamex No Symbol The letter 'm' indicates a primary symbol table entry (symbols 1 & 3). The other entries (a4) are for auxiliary entries. So I consider this output to be for 2 symbols, not 4. |
diggerlin
reviewed
Oct 28, 2023
chenzheng1030
force-pushed
the
sd_symbol
branch
from
902701a
to
52d8c9d
Compare
chenzheng1030
force-pushed
the
sd_symbol
branch
from
1a5e7f6
to
ad3ceda
Compare
jh7370
approved these changes
Nov 15, 2023
There was a problem hiding this comment.
LGTM, barring nit, but please wait for @diggerlin before submitting.
diggerlin
reviewed
Nov 19, 2023
llvm/lib/Object/XCOFFObjectFile.cpp
Outdated
| if (CsectAuxRef.getSymbolType() == XCOFF::XTY_LD) | ||
| return true; | ||
|
|
||
| return false; |
There was a problem hiding this comment.
if the code run into the line, it means symbol has unexpected symbol type. report an error here ?
jh7370
requested changes
Nov 20, 2023
diggerlin
reviewed
Nov 20, 2023
llvm/lib/Object/XCOFFObjectFile.cpp
Outdated
| if (!NameOrErr) | ||
| return NameOrErr.takeError(); | ||
|
|
||
| return createError("csect symbol " + *NameOrErr + |
There was a problem hiding this comment.
can we add Symbol Index into the error as XCOFFSymbolRef::getXCOFFCsectAuxRef() ? since the getName() do not include the storage mapping class, several symbols maybe have the same name. and do we need to add a test case for the emit error code @jh7370 ?
There was a problem hiding this comment.
Done. Cases added and use symbol index like you suggested.
There was a problem hiding this comment.
Yes, all error paths should be tested as much as non-error paths (I see @chenzheng1030 has already addressed this point).
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
suggested in review of llvm#69553 This is actually not an NFC as isFunction() does not return false for some "invalid" object, instead it returns the errors to its caller. But since there is no such invalid object in the LIT tests, so no case changes.
chenzheng1030
force-pushed
the
sd_symbol
branch
from
3744f29
to
4350bab
Compare
jh7370
reviewed
Nov 21, 2023
llvm/lib/Object/XCOFFObjectFile.cpp
Outdated
| @@ -1288,7 +1288,11 @@ Expected<bool> XCOFFSymbolRef::isFunction() const { | |||
| if (CsectAuxRef.getSymbolType() == XCOFF::XTY_LD) | |||
| return true; | |||
|
|
|||
| return createError("csect symbol has no valid symbol type."); | |||
| return createError( | |||
| "Symbol csect aux entry with index " + | |||
There was a problem hiding this comment.
This message doesn't conform to the LLVM coding standards. Please fix.
There was a problem hiding this comment.
Hope I am doing right this time.
llvm/lib/Object/XCOFFObjectFile.cpp
Outdated
| if (!NameOrErr) | ||
| return NameOrErr.takeError(); | ||
|
|
||
| return createError("csect symbol " + *NameOrErr + |
There was a problem hiding this comment.
Yes, all error paths should be tested as much as non-error paths (I see @chenzheng1030 has already addressed this point).
jh7370
approved these changes
Nov 22, 2023
| @@ -54,3 +31,4 @@ Symbols: | |||
| - Type: AUX_CSECT | |||
| SymbolAlignmentAndType: 5 | |||
| StorageMappingClass: XMC_PR | |||
|
|
|||
There was a problem hiding this comment.
Nit: Looks like you've added an additional blank line unnecessarily? Files should end in exactly one \n
diggerlin
approved these changes
Nov 22, 2023
Guzhu-AMD
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Nov 30, 2023
Local branch amd-gfx bb29769 Merged main:3bbed4ee2681 into amd-gfx:4f2762c1457a Remote branch main abc4058 [XCOFF] make related SD symbols as isFunction (llvm#69553)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will help tools like llvm-symbolizer recognizes more functions.