Re: [PATCH] Fix msp430 section name scribbling and tests

[Alan Modra <amodra@gmail.com>]

On Fri, Nov 25, 2022 at 04:00:05PM +0000, Michael Matz via Binutils wrote:
> msp430 upper/lower section selector simply overwrites section names
> without going through bfd_rename_section.  This doesn't really work:

Right.

> as soon as the generic routine to map to output section names is used
> (e.g. by having a complicated selector, or simply by adding an early-out
> into analyze_walk_wild_section_handler for always use the generic
> matcher) it iterates over the new names and so sees e.g. '.either..data'
> and makes that orphan (because of no output statement matching that),
> and then errors out because there may be no output statements for
> .upper.data or .lower.data.  In the testcases that works because that
> .data is actually empty, and when using the non-generic routines for
> section matching it can use the per-bfd section-name-hash.  Because that
> one still is unchanged it will find the section formerly named .data for
> which an output statement exists, put it into that section, and makes it
> not ortphan.  That works because the actual section is empty and hence
> this placement is harmless.
> 
> But it's really not how this all is expected to work, one can't just
> change section->name alone.  So to make it reliably work we either
> would have to force users to write at least .lower.XYZ statements for
> the three usual sections (.text, .data, .bss), even if the input is empty,
> or to check if renaming to .either.XYZ is going to do any good.
> 
> Here we do the latter, we check before renaming and then actually do
> rename properly.

I'm inclined to think that you should just apply the single line
bfd_rename_section change, and adjust the testsuite script file.
The standard scripts do contain the lower/upper/either variants for
.text, .data and .bss.  

Something like this.

	* emultempl/msp430.em (add_region_prefix <REGION_EITHER>): Use
	bfd_rename_section.
	* testsuite/ld-msp430-elf/msp430-tiny-rom.ld: Handle varian data
	and bss input sections.

diff --git a/ld/emultempl/msp430.em b/ld/emultempl/msp430.em
index f188b46b4aa..048e3ebfe5a 100644
--- a/ld/emultempl/msp430.em
+++ b/ld/emultempl/msp430.em
@@ -417,7 +417,7 @@ add_region_prefix (bfd *abfd ATTRIBUTE_UNUSED, asection *s,
       bfd_rename_section (s, concat (".lower", curr_name, NULL));
       break;
     case REGION_EITHER:
-      s->name = concat (".either", curr_name, NULL);
+      bfd_rename_section (s, concat (".either", curr_name, NULL));
       break;
     default:
       /* Unreachable.  */
diff --git a/ld/testsuite/ld-msp430-elf/msp430-tiny-rom.ld b/ld/testsuite/ld-msp430-elf/msp430-tiny-rom.ld
index 3e263796948..c88442377d1 100644
--- a/ld/testsuite/ld-msp430-elf/msp430-tiny-rom.ld
+++ b/ld/testsuite/ld-msp430-elf/msp430-tiny-rom.ld
@@ -26,13 +26,19 @@ SECTIONS
   .data :
   {
     . = ALIGN(2);
+    *(.lower.data.* .lower.data)
     *(.data.* .data)
+    *(.either.data.* .either.data)
+    *(.upper.data.* .upper.data)
   } > RAM AT> ROM
 
   .bss :
   {
     . = ALIGN(2);
+    *(.lower.bss.* .lower.bss)
     *(.bss.* .bss)
+    *(.either.bss.* .either.bss)
+    *(.upper.bss.* .upper.bss)
   } > RAM
 
   .upper.text :


-- 
Alan Modra
Australia Development Lab, IBM