public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add testcase for PR 25662 invalid sh_offset for section
@ 2020-03-26 11:10 Jozef Lawrynowicz
  2020-03-26 21:25 ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: Jozef Lawrynowicz @ 2020-03-26 11:10 UTC (permalink / raw)
  To: binutils

[-- 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add testcase for PR 25662 invalid sh_offset for section
  2020-03-26 11:10 [PATCH] Add testcase for PR 25662 invalid sh_offset for section Jozef Lawrynowicz
@ 2020-03-26 21:25 ` Alan Modra
  2020-03-27 10:59   ` Jozef Lawrynowicz
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2020-03-26 21:25 UTC (permalink / raw)
  To: Jozef Lawrynowicz; +Cc: binutils

On Thu, Mar 26, 2020 at 11:10:36AM +0000, Jozef Lawrynowicz wrote:
> 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?

OK, thanks.  Yes, we are going to have some fails with this new test.
Hmm, objcopy -p doesn't help with the PE date stamp and it seems like
it should.

You might like to simplify the LD test.

> +	set status [remote_exec host $LD -v]
> +	if { [lindex $status 0] != 0 } {

      if {[which $LD] == 0} then {

> +	    untested "objcopy $type ($testname)"
> +	    return
> +	}

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add testcase for PR 25662 invalid sh_offset for section
  2020-03-26 21:25 ` Alan Modra
@ 2020-03-27 10:59   ` Jozef Lawrynowicz
  2020-03-27 21:03     ` Hans-Peter Nilsson
  0 siblings, 1 reply; 7+ messages in thread
From: Jozef Lawrynowicz @ 2020-03-27 10:59 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

On Fri, 27 Mar 2020 07:55:12 +1030
Alan Modra <amodra@gmail.com> wrote:

> On Thu, Mar 26, 2020 at 11:10:36AM +0000, Jozef Lawrynowicz wrote:
> > 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?  
> 
> OK, thanks.  Yes, we are going to have some fails with this new test.
> Hmm, objcopy -p doesn't help with the PE date stamp and it seems like
> it should.
> 
> You might like to simplify the LD test.
> 
> > +	set status [remote_exec host $LD -v]
> > +	if { [lindex $status 0] != 0 } {  
> 
>       if {[which $LD] == 0} then {
> 
> > +	    untested "objcopy $type ($testname)"
> > +	    return
> > +	}  
> 

Thanks for the tip. Fixed that and pushed to master.

Jozef

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add testcase for PR 25662 invalid sh_offset for section
  2020-03-27 10:59   ` Jozef Lawrynowicz
@ 2020-03-27 21:03     ` Hans-Peter Nilsson
  2020-03-28  0:52       ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: Hans-Peter Nilsson @ 2020-03-27 21:03 UTC (permalink / raw)
  To: Jozef Lawrynowicz; +Cc: Alan Modra, binutils

On Fri, 27 Mar 2020, Jozef Lawrynowicz wrote:
> On Fri, 27 Mar 2020 07:55:12 +1030
> Alan Modra <amodra@gmail.com> wrote:
> > OK, thanks.  Yes, we are going to have some fails with this new test.
> > Hmm, objcopy -p doesn't help with the PE date stamp and it seems like
> > it should.
> >
> > You might like to simplify the LD test.
> >
> > > +	set status [remote_exec host $LD -v]
> > > +	if { [lindex $status 0] != 0 } {
> >
> >       if {[which $LD] == 0} then {
> >
> > > +	    untested "objcopy $type ($testname)"
> > > +	    return
> > > +	}
> >
>
> Thanks for the tip. Fixed that and pushed to master.

Your 1fafefd59438 seems to have caused a new failure for my
binutils autotester.  With --target mmix-knuth-mmixware:

(edited copypaste, line breaks may be wrong)

...
Running x/src/binutils/testsuite/binutils-all/objcopy.exp ...
Version x/binutils/objcopy 2.34.50.20200327
FAIL: objcopy executable (pr25662)
...

In binutils.log:
...
Executing on host: x/binutils/objcopy  tmpdir/bintest tmpdir/copy   (timeout = 300)
spawn -ignore SIGHUP x/binutils/objcopy tmpdir/bintest tmpdir/copy
cmp tmpdir/bintest tmpdir/copy
Executing on build: cmp tmpdir/bintest tmpdir/copy   (timeout = 300)
spawn -ignore SIGHUP cmp tmpdir/bintest tmpdir/copy
tmpdir/bintest tmpdir/copy differ: char 18, line 1
tmpdir/bintest tmpdir/copy differ: char 18, line 1

FAIL: objcopy executable (pr25662)
...

Could you please look into this?
Thanks.

brgds, H-P

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add testcase for PR 25662 invalid sh_offset for section
  2020-03-27 21:03     ` Hans-Peter Nilsson
@ 2020-03-28  0:52       ` Alan Modra
  2020-03-28  2:01         ` Hans-Peter Nilsson
  2020-03-28  6:15         ` Alan Modra
  0 siblings, 2 replies; 7+ messages in thread
From: Alan Modra @ 2020-03-28  0:52 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: Jozef Lawrynowicz, binutils

On Fri, Mar 27, 2020 at 05:03:07PM -0400, Hans-Peter Nilsson wrote:
> On Fri, 27 Mar 2020, Jozef Lawrynowicz wrote:
> > On Fri, 27 Mar 2020 07:55:12 +1030
> > Alan Modra <amodra@gmail.com> wrote:
> > > OK, thanks.  Yes, we are going to have some fails with this new test.
> > > Hmm, objcopy -p doesn't help with the PE date stamp and it seems like
> > > it should.
> > >
> > > You might like to simplify the LD test.
> > >
> > > > +	set status [remote_exec host $LD -v]
> > > > +	if { [lindex $status 0] != 0 } {
> > >
> > >       if {[which $LD] == 0} then {
> > >
> > > > +	    untested "objcopy $type ($testname)"
> > > > +	    return
> > > > +	}
> > >
> >
> > Thanks for the tip. Fixed that and pushed to master.
> 
> Your 1fafefd59438 seems to have caused a new failure for my
> binutils autotester.  With --target mmix-knuth-mmixware:
> 

original
00000000: 9809 0101 5e7e 96c9 9808 0050 0000 0002  ....^~.....P....
00000010: 2e64 6174 6100 0000 0000 0043 0000 0000  .data......C....
00000020: 0000 0201 0000 0000 0000 0000 9801 0002  ................
00000030: 0000 0000 0000 0200 0000 0000 9808 0050  ...............P
00000040: 0000 0002 2e74 6578 7400 0000 0000 0033  .....text......3
00000050: 0000 0000 0000 0004 0000 0000 0000 1204  ................
00000060: 9801 0002 0000 0000 0000 1204 0000 0000  ................
00000070: 980a 00ff 0000 0000 0000 1204 980b 0000  ................
00000080: 203a 304d 2061 2069 026e 1204 8120 5f20   :0M a i.n... _ 
00000090: 7320 7420 6120 7202 7412 0482 980c 0007  s t a r.t.......

copy
00000000: 9809 0101 5e7e 96c9 9808 0050 0000 0002  ....^~.....P....
00000010: 2e74 6578 7400 0000 0000 0033 0000 0000  .text......3....
00000020: 0000 0004 0000 0000 0000 1204 9801 0002  ................
00000030: 0000 0000 0000 1204 0000 0000 9808 0050  ...............P
00000040: 0000 0002 2e64 6174 6100 0000 0000 0043  .....data......C
00000050: 0000 0000 0000 0201 0000 0000 0000 0000  ................
00000060: 9801 0002 0000 0000 0000 0200 0000 0000  ................
00000070: 980a 00ff 0000 0000 0000 1204 980b 0000  ................
00000080: 203a 304d 2061 2069 026e 1204 8120 5f20   :0M a i.n... _ 
00000090: 7320 7420 6120 7202 7412 0482 980c 0007  s t a r.t.......

I think that is just a rearrangement of the object file records, so
I'm inclined to xfail the test for mmix.  I'll leave that to you
Hans-Peter since you know your object format much better than I can
understand from the occasional glance at bfd/mmo.c.

Meanwhile, to fix an XPASS on tic54x and to xfail some others for the
new test, I'm committing the following.

xfails spu due to a note section getting a different vma, and some
mips targets that give section symbols a name string.  I added -p
for the executable test in an attempt to fix all the pe target fails,
but that doesn't preserve the date/time for some reason.

	* testsuite/binutils-all/objcopy.exp (objcopy_test): Move xfails
	from here to calls.  Remove "m8*-*-*" entry.  Don't xfail tic54x
	but do xfail spu, mipstx39 and mips-sgi-irix for the executable
	test.  Pass "-p" to objcopy for the executable test.

diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index b5c1895f41..0eafcdbede 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -59,6 +59,7 @@ proc objcopy_test {testname srcfile type asflags ldflags} {
 	return
     }
 
+    set xflags ""
     if { $type == "executable" } {
 	global LD
 	# Check that LD exists
@@ -66,6 +67,7 @@ proc objcopy_test {testname srcfile type asflags ldflags} {
 	    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
@@ -74,9 +76,10 @@ proc objcopy_test {testname srcfile type asflags ldflags} {
 	    unresolved "objcopy $type ($testname)"
 	    return
 	}
+	set xflags "-p"
     }
 
-    set got [binutils_run $OBJCOPY "$OBJCOPYFLAGS $t_tempfile $t_copyfile"]
+    set got [binutils_run $OBJCOPY "$OBJCOPYFLAGS $xflags $t_tempfile $t_copyfile"]
 
     if ![string equal "" $got] then {
 	fail "objcopy $type ($testname)"
@@ -96,19 +99,6 @@ proc objcopy_test {testname srcfile type asflags ldflags} {
 	set exec_output [lindex $status 1]
 	set exec_output [prune_warnings $exec_output]
 
-	# On some systems the result of objcopy will not be identical.
-	# Usually this is just because gas isn't using bfd to write the
-	# files in the first place, and may order things a little
-	# differently. Those systems should use setup_xfail here.
-
-	setup_xfail "hppa*-*-*"
-	setup_xfail "m8*-*"
-	setup_xfail "sh-*-coff*"
-	setup_xfail "tic54x-*-*"
-
-	clear_xfail "hppa*64*-*-hpux*" "hppa*-*-linux*" "hppa*-*-lites*"
-	clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
-
 	if [string equal "" $exec_output] then {
 	    pass "objcopy $type ($testname)"
 	} else {
@@ -123,6 +113,11 @@ proc objcopy_test {testname srcfile type asflags ldflags} {
     }
 }
 
+setup_xfail "hppa*-*-*"
+setup_xfail "sh-*-coff*"
+setup_xfail "tic54x-*-*"
+clear_xfail "hppa*64*-*-hpux*" "hppa*-*-linux*" "hppa*-*-lites*"
+clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
 objcopy_test "simple copy" bintest.s object "" ""
 
 # Test verilog data width
@@ -1112,7 +1107,14 @@ set elf64 ""
 if [is_elf_format] {
     objcopy_test_symbol_manipulation
     objcopy_test_elf_common_symbols
+
+    setup_xfail "hppa*-*-*"
+    setup_xfail "sh-*-coff*"
+    setup_xfail "tic54x-*-*"
+    clear_xfail "hppa*64*-*-hpux*" "hppa*-*-linux*" "hppa*-*-lites*"
+    clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
     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
@@ -1341,4 +1343,9 @@ run_dump_test "pr23633"
 
 run_dump_test "set-section-alignment"
 
+setup_xfail "hppa*-*-*"
+setup_xfail "sh-*-coff*"
+setup_xfail "mips-*-irix" "mipstx39-*-*" "spu-*-*"
+clear_xfail "hppa*64*-*-hpux*" "hppa*-*-linux*" "hppa*-*-lites*"
+clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
 objcopy_test "pr25662" pr25662.s executable "" "-T$srcdir/$subdir/pr25662.ld"

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add testcase for PR 25662 invalid sh_offset for section
  2020-03-28  0:52       ` Alan Modra
@ 2020-03-28  2:01         ` Hans-Peter Nilsson
  2020-03-28  6:15         ` Alan Modra
  1 sibling, 0 replies; 7+ messages in thread
From: Hans-Peter Nilsson @ 2020-03-28  2:01 UTC (permalink / raw)
  To: Alan Modra; +Cc: Jozef Lawrynowicz, binutils

On Sat, 28 Mar 2020, Alan Modra wrote:
> I think that is just a rearrangement of the object file records, so
> I'm inclined to xfail the test for mmix.  I'll leave that to you
> Hans-Peter since you know your object format much better than I can
> understand from the occasional glance at bfd/mmo.c.

Thanks for looking.  That seems likely; I intent to look at
details later in the weekend.

> Meanwhile, to fix an XPASS on tic54x and to xfail some others for the
> new test, I'm committing the following.

I'm a bit worried of that use of xfail here, because that means
people get an XPASS if the test starts passing, and if the only
reason is that gas and/or ld vs. objcopy disagrees on content
ordering for random perhaps build-system-specific reasons, then
it's spurious and will cause wild goose chases.  I suggest just
skipping the tests for those systems where the failure isn't due
to some known flaw (one unlikely to randomly disappear).

brgds, H-P

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add testcase for PR 25662 invalid sh_offset for section
  2020-03-28  0:52       ` Alan Modra
  2020-03-28  2:01         ` Hans-Peter Nilsson
@ 2020-03-28  6:15         ` Alan Modra
  1 sibling, 0 replies; 7+ messages in thread
From: Alan Modra @ 2020-03-28  6:15 UTC (permalink / raw)
  To: binutils

On Sat, Mar 28, 2020 at 11:22:35AM +1030, Alan Modra wrote:
> xfails spu due to a note section getting a different vma, and some
> mips targets that give section symbols a name string.  I added -p
> for the executable test in an attempt to fix all the pe target fails,
> but that doesn't preserve the date/time for some reason.
> 
> 	* testsuite/binutils-all/objcopy.exp (objcopy_test): Move xfails
> 	from here to calls.  Remove "m8*-*-*" entry.  Don't xfail tic54x
> 	but do xfail spu, mipstx39 and mips-sgi-irix for the executable
> 	test.  Pass "-p" to objcopy for the executable test.

Last patch didn't manage to xfail spu due to clear_xfail *-*-*elf*.
Clearing *-*-*elf* dates back to a time when we had rather a lot more
setup_xfail patterns, so limiting it to hppa*-*-*elf*.  Also,
mips-*-irix ought to have been mips-*-irix* and I'm having second
thoughts about xfailing mips and hiding what looks like a problem: If
the mips target is supposed to emit names for local section symbols
and does so for objcopy, why isn't it doing the same for ld?  Also,
lots more mips targets would be subject to this test failing.  So I'm
backing out those xfails and leaving it to someone more knowledgeable
about mips.

	* testsuite/binutils-all/objcopy.exp (objcopy_test): Only
	clear_xfail hppa*-*-*elf*.  Revert mips xfails.

diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index 0eafcdbede..e4eb53cf84 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -117,7 +117,7 @@ setup_xfail "hppa*-*-*"
 setup_xfail "sh-*-coff*"
 setup_xfail "tic54x-*-*"
 clear_xfail "hppa*64*-*-hpux*" "hppa*-*-linux*" "hppa*-*-lites*"
-clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
+clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "hppa*-*-*elf*"
 objcopy_test "simple copy" bintest.s object "" ""
 
 # Test verilog data width
@@ -1112,7 +1112,7 @@ if [is_elf_format] {
     setup_xfail "sh-*-coff*"
     setup_xfail "tic54x-*-*"
     clear_xfail "hppa*64*-*-hpux*" "hppa*-*-linux*" "hppa*-*-lites*"
-    clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
+    clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "hppa*-*-*elf*"
     objcopy_test "ELF unknown section type" unknown.s object "" ""
 
     objcopy_test_readelf "ELF group 1" group.s
@@ -1345,7 +1345,7 @@ run_dump_test "set-section-alignment"
 
 setup_xfail "hppa*-*-*"
 setup_xfail "sh-*-coff*"
-setup_xfail "mips-*-irix" "mipstx39-*-*" "spu-*-*"
+setup_xfail "spu-*-*"
 clear_xfail "hppa*64*-*-hpux*" "hppa*-*-linux*" "hppa*-*-lites*"
-clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "*-*-*elf*"
+clear_xfail "hppa*-*-*n*bsd*" "hppa*-*-rtems*" "hppa*-*-*elf*"
 objcopy_test "pr25662" pr25662.s executable "" "-T$srcdir/$subdir/pr25662.ld"

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-28  7:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 11:10 [PATCH] Add testcase for PR 25662 invalid sh_offset for section Jozef Lawrynowicz
2020-03-26 21:25 ` Alan Modra
2020-03-27 10:59   ` Jozef Lawrynowicz
2020-03-27 21:03     ` Hans-Peter Nilsson
2020-03-28  0:52       ` Alan Modra
2020-03-28  2:01         ` Hans-Peter Nilsson
2020-03-28  6:15         ` Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).