[PATCH] MIPS: N64, mark .interp as INITIAL_READONLY_SECTIONS

[Maciej W. Rozycki]

On Fri, 30 Jun 2023, YunQiang Su wrote:

> In ld/emulparams/elf64bmip-defs.sh, there is no .interp, which make
> the .interp section appears after some other less important sections.
> Let's add it, and mark it as INITIAL_READONLY_SECTIONS, just like
> O32/N32 do.
> 
> This changes fixes ld/pr23658-2.

 It would have helped with the review if the actual difference between the 
output expected by the test case and one produced was given along with the 
submission, perhaps in the discussion section.

 Without that I had to bend backwards to make this test case run with n64 
MIPS at all and see what is going on here as I don't have a compiler 
available for OpenBSD, which is my reference target for n64 verification.  
This revealed a minor discrepancy only between the output produced:

   03     .MIPS.abiflags .MIPS.options .dynamic .hash .dynsym .dynstr .text .interp .note.4 .note.1 .note.2 .note.3

vs expected:

 +[0-9]+ +\.interp \.note.4 \.note.1 \.note.2 \.note.3.*

and made me question whether the test case shouldn't be relaxed instead.  
Eventually I realised this is actually a fix for a regression back from 
2006, so I chose to give your change a go-ahead, even though clearly the 
current arrangement is good enough not to have caused any issues through 
17 years.

 Overall compiled tests are hard to run in a multi-target environment 
where compilers for all the exotic configurations are not necessarily 
available and therefore these tests receive less attention.  As compiler 
configurations vary these tests may even produce incorrect results despite 
no actual issue being present with binutils.  They're a compromise really 
for issues discovered in the past for which no effort could have been 
allocated to prepare specific per-target tests that rely on tools coming 
with binutils only.

 It also means we need proper coverage for section ordering, as it would 
have prevented this bug from having been introduced in the first place.

> ld/ChangeLog:
> 	* emulparams/elf64bmip-defs.sh: mark .interp as
> 	INITIAL_READONLY_SECTIONS.
> 	* testsuite/ld-mips-elf/pie-n64.d: adjust addresses.

 We'd best avoid code duplication we have here, which leads to situations 
exactly such as this one.  But that'd be for a larger rewrite.  Meanwhile 
your change seems good to me as an interim fix, with typos fixed in the 
change description.

 As I have observed above a test case is needed, so I have now pushed an 
updated version including a suitable set of tests, as recorded here: 
<https://sourceware.org/pipermail/binutils/2023-July/128674.html>.

 In the future please try to come up with testsuite cases for bug fixes 
where feasible.  I appreciate that making a test case often takes more 
effort than the bug fix itself, but we need these tests so as not to 
reintroduce similar issues in the future, such as e.g. with a rewrite I 
have suggested above.

 Thank you for your contribution.

  Maciej