[llvm] [Object][ELF] Fix section header zero check (PR #181796)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 7 07:09:29 PST 2026
https://github.com/aokblast updated https://github.com/llvm/llvm-project/pull/181796
>From 817ef8ccc1eafde5a0329e9c05c633b3523afde2 Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Tue, 17 Feb 2026 18:11:52 +0800
Subject: [PATCH 1/6] [Object][ELF] Fix section header zero check
The PN_XNUM is a necessary condition for reading shdr0 regardless of the
value of e_shoff. Without this, readShdrZero falsely returns the garbage
value in ELF header instead of emitting warning.
---
llvm/include/llvm/Object/ELF.h | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index df16e50dc1c4f..68807f37a978e 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -818,12 +818,11 @@ Expected<StringRef>
ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections,
WarningHandler WarnHandler) const {
Expected<uint32_t> ShStrNdxOrErr = getShStrNdx();
- if (!ShStrNdxOrErr)
- return ShStrNdxOrErr.takeError();
-
- if (*ShStrNdxOrErr == ELF::SHN_XINDEX && Sections.empty())
+ if (!ShStrNdxOrErr) {
+ llvm::consumeError(ShStrNdxOrErr.takeError());
return createError(
"e_shstrndx == SHN_XINDEX, but the section header table is empty");
+ }
uint32_t Index = *ShStrNdxOrErr;
// There is no section name string table. Return FakeSectionStrings which
@@ -935,9 +934,15 @@ template <class ELFT> ELFFile<ELFT>::ELFFile(StringRef Object) : Buf(Object) {}
template <class ELFT> Error ELFFile<ELFT>::readShdrZero() {
const Elf_Ehdr &Header = getHeader();
- if ((Header.e_phnum == ELF::PN_XNUM || Header.e_shnum == 0 ||
- Header.e_shstrndx == ELF::SHN_XINDEX) &&
- Header.e_shoff != 0) {
+ // If e_shnum == 0 && e_shoff == 0, this indicates that there are no sections,
+ // which is valid for an ELF file.
+ //
+ // However, if e_phnum == PN_XNUM or e_shstrndx == SHN_XINDEX while
+ // e_shoff == 0, the file is inconsistent. In that case, an error will be
+ // triggered later when getSection() is called and detects that e_shoff == 0.
+ if ((Header.e_phnum == ELF::PN_XNUM ||
+ (Header.e_shnum == 0 && Header.e_shoff != 0) ||
+ Header.e_shstrndx == ELF::SHN_XINDEX)) {
// Pretend we have section 0 or sections() would call getShNum and thus
// become an infinite recursion.
RealShNum = 1;
>From 5d8adcb1ea93fc6071642fd78b8d670fc882995f Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Thu, 19 Feb 2026 16:50:26 +0800
Subject: [PATCH 2/6] fixup! [Object][ELF] Fix section header zero check
---
llvm/include/llvm/Object/ELF.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index 68807f37a978e..5c4cc9727421d 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -938,8 +938,11 @@ template <class ELFT> Error ELFFile<ELFT>::readShdrZero() {
// which is valid for an ELF file.
//
// However, if e_phnum == PN_XNUM or e_shstrndx == SHN_XINDEX while
- // e_shoff == 0, the file is inconsistent. In that case, an error will be
- // triggered later when getSection() is called and detects that e_shoff == 0.
+ // e_shoff == 0, the file is inconsistent, because such entries indicate
+ // information should be stored in the index 0 section header, whereas e_shoff
+ // 0 indicates that there are no section headers. In that case, an error will
+ // be triggered later when getSection() is called and detects that e_shoff ==
+ // 0.
if ((Header.e_phnum == ELF::PN_XNUM ||
(Header.e_shnum == 0 && Header.e_shoff != 0) ||
Header.e_shstrndx == ELF::SHN_XINDEX)) {
>From 004bfd2d821138ea25ae083c24307733989d343b Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Tue, 24 Feb 2026 22:08:52 +0800
Subject: [PATCH 3/6] fixup! [Object][ELF] Fix section header zero check
---
llvm/include/llvm/Object/ELF.h | 36 ++++++++++++++++++++++++++++++----
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index 5c4cc9727421d..1bae6d620de6b 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -300,24 +300,45 @@ class ELFFile {
public:
Expected<uint32_t> getPhNum() const {
if (!RealPhNum) {
- if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero())
+ if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero()) {
+ // If RealPhNum is set, the error is not emitted because PhNum is being
+ // read.
+ if (RealPhNum) {
+ llvm::consumeError(std::move(E));
+ return *RealPhNum;
+ }
return std::move(E);
+ }
}
return *RealPhNum;
}
Expected<uint64_t> getShNum() const {
if (!RealShNum) {
- if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero())
+ if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero()) {
+ // If RealShNum is set, the error is not emitted because ShNum is being
+ // read.
+ if (RealShNum) {
+ llvm::consumeError(std::move(E));
+ return *RealShNum;
+ }
return std::move(E);
+ }
}
return *RealShNum;
}
Expected<uint32_t> getShStrNdx() const {
if (!RealShStrNdx) {
- if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero())
+ if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero()) {
+ // If RealShStrNdx is set, the error is not emitted because of reading
+ // ShStrNdx
+ if (RealShStrNdx) {
+ llvm::consumeError(std::move(E));
+ return *RealShStrNdx;
+ }
return std::move(E);
+ }
}
return *RealShStrNdx;
}
@@ -951,7 +972,14 @@ template <class ELFT> Error ELFFile<ELFT>::readShdrZero() {
RealShNum = 1;
auto SecOrErr = getSection(0);
if (!SecOrErr) {
- RealShNum = std::nullopt;
+ if (Header.e_shnum != 0)
+ RealShNum = Header.e_shnum;
+ else
+ RealShNum = std::nullopt;
+ if (Header.e_phnum != ELF::PN_XNUM)
+ RealPhNum = Header.e_phnum;
+ if (Header.e_shstrndx != ELF::SHN_XINDEX)
+ RealShStrNdx = Header.e_shstrndx;
return SecOrErr.takeError();
}
>From 7804b281dc8e6294759659edf4b697769891f603 Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Wed, 25 Feb 2026 17:25:08 +0800
Subject: [PATCH 4/6] fixup! [Object][ELF] Fix section header zero check
---
llvm/include/llvm/Object/ELF.h | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index 1bae6d620de6b..21490c6212fc4 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -301,10 +301,10 @@ class ELFFile {
Expected<uint32_t> getPhNum() const {
if (!RealPhNum) {
if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero()) {
- // If RealPhNum is set, the error is not emitted because PhNum is being
- // read.
+ // If RealPhNum is set, the error was not emitted due to reading the
+ // program header count, so we can ignore it in this context.
if (RealPhNum) {
- llvm::consumeError(std::move(E));
+ consumeError(std::move(E));
return *RealPhNum;
}
return std::move(E);
@@ -316,10 +316,10 @@ class ELFFile {
Expected<uint64_t> getShNum() const {
if (!RealShNum) {
if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero()) {
- // If RealShNum is set, the error is not emitted because ShNum is being
- // read.
+ // If RealShNum is set, the error was not emitted due to reading the
+ // section header count, so we can ignore it in this context.
if (RealShNum) {
- llvm::consumeError(std::move(E));
+ consumeError(std::move(E));
return *RealShNum;
}
return std::move(E);
@@ -331,10 +331,10 @@ class ELFFile {
Expected<uint32_t> getShStrNdx() const {
if (!RealShStrNdx) {
if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero()) {
- // If RealShStrNdx is set, the error is not emitted because of reading
- // ShStrNdx
+ // If RealShStrNdx is set, the error was not emitted due to reading the
+ // section header table index, so we can ignore it in this context.
if (RealShStrNdx) {
- llvm::consumeError(std::move(E));
+ consumeError(std::move(E));
return *RealShStrNdx;
}
return std::move(E);
@@ -840,7 +840,7 @@ ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections,
WarningHandler WarnHandler) const {
Expected<uint32_t> ShStrNdxOrErr = getShStrNdx();
if (!ShStrNdxOrErr) {
- llvm::consumeError(ShStrNdxOrErr.takeError());
+ consumeError(ShStrNdxOrErr.takeError());
return createError(
"e_shstrndx == SHN_XINDEX, but the section header table is empty");
}
>From c4bae7f1e4df78c68a685c8d66a15560f5270b79 Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Thu, 26 Feb 2026 17:32:30 +0800
Subject: [PATCH 5/6] fixup! [Object][ELF] Fix section header zero check
---
llvm/include/llvm/Object/ELF.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index 21490c6212fc4..f54a94ecd8d4f 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -332,7 +332,8 @@ class ELFFile {
if (!RealShStrNdx) {
if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero()) {
// If RealShStrNdx is set, the error was not emitted due to reading the
- // section header table index, so we can ignore it in this context.
+ // section header string table index, so we can ignore it in this
+ // context.
if (RealShStrNdx) {
consumeError(std::move(E));
return *RealShStrNdx;
>From 7b081bf6092c34f7c2f3620fb2554118755014b0 Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Fri, 27 Feb 2026 22:08:14 +0800
Subject: [PATCH 6/6] fixup! [Object][ELF] Fix section header zero check
---
llvm/include/llvm/Object/ELF.h | 7 +++----
llvm/test/tools/llvm-readobj/ELF/many-sections.s | 2 +-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index f54a94ecd8d4f..e052519e27e8f 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -840,11 +840,10 @@ Expected<StringRef>
ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections,
WarningHandler WarnHandler) const {
Expected<uint32_t> ShStrNdxOrErr = getShStrNdx();
- if (!ShStrNdxOrErr) {
- consumeError(ShStrNdxOrErr.takeError());
+ if (!ShStrNdxOrErr)
return createError(
- "e_shstrndx == SHN_XINDEX, but the section header table is empty");
- }
+ "e_shstrndx == SHN_XINDEX, but cannot read section header 0: " +
+ toString(ShStrNdxOrErr.takeError()));
uint32_t Index = *ShStrNdxOrErr;
// There is no section name string table. Return FakeSectionStrings which
diff --git a/llvm/test/tools/llvm-readobj/ELF/many-sections.s b/llvm/test/tools/llvm-readobj/ELF/many-sections.s
index cf70149bf988c..544a4ca4fa341 100644
--- a/llvm/test/tools/llvm-readobj/ELF/many-sections.s
+++ b/llvm/test/tools/llvm-readobj/ELF/many-sections.s
@@ -42,7 +42,7 @@ Sections:
# GNU2-EMPTY:
# GNU2: There are no sections in this file.
-# GNU2-NEXT: warning: '[[FILE]]': e_shstrndx == SHN_XINDEX, but the section header table is empty
+# GNU2-NEXT: warning: '[[FILE]]': e_shstrndx == SHN_XINDEX, but cannot read section header 0: invalid section index: 0
# RUN: llvm-readobj --file-headers --sections %t2 | \
# RUN: FileCheck %s --check-prefix=LLVM2 --implicit-check-not="warning:"
More information about the llvm-commits
mailing list