[llvm-objdump] Rework .gnu.version_d dumping #128434
Open
+98
−52
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
Created using spr 1.3.5-bogner
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Fangrui Song (MaskRay) Changesand fix crash when vd_aux is invalid (#86611). vd_version, vd_flags, vd_ndx, and vd_cnt in Elf{32,64}_Verdef are vda_name specifies a NUL-terminated string. Update getVersionDefinitions Full diff: https://github.com/llvm/llvm-project/pull/128434.diff 8 Files Affected:
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index 3aa1d7864fcb7..57a6db6c4e5aa 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -41,10 +41,10 @@ struct VerdAux {
struct VerDef {
unsigned Offset;
- unsigned Version;
- unsigned Flags;
- unsigned Ndx;
- unsigned Cnt;
+ uint16_t Version;
+ uint16_t Flags;
+ uint16_t Ndx;
+ uint16_t Cnt;
unsigned Hash;
std::string Name;
std::vector<VerdAux> AuxV;
@@ -1057,8 +1057,8 @@ ELFFile<ELFT>::getVersionDefinitions(const Elf_Shdr &Sec) const {
VerdAux Aux;
Aux.Offset = VerdauxBuf - Start;
- if (Verdaux->vda_name <= StrTabOrErr->size())
- Aux.Name = std::string(StrTabOrErr->drop_front(Verdaux->vda_name));
+ if (Verdaux->vda_name < StrTabOrErr->size())
+ Aux.Name = std::string(StrTabOrErr->drop_front(Verdaux->vda_name).data());
else
Aux.Name = ("<invalid vda_name: " + Twine(Verdaux->vda_name) + ">").str();
return Aux;
diff --git a/llvm/test/tools/llvm-objdump/ELF/private-headers.test b/llvm/test/tools/llvm-objdump/ELF/private-headers.test
index eefdc8440385c..15a721895525b 100644
--- a/llvm/test/tools/llvm-objdump/ELF/private-headers.test
+++ b/llvm/test/tools/llvm-objdump/ELF/private-headers.test
@@ -37,6 +37,7 @@ Sections:
Value: 0x0
- Name: .gnu.version_d
Type: SHT_GNU_verdef
+ AddressAlign: 4
Entries:
- Version: 1
Flags: 1
diff --git a/llvm/test/tools/llvm-objdump/ELF/verdef-invalid.test b/llvm/test/tools/llvm-objdump/ELF/verdef-invalid.test
new file mode 100644
index 0000000000000..8d0199244752a
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/ELF/verdef-invalid.test
@@ -0,0 +1,50 @@
+## Adapted from test/llvm-readobj/ELF/verdef-invalid.test
+## Check that we report a warning when a SHT_GNU_verdef section contains a version definition
+## that refers to an auxiliary entry that goes past the end of the section.
+
+# RUN: yaml2obj %s -o %t5
+# RUN: llvm-objdump -p %t5 2>&1 | FileCheck %s --check-prefix=AUX-PAST-END -DFILE=%t5
+# RUN: llvm-objdump -p %t5 2>&1 | FileCheck %s --check-prefix=AUX-PAST-END -DFILE=%t5
+
+# AUX-PAST-END: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 1: version definition 1 refers to an auxiliary entry that goes past the end of the section
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_DYN
+Sections:
+ - Name: .gnu.version_d
+ Type: SHT_GNU_verdef
+ Entries:
+ - Names:
+ - FOO
+ ShSize: 21
+DynamicSymbols:
+ - Name: foo
+
+## Check we report a warning when a version definition is not correctly aligned in memory.
+
+# RUN: yaml2obj %s --docnum=2 -o %t7
+# RUN: llvm-objdump -p %t7 2>&1 | FileCheck %s --check-prefix=MISALIGNED-DEF -DFILE=%t7
+# RUN: llvm-objdump -p %t7 2>&1 | FileCheck %s --check-prefix=MISALIGNED-DEF -DFILE=%t7
+
+# MISALIGNED-DEF: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 1: found a misaligned version definition entry at offset 0x0
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_DYN
+Sections:
+ - Type: Fill
+ Size: 0x1
+ - Name: .gnu.version_d
+ Type: SHT_GNU_verdef
+ Link: .dynstr
+ Info: 0x1
+ Entries:
+ - Names:
+ - FOO
+DynamicSymbols:
+ - Name: foo
diff --git a/llvm/test/tools/llvm-objdump/ELF/verdef.test b/llvm/test/tools/llvm-objdump/ELF/verdef.test
index e4ae33853deb4..dbb10bf87cbea 100644
--- a/llvm/test/tools/llvm-objdump/ELF/verdef.test
+++ b/llvm/test/tools/llvm-objdump/ELF/verdef.test
@@ -1,12 +1,14 @@
# RUN: yaml2obj %s -o %t
-# RUN: llvm-objdump -p %t | FileCheck --strict-whitespace %s
+# RUN: llvm-objdump -p %t | FileCheck --match-full-lines --strict-whitespace %s
-# CHECK: Dynamic Section:
-# CHECK-EMPTY:
-# CHECK-NEXT: Version definitions:
-# CHECK-NEXT: 1 0x01 0x075bcd15 foo
-# CHECK-NEXT: 2 0x02 0x3ade68b1 VERSION_1
-# CHECK-NEXT: VERSION_2
+# CHECK:Dynamic Section:
+#CHECK-EMPTY:
+# CHECK-NEXT:Version definitions:
+# CHECK-NEXT:2 0x01 0x075bcd15 foo
+# CHECK-NEXT:3 0x02 0x3ade68b1 VERSION_1
+# CHECK-NEXT: VERSION_2
+# CHECK-NEXT:4 0x00 0x0000007b VERSION_3
+# CHECK-NEXT: VERSION_4 VERSION_5
--- !ELF
FileHeader:
@@ -24,17 +26,25 @@ Sections:
Entries:
- Version: 1
Flags: 1
- VersionNdx: 1
+ VersionNdx: 2
Hash: 123456789
Names:
- foo
- Version: 1
Flags: 2
- VersionNdx: 2
+ VersionNdx: 3
Hash: 987654321
Names:
- VERSION_1
- VERSION_2
+ - Version: 1
+ Flags: 0
+ VersionNdx: 4
+ Hash: 123
+ Names:
+ - VERSION_3
+ - VERSION_4
+ - VERSION_5
DynamicSymbols:
- Name: bar
Binding: STB_GLOBAL
diff --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp
index e9e5b059f1786..0c9b1f3479f83 100644
--- a/llvm/tools/llvm-objdump/ELFDump.cpp
+++ b/llvm/tools/llvm-objdump/ELFDump.cpp
@@ -378,38 +378,6 @@ void ELFDumper<ELFT>::printSymbolVersionDependency(
}
}
-template <class ELFT>
-static void printSymbolVersionDefinition(const typename ELFT::Shdr &Shdr,
- ArrayRef<uint8_t> Contents,
- StringRef StrTab) {
- outs() << "\nVersion definitions:\n";
-
- const uint8_t *Buf = Contents.data();
- uint32_t VerdefIndex = 1;
- // sh_info contains the number of entries in the SHT_GNU_verdef section. To
- // make the index column have consistent width, we should insert blank spaces
- // according to sh_info.
- uint16_t VerdefIndexWidth = std::to_string(Shdr.sh_info).size();
- while (Buf) {
- auto *Verdef = reinterpret_cast<const typename ELFT::Verdef *>(Buf);
- outs() << format_decimal(VerdefIndex++, VerdefIndexWidth) << " "
- << format("0x%02" PRIx16 " ", (uint16_t)Verdef->vd_flags)
- << format("0x%08" PRIx32 " ", (uint32_t)Verdef->vd_hash);
-
- const uint8_t *BufAux = Buf + Verdef->vd_aux;
- uint16_t VerdauxIndex = 0;
- while (BufAux) {
- auto *Verdaux = reinterpret_cast<const typename ELFT::Verdaux *>(BufAux);
- if (VerdauxIndex)
- outs() << std::string(VerdefIndexWidth + 17, ' ');
- outs() << StringRef(StrTab.drop_front(Verdaux->vda_name).data()) << '\n';
- BufAux = Verdaux->vda_next ? BufAux + Verdaux->vda_next : nullptr;
- ++VerdauxIndex;
- }
- Buf = Verdef->vd_next ? Buf + Verdef->vd_next : nullptr;
- }
-}
-
template <class ELFT> void ELFDumper<ELFT>::printSymbolVersion() {
const ELFFile<ELFT> &Elf = getELFFile();
StringRef FileName = Obj.getFileName();
@@ -426,10 +394,26 @@ template <class ELFT> void ELFDumper<ELFT>::printSymbolVersion() {
unwrapOrError(Elf.getSection(Shdr.sh_link), FileName);
StringRef StrTab = unwrapOrError(Elf.getStringTable(*StrTabSec), FileName);
- if (Shdr.sh_type == ELF::SHT_GNU_verneed)
+ if (Shdr.sh_type == ELF::SHT_GNU_verneed) {
printSymbolVersionDependency(Shdr);
- else
- printSymbolVersionDefinition<ELFT>(Shdr, Contents, StrTab);
+ } else {
+ OS << "\nVersion definitions:\n";
+ Expected<std::vector<VerDef>> V =
+ getELFFile().getVersionDefinitions(Shdr);
+ if (!V) {
+ this->reportUniqueWarning(V.takeError());
+ continue;
+ }
+ for (const VerDef &Def : *V) {
+ OS << Def.Ndx << ' ' << format_hex(Def.Flags, 4) << ' '
+ << format_hex(Def.Hash, 10) << ' ' << Def.Name << '\n';
+ if (!Def.AuxV.empty()) {
+ for (auto [I, Aux] : enumerate(Def.AuxV))
+ OS << (I ? ' ' : '\t') << Aux.Name;
+ OS << '\n';
+ }
+ }
+ }
}
}
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 99e0440dce78d..115f04a4df778 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -360,7 +360,7 @@ static StringRef ToolName;
std::unique_ptr<BuildIDFetcher> BIDFetcher;
-Dumper::Dumper(const object::ObjectFile &O) : O(O) {
+Dumper::Dumper(const object::ObjectFile &O) : O(O), OS(outs()) {
WarningHandler = [this](const Twine &Msg) {
if (Warnings.insert(Msg.str()).second)
reportWarning(Msg, this->O.getFileName());
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.h b/llvm/tools/llvm-objdump/llvm-objdump.h
index 7253cc3f4d91b..25d9c1e106a6c 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.h
+++ b/llvm/tools/llvm-objdump/llvm-objdump.h
@@ -77,6 +77,7 @@ class Dumper {
StringSet<> Warnings;
protected:
+ llvm::raw_ostream &OS;
std::function<Error(const Twine &Msg)> WarningHandler;
public:
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index fdae09ac767e6..e7825419ef9ec 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -7668,7 +7668,7 @@ void LLVMELFDumper<ELFT>::printVersionDefinitionSection(const Elf_Shdr *Sec) {
W.printFlags("Flags", D.Flags, ArrayRef(SymVersionFlags));
W.printNumber("Index", D.Ndx);
W.printNumber("Hash", D.Hash);
- W.printString("Name", D.Name.c_str());
+ W.printString("Name", D.Name);
W.printList(
"Predecessors", D.AuxV,
[](raw_ostream &OS, const VerdAux &Aux) { OS << Aux.Name.c_str(); });
|
jh7370
reviewed
Feb 25, 2025
| Entries: | ||
| - Names: | ||
| - FOO | ||
| ShSize: 21 |
There was a problem hiding this comment.
What should the size actually be (perhaps worth a comment, or even a test case showing that)?
| ## Check that we report a warning when a SHT_GNU_verdef section contains a version definition | ||
| ## that refers to an auxiliary entry that goes past the end of the section. | ||
|
|
||
| # RUN: yaml2obj %s -o %t5 |
There was a problem hiding this comment.
Nit: no need for this to be %t5, since this is the first file.
|
|
||
| ## Check we report a warning when a version definition is not correctly aligned in memory. | ||
|
|
||
| # RUN: yaml2obj %s --docnum=2 -o %t7 |
There was a problem hiding this comment.
Same as nit above (%t7 -> e.g. %t2)
|
|
||
| # RUN: yaml2obj %s -o %t5 | ||
| # RUN: llvm-objdump -p %t5 2>&1 | FileCheck %s --check-prefix=AUX-PAST-END -DFILE=%t5 | ||
| # RUN: llvm-objdump -p %t5 2>&1 | FileCheck %s --check-prefix=AUX-PAST-END -DFILE=%t5 |
There was a problem hiding this comment.
What's the motivation for the double RUN? Same goes below.
| else | ||
| printSymbolVersionDefinition<ELFT>(Shdr, Contents, StrTab); | ||
| } else { | ||
| OS << "\nVersion definitions:\n"; |
There was a problem hiding this comment.
What's the motivation to use a stored OS here rather than outs() directly, like in the old code?
| @@ -1057,8 +1057,8 @@ ELFFile<ELFT>::getVersionDefinitions(const Elf_Shdr &Sec) const { | |||
|
|
|||
| VerdAux Aux; | |||
| Aux.Offset = VerdauxBuf - Start; | |||
| if (Verdaux->vda_name <= StrTabOrErr->size()) | |||
| Aux.Name = std::string(StrTabOrErr->drop_front(Verdaux->vda_name)); | |||
| if (Verdaux->vda_name < StrTabOrErr->size()) | |||
There was a problem hiding this comment.
Do we need a test case to test this change?
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.
and fix crash when vd_aux is invalid (#86611).
vd_version, vd_flags, vd_ndx, and vd_cnt in Elf{32,64}_Verdef are
16-bit. Change VerDef to use uint16_t instead.
vda_name specifies a NUL-terminated string. Update getVersionDefinitions
to remove some
.c_str().