Re: [PATCH] Use separate files for strip outputs
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
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
---
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
+
+ * strip.c (handle_elf): Fix check test for SHN_XINDEX symbol.
+
2019-01-22 Mark Wielaard
* 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
+
+ * 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
* 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, ) < 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 +
Re: [PATCH] Use separate files for strip outputs
On Fri, Jan 18, 2019 at 12:56:28PM +, Ulf Hermann wrote: > Let's see if this works: Apparently I cannot get a properly formatted > inline diff through. Therefore, please find the pull request, including > diff, as attachment. That certainly works. git pull https://codereview.qt-project.org/qt-creator/elfutils changes/37/250337/3 git rebase master Applies cleanly. The only thing I would change is removing the Change-ID line. 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. Thanks, Mark
[PATCH] Use separate files for strip outputs
Let's see if this works: Apparently I cannot get a properly formatted
inline diff through. Therefore, please find the pull request, including
diff, as attachment.
regards,
Ulf
The following changes since commit e65d91d21cb09d83b001fef9435e576ba447db32:
libelf: Correct overflow check in note_xlate. (2019-01-16 12:25:57 +0100)
are available in the git repository at:
https://codereview.qt-project.org/qt-creator/elfutils changes/37/250337/3
for you to fetch changes up to ce0ea06597bbba665ad6c26cef50d20895d246de:
Use separate files for strip outputs (2019-01-18 13:53:52 +0100)
Ulf Hermann (1):
Use separate files for strip outputs
tests/ChangeLog | 6 +
tests/run-annobingroup.sh| 20 -
tests/run-strip-test-many.sh | 53 +---
3 files changed, 51 insertions(+), 28 deletions(-)
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 8c9e7807..19879269 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2019-01-18 Ulf Hermann
+
+ * run-annobingroup.sh: Use different files for strip output.
+ * run-strip-test-many.sh: Use different files for strip output,
+ check results of strip, unstrip, elflint.
+
2019-01-09 Ulf Hermann
* run-readelf-compressed.sh: Skip if USE_BZIP2 not found.
diff --git a/tests/run-annobingroup.sh b/tests/run-annobingroup.sh
index fd36e4ac..16b031a1 100755
--- a/tests/run-annobingroup.sh
+++ b/tests/run-annobingroup.sh
@@ -25,7 +25,7 @@
# gcc -g -O2 -fplugin=annobin -c testfile-annobingroup.c
testfiles testfile-annobingroup.o
-tempfiles merged.elf stripped.elf debugfile.elf remerged.elf
+tempfiles merged.elf stripped.elf debugfile1.elf debugfile2.elf debugfile3.elf
remerged.elf
testrun_compare ${abs_top_builddir}/src/readelf -g testfile-annobingroup.o <<
EOF
@@ -35,7 +35,7 @@ Section group [ 1] '.group' with signature
'.text.unlikely.group' contains 3 ent
[ 9] .text.unlikely
EOF
-testrun ${abs_top_builddir}/src/strip -o stripped.elf -f debugfile.elf
testfile-annobingroup.o
+testrun ${abs_top_builddir}/src/strip -o stripped.elf -f debugfile1.elf
testfile-annobingroup.o
testrun_compare ${abs_top_builddir}/src/readelf -g stripped.elf << EOF
@@ -45,7 +45,7 @@ Section group [ 1] '.group' with signature
'.text.unlikely.group' contains 3 ent
[ 9] .text.unlikely
EOF
-testrun_compare ${abs_top_builddir}/src/readelf -g debugfile.elf << EOF
+testrun_compare ${abs_top_builddir}/src/readelf -g debugfile1.elf << EOF
Section group [ 1] '.group' with signature '.text.unlikely.group' contains 3
entries:
[ 7] .gnu.build.attributes..text.unlikely
@@ -53,7 +53,7 @@ Section group [ 1] '.group' with signature
'.text.unlikely.group' contains 3 ent
[ 9] .text.unlikely
EOF
-testrun ${abs_top_builddir}/src/unstrip -o remerged.elf stripped.elf
debugfile.elf
+testrun ${abs_top_builddir}/src/unstrip -o remerged.elf stripped.elf
debugfile1.elf
testrun_compare ${abs_top_builddir}/src/readelf -g remerged.elf << EOF
@@ -81,7 +81,7 @@ COMDAT section group [ 2] '.group' with signature
'__x86.get_pc_thunk.ax' contai
[13] .text.__x86.get_pc_thunk.ax
EOF
-testrun ${abs_top_builddir}/src/strip -o stripped.elf -f debugfile.elf
testfile-annobingroup-i386.o
+testrun ${abs_top_builddir}/src/strip -o stripped.elf -f debugfile2.elf
testfile-annobingroup-i386.o
testrun_compare ${abs_top_builddir}/src/readelf -g stripped.elf << EOF
@@ -94,7 +94,7 @@ COMDAT section group [ 2] '.group' with signature
'__x86.get_pc_thunk.ax' contai
[13] .text.__x86.get_pc_thunk.ax
EOF
-testrun_compare ${abs_top_builddir}/src/readelf -g debugfile.elf << EOF
+testrun_compare ${abs_top_builddir}/src/readelf -g debugfile2.elf << EOF
Section group [ 1] '.group' with signature '.text.unlikely.group' contains 3
entries:
[ 8] .gnu.build.attributes..text.unlikely
@@ -105,7 +105,7 @@ COMDAT section group [ 2] '.group' with signature
'__x86.get_pc_thunk.ax' contai
[13] .text.__x86.get_pc_thunk.ax
EOF
-testrun ${abs_top_builddir}/src/unstrip -o remerged.elf stripped.elf
debugfile.elf
+testrun ${abs_top_builddir}/src/unstrip -o remerged.elf stripped.elf
debugfile2.elf
testrun_compare ${abs_top_builddir}/src/readelf -g remerged.elf << EOF
@@ -143,13 +143,13 @@ Section group [ 4] '.group' with signature
'.text.unlikely..group' contains 1 en
[27] .text.unlikely
EOF
-testrun ${abs_top_builddir}/src/strip -o stripped.elf -f debugfile.elf
testfile-annobingroup-x86_64.o
+testrun ${abs_top_builddir}/src/strip -o stripped.elf -f debugfile3.elf
testfile-annobingroup-x86_64.o
# This would/should work, except for the unknown NOTEs.
# testrun ${abs_top_builddir}/src/elflint --gnu stripped.elf
-# testrun ${abs_top_builddir}/src/elflint --gnu --debug debugfile.elf
+# testrun ${abs_top_builddir}/src/elflint --gnu --debug debugfile3.elf
-testrun
