On Wed, 2019-01-23 at 23:19 +0100, Mark Wielaard wrote:
> As you say in your commit message this exposes that the
> run-strip-test-many.sh actually fails. Indeed, even without
> you patch you can see src/strip: illformed file 'testfile'
> in the run-strip-test-many.sh.log (but without actually failing
> the test...
>
> I am still investgating why/how it is failing.
> Will apply your patch afterwards.
Turned out to it was a onliner:
diff --git a/src/strip.c b/src/strip.c
index 1518073..a73009d 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -1944,7 +1944,7 @@ handle_elf (int fd, Elf *elf, const char *prefix,
const char *fname,
INTERNAL_ERROR (fname);
if (sym->st_shndx == SHN_UNDEF
- || (sym->st_shndx >= shnum
+ || (sym->st_shndx >= SHN_LORESERVE
&& sym->st_shndx != SHN_XINDEX))
{
/* This is no section index, leave it alone
This caused special section numbers like SHN_ABS and SHN_COMMON to not
be recognized as special, so they got "renumbered". Which was obviously
bogus.
Fixing this did expose that elflint is very strict with regards to the
.shstrtab name and type (with strip -g we will not move the old
.shstrtab section and so change its type to SHT_NOBITS). This might be
a bit overzealous of elflint. But for now I just changed the
addsections test program to change the old name and name the new one
.shstrtab.
With that your cleaned up test case passes.
Pushed both to master.
Thanks,
Mark
From 4540ea98ce3314c84273f5c9cbb8b774f1463dc4 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Thu, 24 Jan 2019 16:00:49 +0100
Subject: [PATCH] strip: Fix check test for SHN_XINDEX symbol.
The check for whether a symbol used the extended section table was
wrong causing the run-strip-test-many.sh testcase to declare the
testfile was an illformed file.
Fixing this exposed a strict elfutils check for the '.shstrtab'
section having this exact name and a SHT_STRTAB type. This might
be a little too strict, but easily worked around by changing the
name of the "old" shstrtab section in the addsections program.
Signed-off-by: Mark Wielaard <m...@klomp.org>
---
src/ChangeLog | 4 ++++
src/strip.c | 2 +-
tests/ChangeLog | 6 ++++++
tests/addsections.c | 37 ++++++++++++++++++++++++++++---------
4 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 0ea106c..0544fce 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2019-01-24 Mark Wielaard <m...@klomp.org>
+
+ * strip.c (handle_elf): Fix check test for SHN_XINDEX symbol.
+
2019-01-22 Mark Wielaard <m...@klomp.org>
* readelf.c (print_debug_line_section): Check we are not at end of
diff --git a/src/strip.c b/src/strip.c
index 1518073..a73009d 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -1944,7 +1944,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
INTERNAL_ERROR (fname);
if (sym->st_shndx == SHN_UNDEF
- || (sym->st_shndx >= shnum
+ || (sym->st_shndx >= SHN_LORESERVE
&& sym->st_shndx != SHN_XINDEX))
{
/* This is no section index, leave it alone
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 8c9e780..2dd8026 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2019-01-24 Mark Wielaard <m...@klomp.org>
+
+ * addsections.c (add_sections): Change the name of the old shstrtab
+ section to ".old_shstrtab" and give the old shstrtab name to the
+ new shstrtab section.
+
2019-01-09 Ulf Hermann <ulf.herm...@qt.io>
* run-readelf-compressed.sh: Skip if USE_BZIP2 not found.
diff --git a/tests/addsections.c b/tests/addsections.c
index 391c5b4..cc8d065 100644
--- a/tests/addsections.c
+++ b/tests/addsections.c
@@ -92,7 +92,7 @@ add_sections (const char *name, size_t nr, int use_mmap)
/* We will add a new shstrtab section with two new names at the end.
Just get the current shstrtab table and add two entries '.extra'
- and '.new_shstrtab' at the end of the table, so all existing indexes
+ and '.old_shstrtab' at the end of the table, so all existing indexes
are still valid. */
size_t shstrndx;
if (elf_getshdrstrndx (elf, &shstrndx) < 0)
@@ -115,7 +115,7 @@ add_sections (const char *name, size_t nr, int use_mmap)
}
size_t new_shstrtab_size = (shstrtab_data->d_size
+ strlen (".extra") + 1
- + strlen (".new_shstrtab") + 1);
+ + strlen (".old_shstrtab") + 1);
void *new_shstrtab_buf = malloc (new_shstrtab_size);
if (new_shstrtab_buf == NULL)
{
@@ -124,9 +124,30 @@ add_sections (const char *name, size_t nr, int use_mmap)
}
memcpy (new_shstrtab_buf, shstrtab_data->d_buf, shstrtab_data->d_size);
size_t extra_idx = shstrtab_data->d_size;
- size_t new_shstrtab_idx = extra_idx + strlen (".extra") + 1;
+ size_t old_shstrtab_idx = extra_idx + strlen (".extra") + 1;
strcpy (new_shstrtab_buf + extra_idx, ".extra");
- strcpy (new_shstrtab_buf + new_shstrtab_idx, ".new_shstrtab");
+ strcpy (new_shstrtab_buf + old_shstrtab_idx, ".old_shstrtab");
+
+ /* Change the name of the old shstrtab section, because elflint
+ has a strict check on the name/type for .shstrtab. */
+ GElf_Shdr shdr_mem;
+ GElf_Shdr *shdr = gelf_getshdr (shstrtab_scn, &shdr_mem);
+ if (shdr == NULL)
+ {
+ printf ("cannot get header for old shstrtab section: %s\n",
+ elf_errmsg (-1));
+ exit (1);
+ }
+
+ size_t shstrtab_idx = shdr->sh_name;
+ shdr->sh_name = old_shstrtab_idx;
+
+ if (gelf_update_shdr (shstrtab_scn, shdr) == 0)
+ {
+ printf ("cannot update old shstrtab section header: %s\n",
+ elf_errmsg (-1));
+ exit (1);
+ }
// Add lots of .extra sections...
size_t cnt = 0;
@@ -153,8 +174,7 @@ add_sections (const char *name, size_t nr, int use_mmap)
data->d_type = ELF_T_BYTE;
data->d_align = 1;
- GElf_Shdr shdr_mem;
- GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
+ shdr = gelf_getshdr (scn, &shdr_mem);
if (shdr == NULL)
{
printf ("cannot get header for new section (%zd): %s\n", cnt,
@@ -201,8 +221,7 @@ add_sections (const char *name, size_t nr, int use_mmap)
new_shstrtab_data->d_type = ELF_T_BYTE;
new_shstrtab_data->d_align = 1;
- GElf_Shdr shdr_mem;
- GElf_Shdr *shdr = gelf_getshdr (new_shstrtab_scn, &shdr_mem);
+ shdr = gelf_getshdr (new_shstrtab_scn, &shdr_mem);
if (shdr == NULL)
{
printf ("cannot get header for new shstrtab section: %s\n",
@@ -218,7 +237,7 @@ add_sections (const char *name, size_t nr, int use_mmap)
shdr->sh_addralign = 1;
shdr->sh_entsize = 0;
shdr->sh_size = new_shstrtab_size;
- shdr->sh_name = new_shstrtab_idx;
+ shdr->sh_name = shstrtab_idx;
// Finished new shstrtab section, update the header.
if (gelf_update_shdr (new_shstrtab_scn, shdr) == 0)
--
1.8.3.1
