[PATCH] ld/testsuite: Fix test failures in pe.exp caused by 8819b236

[PATCH] ld/testsuite: Fix test failures in pe.exp caused by 8819b236

[Mark Harmstone]

This fixes three test failures caused by the change in .idata alignment
on x86_64-w64-mingw32.

The secrel and secidx tests were dumping .idata, so this is easy enough
to solve - it's irrelevant to the tests anyway.

As for the CFI test, it looks like the debug_frame code inserts an empty
entry at the beginning to pad this to an 8-byte boundary, so I've
changed the section alignment for the test to fix this.

---
 ld/testsuite/ld-pe/cfi.d       | 6 +++---
 ld/testsuite/ld-pe/secidx_64.d | 2 +-
 ld/testsuite/ld-pe/secrel_64.d | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ld/testsuite/ld-pe/cfi.d b/ld/testsuite/ld-pe/cfi.d
index 55ebaca1aef..c4d9b6ef2c3 100644
--- a/ld/testsuite/ld-pe/cfi.d
+++ b/ld/testsuite/ld-pe/cfi.d
@@ -1,10 +1,10 @@
 #source: cfia.s
 #source: cfib.s
-#ld: --file-align 1 --section-align 1
+#ld: --file-align 8 --section-align 8
 #objdump: -Wf
 
 #...
-0+4 0+14 0*ffffffff CIE
+0+ 0+14 0*ffffffff CIE
   Version:               1
   Augmentation:          ""
   Code alignment factor: 1
@@ -20,7 +20,7 @@
   DW_CFA_nop
   DW_CFA_nop
 
-0+1c 0+24 0+4 FDE cie=0+4 pc=.*
+0+18 0+24 0+ FDE cie=0+ pc=.*
   DW_CFA_advance_loc: 4 to .*
   DW_CFA_def_cfa_offset: 16
   DW_CFA_offset: r6 \(rbp\) at cfa\-16
diff --git a/ld/testsuite/ld-pe/secidx_64.d b/ld/testsuite/ld-pe/secidx_64.d
index ddf4aec74f9..6f034f7ac9e 100644
--- a/ld/testsuite/ld-pe/secidx_64.d
+++ b/ld/testsuite/ld-pe/secidx_64.d
@@ -24,4 +24,4 @@ Contents of section \.rdata:
  .*3030 3c3c3c3e 3e3e3e3e 3e000000 00000000  <<<>>>>>>.......
 Contents of section \.idata:
  .*4000 00000000 00000000 00000000 00000000  ................
- .*4010 00000000                             ....            
+ .*4010 00000000 .*
diff --git a/ld/testsuite/ld-pe/secrel_64.d b/ld/testsuite/ld-pe/secrel_64.d
index aba1bf11c69..2997cac691f 100644
--- a/ld/testsuite/ld-pe/secrel_64.d
+++ b/ld/testsuite/ld-pe/secrel_64.d
@@ -25,4 +25,4 @@ Contents of section \.rdata:
  .*3020 3e3e3e3e 00000000 00000000 00000000  >>>>............
 Contents of section \.idata:
  .*4000 00000000 00000000 00000000 00000000  ................
- .*4010 00000000                             ....            
+ .*4010 00000000 .*
\ No newline at end of file
-- 
2.37.4

Re: [PATCH] ld/testsuite: Fix test failures in pe.exp caused by 8819b236

[Jan Beulich]

On 05.01.2023 01:27, Mark Harmstone wrote:
> This fixes three test failures caused by the change in .idata alignment
> on x86_64-w64-mingw32.
> 
> The secrel and secidx tests were dumping .idata, so this is easy enough
> to solve - it's irrelevant to the tests anyway.
> 
> As for the CFI test, it looks like the debug_frame code inserts an empty
> entry at the beginning to pad this to an 8-byte boundary, so I've
> changed the section alignment for the test to fix this.

I have to admit that I'm wary of changes like this, without it being clear
whether the options were put there on purpose (in exactly that form). Nick,
it's been over 10 years, but I still wonder whether you recall.

I further think that cfi32.d would then want similar adjustment (perhaps
using 4 instead of 8), even if not strictly needed to address the fallout.
Altogether I think that part of the change would better be a separate
change, not the least to allow easy reverting if need be.

> --- a/ld/testsuite/ld-pe/cfi.d
> +++ b/ld/testsuite/ld-pe/cfi.d
> @@ -1,10 +1,10 @@
>  #source: cfia.s
>  #source: cfib.s
> -#ld: --file-align 1 --section-align 1
> +#ld: --file-align 8 --section-align 8
>  #objdump: -Wf
>  
>  #...
> -0+4 0+14 0*ffffffff CIE
> +0+ 0+14 0*ffffffff CIE

I have to admit that I'm puzzled by this having been like it was, i.e.
expecting 4 here instead of 0 as the starting section offset. I can't
help thinking that the expectation was wrong, and hence a change
elsewhere is needed (and then the .idata alignment change would not
have caused any fallout).

> --- a/ld/testsuite/ld-pe/secrel_64.d
> +++ b/ld/testsuite/ld-pe/secrel_64.d
> @@ -25,4 +25,4 @@ Contents of section \.rdata:
>   .*3020 3e3e3e3e 00000000 00000000 00000000  >>>>............
> 
>  Contents of section \.idata:
> 
>   .*4000 00000000 00000000 00000000 00000000  ................
> 
> - .*4010 00000000                             ....            
> 
> + .*4010 00000000 .*
> \ No newline at end of file

May I ask that you take the opportunity and insert the missing newline
here?

With the CFI part split off the remainder is okay to put in.

Jan

Re: [PATCH] ld/testsuite: Fix test failures in pe.exp caused by 8819b236

[Nick Clifton]

Hi Mark,

> This fixes three test failures caused by the change in .idata alignment
> on x86_64-w64-mingw32.
> 
> The secrel and secidx tests were dumping .idata, so this is easy enough
> to solve - it's irrelevant to the tests anyway.
> 
> As for the CFI test, it looks like the debug_frame code inserts an empty
> entry at the beginning to pad this to an 8-byte boundary, so I've
> changed the section alignment for the test to fix this.

Patch approved - please apply.

Thanks for fixing this problem!

Cheers
   Nick

Re: [PATCH] ld/testsuite: Fix test failures in pe.exp caused by 8819b236

[Alan Modra]

On Thu, Jan 05, 2023 at 08:21:35AM +0000, Nick Clifton via Binutils wrote:
> Hi Mark,
> 
> > This fixes three test failures caused by the change in .idata alignment
> > on x86_64-w64-mingw32.
> > 
> > The secrel and secidx tests were dumping .idata, so this is easy enough
> > to solve - it's irrelevant to the tests anyway.
> > 
> > As for the CFI test, it looks like the debug_frame code inserts an empty
> > entry at the beginning to pad this to an 8-byte boundary, so I've
> > changed the section alignment for the test to fix this.
> 
> Patch approved - please apply.
> 
> Thanks for fixing this problem!

I already fixed this with commit 478eebf83198.  I guess I should put
it on the branch too.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] ld/testsuite: Fix test failures in pe.exp caused by 8819b236

[Nick Clifton]

Hi Alan,

> I already fixed this with commit 478eebf83198. 

Ah - sorry - I missed that.

> I guess I should put it on the branch too.

Done.

Cheers
   Nick