[llvm] 8bb74d1 - [llvm-objcopy/strip] Fix off-by-one error in SYMTAB_SHNDX need check
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 4 02:24:23 PST 2021
Author: James Henderson
Date: 2021-03-04T10:23:45Z
New Revision: 8bb74d16ef04d83f71b1873e7c2e652fb8287b29
URL: https://github.com/llvm/llvm-project/commit/8bb74d16ef04d83f71b1873e7c2e652fb8287b29
DIFF: https://github.com/llvm/llvm-project/commit/8bb74d16ef04d83f71b1873e7c2e652fb8287b29.diff
LOG: [llvm-objcopy/strip] Fix off-by-one error in SYMTAB_SHNDX need check
The check for whether an extended symbol index table was required
dropped the first SHN_LORESERVE sections from the sections array before
checking whether the remaining sections had symbols. Unfortunately, the
null section header is not present in this list, so the check was
skipping the first section that might be important. If that section
contained a symbol, and no subsequent ones did, the .symtab_shndx
section would not be emitted, leading to a corrupt object.
Also consolidate and expand test coverage in the area to cover this bug
and other aspects of the SYMTAB_SHNDX section.
Reviewed by: alexshap, MaskRay
Differential Revision: https://reviews.llvm.org/D97661
Added:
llvm/test/tools/llvm-objcopy/ELF/auto-remove-add-symtab-shndx.test
Modified:
llvm/tools/llvm-objcopy/ELF/Object.cpp
Removed:
llvm/test/tools/llvm-objcopy/ELF/auto-remove-shndx.test
llvm/test/tools/llvm-objcopy/ELF/strict-no-add.test
################################################################################
diff --git a/llvm/test/tools/llvm-objcopy/ELF/auto-remove-add-symtab-shndx.test b/llvm/test/tools/llvm-objcopy/ELF/auto-remove-add-symtab-shndx.test
new file mode 100644
index 000000000000..e501a8b9b49d
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/ELF/auto-remove-add-symtab-shndx.test
@@ -0,0 +1,47 @@
+# This test makes sure that the .symtab_shndx section is automatically removed
+# when it is not needed and added when it is.
+
+RUN: %python %p/../Inputs/ungzip.py %p/Inputs/many-sections.o.gz > %t.0
+RUN: echo 'foo' > %t
+# many-sections.o contains sections from s1 to s65535, each with a symbol.
+# Remove enough so that the last section with a symbol has index
+# SHN_LORESERVE (i.e. s65280), and show the section hasn't been removed.
+RUN: llvm-objcopy -R 's65[3-9][0-9][0-9]' -R 's6529[0-9]' -R 's6528[1-9]' %t.0 %t2
+RUN: llvm-readobj --sections %t2 | FileCheck --check-prefix=SHNDX %s
+# Remove one more to cause removal of the section.
+RUN: llvm-objcopy -R s65280 %t2 %t3
+RUN: llvm-readobj --sections %t3 | FileCheck --check-prefix=NOSHNDX %s
+
+# Add additional sections. No .symtab_shndx should be added, as they contain no
+# symbols.
+RUN: llvm-objcopy --add-section=newsec1=%t --add-section=newsec2=%t --add-section=newsec3=%t %t3 %t4
+RUN: llvm-readobj --sections %t4 | FileCheck --check-prefix=NOSHNDX %s
+
+# Add a symbol to one of these sections and show that the .symtab_shndx
+# section has been created and that the new symbol has the right section index.
+RUN: llvm-objcopy --add-symbol=newsym=newsec1:0 %t4 %t5
+RUN: llvm-readobj --symbols --sections %t5 | \
+RUN: FileCheck --check-prefixes=SHNDX,SHNDX-SYM -DSECTION=newsec1 %s
+
+# Show that removing and adding symbols, but not their sections, has the same
+# effect regarding removal/addition of the .symtab_shndx section.
+# FIXME: llvm-objcopy does not currently remove this section in these
+# circumstances. See https://bugs.llvm.org/show_bug.cgi?id=49364
+# Test is left here to document the current behaviour.
+RUN: llvm-objcopy -N 'sym65[3-9][0-9][0-9]' -N 'sym652[89][0-9]' -w %t.0 %t6
+RUN: llvm-readobj --sections %t6 | not FileCheck --check-prefix=NOSHNDX %s
+# FIXME: Remove these test lines once the bug has been fixed. They simply show
+# that the section is removed by a run without any further operations.
+RUN: llvm-objcopy %t6 %t6b
+RUN: llvm-readobj --sections %t6b | FileCheck --check-prefix=NOSHNDX %s
+
+# Add a symbol to ensure the section is reinstated.
+RUN: llvm-objcopy --add-symbol=newsym=s65280:0 -w %t6 %t7
+RUN: llvm-readobj --sections --symbols %t7 | \
+RUN: FileCheck --check-prefixes=SHNDX,SHNDX-SYM -DSECTION=s65280 %s
+
+NOSHNDX-NOT: Name: .symtab_shndx
+SHNDX: Name: .symtab_shndx
+SHNDX-SYM: Name: newsym
+SHNDX-SYM: Section:
+SHNDX-SYM-SAME: [[SECTION]]
diff --git a/llvm/test/tools/llvm-objcopy/ELF/auto-remove-shndx.test b/llvm/test/tools/llvm-objcopy/ELF/auto-remove-shndx.test
deleted file mode 100644
index 970b218479d2..000000000000
--- a/llvm/test/tools/llvm-objcopy/ELF/auto-remove-shndx.test
+++ /dev/null
@@ -1,5 +0,0 @@
-# RUN: %python %p/../Inputs/ungzip.py %p/Inputs/many-sections.o.gz > %t
-# RUN: llvm-objcopy -R 's65[3-9][0-9][0-9]' -R 's6529[0-9]' -R 's6528[1-9]' %t %t2
-# RUN: llvm-readobj --sections %t2 | FileCheck --check-prefix=SECS %s
-
-# SECS-NOT: Name: .symtab_shndx
diff --git a/llvm/test/tools/llvm-objcopy/ELF/strict-no-add.test b/llvm/test/tools/llvm-objcopy/ELF/strict-no-add.test
deleted file mode 100644
index 1eafc41eaaf6..000000000000
--- a/llvm/test/tools/llvm-objcopy/ELF/strict-no-add.test
+++ /dev/null
@@ -1,10 +0,0 @@
-# This test makes sure that sections added at the end that don't have symbols
-# defined in them don't trigger the creation of a large index table.
-
-RUN: %python %p/../Inputs/ungzip.py %p/Inputs/many-sections.o.gz > %t.0
-RUN: echo 'foo' > %t
-RUN: llvm-objcopy -R 's65[3-9][0-9][0-9]' -R 's6529[0-9]' -R 's652[89][0-9]' %t.0 %t2
-RUN: llvm-objcopy --add-section=.s0=%t --add-section=.s1=%t --add-section=.s2=%t %t2 %t2
-RUN: llvm-readobj --sections %t2 | FileCheck --check-prefix=SECS %s
-
-SECS-NOT: Name: .symtab_shndx
diff --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp
index 7faf415644fb..fe68c510c91a 100644
--- a/llvm/tools/llvm-objcopy/ELF/Object.cpp
+++ b/llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -2450,8 +2450,10 @@ template <class ELFT> Error ELFWriter<ELFT>::finalize() {
bool NeedsLargeIndexes = false;
if (Obj.sections().size() >= SHN_LORESERVE) {
SectionTableRef Sections = Obj.sections();
+ // Sections doesn't include the null section header, so account for this
+ // when skipping the first N sections.
NeedsLargeIndexes =
- any_of(drop_begin(Sections, SHN_LORESERVE),
+ any_of(drop_begin(Sections, SHN_LORESERVE - 1),
[](const SectionBase &Sec) { return Sec.HasSymbol; });
// TODO: handle case where only one section needs the large index table but
// only needs it because the large index table hasn't been removed yet.
More information about the llvm-commits
mailing list