[PATCH] Add testcase for PR 25662 invalid sh_offset for section

[Jozef Lawrynowicz <jozef.l@mittosystems.com>]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

The attached patch has a testcase for PR binutils/25662, "objcopy sets invalid
sh_offset for the first section in a no_contents segment containing program
headers".

The patch also extends the objcopy_test procedure from objcopy.exp, allowing
either object files or linked executables to be tested.

I verified that the testcase does not error for msp430-elf,
x86_64-pc-linux-gnu, arm-eabi, ia64-vms (has no linker so the test is
untested) and i386-pe (is not an ELF target).

Note that the test fails for i386-pe, objdump -x output shows that the
"Time/Date" field has been reset to epoch 0.

Ok to apply?

[-- Attachment #2: 0001-Add-testcase-for-PR25662.patch --]
[-- Type: text/x-patch, Size: 7455 bytes --]

From 84b7e554ca7e0d4751239443257d32f41486ac83 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Wed, 25 Mar 2020 14:25:08 +0000
Subject: [PATCH] Add testcase for PR 25662 invalid sh_offset for section

binutils/ChangeLog:

2020-03-26  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	PR binutils/25662
	* testsuite/binutils-all/objcopy.exp (objcopy_test): Add argument to
	specify whether an object file or executable should be built and tested.
	Change test names to report whether an object file or executable is
	being tested.
	* testsuite/binutils-all/pr25662.ld: New test.
	* testsuite/binutils-all/pr25662.s: New test.
---
 binutils/testsuite/binutils-all/objcopy.exp | 65 +++++++++++++++------
 binutils/testsuite/binutils-all/pr25662.ld  | 15 +++++
 binutils/testsuite/binutils-all/pr25662.s   | 34 +++++++++++
 3 files changed, 95 insertions(+), 19 deletions(-)
 create mode 100644 binutils/testsuite/binutils-all/pr25662.ld
 create mode 100644 binutils/testsuite/binutils-all/pr25662.s

diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index 549b064e96..2856ba658d 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -37,36 +37,61 @@ if ![is_remote host] {
 }
 
 # Test that objcopy does not modify a file when copying it.
+# "object" or "executable" values for type are supported.
 
-proc objcopy_test {testname srcfile} {
+proc objcopy_test {testname srcfile type asflags ldflags} {
     global OBJCOPY
     global OBJCOPYFLAGS
     global srcdir
     global subdir
     global tempfile
     global copyfile
+    set t_tempfile $tempfile
+    set t_copyfile ${copyfile}.o
 
-    if {![binutils_assemble $srcdir/$subdir/${srcfile} $tempfile]} then {
-	unresolved "objcopy ($testname)"
-	remote_file host delete $tempfile
+    if { $type != "object" && $type != "executable" } {
+	error "objcopy_test accepts only \"object\" or \"executable\" values for type"
+    }
+
+    if {![binutils_assemble_flags $srcdir/$subdir/${srcfile} $t_tempfile "$asflags"]} then {
+	unresolved "objcopy $type ($testname)"
+	remote_file host delete $t_tempfile
 	return
     }
 
-    set got [binutils_run $OBJCOPY "$OBJCOPYFLAGS $tempfile ${copyfile}.o"]
+    if { $type == "executable" } {
+	global LD
+	# Check that LD exists and works
+	set status [remote_exec host $LD -v]
+	if { [lindex $status 0] != 0 } {
+	    untested "objcopy $type ($testname)"
+	    return
+	}
+	# Use tempfile and copyfile without the .o extension for executable files
+	set t_tempfile [string range $tempfile 0 end-2]
+	set t_copyfile $copyfile
+	set got [binutils_run $LD "$tempfile -o $t_tempfile $ldflags"]
+	if { ![string equal "" $got] } then {
+	    unresolved "objcopy $type ($testname)"
+	    return
+	}
+    }
+
+    set got [binutils_run $OBJCOPY "$OBJCOPYFLAGS $t_tempfile $t_copyfile"]
 
     if ![string equal "" $got] then {
-	fail "objcopy ($testname)"
+	fail "objcopy $type ($testname)"
     } else {
-	send_log "cmp $tempfile ${copyfile}.o\n"
-	verbose "cmp $tempfile ${copyfile}.o"
+	send_log "cmp $t_tempfile $t_copyfile\n"
+	verbose "cmp $t_tempfile $t_copyfile"
 	if [is_remote host] {
-	    set src1 tmpdir/bintest.o
-	    set src2 tmpdir/copy.o
-	    remote_upload host $tempfile $src1
-	    remote_upload host ${copyfile}.o $src2
+	    set src1 tmpdir/bintest
+	    set src2 tmpdir/copy
+	    remote_upload host $t_tempfile $src1
+	    remote_upload host $t_copyfile $src2
 	} else {
-	    set src1 ${tempfile}
-	    set src2 ${copyfile}.o
+	    set src1 $t_tempfile
+	    set src2 $t_copyfile
 	}
 	set status [remote_exec build cmp "${src1} ${src2}"]
 	set exec_output [lindex $status 1]
@@ -86,7 +111,7 @@ proc objcopy_test {testname srcfile} {
 	clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
 
 	if [string equal "" $exec_output] then {
-	    pass "objcopy ($testname)"
+	    pass "objcopy $type ($testname)"
 	} else {
 	    send_log "$exec_output\n"
 	    verbose "$exec_output" 1
@@ -94,12 +119,12 @@ proc objcopy_test {testname srcfile} {
 	    # On OSF/1, this succeeds with gas and fails with /bin/as.
 	    setup_xfail "alpha*-*-osf*"
 
-	    fail "objcopy ($testname)"
+	    fail "objcopy $type ($testname)"
 	}
     }
 }
 
-objcopy_test "simple copy" bintest.s
+objcopy_test "simple copy" bintest.s object "" ""
 
 # Test verilog data width
 proc objcopy_test_verilog {testname} {
@@ -1080,7 +1105,7 @@ proc objcopy_test_elf_common_symbols {} {
 # ia64 specific tests
 if { ([istarget "ia64-*-elf*"]
        || [istarget "ia64-*-linux*"]) } {
-    objcopy_test "ia64 link order" link-order.s
+    objcopy_test "ia64 link order" link-order.s object "" ""
 }
 
 # ELF specific tests
@@ -1088,7 +1113,7 @@ set elf64 ""
 if [is_elf_format] {
     objcopy_test_symbol_manipulation
     objcopy_test_elf_common_symbols
-    objcopy_test "ELF unknown section type" unknown.s
+    objcopy_test "ELF unknown section type" unknown.s object "" ""
     objcopy_test_readelf "ELF group 1" group.s
     objcopy_test_readelf "ELF group 2" group-2.s
     objcopy_test_readelf "ELF group 3" group-3.s
@@ -1316,3 +1341,5 @@ objcopy_remove_relocations_from_executable
 run_dump_test "pr23633"
 
 run_dump_test "set-section-alignment"
+
+objcopy_test "pr25662" pr25662.s executable "" "-T$srcdir/$subdir/pr25662.ld"
diff --git a/binutils/testsuite/binutils-all/pr25662.ld b/binutils/testsuite/binutils-all/pr25662.ld
new file mode 100644
index 0000000000..19ef1391f8
--- /dev/null
+++ b/binutils/testsuite/binutils-all/pr25662.ld
@@ -0,0 +1,15 @@
+ENTRY(_start)
+MEMORY
+{
+  RAM	: ORIGIN = 0x0000, LENGTH = 0x0FFF
+  ROM	: ORIGIN = 0x1000, LENGTH = 0x0FFF
+}
+
+SECTIONS
+{
+  .data : { *(.data) } > RAM AT>ROM
+
+  .text : { *(.text) } > ROM
+
+  .bss : { *(.bss) } > RAM
+}
diff --git a/binutils/testsuite/binutils-all/pr25662.s b/binutils/testsuite/binutils-all/pr25662.s
new file mode 100644
index 0000000000..0b4db05026
--- /dev/null
+++ b/binutils/testsuite/binutils-all/pr25662.s
@@ -0,0 +1,34 @@
+/* PR 25662: objcopy sets invalid sh_offset for the first section in a
+   no_contents segment containing program headers.
+
+   Several conditions are required for the bug to manifest:
+   - The first loadable segment (which contains the program headers) must only
+     contain SHT_NOBITS sections. .bss is the SHT_NOBITS section in this test.
+   - The next loadable segment must have a !SHT_NOBITS loadable section. .data
+     is the !SHT_NOBITS section in this test.
+   - .bss must be positioned after .data in the executable file itself.
+   - The size of .data must be such that the calculated VMA of the .bss
+     section that follows it is not congruent with the file offset of .bss,
+     modulo the p_align of its segment, i.e.:
+       (VMA(.data) + sizeof(.data)) % (.bss_segment.p_align) != 0
+     This will force the sh_offset of .bss to be aligned so it appears within
+     .data.
+   - The size of .data must be larger than the program headers in the first
+     loadable segment, so that the file offset of .bss is immediately
+     after .data, and not padded to a valid alignment by the program headers.
+
+   The bug originally only manifested for ELF targets, but there's no reason not
+   to run this testcase for other file formats.  */
+
+	.section .bss
+a:
+	.zero	0x2
+
+	.section .data
+c:
+	.zero	0x201
+
+	.section .text
+	.global	_start
+_start:
+	.long 0
-- 
2.17.1