[PATCH 1/3] readelf: Fix incorrect "Version definition past end of section" message
Maciej W. Rozycki
macro@imgtec.com
Thu Feb 23 18:16:00 GMT 2017
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.
---
Hi,
The case of `ent.vd_next' being 0 could perhaps be handled in a more
sophisticated way as it's only really valid with the last entry, but I
have decided to keep it simple and optimise for the legitimate rather than
the broken case. So in the end `readelf --version-info' may take longer
to proceed through corrupt binaries or plain random gibberish, but it will
eventually complete anyway.
No regressions against my usual targets; I have verified the change
against the original PR 17531 test case too.
The new test case does fail for the `arc-linux' target though. The
reason is an assertion failure in the linker:
.../ld-new: BFD (GNU Binutils) 2.28.51.20170217 assertion fail .../bfd/elf-strtab.c:302
which comes from `BFD_ASSERT (tab->array[i]->refcount == 0)' in
`_bfd_elf_strtab_emit'. Consequently output necessary for `readelf' to
process is not produced, so it cannot be considered a bug in the test
case. I have not investigated the problem further and noted that the ARC
port has had no maintainer since 2003.
Cupertino, Claudiu: you have been particularly active with the ARC port
recently, so perhaps you could look into this problem, and maybe also
consider taking the maintainer's post for the port as well?
Another anomaly is with the `tic6x-uclinux' port, which produces a
different dynamic symbol table including an extra entry for the `.got.plt'
section. This is due to unusual processing in
`elf32_tic6x_link_omit_section_dynsym', which always enters section
symbols into the dynamic symbol table for SHT_PROGBITS and SHT_NOBITS
sections. According to a comment there the port requires "dynamic symbols
for every section, since segments can relocate independently", although I
find it questionable for regular (i.e. non-PIE) executables, so maybe this
is a missed case worth looking into?
Until/unless that has been resolved though there's nothing technically
invalid with the extra dynamic symbol table entries, even if they only
serve as clutter, so I have covered `tic6x-*-*' targets with a separate
dedicated output template.
With the observations above noted, OK to apply?
Maciej
binutils-readelf-verdef-past-end.diff
Index: binutils/binutils/readelf.c
===================================================================
--- binutils.orig/binutils/readelf.c 2017-02-21 16:02:42.875666703 +0000
+++ binutils/binutils/readelf.c 2017-02-22 16:50:00.758591121 +0000
@@ -9992,6 +9992,7 @@ process_version_sections (FILE * file)
Elf_External_Verdef * edefs;
unsigned int idx;
unsigned int cnt;
+ unsigned int end;
char * endbuf;
found = 1;
@@ -10013,7 +10014,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;
@@ -10092,8 +10096,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;
Index: binutils/ld/testsuite/ld-elf/readelf.exp
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/readelf.exp 2017-02-22 02:08:54.096944015 +0000
@@ -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"]]
+}
Index: binutils/ld/testsuite/ld-elf/ver_def-tic6x.vd
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/ver_def-tic6x.vd 2017-02-22 16:57:45.717342846 +0000
@@ -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
Index: binutils/ld/testsuite/ld-elf/ver_def.ld
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/ver_def.ld 2017-02-21 21:32:04.672292685 +0000
@@ -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/ : { *(*) }
+}
Index: binutils/ld/testsuite/ld-elf/ver_def.s
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/ver_def.s 2017-02-21 21:44:58.813677043 +0000
@@ -0,0 +1,5 @@
+ .data
+ .globl new_foo
+ .type new_foo, %object
+new_foo:
+ .symver new_foo, foo@@ver_foo
Index: binutils/ld/testsuite/ld-elf/ver_def.vd
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/ver_def.vd 2017-02-21 20:54:08.589722908 +0000
@@ -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
Index: binutils/ld/testsuite/ld-elf/ver_def.ver
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/ver_def.ver 2017-02-21 16:02:54.000000000 +0000
@@ -0,0 +1 @@
+{ global: *foo*; local: *; };
More information about the Binutils
mailing list