[PATCH] section-match: Check parent archive name as well

[PATCH] section-match: Check parent archive name as well

[Michael Matz]

rewriting the section matching routines lost a special case
of matching: section statements of the form

    NAME(section-glob)

normally match against NAME being an object file, but like in
the exclude list we happened to accept archive names as NAME
(undocumented).  The documented way to specify (all) archive members
is by using e.g.

    lib.a:(section-glob)

(that does work also with the prefix tree matcher).

But I intended to not actually change behaviour with the prefix
tree implementation.  So, let's also implement checking against
archive names with a similar FIXME comment we already have in
walk_wild_file_in_exclude_list.
---
Regtesting on Alans targets in progress, on x86_64-linux it finished 
already successfully.  Okay for master?

---
 ld/ldlang.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 78716f17729..e359a89fcc0 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -445,8 +445,19 @@ walk_wild_section_match (lang_wild_statement_type *ptr,
 	 about unset local_sym_name (in which case lookup_name simply adds
 	 the input file again).  */
       const char *filename = file->local_sym_name;
-      if (filename == NULL
-	  || filename_cmp (filename, file_spec) != 0)
+      lang_input_statement_type *arch_is;
+      if (filename && filename_cmp (filename, file_spec) == 0)
+	;
+      /* FIXME: see also walk_wild_file_in_exclude_list for why we
+	 also check parents BFD (local_sym_)name to match input statements
+	 with unadorned archive names.  */
+      else if (file->the_bfd
+	       && file->the_bfd->my_archive
+	       && (arch_is = bfd_usrdata (file->the_bfd->my_archive))
+	       && arch_is->local_sym_name
+	       && filename_cmp (arch_is->local_sym_name, file_spec) == 0)
+	;
+      else
 	return;
     }
 
-- 
2.39.1

Re: [PATCH] section-match: Check parent archive name as well

[Michael Matz]

Lame self-reply:

On Mon, 26 Jun 2023, Michael Matz via Binutils wrote:

> Regtesting on Alans targets in progress,

that finished uneventful as well.


Ciao,
Michael.

Re: [PATCH] section-match: Check parent archive name as well

[Nick Clifton]

Hi Michael,

> rewriting the section matching routines lost a special case
> of matching: section statements of the form
> 
>      NAME(section-glob)
> 
> normally match against NAME being an object file, but like in
> the exclude list we happened to accept archive names as NAME
> (undocumented).  The documented way to specify (all) archive members
> is by using e.g.
> 
>      lib.a:(section-glob)
> 
> (that does work also with the prefix tree matcher).
> 
> But I intended to not actually change behaviour with the prefix
> tree implementation.  So, let's also implement checking against
> archive names with a similar FIXME comment we already have in
> walk_wild_file_in_exclude_list.

Looks good, but please could also update the documentation so that
this use is explicitly cited as being supported ?

Also would you mind adding a link to your post to this list to
the PR 30590 so that anyone reading that bug report can see how
you fixed it.  Thanks

   https://sourceware.org/bugzilla/show_bug.cgi?id=30590

Cheers
   Nick

Re: [PATCH] section-match: Check parent archive name as well

[Michael Matz]

Hello Nick,

On Tue, 27 Jun 2023, Nick Clifton wrote:

> >      NAME(section-glob)
> > 
> > normally match against NAME being an object file, but like in
> > the exclude list we happened to accept archive names as NAME
> > (undocumented).  The documented way to specify (all) archive members
> > is by using e.g.
> > 
> >      lib.a:(section-glob)
> > 
> > (that does work also with the prefix tree matcher).
> > 
> > But I intended to not actually change behaviour with the prefix
> > tree implementation.  So, let's also implement checking against
> > archive names with a similar FIXME comment we already have in
> > walk_wild_file_in_exclude_list.
> 
> Looks good, but please could also update the documentation so that
> this use is explicitly cited as being supported ?

Actually, I'm reluctant to do that and would like to keep this 
undocumented.  We wouldn't be able to fix the FIXMEs in 
walk_wild_file_in_exclude_list anymore if such usage was documented, and 
we have a different (documented) way to specify what the user wanted.

In either case that should be a separate and conscious patch as it 
would introduce (formally) new behaviour.  If you still want me to amend 
the patch with a documentation change, I will, but only on second request 
;-)

> Also would you mind adding a link to your post to this list to
> the PR 30590 so that anyone reading that bug report can see how
> you fixed it.  Thanks
> 
>   https://sourceware.org/bugzilla/show_bug.cgi?id=30590

Oh yes, I haven't yet seen that a bugzilla entry was created for that.
The commit will have it.


Ciao,
Michael.

Re: [PATCH] section-match: Check parent archive name as well

[Nick Clifton]

Hi Michael,

>> Looks good, but please could also update the documentation so that
>> this use is explicitly cited as being supported ?
> 
> Actually, I'm reluctant to do that and would like to keep this
> undocumented.  We wouldn't be able to fix the FIXMEs in
> walk_wild_file_in_exclude_list anymore if such usage was documented, and
> we have a different (documented) way to specify what the user wanted.
> 
> In either case that should be a separate and conscious patch as it
> would introduce (formally) new behaviour.  If you still want me to amend
> the patch with a documentation change, I will, but only on second request
> ;-)

Fair enough.


>> Also would you mind adding a link to your post to this list to
>> the PR 30590 so that anyone reading that bug report can see how
>> you fixed it.  Thanks
>>
>>    https://sourceware.org/bugzilla/show_bug.cgi?id=30590
> 
> Oh yes, I haven't yet seen that a bugzilla entry was created for that.
> The commit will have it.

Great - patch approved - please apply.

Cheers
   Nick

PS. A patch to add a test for this bug to the linker testsuite is pre-approved,
   should you feel like writing one ....