public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Michael Matz <matz@suse.de>
To: binutils@sourceware.org
Subject: [PATCH] Fix msp430 section name scribbling and tests
Date: Fri, 25 Nov 2022 16:00:05 +0000 (UTC)	[thread overview]
Message-ID: <alpine.LSU.2.20.2211251556330.24878@wotan.suse.de> (raw)

msp430 upper/lower section selector simply overwrites section names
without going through bfd_rename_section.  This doesn't really work:
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.

FWIW, to see the testsuite breaking without this patch one could apply
this patch that forces the generic matching routine to always be used:

--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -839,6 +839,7 @@ analyze_walk_wild_section_handler (lang_wild_statement_type *ptr)
   ptr->handler_data[2] = NULL;
   ptr->handler_data[3] = NULL;
   ptr->tree = NULL;
+  return;

   /* Count how many wildcard_specs there are, and how many of those
      actually use wildcards in the name.  Also, bail out if any of the
---
Regtested for msp430-elf.  This problem prevented some other work in 
changing how sections are selected, so I want to get rid of this first.

Okay for master?

 ld/emultempl/msp430.em | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/ld/emultempl/msp430.em b/ld/emultempl/msp430.em
index f188b46b4aa..c8a39f265f2 100644
--- a/ld/emultempl/msp430.em
+++ b/ld/emultempl/msp430.em
@@ -388,6 +388,38 @@ change_output_section (lang_statement_union_type **head,
   return false;
 }
 
+static bool
+has_upper_or_lower (const char *secname)
+{
+  char * lower_name;
+  char * upper_name;
+  char * name;
+  bool ret = false;
+  /* Compute the names of the corresponding upper and lower
+     sections.  If the input section name contains another period,
+     only use the part of the name before the second dot.  */
+  if (strchr (secname + 1, '.') != NULL)
+    {
+      name = xstrdup (secname);
+
+      * strchr (name + 1, '.') = 0;
+    }
+  else
+    name = (char *) secname;
+
+  lower_name = concat (".lower", name, NULL);
+  upper_name = concat (".upper", name, NULL);
+  if (lang_output_section_find (lower_name)
+      || lang_output_section_find (upper_name))
+    ret = true;
+
+  free (upper_name);
+  free (lower_name);
+  if (name != (char*) secname)
+    free (name);
+  return ret;
+}
+
 static void
 add_region_prefix (bfd *abfd ATTRIBUTE_UNUSED, asection *s,
 		   void *unused ATTRIBUTE_UNUSED)
@@ -417,7 +449,8 @@ 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);
+      if (has_upper_or_lower (curr_name))
+	bfd_rename_section (s, concat (".either", curr_name, NULL));
       break;
     default:
       /* Unreachable.  */
-- 
2.36.1

             reply	other threads:[~2022-11-25 16:00 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-25 16:00 Michael Matz [this message]
2022-11-28  2:02 ` Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.20.2211251556330.24878@wotan.suse.de \
    --to=matz@suse.de \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).