[binutils-gdb] readelf: Fix incorrect "Version definition past end of section" message

[Maciej W.Rozycki <macro at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c9f02c3e29498fd9ecb1a9719c317c305fe509ae

commit c9f02c3e29498fd9ecb1a9719c317c305fe509ae
Author: Maciej W. Rozycki <macro@imgtec.com>
Date:   Thu Feb 23 18:16:11 2017 +0000

    readelf: Fix incorrect "Version definition past end of section" message
    
    Fix a commit 74e1a04b9787 ("More fixes for reading corrupt ELF files.")
    `readelf --version-info' regression that caused "Version definition past
    end of section" to be always printed at the end, even with good section
    data.
    
    For example with the `mips-linux' target we get:
    
    $ cat ver_def.s
    	.data
    	.globl	new_foo
    	.type	new_foo, %object
    new_foo:
    	.symver	new_foo, foo@@ver_foo
    $ cat ver_def.ver
    { global: *foo*; local: *; };
    $ as -o ver_def.o ver_def.s
    $ ld -e 0 --export-dynamic --version-script=ver_def.ver -o ver_def ver_def.o
    $ readelf -V ver_def
    
    Version symbols section '.gnu.version' contains 4 entries:
     Addr: 000000000000007e  Offset: 0x01007e  Link: 2 (.dynsym)
      000:   0 (*local*)       2 (ver_foo)       1 (*global*)      2 (ver_foo)
    
    Version definition section '.gnu.version_d' contains 2 entries:
      Addr: 0x0000000000000088  Offset: 0x010088  Link: 3 (.dynstr)
      000000: Rev: 1  Flags: BASE   Index: 1  Cnt: 1  Name: ver_def
      0x001c: Rev: 1  Flags: none  Index: 2  Cnt: 1  Name: ver_foo
      Version definition past end of section
    $
    
    The cause is the `if (idx + ent.vd_next <= idx)' condition introduced to
    ensure forward progress, which however always triggers for good version
    definition section data as the last entry will have its `vd_next' value
    set to 0.
    
    Adjust the condition then, to say `if (idx + ent.vd_next < idx)' instead
    and to ensure forward progress limit the number of entries processed to
    the size of the version definition section, removing the problematic
    message from output quoted above, while ensuring the original PR 17531
    test case is still handled gracefully.
    
    Add a suitable test case so that we have `readelf --version-info'
    coverage; due to the lack of infrastructure needed to run the linker in
    the `binutils' test suite and limited justification to implement it add
    a new `readelf.exp' script to the `ld' test suite instead, intended to
    gather any `readelf' test cases that require the linker to be run.  If
    ever we decide to have linker infrastructure added to the `binutils'
    test suite, then the script can be moved between the test suites.
    
    	binutils/
    	* readelf.c (process_version_sections) <SHT_GNU_verdef>: Limit
    	the number of entries processed by the section size.  Don't
    	break out of the loop if `ent.vd_next' is 0.
    
    	ld/
    	* testsuite/ld-elf/ver_def.d: New test.
    	* testsuite/ld-elf/ver_def.ld: New test linker script.
    	* testsuite/ld-elf/ver_def.ver: New test version script.
    	* testsuite/ld-elf/ver_def.s: New test source.
    	* testsuite/ld-elf/readelf.exp: New test script.

Diff:
---
 binutils/ChangeLog                   |  6 +++++
 binutils/readelf.c                   | 11 +++++---
 ld/ChangeLog                         |  8 ++++++
 ld/testsuite/ld-elf/readelf.exp      | 52 ++++++++++++++++++++++++++++++++++++
 ld/testsuite/ld-elf/ver_def-tic6x.vd | 20 ++++++++++++++
 ld/testsuite/ld-elf/ver_def.ld       | 17 ++++++++++++
 ld/testsuite/ld-elf/ver_def.s        |  5 ++++
 ld/testsuite/ld-elf/ver_def.vd       | 15 +++++++++++
 ld/testsuite/ld-elf/ver_def.ver      |  1 +
 9 files changed, 132 insertions(+), 3 deletions(-)

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index a21e36e..a528db4 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,9 @@
+2017-02-24  Maciej W. Rozycki  <macro@imgtec.com>
+
+	* readelf.c (process_version_sections) <SHT_GNU_verdef>: Limit
+	the number of entries processed by the section size.  Don't
+	break out of the loop if `ent.vd_next' is 0.
+
 2017-02-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* testsuite/binutils-all/dw5.S: New file.
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 9caa4da..cb0da10 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -9994,6 +9994,7 @@ process_version_sections (FILE * file)
 	    Elf_External_Verdef * edefs;
 	    unsigned int idx;
 	    unsigned int cnt;
+	    unsigned int end;
 	    char * endbuf;
 
 	    found = 1;
@@ -10015,7 +10016,10 @@ process_version_sections (FILE * file)
 	      break;
 	    endbuf = (char *) edefs + section->sh_size;
 
-	    for (idx = cnt = 0; cnt < section->sh_info; ++cnt)
+	    /* PR 17531: file: id:000001,src:000172+005151,op:splice,rep:2.  */
+	    end = (section->sh_info < section->sh_size
+		   ? section->sh_info : section->sh_size);
+	    for (idx = cnt = 0; cnt < end; ++cnt)
 	      {
 		char * vstart;
 		Elf_External_Verdef * edef;
@@ -10094,8 +10098,9 @@ process_version_sections (FILE * file)
 		if (j < ent.vd_cnt)
 		  printf (_("  Version def aux past end of section\n"));
 
-		/* PR 17531: file: id:000001,src:000172+005151,op:splice,rep:2.  */
-		if (idx + ent.vd_next <= idx)
+		/* PR 17531:
+		   file: id:000001,src:000172+005151,op:splice,rep:2.  */
+		if (idx + ent.vd_next < idx)
 		  break;
 
 		idx += ent.vd_next;
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 420a9ab..5236ae8 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,11 @@
+2017-02-24  Maciej W. Rozycki  <macro@imgtec.com>
+
+	* testsuite/ld-elf/ver_def.d: New test.
+	* testsuite/ld-elf/ver_def.ld: New test linker script.
+	* testsuite/ld-elf/ver_def.ver: New test version script.
+	* testsuite/ld-elf/ver_def.s: New test source.
+	* testsuite/ld-elf/readelf.exp: New test script.
+
 2017-02-23  Maciej W. Rozycki  <macro@imgtec.com>
 
 	* testsuite/ld-mips-elf/relax-jalr-n32.d: Remove `--relax'
diff --git a/ld/testsuite/ld-elf/readelf.exp b/ld/testsuite/ld-elf/readelf.exp
new file mode 100644
index 0000000..92ca1c5
--- /dev/null
+++ b/ld/testsuite/ld-elf/readelf.exp
@@ -0,0 +1,52 @@
+# Expect script for `readelf' tests that depend on LD.
+#   Copyright (C) 2017 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+# These tests really belong to the `binutils' testsuite as they verify
+# `readelf' rather than LD, however they require LD to be run and we have
+# no infrastructure in the `binutils' testsuite to run LD, so we place
+# them here.
+
+# Exclude non-ELF targets.
+
+if ![is_elf_format] {
+    return
+}
+
+# This target requires a non-default emulation for successful shared
+# library/executable builds, and has dump variances.
+set LFLAGS ""
+set DUMP ""
+if [istarget "tic6x-*-*"] {
+    append LFLAGS " -melf32_tic6x_le"
+    set DUMP "-tic6x"
+}
+
+if [check_shared_lib_support] {
+    run_ld_link_tests [list \
+	[list \
+	    "readelf version information" \
+	    "$LFLAGS -e 0 --export-dynamic -T ver_def.ld\
+	     --version-script=ver_def.ver" \
+	    "" "" \
+	    {ver_def.s} \
+	    [list [list readelf --version-info ver_def$DUMP.vd]] \
+	    "ver_def"]]
+}
diff --git a/ld/testsuite/ld-elf/ver_def-tic6x.vd b/ld/testsuite/ld-elf/ver_def-tic6x.vd
new file mode 100644
index 0000000..9c3e535
--- /dev/null
+++ b/ld/testsuite/ld-elf/ver_def-tic6x.vd
@@ -0,0 +1,20 @@
+# Verify correct version information output from `readelf' and that there
+# is no:
+#
+#   Version definition past end of section
+#
+# line at the end in particular (hence #pass must not be used here).
+
+# TI C6X special variant covering an extra `.got.plt' dynamic symbol
+# table entry and consequently its `.gnu.version' record made as a
+# result of unusual processing in `elf32_tic6x_link_omit_section_dynsym'.
+
+Version symbols section '\.gnu\.version' contains 5 entries:
+ Addr: [0-9a-f]+ +Offset: 0x[0-9a-f]+ +Link: 2 \(\.dynsym\)
+ +000: +0 \(\*local\*\) +0 \(\*local\*\) +2 \(ver_foo\) +1 \(\*global\*\) +
+ +004: +2 \(ver_foo\) +
+
+Version definition section '\.gnu\.version_d' contains 2 entries:
+ +Addr: 0x[0-9a-f]+ +Offset: 0x[0-9a-f]+ +Link: 3 \(\.dynstr\)
+ +000000: Rev: 1 +Flags: BASE +Index: 1 +Cnt: 1 +Name: ver_def
+ +0x001c: Rev: 1 +Flags: none +Index: 2 +Cnt: 1 +Name: ver_foo
diff --git a/ld/testsuite/ld-elf/ver_def.ld b/ld/testsuite/ld-elf/ver_def.ld
new file mode 100644
index 0000000..9a66752
--- /dev/null
+++ b/ld/testsuite/ld-elf/ver_def.ld
@@ -0,0 +1,17 @@
+SECTIONS
+{
+  .hash : { *(.hash) }
+  .dynsym : { *(.dynsym) }
+  .dynstr : { *(.dynstr) }
+  .gnu.version : { *(.gnu.version) }
+  .gnu.version_d : { *(.gnu.version_d) }
+  .dynamic : { *(.dynamic) }
+  .data : { *(.data) }
+  .symtab : { *(.symtab) }
+  .strtab : { *(.strtab) }
+  .shstrtab : { *(.shstrtab) }
+  .plt : { *(.plt) }
+  .got.plt : { *(.got.plt) }
+  .got : { *(.got) }
+  /DISCARD/ : { *(*) }
+}
diff --git a/ld/testsuite/ld-elf/ver_def.s b/ld/testsuite/ld-elf/ver_def.s
new file mode 100644
index 0000000..22b590c
--- /dev/null
+++ b/ld/testsuite/ld-elf/ver_def.s
@@ -0,0 +1,5 @@
+	.data
+	.globl	new_foo
+	.type	new_foo, %object
+new_foo:
+	.symver	new_foo, foo@@ver_foo
diff --git a/ld/testsuite/ld-elf/ver_def.vd b/ld/testsuite/ld-elf/ver_def.vd
new file mode 100644
index 0000000..f75e15e
--- /dev/null
+++ b/ld/testsuite/ld-elf/ver_def.vd
@@ -0,0 +1,15 @@
+# Verify correct version information output from `readelf' and that there
+# is no:
+#
+#   Version definition past end of section
+#
+# line at the end in particular (hence #pass must not be used here).
+
+Version symbols section '\.gnu\.version' contains 4 entries:
+ Addr: [0-9a-f]+ +Offset: 0x[0-9a-f]+ +Link: 2 \(\.dynsym\)
+ +000: +0 \(\*local\*\) +2 \(ver_foo\) +1 \(\*global\*\) +2 \(ver_foo\) +
+
+Version definition section '\.gnu\.version_d' contains 2 entries:
+ +Addr: 0x[0-9a-f]+ +Offset: 0x[0-9a-f]+ +Link: 3 \(\.dynstr\)
+ +000000: Rev: 1 +Flags: BASE +Index: 1 +Cnt: 1 +Name: ver_def
+ +0x001c: Rev: 1 +Flags: none +Index: 2 +Cnt: 1 +Name: ver_foo
diff --git a/ld/testsuite/ld-elf/ver_def.ver b/ld/testsuite/ld-elf/ver_def.ver
new file mode 100644
index 0000000..306cf9f
--- /dev/null
+++ b/ld/testsuite/ld-elf/ver_def.ver
@@ -0,0 +1 @@
+{ global: *foo*; local: *; };