[PATCH] readelf: Reset file position to beginning for thin archive members

[PATCH] readelf: Reset file position to beginning for thin archive members

[Andrei Homescu]

During readelf parsing of thin archives, open_file internally calls
get_file_header and advances the file position while process_object
expects it at the beginning of the file. This patch inserts a call
to rewind before process_object, fixing the following error:

$ touch test1.c
$ gcc -c test1.c
$ ar crT test1.a test1.o
$ readelf -a test1.a
readelf: Error: test1.a[test1.o]: Failed to read file header

binutils/ChangeLog:

	* readelf.c (process_archive): Reset file position to the
	beginning when calling process_object for thin archive members.
	* testsuite/binutils-all/readelf.exp: New test.
	* testsuite/binutils-all/readelf.h.thin: New test.
---
 binutils/readelf.c                            |  3 ++
 binutils/testsuite/binutils-all/readelf.exp   | 28 +++++++++++++++++++
 .../testsuite/binutils-all/readelf.h.thin     | 22 +++++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 binutils/testsuite/binutils-all/readelf.h.thin

diff --git a/binutils/readelf.c b/binutils/readelf.c
index f7c64329f37..f42be19f619 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -21792,6 +21792,9 @@ process_archive (Filedata * filedata, bool is_thin_archive)
 	  filedata->archive_file_offset = arch.nested_member_origin;
 	  member_filedata->file_name = qualified_name;
 
+	  /* process_object expects the file position at the beginning. */
+	  rewind (member_filedata->handle);
+
 	  if (! process_object (member_filedata))
 	    ret = false;
 
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index 4884a5df40f..cf5fb64d2ac 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -310,6 +310,32 @@ proc readelf_dump_test {} {
     # XXX FIXME: Add test of readelf -x here
 }
 
+# Tests whether readelf can read thin archives
+proc readelf_thin_archive_test {} {
+    global AR
+
+    if ![is_remote host] {
+	set tempfile tmpdir/bintest.o
+        set templib  tmpdir/bintest.thin.a
+        set libname  tmpdir/bintest.thin.a
+    } else {
+	set tempfile [remote_download host tmpdir/bintest.o]
+        set templib  [remote_download host tmpdir/bintest.thin.a]
+        set libname  bintest.thin.a
+    }
+
+    set testname "readelf -h bintest.thin"
+    set got [binutils_run $AR "rcT $libname ${tempfile}"]
+    if ![string match "" $got] {
+        fail $testname
+        return
+    }
+
+    readelf_test -h $templib readelf.h.thin
+
+    pass $testname
+}
+
 if ![is_remote host] {
     if {[which $READELF] == 0} then {
         perror "$READELF does not exist"
@@ -344,6 +370,8 @@ if {![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.o]} then {
     setup_xfail "mips-*-*irix*"
     readelf_test -s $tempfile readelf.ss
     readelf_test -r $tempfile readelf.r
+
+    readelf_thin_archive_test
 }
 
 # Test demangling symbol names.
diff --git a/binutils/testsuite/binutils-all/readelf.h.thin b/binutils/testsuite/binutils-all/readelf.h.thin
new file mode 100644
index 00000000000..32ea2b7d6f1
--- /dev/null
+++ b/binutils/testsuite/binutils-all/readelf.h.thin
@@ -0,0 +1,22 @@
+
+File: .*bintest\.thin\.a.*
+ELF Header:
+  Magic:   7f 45 4c 46 0[12] 0[12] 01 .. .. 00 00 00 00 00 00 00 
+  Class:                             ELF[36][24]
+  Data:                              2's complement,.* endian
+  Version:                           1 \(current\)
+  OS/ABI:                            .*
+  ABI Version:                       .*
+  Type:                              REL \(Relocatable file\)
+  Machine:                           .*
+  Version:                           0x1
+  Entry point address:               0x0
+  Start of program headers:          0 \(bytes into file\)
+  Start of section headers:          .* \(bytes into file\)
+  Flags:                             .*
+  Size of this header:               .* \(bytes\)
+  Size of program headers:           0 \(bytes\)
+  Number of program headers:         0
+  Size of section headers:           .* \(bytes\)
+  Number of section headers:         .*
+  Section header string table index: .*
-- 
2.32.0

Re: [PATCH] readelf: Reset file position to beginning for thin archive members

[Nick Clifton]

Hi Andrei,

   Patch approved and applied.

   Thanks very much for reporting this problem, and even more including
   a testcase with the patch - that was very welcome.

   I do have one request however: In the future if you find bugs in the
   binutils, please could you report them via the binutuls bugzilla
   system:

https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils

   Cheers

Nick

Re: [PATCH] readelf: Reset file position to beginning for thin archive members

[Andrei Homescu]

On Thu, Jul 1, 2021 at 4:05 AM Nick Clifton <nickc@redhat.com> wrote:

> Hi Andrei,
>
>    Patch approved and applied.
>
That's great, thanks!


>
>    Thanks very much for reporting this problem, and even more including
>    a testcase with the patch - that was very welcome.
>
>    I do have one request however: In the future if you find bugs in the
>    binutils, please could you report them via the binutuls bugzilla
>    system:
>
> https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils

Got it! It wasn't clear to me whether I needed to do that since I
already had a fix.
Would it make sense for me to retroactively open a bugzilla report?

Andrei

Re: [PATCH] readelf: Reset file position to beginning for thin archive members

[Nick Clifton]

Hi Andrei,

>         I do have one request however: In the future if you find bugs in the
>         binutils, please could you report them via the binutuls bugzilla
>         system:
> 
>     https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils <https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils>

> Got it! It wasn't clear to me whether I needed to do that since I already had a fix.

Even when you have a fix (which is fabulous) opening a bug report helps.  There is a
facility to attach files to a bug report, so that is how patches are often uploaded.

Having a bug report allows us (and future binutils users) to track the bug and follow
any discussions about the problem and the fix.  Plus it allows us to put a reference
into the changelog and/or git commits pointing to the report.


> Would it make sense for me to retroactively open a bugzilla report?

Oh why not - it is good practice. :-)

Cheers
   Nick