[PATCH] elf: Keep zero-sized relocation section in section group

[PATCH] elf: Keep zero-sized relocation section in section group

[H.J. Lu]

We must keep zero-sized relocation section in a section group.  Otherwise,
the relocation section will be missing from the section group.

	PR ld/25767
	* elflink.c (bfd_elf_final_link): Keep zero-sized relocation
	section in section group.
---
 bfd/elflink.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 7c0849423a..3b29cc127a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -12058,11 +12058,15 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
 
       if (o->reloc_count > 0)
 	o->flags |= SEC_RELOC;
-      else
+      else if (info->resolve_section_groups || !elf_group_name (o))
 	{
 	  /* Explicitly clear the SEC_RELOC flag.  The linker tends to
 	     set it (this is probably a bug) and if it is set
-	     assign_section_numbers will create a reloc section.  */
+	     assign_section_numbers will create a reloc section.
+
+	     PR ld/25767: We must keep zero-sized relocation section
+	     in a section group.  Otherwise, that relocation section
+	     will be missing from the section group.  */
 	  o->flags &=~ SEC_RELOC;
 	}
 
-- 
2.25.1

Re: [PATCH] elf: Keep zero-sized relocation section in section group

[Alan Modra]

On Thu, Apr 02, 2020 at 08:41:18PM -0700, H.J. Lu via Binutils wrote:
> We must keep zero-sized relocation section in a section group.  Otherwise,
> the relocation section will be missing from the section group.

Wouldn't it be better to get rid of these bogus relocation sections?
Should be possible with a change to _bfd_elf_fixup_group_sections, I
think.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] elf: Keep zero-sized relocation section in section group

[Fangrui Song]

On 2020-04-03, Alan Modra via Binutils wrote:
>On Thu, Apr 02, 2020 at 08:41:18PM -0700, H.J. Lu via Binutils wrote:
>> We must keep zero-sized relocation section in a section group.  Otherwise,
>> the relocation section will be missing from the section group.
>
>Wouldn't it be better to get rid of these bogus relocation sections?
>Should be possible with a change to _bfd_elf_fixup_group_sections, I
>think.
>
>-- 
>Alan Modra
>Australia Development Lab, IBM

"19990502 sourceware import" contains

+      if (o->reloc_count > 0)
+       o->flags |= SEC_RELOC;
+      else
+       {
+         /* Explicitly clear the SEC_RELOC flag.  The linker tends to
+            set it (this is probably a bug) and if it is set
+            assign_section_numbers will create a reloc section.  */
+         o->flags &=~ SEC_RELOC;
+       }

"this is probably a bug" Shouldn't we fix the bug?

[PATCH] elf: Remove zero-sized relocation section from section group

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

On Thu, Apr 2, 2020 at 9:45 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Thu, Apr 02, 2020 at 08:41:18PM -0700, H.J. Lu via Binutils wrote:
> > We must keep zero-sized relocation section in a section group.  Otherwise,
> > the relocation section will be missing from the section group.
>
> Wouldn't it be better to get rid of these bogus relocation sections?
> Should be possible with a change to _bfd_elf_fixup_group_sections, I
> think.

Here is the patch which does that.  OK for master?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-elf-Remove-zero-sized-relocation-section-from-sectio.patch --]
[-- Type: text/x-patch, Size: 2953 bytes --]

From db62e0099b194aa7a91bf7b705feb23d6f3f1756 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 2 Apr 2020 20:38:02 -0700
Subject: [PATCH] elf: Remove zero-sized relocation section from section group

Remove zero-sized relocation section from a section group since it has
been removed from the output.

	PR ld/25767
	* elflink.c (_bfd_elf_fixup_group_sections): Remove zero-sized
	relocation section from section group.
---
 bfd/elf.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 86dadea05c..249cb3ff1f 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -7929,19 +7929,55 @@ _bfd_elf_fixup_group_sections (bfd *ibfd, asection *discarded)
 		elf_section_flags (s->output_section) &= ~SHF_GROUP;
 		elf_group_name (s->output_section) = NULL;
 	      }
-	    /* Conversely, if the member section is not being output
-	       but the SHT_GROUP section is, then adjust its size.  */
-	    else if (s->output_section == discarded
-		     && isec->output_section != discarded)
+	    else
 	      {
 		struct bfd_elf_section_data *elf_sec = elf_section_data (s);
-		removed += 4;
-		if (elf_sec->rel.hdr != NULL
-		    && (elf_sec->rel.hdr->sh_flags & SHF_GROUP) != 0)
-		  removed += 4;
-		if (elf_sec->rela.hdr != NULL
-		    && (elf_sec->rela.hdr->sh_flags & SHF_GROUP) != 0)
-		  removed += 4;
+		if (s->output_section == discarded
+		    && isec->output_section != discarded)
+		  {
+		    /* Conversely, if the member section is not being
+		       output but the SHT_GROUP section is, then adjust
+		       its size.  */
+		    removed += 4;
+		    if (elf_sec->rel.hdr != NULL
+			&& (elf_sec->rel.hdr->sh_flags & SHF_GROUP) != 0)
+		      removed += 4;
+		    if (elf_sec->rela.hdr != NULL
+			&& (elf_sec->rela.hdr->sh_flags & SHF_GROUP) != 0)
+		      removed += 4;
+		  }
+		else
+		  {
+		    const char *name;
+		    /* Also adjust for zero-sized relocation member
+		       section.  */
+		    if (elf_sec->rel.hdr != NULL
+			&& elf_sec->rel.hdr->sh_size == 0)
+		      {
+			removed += 4;
+			name = bfd_elf_string_from_elf_section
+			  (ibfd, (elf_elfheader (ibfd)->e_shstrndx),
+			   elf_sec->rel.hdr->sh_name);
+			_bfd_error_handler
+			  /* xgettext:c-format */
+			  (_("%pB: warning: zero-sized relocation section "
+			     "%s' for section %pA found - ignoring"),
+			   ibfd, name, s);
+		      }
+		    if (elf_sec->rela.hdr != NULL
+			&& elf_sec->rela.hdr->sh_size == 0)
+		      {
+			removed += 4;
+			name = bfd_elf_string_from_elf_section
+			  (ibfd, (elf_elfheader (ibfd)->e_shstrndx),
+			   elf_sec->rela.hdr->sh_name);
+			_bfd_error_handler
+			  /* xgettext:c-format */
+			  (_("%pB: warning: zero-sized relocation section "
+			     "%s' for section %pA found - ignoring"),
+			   ibfd, name, s);
+		      }
+		  }
 	      }
 	    s = elf_next_in_group (s);
 	    if (s == first)
-- 
2.25.1

Re: [PATCH] elf: Keep zero-sized relocation section in section group

[H.J. Lu]

On Thu, Apr 2, 2020 at 11:34 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2020-04-03, Alan Modra via Binutils wrote:
> >On Thu, Apr 02, 2020 at 08:41:18PM -0700, H.J. Lu via Binutils wrote:
> >> We must keep zero-sized relocation section in a section group.  Otherwise,
> >> the relocation section will be missing from the section group.
> >
> >Wouldn't it be better to get rid of these bogus relocation sections?
> >Should be possible with a change to _bfd_elf_fixup_group_sections, I
> >think.
> >
> >--
> >Alan Modra
> >Australia Development Lab, IBM
>
> "19990502 sourceware import" contains
>
> +      if (o->reloc_count > 0)
> +       o->flags |= SEC_RELOC;
> +      else
> +       {
> +         /* Explicitly clear the SEC_RELOC flag.  The linker tends to
> +            set it (this is probably a bug) and if it is set
> +            assign_section_numbers will create a reloc section.  */
> +         o->flags &=~ SEC_RELOC;
> +       }
>
> "this is probably a bug" Shouldn't we fix the bug?

Yes, the real bug will be fixed :-).

-- 
H.J.

Re: [PATCH] elf: Remove zero-sized relocation section from section group

[Alan Modra]

On Fri, Apr 03, 2020 at 01:48:42PM -0700, H.J. Lu wrote:
> On Thu, Apr 2, 2020 at 9:45 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Thu, Apr 02, 2020 at 08:41:18PM -0700, H.J. Lu via Binutils wrote:
> > > We must keep zero-sized relocation section in a section group.  Otherwise,
> > > the relocation section will be missing from the section group.
> >
> > Wouldn't it be better to get rid of these bogus relocation sections?
> > Should be possible with a change to _bfd_elf_fixup_group_sections, I
> > think.
> 
> Here is the patch which does that.  OK for master?

OK, thanks.

Hmm, do we want a warning message?  I'm OK with it as a reminder to
people to fix their compilers and/or assembly, but other than that it
seems to me that removing an empty relocation section is hardly of
concern to users.  How was your testcase generated?

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] elf: Remove zero-sized relocation section from section group

[H.J. Lu]

On Fri, Apr 3, 2020 at 5:36 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Fri, Apr 03, 2020 at 01:48:42PM -0700, H.J. Lu wrote:
> > On Thu, Apr 2, 2020 at 9:45 PM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Thu, Apr 02, 2020 at 08:41:18PM -0700, H.J. Lu via Binutils wrote:
> > > > We must keep zero-sized relocation section in a section group.  Otherwise,
> > > > the relocation section will be missing from the section group.
> > >
> > > Wouldn't it be better to get rid of these bogus relocation sections?
> > > Should be possible with a change to _bfd_elf_fixup_group_sections, I
> > > think.
> >
> > Here is the patch which does that.  OK for master?
>
> OK, thanks.
>
> Hmm, do we want a warning message?  I'm OK with it as a reminder to

I will remove the warning.

> people to fix their compilers and/or assembly, but other than that it
> seems to me that removing an empty relocation section is hardly of
> concern to users.  How was your testcase generated?
>

I was given an object file generated by an "unnamed" compiler :-).
I have told them to fix their compiler.

Thanks.

-- 
H.J.

Re: [PATCH] elf: Remove zero-sized relocation section from section group

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

On Fri, Apr 3, 2020 at 6:56 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Apr 3, 2020 at 5:36 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Fri, Apr 03, 2020 at 01:48:42PM -0700, H.J. Lu wrote:
> > > On Thu, Apr 2, 2020 at 9:45 PM Alan Modra <amodra@gmail.com> wrote:
> > > >
> > > > On Thu, Apr 02, 2020 at 08:41:18PM -0700, H.J. Lu via Binutils wrote:
> > > > > We must keep zero-sized relocation section in a section group.  Otherwise,
> > > > > the relocation section will be missing from the section group.
> > > >
> > > > Wouldn't it be better to get rid of these bogus relocation sections?
> > > > Should be possible with a change to _bfd_elf_fixup_group_sections, I
> > > > think.
> > >
> > > Here is the patch which does that.  OK for master?
> >
> > OK, thanks.
> >
> > Hmm, do we want a warning message?  I'm OK with it as a reminder to
>
> I will remove the warning.
>
> > people to fix their compilers and/or assembly, but other than that it
> > seems to me that removing an empty relocation section is hardly of
> > concern to users.  How was your testcase generated?
> >
>
> I was given an object file generated by an "unnamed" compiler :-).
> I have told them to fix their compiler.
>
> Thanks.

This is what I am checking in.

-- 
H.J.

[-- Attachment #2: 0001-elf-Remove-zero-sized-relocation-section-from-sectio.patch --]
[-- Type: text/x-patch, Size: 2253 bytes --]

From fad689e7b5d92d50986ec040071e84b5eb50604f Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 2 Apr 2020 20:38:02 -0700
Subject: [PATCH] elf: Remove zero-sized relocation section from section group

Remove zero-sized relocation section from a section group since it has
been removed from the output.

	PR ld/25767
	* elf.c (_bfd_elf_fixup_group_sections): Remove zero-sized
	relocation section from section group.
---
 bfd/elf.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 86dadea05c..1780074f5a 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -7929,19 +7929,34 @@ _bfd_elf_fixup_group_sections (bfd *ibfd, asection *discarded)
 		elf_section_flags (s->output_section) &= ~SHF_GROUP;
 		elf_group_name (s->output_section) = NULL;
 	      }
-	    /* Conversely, if the member section is not being output
-	       but the SHT_GROUP section is, then adjust its size.  */
-	    else if (s->output_section == discarded
-		     && isec->output_section != discarded)
+	    else
 	      {
 		struct bfd_elf_section_data *elf_sec = elf_section_data (s);
-		removed += 4;
-		if (elf_sec->rel.hdr != NULL
-		    && (elf_sec->rel.hdr->sh_flags & SHF_GROUP) != 0)
-		  removed += 4;
-		if (elf_sec->rela.hdr != NULL
-		    && (elf_sec->rela.hdr->sh_flags & SHF_GROUP) != 0)
-		  removed += 4;
+		if (s->output_section == discarded
+		    && isec->output_section != discarded)
+		  {
+		    /* Conversely, if the member section is not being
+		       output but the SHT_GROUP section is, then adjust
+		       its size.  */
+		    removed += 4;
+		    if (elf_sec->rel.hdr != NULL
+			&& (elf_sec->rel.hdr->sh_flags & SHF_GROUP) != 0)
+		      removed += 4;
+		    if (elf_sec->rela.hdr != NULL
+			&& (elf_sec->rela.hdr->sh_flags & SHF_GROUP) != 0)
+		      removed += 4;
+		  }
+		else
+		  {
+		    /* Also adjust for zero-sized relocation member
+		       section.  */
+		    if (elf_sec->rel.hdr != NULL
+			&& elf_sec->rel.hdr->sh_size == 0)
+		      removed += 4;
+		    if (elf_sec->rela.hdr != NULL
+			&& elf_sec->rela.hdr->sh_size == 0)
+		      removed += 4;
+		  }
 	      }
 	    s = elf_next_in_group (s);
 	    if (s == first)
-- 
2.25.1