[PATCH] readelf: Warn zero-sized relocation sections

[PATCH] readelf: Warn zero-sized relocation sections

[H.J. Lu]

Older linkers may fail with zero-sized relocation section in section
group.

	PR ld/25767
	* readelf.c (process_section_headers): Warn zero-sized
	relocation sections.
---
 binutils/readelf.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/binutils/readelf.c b/binutils/readelf.c
index cd456b0290..535d47a58e 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -6315,9 +6315,19 @@ process_section_headers (Filedata * filedata)
       else if (section->sh_type == SHT_GROUP)
 	CHECK_ENTSIZE_VALUES (section, i, GRP_ENTRY_SIZE, GRP_ENTRY_SIZE);
       else if (section->sh_type == SHT_REL)
-	CHECK_ENTSIZE (section, i, Rel);
+	{
+	  CHECK_ENTSIZE (section, i, Rel);
+	  if (section->sh_size == 0)
+	    warn (_("Section '%s': zero-sized relocation section\n"),
+		  name);
+	}
       else if (section->sh_type == SHT_RELA)
-	CHECK_ENTSIZE (section, i, Rela);
+	{
+	  CHECK_ENTSIZE (section, i, Rela);
+	  if (section->sh_size == 0)
+	    warn (_("Section '%s': zero-sized relocation section\n"),
+		  name);
+	}
       else if ((do_debugging || do_debug_info || do_debug_abbrevs
 		|| do_debug_lines || do_debug_pubnames || do_debug_pubtypes
 		|| do_debug_aranges || do_debug_frames || do_debug_macinfo
-- 
2.25.2

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Jan Beulich]

On 14.04.2020 14:25, H.J. Lu via Binutils wrote:
> Older linkers may fail with zero-sized relocation section in section
> group.
> 
> 	PR ld/25767
> 	* readelf.c (process_section_headers): Warn zero-sized
> 	relocation sections.

Zero-sized sections of whatever kind are perfectly fine in ELF. I
see no reason why one would want to see a warning for such by
default.

Jan

> ---
>  binutils/readelf.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index cd456b0290..535d47a58e 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -6315,9 +6315,19 @@ process_section_headers (Filedata * filedata)
>        else if (section->sh_type == SHT_GROUP)
>  	CHECK_ENTSIZE_VALUES (section, i, GRP_ENTRY_SIZE, GRP_ENTRY_SIZE);
>        else if (section->sh_type == SHT_REL)
> -	CHECK_ENTSIZE (section, i, Rel);
> +	{
> +	  CHECK_ENTSIZE (section, i, Rel);
> +	  if (section->sh_size == 0)
> +	    warn (_("Section '%s': zero-sized relocation section\n"),
> +		  name);
> +	}
>        else if (section->sh_type == SHT_RELA)
> -	CHECK_ENTSIZE (section, i, Rela);
> +	{
> +	  CHECK_ENTSIZE (section, i, Rela);
> +	  if (section->sh_size == 0)
> +	    warn (_("Section '%s': zero-sized relocation section\n"),
> +		  name);
> +	}
>        else if ((do_debugging || do_debug_info || do_debug_abbrevs
>  		|| do_debug_lines || do_debug_pubnames || do_debug_pubtypes
>  		|| do_debug_aranges || do_debug_frames || do_debug_macinfo
> 

Re: [PATCH] readelf: Warn zero-sized relocation sections

[H.J. Lu]

On Tue, Apr 14, 2020 at 5:42 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.04.2020 14:25, H.J. Lu via Binutils wrote:
> > Older linkers may fail with zero-sized relocation section in section
> > group.
> >
> >       PR ld/25767
> >       * readelf.c (process_section_headers): Warn zero-sized
> >       relocation sections.
>
> Zero-sized sections of whatever kind are perfectly fine in ELF. I
> see no reason why one would want to see a warning for such by
> default.

Linkers older than 2.35 may fail with

./ld: BFD (GNU Binutils) 2.34.50.20200402 assertion fail
/export/gnu/import/git/gitlab/x86-binutils/bfd/elf.c:3652

> Jan
>
> > ---
> >  binutils/readelf.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/binutils/readelf.c b/binutils/readelf.c
> > index cd456b0290..535d47a58e 100644
> > --- a/binutils/readelf.c
> > +++ b/binutils/readelf.c
> > @@ -6315,9 +6315,19 @@ process_section_headers (Filedata * filedata)
> >        else if (section->sh_type == SHT_GROUP)
> >       CHECK_ENTSIZE_VALUES (section, i, GRP_ENTRY_SIZE, GRP_ENTRY_SIZE);
> >        else if (section->sh_type == SHT_REL)
> > -     CHECK_ENTSIZE (section, i, Rel);
> > +     {
> > +       CHECK_ENTSIZE (section, i, Rel);
> > +       if (section->sh_size == 0)
> > +         warn (_("Section '%s': zero-sized relocation section\n"),
> > +               name);
> > +     }
> >        else if (section->sh_type == SHT_RELA)
> > -     CHECK_ENTSIZE (section, i, Rela);
> > +     {
> > +       CHECK_ENTSIZE (section, i, Rela);
> > +       if (section->sh_size == 0)
> > +         warn (_("Section '%s': zero-sized relocation section\n"),
> > +               name);
> > +     }
> >        else if ((do_debugging || do_debug_info || do_debug_abbrevs
> >               || do_debug_lines || do_debug_pubnames || do_debug_pubtypes
> >               || do_debug_aranges || do_debug_frames || do_debug_macinfo
> >
>


-- 
H.J.

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Jan Beulich]

On 14.04.2020 15:03, H.J. Lu wrote:
> On Tue, Apr 14, 2020 at 5:42 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 14.04.2020 14:25, H.J. Lu via Binutils wrote:
>>> Older linkers may fail with zero-sized relocation section in section
>>> group.
>>>
>>>       PR ld/25767
>>>       * readelf.c (process_section_headers): Warn zero-sized
>>>       relocation sections.
>>
>> Zero-sized sections of whatever kind are perfectly fine in ELF. I
>> see no reason why one would want to see a warning for such by
>> default.
> 
> Linkers older than 2.35 may fail with
> 
> ./ld: BFD (GNU Binutils) 2.34.50.20200402 assertion fail
> /export/gnu/import/git/gitlab/x86-binutils/bfd/elf.c:3652

Which is a bug, yes, but imo no reason to clutter readelf output.

Jan

Re: [PATCH] readelf: Warn zero-sized relocation sections

[H.J. Lu]

On Tue, Apr 14, 2020 at 6:25 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.04.2020 15:03, H.J. Lu wrote:
> > On Tue, Apr 14, 2020 at 5:42 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 14.04.2020 14:25, H.J. Lu via Binutils wrote:
> >>> Older linkers may fail with zero-sized relocation section in section
> >>> group.
> >>>
> >>>       PR ld/25767
> >>>       * readelf.c (process_section_headers): Warn zero-sized
> >>>       relocation sections.
> >>
> >> Zero-sized sections of whatever kind are perfectly fine in ELF. I
> >> see no reason why one would want to see a warning for such by
> >> default.
> >
> > Linkers older than 2.35 may fail with
> >
> > ./ld: BFD (GNU Binutils) 2.34.50.20200402 assertion fail
> > /export/gnu/import/git/gitlab/x86-binutils/bfd/elf.c:3652
>
> Which is a bug, yes, but imo no reason to clutter readelf output.
>

When such linker failures happen, this is useful to identify why.

-- 
H.J.

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Jan Beulich]

On 14.04.2020 15:30, H.J. Lu wrote:
> On Tue, Apr 14, 2020 at 6:25 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 14.04.2020 15:03, H.J. Lu wrote:
>>> On Tue, Apr 14, 2020 at 5:42 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 14.04.2020 14:25, H.J. Lu via Binutils wrote:
>>>>> Older linkers may fail with zero-sized relocation section in section
>>>>> group.
>>>>>
>>>>>       PR ld/25767
>>>>>       * readelf.c (process_section_headers): Warn zero-sized
>>>>>       relocation sections.
>>>>
>>>> Zero-sized sections of whatever kind are perfectly fine in ELF. I
>>>> see no reason why one would want to see a warning for such by
>>>> default.
>>>
>>> Linkers older than 2.35 may fail with
>>>
>>> ./ld: BFD (GNU Binutils) 2.34.50.20200402 assertion fail
>>> /export/gnu/import/git/gitlab/x86-binutils/bfd/elf.c:3652
>>
>> Which is a bug, yes, but imo no reason to clutter readelf output.
>>
> 
> When such linker failures happen, this is useful to identify why.

And hence an option to enable such a warning would seem reasonable,
but not making this default behavior.

Jan

Re: [PATCH] readelf: Warn zero-sized relocation sections

[H.J. Lu]

On Tue, Apr 14, 2020 at 6:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.04.2020 15:30, H.J. Lu wrote:
> > On Tue, Apr 14, 2020 at 6:25 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 14.04.2020 15:03, H.J. Lu wrote:
> >>> On Tue, Apr 14, 2020 at 5:42 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 14.04.2020 14:25, H.J. Lu via Binutils wrote:
> >>>>> Older linkers may fail with zero-sized relocation section in section
> >>>>> group.
> >>>>>
> >>>>>       PR ld/25767
> >>>>>       * readelf.c (process_section_headers): Warn zero-sized
> >>>>>       relocation sections.
> >>>>
> >>>> Zero-sized sections of whatever kind are perfectly fine in ELF. I
> >>>> see no reason why one would want to see a warning for such by
> >>>> default.
> >>>
> >>> Linkers older than 2.35 may fail with
> >>>
> >>> ./ld: BFD (GNU Binutils) 2.34.50.20200402 assertion fail
> >>> /export/gnu/import/git/gitlab/x86-binutils/bfd/elf.c:3652
> >>
> >> Which is a bug, yes, but imo no reason to clutter readelf output.
> >>
> >
> > When such linker failures happen, this is useful to identify why.
>
> And hence an option to enable such a warning would seem reasonable,
> but not making this default behavior.
>

The idea is that compilers shouldn't generate such objects so that
readelf won't output this warning and older linkers work OK.  Otherwise,
users will have a very hard time to figure out why linker fails.

-- 
H.J.

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Nick Clifton]

Hi Guys,

  I am of a mind to allow this patch, given that one of readelf's
  main functions is to diagnose problems with ELF files.  If these
  warnings really are a problem for users however then we could
  provide an option to filter them out.

  That said, the patch as it currently exists causes lots of new
  failures in the linker testsuite for various targets.  Especially
  powerpc64-linux.  There may well be others, eg alpha-linux-gnu, 
  but my regression tester is still running.  The failures are from
  messages like this:

    readelf: Warning: Section '.rela.plt': zero-sized relocation section

  and

    readelf: Warning: Section '.rela.dyn': zero-sized relocation section

  So H.J. if you want to persist with this feature you will need to
  either fix the tests or the check in readelf.

Cheers
  Nick

Re: [PATCH] readelf: Warn zero-sized relocation sections

[H.J. Lu]

On Fri, Apr 17, 2020 at 8:46 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi Guys,
>
>   I am of a mind to allow this patch, given that one of readelf's
>   main functions is to diagnose problems with ELF files.  If these
>   warnings really are a problem for users however then we could
>   provide an option to filter them out.
>
>   That said, the patch as it currently exists causes lots of new
>   failures in the linker testsuite for various targets.  Especially
>   powerpc64-linux.  There may well be others, eg alpha-linux-gnu,
>   but my regression tester is still running.  The failures are from
>   messages like this:
>
>     readelf: Warning: Section '.rela.plt': zero-sized relocation section
>
>   and
>
>     readelf: Warning: Section '.rela.dyn': zero-sized relocation section
>
>   So H.J. if you want to persist with this feature you will need to
>   either fix the tests or the check in readelf.

Hi Alan,

Any ideas why ppc64 backend generates zero-sized relocation section:

tmpdir/libcomm-data.so:
...
  [ 5] .rela.dyn         RELA            0000000000000160 000160
000000 18   A  3   0  8
...

Is this required by psABI?

-- 
H.J.

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Alan Modra]

On Fri, Apr 17, 2020 at 10:57:04AM -0700, H.J. Lu wrote:
> Any ideas why ppc64 backend generates zero-sized relocation section:
> 
> tmpdir/libcomm-data.so:
> ...
>   [ 5] .rela.dyn         RELA            0000000000000160 000160
> 000000 18   A  3   0  8
> ...
> 
> Is this required by psABI?

No, it just looks to be something that isn't tidied.  The .rela.dyn
output section is created due to .rela.branch_lt being created as a
dynamic section, and not being sized early enough for
size_dynamic_sections to set SEC_EXCLUDE.  I'll see if I can make it
go away.

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH] elf: Strip zero-sized dynamic sections

[H.J. Lu]

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

On Fri, Apr 17, 2020 at 5:26 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Fri, Apr 17, 2020 at 10:57:04AM -0700, H.J. Lu wrote:
> > Any ideas why ppc64 backend generates zero-sized relocation section:
> >
> > tmpdir/libcomm-data.so:
> > ...
> >   [ 5] .rela.dyn         RELA            0000000000000160 000160
> > 000000 18   A  3   0  8
> > ...
> >
> > Is this required by psABI?
>
> No, it just looks to be something that isn't tidied.  The .rela.dyn
> output section is created due to .rela.branch_lt being created as a
> dynamic section, and not being sized early enough for
> size_dynamic_sections to set SEC_EXCLUDE.  I'll see if I can make it
> go away.
>

I opened:

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

Here is a patch.  OK for master?

-- 
H.J.

[-- Attachment #2: 0001-elf-Strip-zero-sized-dynamic-sections.patch --]
[-- Type: application/x-patch, Size: 14980 bytes --]

Re: [PATCH] elf: Strip zero-sized dynamic sections

[Alan Modra]

On Sat, Apr 18, 2020 at 09:51:53AM -0700, H.J. Lu wrote:
> On Fri, Apr 17, 2020 at 5:26 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Fri, Apr 17, 2020 at 10:57:04AM -0700, H.J. Lu wrote:
> > > Any ideas why ppc64 backend generates zero-sized relocation section:
> > >
> > > tmpdir/libcomm-data.so:
> > > ...
> > >   [ 5] .rela.dyn         RELA            0000000000000160 000160
> > > 000000 18   A  3   0  8
> > > ...
> > >
> > > Is this required by psABI?
> >
> > No, it just looks to be something that isn't tidied.  The .rela.dyn
> > output section is created due to .rela.branch_lt being created as a
> > dynamic section, and not being sized early enough for
> > size_dynamic_sections to set SEC_EXCLUDE.  I'll see if I can make it
> > go away.
> >
> 
> I opened:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=25849
> 
> Here is a patch.  OK for master?

I don't think readelf should warn until all ELF targets are tidied.

If someone sees a warning on alpha-linux, say, they have no way to
avoid producing object files that give a warning.

-- 
Alan Modra
Australia Development Lab, IBM

V2 [PATCH] elf: Strip zero-sized dynamic sections

[H.J. Lu]

On Mon, Apr 20, 2020 at 07:05:47PM +0930, Alan Modra wrote:
> 
> If someone sees a warning on alpha-linux, say, they have no way to
> avoid producing object files that give a warning.
> 

Here is the updated patch to remove zero-sized dynamic sections.

OK for master?

H.J.
---
ELF size_dynamic_sections is called by the ELF backend linker after all
the linker input files have been seen but before the section sizes have
been set.  After the sections sizes have been set, target-specific,
global optimizations may make some dynamic sections zero-sized if they
are no longer needed.

Add ELF strip_zero_sized_dynamic_sections so that ELF backend linker can
strip zero-sized dynamic sections after the sections sizes have been set.

bfd/

	PR ld/25849
	* elf-bfd.h (elf_backend_data): Add
	elf_backend_strip_zero_sized_dynamic_sections.
	(_bfd_elf_strip_zero_sized_dynamic_sections): New prototype.
	* elf64-alpha.c (elf_backend_strip_zero_sized_dynamic_sections):
	New macro.
	* elflink.c (_bfd_elf_strip_zero_sized_dynamic_sections): New
	function.
	* elfxx-target.h (elf_backend_strip_zero_sized_dynamic_sections):
	New macro.
	(elfNN_bed): Add elf_backend_strip_zero_sized_dynamic_sections.

ld/

	PR ld/25849
	* ldelfgen.c (ldelf_map_segments): Call
	elf_backend_strip_zero_sized_dynamic_sections.
	* testsuite/ld-alpha/tlsbinr.rd: Updated.
---
 bfd/elf-bfd.h                    |  8 +++
 bfd/elf64-alpha.c                |  3 +
 bfd/elflink.c                    | 98 +++++++++++++++++++++++++++++++
 bfd/elfxx-target.h               |  4 ++
 ld/ldelfgen.c                    | 13 +++++
 ld/testsuite/ld-alpha/tlsbinr.rd | 99 +++++++++++++++-----------------
 6 files changed, 173 insertions(+), 52 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index b08502cd1c..3ae98425e8 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1083,6 +1083,12 @@ struct elf_backend_data
   bfd_boolean (*elf_backend_size_dynamic_sections)
     (bfd *output_bfd, struct bfd_link_info *info);
 
+  /* The STRIP_ZERO_SIZED_DYNAMIC_SECTIONS function is called by the
+     ELF backend linker to strip zero-sized dynamic sections after
+     the section sizes have been set.  */
+  bfd_boolean (*elf_backend_strip_zero_sized_dynamic_sections)
+    (struct bfd_link_info *info);
+
   /* Set TEXT_INDEX_SECTION and DATA_INDEX_SECTION, the output sections
      we keep to use as a base for relocs and symbols.  */
   void (*elf_backend_init_index_section)
@@ -2520,6 +2526,8 @@ extern bfd_boolean bfd_elf_link_add_symbols
   (bfd *, struct bfd_link_info *);
 extern bfd_boolean _bfd_elf_add_dynamic_entry
   (struct bfd_link_info *, bfd_vma, bfd_vma);
+extern bfd_boolean _bfd_elf_strip_zero_sized_dynamic_sections
+  (struct bfd_link_info *);
 extern int bfd_elf_add_dt_needed_tag
   (bfd *, struct bfd_link_info *);
 extern bfd_boolean _bfd_elf_link_check_relocs
diff --git a/bfd/elf64-alpha.c b/bfd/elf64-alpha.c
index ca15944e60..edd16e241b 100644
--- a/bfd/elf64-alpha.c
+++ b/bfd/elf64-alpha.c
@@ -5525,6 +5525,9 @@ static const struct elf_size_info alpha_elf_size_info =
 #define elf_backend_special_sections \
   elf64_alpha_special_sections
 
+#define elf_backend_strip_zero_sized_dynamic_sections \
+  _bfd_elf_strip_zero_sized_dynamic_sections
+
 /* A few constants that determine how the .plt section is set up.  */
 #define elf_backend_want_got_plt 0
 #define elf_backend_plt_readonly 0
diff --git a/bfd/elflink.c b/bfd/elflink.c
index eb6b3eeca5..6624864bf5 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -3501,6 +3501,104 @@ _bfd_elf_add_dynamic_entry (struct bfd_link_info *info,
   return TRUE;
 }
 
+/* Strip zero-sized dynamic sections.  */
+
+bfd_boolean
+_bfd_elf_strip_zero_sized_dynamic_sections (struct bfd_link_info *info)
+{
+  struct elf_link_hash_table *hash_table;
+  const struct elf_backend_data *bed;
+  asection *s, *sdynamic, **pp;
+  asection *rela_dyn, *rel_dyn;
+  Elf_Internal_Dyn dyn;
+  bfd_byte *extdyn, *next;
+  void (*swap_dyn_in) (bfd *, const void *, Elf_Internal_Dyn *);
+  bfd_boolean strip_zero_sized;
+  bfd_boolean strip_zero_sized_plt;
+
+  if (bfd_link_relocatable (info))
+    return TRUE;
+
+  hash_table = elf_hash_table (info);
+  if (!is_elf_hash_table (hash_table))
+    return FALSE;
+
+  if (!hash_table->dynobj)
+    return TRUE;
+
+  sdynamic= bfd_get_linker_section (hash_table->dynobj, ".dynamic");
+  if (!sdynamic)
+    return TRUE;
+
+  bed = get_elf_backend_data (hash_table->dynobj);
+  swap_dyn_in = bed->s->swap_dyn_in;
+
+  strip_zero_sized = FALSE;
+  strip_zero_sized_plt = FALSE;
+
+  /* Strip zero-sized dynamic sections.  */
+  rela_dyn = bfd_get_section_by_name (info->output_bfd, ".rela.dyn");
+  rel_dyn = bfd_get_section_by_name (info->output_bfd, ".rel.dyn");
+  for (pp = &info->output_bfd->sections; (s = *pp) != NULL;)
+    if (s->size == 0
+	&& (s == rela_dyn
+	    || s == rel_dyn
+	    || s == hash_table->srelplt->output_section
+	    || s == hash_table->splt->output_section))
+      {
+	*pp = s->next;
+	info->output_bfd->section_count--;
+	strip_zero_sized = TRUE;
+	if (s == rela_dyn)
+	  s = rela_dyn;
+	if (s == rel_dyn)
+	  s = rel_dyn;
+	else if (s == hash_table->splt->output_section)
+	  {
+	    s = hash_table->splt;
+	    strip_zero_sized_plt = TRUE;
+	  }
+	else
+	  s = hash_table->srelplt;
+	s->flags |= SEC_EXCLUDE;
+	s->output_section = bfd_abs_section_ptr;
+      }
+    else
+      pp = &s->next;
+
+  if (strip_zero_sized_plt)
+    for (extdyn = sdynamic->contents;
+	 extdyn < sdynamic->contents + sdynamic->size;
+	 extdyn = next)
+      {
+	next = extdyn + bed->s->sizeof_dyn;
+	swap_dyn_in (hash_table->dynobj, extdyn, &dyn);
+	switch (dyn.d_tag)
+	  {
+	  default:
+	    break;
+	  case DT_JMPREL:
+	  case DT_PLTRELSZ:
+	  case DT_PLTREL:
+	    /* Strip DT_PLTRELSZ, DT_JMPREL and DT_PLTREL entries if
+	       the procedure linkage table (the .plt section) has been
+	       removed.  */
+	    memmove (extdyn, next,
+		     sdynamic->size - (next - sdynamic->contents));
+	    next = extdyn;
+	  }
+      }
+
+  if (strip_zero_sized)
+    {
+      /* Regenerate program headers.  */
+      elf_seg_map (info->output_bfd) = NULL;
+      return _bfd_elf_map_sections_to_segments (info->output_bfd, info);
+    }
+
+  return TRUE;
+}
+
 /* Add a DT_NEEDED entry for this dynamic object.  Returns -1 on error,
    1 if a DT_NEEDED tag already exists, and 0 on success.  */
 
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index 1ae17f45ee..b41fcde0ca 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -485,6 +485,9 @@
 #ifndef elf_backend_size_dynamic_sections
 #define elf_backend_size_dynamic_sections 0
 #endif
+#ifndef elf_backend_strip_zero_sized_dynamic_sections
+#define elf_backend_strip_zero_sized_dynamic_sections 0
+#endif
 #ifndef elf_backend_init_index_section
 #define elf_backend_init_index_section _bfd_void_bfd_link
 #endif
@@ -831,6 +834,7 @@ static struct elf_backend_data elfNN_bed =
   elf_backend_adjust_dynamic_symbol,
   elf_backend_always_size_sections,
   elf_backend_size_dynamic_sections,
+  elf_backend_strip_zero_sized_dynamic_sections,
   elf_backend_init_index_section,
   elf_backend_relocate_section,
   elf_backend_finish_dynamic_symbol,
diff --git a/ld/ldelfgen.c b/ld/ldelfgen.c
index 21739b19ca..c0568f169f 100644
--- a/ld/ldelfgen.c
+++ b/ld/ldelfgen.c
@@ -73,6 +73,19 @@ ldelf_map_segments (bfd_boolean need_layout)
 
   if (tries == 0)
     einfo (_("%F%P: looping in map_segments"));
+
+  if (link_info.output_bfd->xvec->flavour == bfd_target_elf_flavour
+      && lang_phdr_list == NULL)
+    {
+      /* If we don't have user supplied phdrs, strip zero-sized dynamic
+	 sections and regenerate program headers.  */
+      const struct elf_backend_data *bed
+	= get_elf_backend_data (link_info.output_bfd);
+      if (bed->elf_backend_strip_zero_sized_dynamic_sections
+	  && !bed->elf_backend_strip_zero_sized_dynamic_sections
+		(&link_info))
+	  einfo (_("%F%P: failed to strip zero-sized dynamic sections"));
+    }
 }
 
 /* We want to emit CTF early if and only if we are not targetting ELF with this
diff --git a/ld/testsuite/ld-alpha/tlsbinr.rd b/ld/testsuite/ld-alpha/tlsbinr.rd
index d6a70f9102..ea51686640 100644
--- a/ld/testsuite/ld-alpha/tlsbinr.rd
+++ b/ld/testsuite/ld-alpha/tlsbinr.rd
@@ -16,8 +16,6 @@ Section Headers:
  +\[[ 0-9]+\] \.dynsym +.*
  +\[[ 0-9]+\] \.dynstr +.*
  +\[[ 0-9]+\] \.rela\.dyn +.*
- +\[[ 0-9]+\] \.rela\.plt +.*
- +\[[ 0-9]+\] \.plt +PROGBITS +[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 0+ +AX +0 +0 +16
  +\[[ 0-9]+\] \.text +PROGBITS +[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 0+ +AX +0 +0 4096
  +\[[ 0-9]+\] \.eh_frame +PROGBITS +[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 +A +0 +0 +8
  +\[[ 0-9]+\] \.tdata +PROGBITS +[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 0+ WAT +0 +0 +4
@@ -70,59 +68,56 @@ Symbol table '\.symtab' contains [0-9]+ entries:
 [0-9 ]+: [0-9a-f]+ +0 +SECTION +LOCAL +DEFAULT +9 
 [0-9 ]+: [0-9a-f]+ +0 +SECTION +LOCAL +DEFAULT +10 
 [0-9 ]+: [0-9a-f]+ +0 +SECTION +LOCAL +DEFAULT +11 
-[0-9 ]+: [0-9a-f]+ +0 +SECTION +LOCAL +DEFAULT +12 
-[0-9 ]+: [0-9a-f]+ +0 +SECTION +LOCAL +DEFAULT +13 
 .* FILE +LOCAL +DEFAULT +ABS .*
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl1
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl2
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl3
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl4
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl5
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl6
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl7
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +10 sl8
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl1
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl2
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl3
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl4
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl5
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl6
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl7
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +8 sl8
 .* FILE +LOCAL +DEFAULT +ABS .*
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl1
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl2
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl3
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl4
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl5
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl6
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl7
-[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +11 bl8
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl1
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl2
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl3
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl4
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl5
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl6
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl7
+[0-9 ]+: [0-9a-f]+ +0 +TLS +LOCAL +DEFAULT +9 bl8
 .* FILE +LOCAL +DEFAULT +ABS .*
-[0-9 ]+: [0-9a-f]+ +0 +OBJECT +LOCAL +DEFAULT +12 _DYNAMIC
-[0-9 ]+: [0-9a-f]+ +0 +OBJECT +LOCAL +DEFAULT +7 _PROCEDURE_LINKAGE_TABLE_
-[0-9 ]+: [0-9a-f]+ +0 +OBJECT +LOCAL +DEFAULT +13 _GLOBAL_OFFSET_TABLE_
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg8
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg8
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg6
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg3
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg3
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh3
+[0-9 ]+: [0-9a-f]+ +0 +OBJECT +LOCAL +DEFAULT +10 _DYNAMIC
+[0-9 ]+: [0-9a-f]+ +0 +OBJECT +LOCAL +DEFAULT +11 _GLOBAL_OFFSET_TABLE_
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg8
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg8
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg6
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg3
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg3
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh3
 [0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +UND sG2
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg4
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg5
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg5
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg4
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg5
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg5
 [0-9 ]+: [0-9a-f]+ +0 +FUNC +GLOBAL +DEFAULT +UND __tls_get_addr
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh7
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh8
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg1
-[0-9 ]+: [0-9a-f]+ +52 +FUNC +GLOBAL +DEFAULT +8 _start
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh4
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg7
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh5
-[0-9 ]+: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +13 __bss_start
-[0-9 ]+: [0-9a-f]+ +136 +FUNC +GLOBAL +DEFAULT +\[STD GPLOAD\] +8 fn2
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg2
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh7
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh8
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg1
+[0-9 ]+: [0-9a-f]+ +52 +FUNC +GLOBAL +DEFAULT +6 _start
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh4
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg7
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh5
+[0-9 ]+: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +11 __bss_start
+[0-9 ]+: [0-9a-f]+ +136 +FUNC +GLOBAL +DEFAULT +\[STD GPLOAD\] +6 fn2
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg2
 [0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +UND sG1
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh1
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg6
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +10 sg7
-[0-9 ]+: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +13 _edata
-[0-9 ]+: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +13 _end
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh2
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +10 sh6
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg2
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg1
-[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +11 bg4
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh1
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg6
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +8 sg7
+[0-9 ]+: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +11 _edata
+[0-9 ]+: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +11 _end
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh2
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +HIDDEN +8 sh6
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg2
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg1
+[0-9 ]+: [0-9a-f]+ +0 +TLS +GLOBAL +DEFAULT +9 bg4
-- 
2.25.2

Re: V2 [PATCH] elf: Strip zero-sized dynamic sections

[Nick Clifton]

Hi H.J.

> OK for master?

Approved - please apply.

> bfd/
> 
> 	PR ld/25849
> 	* elf-bfd.h (elf_backend_data): Add
> 	elf_backend_strip_zero_sized_dynamic_sections.
> 	(_bfd_elf_strip_zero_sized_dynamic_sections): New prototype.
> 	* elf64-alpha.c (elf_backend_strip_zero_sized_dynamic_sections):
> 	New macro.
> 	* elflink.c (_bfd_elf_strip_zero_sized_dynamic_sections): New
> 	function.
> 	* elfxx-target.h (elf_backend_strip_zero_sized_dynamic_sections):
> 	New macro.
> 	(elfNN_bed): Add elf_backend_strip_zero_sized_dynamic_sections.
> 
> ld/
> 
> 	PR ld/25849
> 	* ldelfgen.c (ldelf_map_segments): Call
> 	elf_backend_strip_zero_sized_dynamic_sections.
> 	* testsuite/ld-alpha/tlsbinr.rd: Updated.
 
Cheers
  Nick

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Jan Beulich]

On 17.04.2020 17:46, Nick Clifton wrote:
>   I am of a mind to allow this patch, given that one of readelf's
>   main functions is to diagnose problems with ELF files.  If these
>   warnings really are a problem for users however then we could
>   provide an option to filter them out.

While I can see the "diagnose problems" aspect, zero-sized relocation
sections aren't something the ELF standard disallows. So there's no
problem with the ELF files in this case, just a _possible_ problem
with consumers thereof.

>   That said, the patch as it currently exists causes lots of new
>   failures in the linker testsuite for various targets.  Especially
>   powerpc64-linux.  There may well be others, eg alpha-linux-gnu, 
>   but my regression tester is still running.  The failures are from
>   messages like this:
> 
>     readelf: Warning: Section '.rela.plt': zero-sized relocation section
> 
>   and
> 
>     readelf: Warning: Section '.rela.dyn': zero-sized relocation section

Aren't these a good enough indication that zero-sized relocation
sections aren't this unusual, and hence a default-on warning is
going too far?

Jan

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Nick Clifton]

Hi Jan,

>>   I am of a mind to allow this patch, given that one of readelf's

> While I can see the "diagnose problems" aspect, zero-sized relocation
> sections aren't something the ELF standard disallows. So there's no
> problem with the ELF files in this case, just a _possible_ problem
> with consumers thereof.

True - but in this case we are talking about relocation sections, and
what possible use is an empty relocation section ?  Its presence is
legal sure, but it can also be an indication that the tool that created
the binary is not quite doing its job properly.

I personally like the idea of giving users as much information as possible
and allowing them to decide if they want it.  After all it is relatively
easy to remove any warning out that is undesired via command line tools.


>>     readelf: Warning: Section '.rela.dyn': zero-sized relocation section
> 
> Aren't these a good enough indication that zero-sized relocation
> sections aren't this unusual, and hence a default-on warning is
> going too far?

Actually no - these failures have now prompted H.J. and Alan to have a look
at the code producing these empty sections and see about fixing them.

Cheers
  Nick

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Jan Beulich]

On 20.04.2020 12:28, Nick Clifton wrote:
>>>   I am of a mind to allow this patch, given that one of readelf's
> 
>> While I can see the "diagnose problems" aspect, zero-sized relocation
>> sections aren't something the ELF standard disallows. So there's no
>> problem with the ELF files in this case, just a _possible_ problem
>> with consumers thereof.
> 
> True - but in this case we are talking about relocation sections, and
> what possible use is an empty relocation section ?  Its presence is
> legal sure, but it can also be an indication that the tool that created
> the binary is not quite doing its job properly.

It is still legitimate for a tool to create _all_ sections it may
possibly put data into. While now causing headaches, that ppc
behavior mentioned further down actually demonstrates this.

> I personally like the idea of giving users as much information as possible
> and allowing them to decide if they want it.  After all it is relatively
> easy to remove any warning out that is undesired via command line tools.

Is it, even when considering the messages getting translated to
other languages? What would my pattern be to filter them out
regardless of language?

Anyway, I merely meant to voice my opinion. You're the maintainer,
so you've got to judge.

>>>     readelf: Warning: Section '.rela.dyn': zero-sized relocation section
>>
>> Aren't these a good enough indication that zero-sized relocation
>> sections aren't this unusual, and hence a default-on warning is
>> going too far?
> 
> Actually no - these failures have now prompted H.J. and Alan to have a look
> at the code producing these empty sections and see about fixing them.

Yes, I've seen this. Not emitting these sections is certainly a
good thing to do, but aiui current behavior is still legitimate
(and hence I'd call this clean up, not "fixing").

Jan

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Hans-Peter Nilsson]

On Mon, 20 Apr 2020, Jan Beulich wrote:

> On 20.04.2020 12:28, Nick Clifton wrote:
> >>>   I am of a mind to allow this patch, given that one of readelf's
> >
> >> While I can see the "diagnose problems" aspect, zero-sized relocation
> >> sections aren't something the ELF standard disallows. So there's no
> >> problem with the ELF files in this case, just a _possible_ problem
> >> with consumers thereof.
> >
> > True - but in this case we are talking about relocation sections, and
> > what possible use is an empty relocation section ?  Its presence is
> > legal sure, but it can also be an indication that the tool that created
> > the binary is not quite doing its job properly.
>
> It is still legitimate for a tool to create _all_ sections it may
> possibly put data into. While now causing headaches, that ppc
> behavior mentioned further down actually demonstrates this.
>
> > I personally like the idea of giving users as much information as possible
> > and allowing them to decide if they want it.  After all it is relatively
> > easy to remove any warning out that is undesired via command line tools.
>
> Is it, even when considering the messages getting translated to
> other languages? What would my pattern be to filter them out
> regardless of language?
>
> Anyway, I merely meant to voice my opinion. You're the maintainer,
> so you've got to judge.

Time to chime in?

I'd like to voice my support for *not* have readelf emit an
empty-section warning by default (whether a relocation section
or not).  Nonetheless it would be very useful as a separate,
non-default, option, say for generally fishy contents.

If it's added anyway, please also add an option to not emit the
warning.

> >>>     readelf: Warning: Section '.rela.dyn': zero-sized relocation section
> >>
> >> Aren't these a good enough indication that zero-sized relocation
> >> sections aren't this unusual, and hence a default-on warning is
> >> going too far?
> >
> > Actually no - these failures have now prompted H.J. and Alan to have a look
> > at the code producing these empty sections and see about fixing them.
>
> Yes, I've seen this. Not emitting these sections is certainly a
> good thing to do, but aiui current behavior is still legitimate
> (and hence I'd call this clean up, not "fixing").
>
> Jan

brgds, H-P

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Nick Clifton]

Hi Guys,

> Time to chime in?

Go ahead... :-)

> I'd like to voice my support for *not* have readelf emit an
> empty-section warning by default (whether a relocation section
> or not).  Nonetheless it would be very useful as a separate,
> non-default, option, say for generally fishy contents.
> 
> If it's added anyway, please also add an option to not emit the
> warning.

I have been thinking about this.  I was wondering what the best form 
of such an option would be.  I see three different possibilities:

  * Just warn about everything, and do not have an option.

  * Have a --enable-warnings option which is off by default
    and which disables *all* warnings unless enabled.
    This would mean changing the current behaviour of readelf...

  * Have a --enable-extra-warnings option which enables the zero
    sized section test but no others.  (For now - in the future
    I would envision us adding more tests).  For this path I
    think that it would also make sense to warn about any zero
    sized section that does not have the SHT_NOBITS type.

Or maybe:

  * Have a --enable-lint (or some such name) which enables testing
    for conformance to the ELF standard, (plus strangeness things
    like zero sized sections).  This would count as a command option
    in its own right, so that unless another option is provided
    then only the warning/error messages would be displayed.

What do people think ?

Cheers
  Nick

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Jan Beulich]

On 21.04.2020 12:01, Nick Clifton wrote:
> Hi Guys,
> 
>> Time to chime in?
> 
> Go ahead... :-)
> 
>> I'd like to voice my support for *not* have readelf emit an
>> empty-section warning by default (whether a relocation section
>> or not).  Nonetheless it would be very useful as a separate,
>> non-default, option, say for generally fishy contents.
>>
>> If it's added anyway, please also add an option to not emit the
>> warning.
> 
> I have been thinking about this.  I was wondering what the best form 
> of such an option would be.  I see three different possibilities:
> 
>   * Just warn about everything, and do not have an option.
> 
>   * Have a --enable-warnings option which is off by default
>     and which disables *all* warnings unless enabled.
>     This would mean changing the current behaviour of readelf...
> 
>   * Have a --enable-extra-warnings option which enables the zero
>     sized section test but no others.  (For now - in the future
>     I would envision us adding more tests).  For this path I
>     think that it would also make sense to warn about any zero
>     sized section that does not have the SHT_NOBITS type.

Of the three I'd vote for the last, but I don't think all zero
sized sections should be warned about. An assembly file
contributing to just .rodata still has empty .text and .data
sections in the resulting object. In this regard I also don't
see why empty NOBITS sections would be any special.

> Or maybe:
> 
>   * Have a --enable-lint (or some such name) which enables testing
>     for conformance to the ELF standard, (plus strangeness things
>     like zero sized sections).  This would count as a command option
>     in its own right, so that unless another option is provided
>     then only the warning/error messages would be displayed.

Perhaps this one's best, albeit - as per above - with there
being a definition of "strangeness" that wouldn't cause warnings
for ELF files coming out of well known (and assumed to be well
behaved) tools (e.g. including gas, gld, gold etc). I've even
want to lobby for a two stage control here - at a lower level
only ELF conformance violations would be warned about, while at
a higher level strangeness would also be complained about.
Eventually there may want to be multiple levels of strangeness.

Jan

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Hans-Peter Nilsson]

On Tue, 21 Apr 2020, Jan Beulich wrote:
> On 21.04.2020 12:01, Nick Clifton wrote:
> > I have been thinking about this.  I was wondering what the best form
> > of such an option would be.  I see three different possibilities:
> >
> >   * Just warn about everything, and do not have an option.

(Not this one.)

> >   * Have a --enable-warnings option which is off by default
> >     and which disables *all* warnings unless enabled.
> >     This would mean changing the current behaviour of readelf...

Hm; regarding "--enable-*".  I don't mind the names, but you're
not going to make these *configure-time* options, right?

> >
> >   * Have a --enable-extra-warnings option which enables the zero
> >     sized section test but no others.  (For now - in the future
> >     I would envision us adding more tests).  For this path I
> >     think that it would also make sense to warn about any zero
> >     sized section that does not have the SHT_NOBITS type.
>
> Of the three I'd vote for the last,
[...]

> > Or maybe:
> >
> >   * Have a --enable-lint (or some such name) which enables testing
> >     for conformance to the ELF standard, (plus strangeness things
> >     like zero sized sections).  This would count as a command option
> >     in its own right, so that unless another option is provided
> >     then only the warning/error messages would be displayed.
>
> Perhaps this one's best, albeit -
[...]

Agree with what Jan wrote (including what I pruned).

brgds, H-P

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Nick Clifton]

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

Hi Guys,

  OK, here is a first draft of a patch to implement a checking mode
  for readelf.  It incorporates H.J.'s zero-size section test, although
  I have extended it to cover any SHT_PROGBITS section as well.  (Mainly
  because it makes writing a test case much easier).

  One thing that the patch does not do, but which maybe it should, is
  disable the normal non-warning output when the checking mode is enabled.
  The effect can be achieved by redirecting stdout to the null device
  of course, but I appreciate that this is not always possible/easy to do.

  Anyway - comments ?

Cheers
  Nick

[-- Attachment #2: enable-checks.patch --]
[-- Type: text/x-patch, Size: 14601 bytes --]

diff --git a/binutils/NEWS b/binutils/NEWS
index 1650a3ac93..47eeeeb511 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -1,5 +1,11 @@
 -*- text -*-
 
+* The readelf tool now has a -L or --enable-checks option which turns on
+  warning messages about possible problems with the file(s) being examined.
+  These checks include things like zero-sized sections, which are allowed by
+  the ELF standard but which nevertheless might be of concern if the user
+  was expecting them to actually contain something.
+
 Changes in 2.34:
 
 * Binutils now supports debuginfod, an HTTP server for distributing
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 0da0492961..ac593c42d6 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -4706,6 +4706,7 @@ readelf [@option{-a}|@option{--all}]
         [@option{-V}|@option{--version-info}]
         [@option{-A}|@option{--arch-specific}]
         [@option{-D}|@option{--use-dynamic}]
+        [@option{-L}|@option{--enable-checks}
         [@option{-x} <number or name>|@option{--hex-dump=}<number or name>]
         [@option{-p} <number or name>|@option{--string-dump=}<number or name>]
         [@option{-R} <number or name>|@option{--relocated-dump=}<number or name>]
@@ -4862,6 +4863,14 @@ symbol table sections.
 When displaying relocations, this option makes @command{readelf}
 display the dynamic relocations rather than the static relocations.
 
+@item -L
+@itemx --enable-checks
+Displays warning messages about possible problems with the file(s)
+being examined.  If used on its own then all of the contents of the
+file(s) will be examined.  If used with one of the dumping options
+then the warning messages will only be produced for the regions being
+displayed.
+
 @item -x <number or name>
 @itemx --hex-dump=<number or name>
 Displays the contents of the indicated section as a hexadecimal bytes.
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index c75059bd93..675b4d016f 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -2008,7 +2008,7 @@ read_and_print_leb128 (unsigned char *        data,
   int status;
   dwarf_vma val = read_leb128 (data, end, is_signed, bytes_read, &status);
   if (status != 0)
-    report_leb_status (status);
+    report_leb_status (status, __FILE__, __LINE__);
   else
     printf ("%s", dwarf_vmatoa (is_signed ? "d" : "u", val));
 }
diff --git a/binutils/dwarf.h b/binutils/dwarf.h
index 2249750f87..27f8a51521 100644
--- a/binutils/dwarf.h
+++ b/binutils/dwarf.h
@@ -263,12 +263,12 @@ extern unsigned char * get_build_id (void *);
 #endif
 
 static inline void
-report_leb_status (int status)
+report_leb_status (int status, const char *file, unsigned long lnum)
 {
   if ((status & 1) != 0)
-    error (_("LEB end of data\n"));
+    error (_("%s:%lu: end of data encountered whilst reading LEB\n"), file, lnum);
   else if ((status & 2) != 0)
-    error (_("LEB value too large\n"));
+    error (_("%s:%lu: read LEB value is too large to store in destination variable\n"), file, lnum);
 }
 
 #define SKIP_ULEB(start, end)					\
@@ -277,7 +277,8 @@ report_leb_status (int status)
       unsigned int _len;					\
       read_leb128 (start, end, FALSE, &_len, NULL);		\
       start += _len;						\
-    } while (0)
+    }								\
+  while (0)
 
 #define SKIP_SLEB(start, end)					\
   do								\
@@ -285,7 +286,8 @@ report_leb_status (int status)
       unsigned int _len;					\
       read_leb128 (start, end, TRUE, &_len, NULL);		\
       start += _len;						\
-    } while (0)
+    }								\
+  while (0)
 
 #define READ_ULEB(var, start, end)				\
   do								\
@@ -299,8 +301,9 @@ report_leb_status (int status)
       (var) = _val;						\
       if ((var) != _val)					\
 	_status |= 2;						\
-      report_leb_status (_status);				\
-    } while (0)
+      report_leb_status (_status, __FILE__, __LINE__);		\
+    }								\
+  while (0)
 
 #define READ_SLEB(var, start, end)				\
   do								\
@@ -314,5 +317,6 @@ report_leb_status (int status)
       (var) = _val;						\
       if ((var) != _val)					\
 	_status |= 2;						\
-      report_leb_status (_status);				\
-    } while (0)
+      report_leb_status (_status, __FILE__, __LINE__);		\
+    }								\
+  while (0)
diff --git a/binutils/elfcomm.c b/binutils/elfcomm.c
index 5ec4690e13..119726e853 100644
--- a/binutils/elfcomm.c
+++ b/binutils/elfcomm.c
@@ -34,6 +34,10 @@
 
 extern char *program_name;
 
+/* Allow the following two functions to be overridden if desired.  */
+void error (const char *, ...) __attribute__((weak));
+void warn (const char *, ...) __attribute__((weak));
+
 void
 error (const char *message, ...)
 {
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 8e8ade8fbe..2d3f839081 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -199,7 +199,8 @@ struct dump_list_entry
 
 /* A dynamic array of flags indicating for which sections a dump
    has been requested via command line switches.  */
-struct dump_data {
+struct dump_data
+{
   dump_type *          dump_sects;
   unsigned int         num_dump_sects;
 };
@@ -230,6 +231,8 @@ static bfd_boolean do_ctf = FALSE;
 static bfd_boolean do_arch = FALSE;
 static bfd_boolean do_notes = FALSE;
 static bfd_boolean do_archive_index = FALSE;
+static bfd_boolean do_checks = FALSE;
+static bfd_boolean check_all = FALSE;
 static bfd_boolean is_32bit_elf = FALSE;
 static bfd_boolean decompress_dumps = FALSE;
 
@@ -384,6 +387,25 @@ bfd_vmatoa (char *fmtch, bfd_vma value)
   return ret;
 }
 
+/* A version of the warn() function that is disabled if do_checks is not active.  */
+
+void
+warn (const char *message, ...)
+{
+  va_list args;
+
+  if (!do_checks)
+    return;
+
+  /* Try to keep warning messages in sync with the program's normal output.  */
+  fflush (stdout);
+
+  va_start (args, message);
+  fprintf (stderr, _("%s: Warning: "), program_name);
+  vfprintf (stderr, message, args);
+  va_end (args);
+}
+
 /* Retrieve NMEMB structures, each SIZE bytes long from FILEDATA starting at
    OFFSET + the offset of the current archive member, if we are examining an
    archive.  Put the retrieved data into VAR, if it is not NULL.  Otherwise
@@ -4478,6 +4500,7 @@ static struct option options[] =
   {"relocs",	       no_argument, 0, 'r'},
   {"notes",	       no_argument, 0, 'n'},
   {"dynamic",	       no_argument, 0, 'd'},
+  {"enable-checks",    no_argument, 0, 'L'},
   {"arch-specific",    no_argument, 0, 'A'},
   {"version-info",     no_argument, 0, 'V'},
   {"use-dynamic",      no_argument, 0, 'D'},
@@ -4534,6 +4557,7 @@ usage (FILE * stream)
   -A --arch-specific     Display architecture specific information (if any)\n\
   -c --archive-index     Display the symbol/file index in an archive\n\
   -D --use-dynamic       Use the dynamic section info when displaying symbols\n\
+  -L --enable-checks     Display warning messages for possible problems\n\
   -x --hex-dump=<number|name>\n\
                          Dump the contents of section <number|name> as bytes\n\
   -p --string-dump=<number|name>\n\
@@ -4662,7 +4686,7 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv)
     usage (stderr);
 
   while ((c = getopt_long
-	  (argc, argv, "ADHINR:SVWacdeghi:lnp:rstuvw::x:z", options, NULL)) != EOF)
+	  (argc, argv, "ADHILNR:SVWacdeghi:lnp:rstuvw::x:z", options, NULL)) != EOF)
     {
       switch (c)
 	{
@@ -4736,6 +4760,9 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv)
 	case 'c':
 	  do_archive_index = TRUE;
 	  break;
+	case 'L':
+	  do_checks = TRUE;
+	  break;
 	case 'x':
 	  request_dump (dumpdata, HEX_DUMP);
 	  break;
@@ -4832,7 +4859,18 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv)
       && !do_histogram && !do_debugging && !do_arch && !do_notes
       && !do_section_groups && !do_archive_index
       && !do_dyn_syms)
-    usage (stderr);
+    {
+      if (do_checks)
+	{
+	  check_all = TRUE;
+	  do_dynamic = do_syms = do_reloc = do_unwind = do_sections = TRUE;
+	  do_segments = do_header = do_dump = do_version = TRUE;
+	  do_histogram = do_debugging = do_arch = do_notes = TRUE;
+	  do_section_groups = do_archive_index = do_dyn_syms = TRUE;
+	}
+      else
+	usage (stderr);
+    }
 }
 
 static const char *
@@ -6277,7 +6315,7 @@ process_section_headers (Filedata * filedata)
   while (0)
 
 #define CHECK_ENTSIZE(section, i, type)					\
-  CHECK_ENTSIZE_VALUES (section, i, sizeof (Elf32_External_##type),	    \
+  CHECK_ENTSIZE_VALUES (section, i, sizeof (Elf32_External_##type),	\
 			sizeof (Elf64_External_##type))
 
   for (i = 0, section = filedata->section_headers;
@@ -6286,8 +6324,11 @@ process_section_headers (Filedata * filedata)
     {
       char * name = SECTION_NAME (section);
 
-      if (section->sh_type == SHT_DYNSYM)
+      /* Run some sanity checks on the headers and
+	 possibly fill in some file data as well.  */
+      switch (section->sh_type)
 	{
+	case SHT_DYNSYM:
 	  if (filedata->dynamic_symbols != NULL)
 	    {
 	      error (_("File contains multiple dynamic symbol tables\n"));
@@ -6297,45 +6338,74 @@ process_section_headers (Filedata * filedata)
 	  CHECK_ENTSIZE (section, i, Sym);
 	  filedata->dynamic_symbols
 	    = GET_ELF_SYMBOLS (filedata, section, &filedata->num_dynamic_syms);
-	}
-      else if (section->sh_type == SHT_STRTAB
-	       && streq (name, ".dynstr"))
-	{
-	  if (filedata->dynamic_strings != NULL)
+	  break;
+
+	case SHT_STRTAB:
+	  if (streq (name, ".dynstr"))
 	    {
-	      error (_("File contains multiple dynamic string tables\n"));
-	      continue;
+	      if (filedata->dynamic_strings != NULL)
+		{
+		  error (_("File contains multiple dynamic string tables\n"));
+		  continue;
+		}
+
+	      filedata->dynamic_strings
+		= (char *) get_data (NULL, filedata, section->sh_offset,
+				     1, section->sh_size, _("dynamic strings"));
+	      filedata->dynamic_strings_length
+		= filedata->dynamic_strings == NULL ? 0 : section->sh_size;
 	    }
+	  break;
+
+	case SHT_SYMTAB_SHNDX:
+	  {
+	    elf_section_list * entry = xmalloc (sizeof * entry);
+
+	    entry->hdr = section;
+	    entry->next = filedata->symtab_shndx_list;
+	    filedata->symtab_shndx_list = entry;
+	  }
+	  break;
+
+	case SHT_SYMTAB:
+	  CHECK_ENTSIZE (section, i, Sym);
+	  break;
+
+	case SHT_GROUP:
+	  CHECK_ENTSIZE_VALUES (section, i, GRP_ENTRY_SIZE, GRP_ENTRY_SIZE);
+	  break;
 
-	  filedata->dynamic_strings
-	    = (char *) get_data (NULL, filedata, section->sh_offset,
-				 1, section->sh_size, _("dynamic strings"));
-	  filedata->dynamic_strings_length
-	    = filedata->dynamic_strings == NULL ? 0 : section->sh_size;
-	}
-      else if (section->sh_type == SHT_SYMTAB_SHNDX)
-	{
-	  elf_section_list * entry = xmalloc (sizeof * entry);
-
-	  entry->hdr = section;
-	  entry->next = filedata->symtab_shndx_list;
-	  filedata->symtab_shndx_list = entry;
-	}
-      else if (section->sh_type == SHT_SYMTAB)
-	CHECK_ENTSIZE (section, i, Sym);
-      else if (section->sh_type == SHT_GROUP)
-	CHECK_ENTSIZE_VALUES (section, i, GRP_ENTRY_SIZE, GRP_ENTRY_SIZE);
-      else if (section->sh_type == SHT_REL)
-	CHECK_ENTSIZE (section, i, Rel);
-      else if (section->sh_type == SHT_RELA)
-	CHECK_ENTSIZE (section, i, Rela);
-      else if ((do_debugging || do_debug_info || do_debug_abbrevs
-		|| do_debug_lines || do_debug_pubnames || do_debug_pubtypes
-		|| do_debug_aranges || do_debug_frames || do_debug_macinfo
-		|| do_debug_str || do_debug_loc || do_debug_ranges
-		|| do_debug_addr || do_debug_cu_index || do_debug_links)
-	       && (const_strneq (name, ".debug_")
-                   || const_strneq (name, ".zdebug_")))
+	case SHT_REL:
+	  CHECK_ENTSIZE (section, i, Rel);
+	  if (section->sh_size == 0)
+	    warn (_("Section '%s': zero-sized relocation section\n"), name);
+	  break;
+
+	case SHT_RELA:
+	  CHECK_ENTSIZE (section, i, Rela);
+	  if (section->sh_size == 0)
+	    warn (_("Section '%s': zero-sized relocation section\n"), name);
+	  break;
+
+	case SHT_NOTE:
+	case SHT_PROGBITS:
+	  if (section->sh_size == 0)
+	    /* This is not illegal according to the ELF standard, but
+	       it might be an indication that something is wrong.  */
+	    warn (_("Section '%s': has a size of zero - is this intended ?\n"), name);
+	  break;
+
+	default:
+	  break;
+	}
+
+      if ((do_debugging || do_debug_info || do_debug_abbrevs
+	   || do_debug_lines || do_debug_pubnames || do_debug_pubtypes
+	   || do_debug_aranges || do_debug_frames || do_debug_macinfo
+	   || do_debug_str || do_debug_loc || do_debug_ranges
+	   || do_debug_addr || do_debug_cu_index || do_debug_links)
+	  && (const_strneq (name, ".debug_")
+	      || const_strneq (name, ".zdebug_")))
 	{
           if (name[1] == 'z')
             name += sizeof (".zdebug_") - 1;
@@ -20667,7 +20737,7 @@ process_file (char * file_name)
     }
   else
     {
-      if (do_archive_index)
+      if (do_archive_index && !check_all)
 	error (_("File %s is not an archive so its index cannot be displayed.\n"),
 	       file_name);
 
@@ -20733,9 +20803,14 @@ main (int argc, char ** argv)
   parse_args (& cmdline, argc, argv);
 
   if (optind < (argc - 1))
+    /* When displaying information for more than one file,
+       prefix the information with the file name.  */
     show_name = TRUE;
   else if (optind >= argc)
     {
+      /* Ensure that the warning is always displayed.  */
+      do_checks = TRUE;
+
       warn (_("Nothing to do.\n"));
       usage (stderr);
     }
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index cc78e66ea3..6b385fda2d 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -506,4 +506,15 @@ if {![binutils_assemble $srcdir/$subdir/dwo.s tmpdir/dwo.o]} then {
     readelf_test {--debug-dump=links} $tempfile readelf.k2  {}
 }
 
+if {![binutils_assemble $srcdir/$subdir/zero-sec.s tmpdir/zero-sec.o]} then {
+    unresolved "readelf --enable-checks (failed to assemble zero-sec.s)"
+} else {
+    if ![is_remote host] {
+	set tempfile tmpdir/zero-sec.o
+    } else {
+	set tempfile [remote_download host tmpdir/zero-sec.o]
+    }
+
+    readelf_test {--enable-checks --sections --wide} $tempfile zero-sec.r {}
+}
 
--- /dev/null	2020-04-24 08:11:48.809179026 +0100
+++ binutils/testsuite/binutils-all/zero-sec.r	2020-04-24 12:46:18.979898729 +0100
@@ -0,0 +1,3 @@
+#...
+.*Warning: Section '.*': has a size of zero - is this intended.*
+#...
--- /dev/null	2020-04-24 08:11:48.809179026 +0100
+++ binutils/testsuite/binutils-all/zero-sec.s	2020-04-24 12:28:13.818534149 +0100
@@ -0,0 +1 @@
+	.data

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Hans-Peter Nilsson]

On Fri, 24 Apr 2020, Nick Clifton wrote:

> Hi Guys,
>
>   OK, here is a first draft of a patch to implement a checking mode
>   for readelf.  It incorporates H.J.'s zero-size section test, although
>   I have extended it to cover any SHT_PROGBITS section as well.  (Mainly
>   because it makes writing a test case much easier).
>
>   One thing that the patch does not do, but which maybe it should, is
>   disable the normal non-warning output when the checking mode is enabled.
>   The effect can be achieved by redirecting stdout to the null device
>   of course, but I appreciate that this is not always possible/easy to do.
>
>   Anyway - comments ?

LGTM!

brgds, H-P

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Nick Clifton]

Hi Guys,

  OK - I have checked my patch in, with the addition of an alias for --enable-checks
  called --lint.  I am sure that the will be issues that we need to resolve in the 
  future but at least now we have a start.

Cheers
  Nick

binutils/ChangeLog
2020-04-29  Nick Clifton  <nickc@redhat.com>

	* readelf.c (warn): New function - like elfcomm.c version but only
	produces output if warnings are enabled.
	(struct options): Add --lint and --enable-checks.
	(usage): Add entry for --lint.
	(parse_args): Handle -L.  If checks are enabled but no dumps have
	been selected then enable all dumps.
	(process_section_headers): Replace long if-then-else sequence with
	a switch.  Add warning messages for empty SHT_REL, SHT_RELA and
	SHT_PROGBITS sections.
	(process_file): Do not complain if the file is an archive and lint
	mode has been enabled.
	* elfcomm.c (error): Make the function weak.
	(warn): Likewise.
	* NEWS: Mention the new feature.
	* doc/binutils.texi: Document the new feature.
	* dwarf.h (report_leb_status): Add file name and line number
	parameters.  Include them in the diagnostic output.
	(READ_ULEB): Pass file and line number to report_leb_status.
	(READ_SLEB): Likewise.
	* dwarf.c (read_and_print_leb128): Pass file and line number to
	report_leb_status.
	* testsuite/binutils-all/readelf.exp: Add test of new feature.
	* testsuite/binutils-all/zero-sec.s: New test source file.
	* testsuite/binutils-all/zero-sec.r: Expected output from new
	test.

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Fangrui Song]

On 2020-04-24, Nick Clifton via Binutils wrote:
>Hi Guys,
>
>  OK, here is a first draft of a patch to implement a checking mode
>  for readelf.  It incorporates H.J.'s zero-size section test, although
>  I have extended it to cover any SHT_PROGBITS section as well.  (Mainly
>  because it makes writing a test case much easier).
>
>  One thing that the patch does not do, but which maybe it should, is
>  disable the normal non-warning output when the checking mode is enabled.
>  The effect can be achieved by redirecting stdout to the null device
>  of course, but I appreciate that this is not always possible/easy to do.
>
>  Anyway - comments ?
>
>Cheers
>  Nick

Certain linker script constructs can create empty output sections, e.g.

   .keep : { KEEP(*(.keep)) }

It is convenient to write a general linker script with lots of output
section descriptions but many omitted in some cases. So empty sections
may be common, especially in embedded systems. Not warning by default
seems sensible.

(In lld, there are a few more cases, e.g.

   .bar : { PROVIDE(foo = .) }  # when foo is referenced, .bar is kept by lld but discarded by GNU ld/gold

   .empty : { *(.empty) }  # this is insufficient to keep .empty
   .data : AT(ADDR(.empty)+0x2000) { ... }  # .empty is referenced an expression and thus retained)

Took a rough look at the patch. Looks great! Just one question, -L's
mnemonic is *L*int. Why is the long option --enable-checks, instead of
--lint?

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Nick Clifton]

Hi Fangrui,

> It is convenient to write a general linker script with lots of output
> section descriptions but many omitted in some cases. So empty sections
> may be common, especially in embedded systems. Not warning by default
> seems sensible.

Right - this seems to be the opinion of several other contributors as well.

> Took a rough look at the patch. Looks great! Just one question, -L's
> mnemonic is *L*int. Why is the long option --enable-checks, instead of
> --lint?

Eh - no good reason - and I am happy to change the option name if people
think that --lint is better.  I used -L as it was reminiscent of "lint"
(and "l" was already taken).  But I picked --enable-checks as this seemed
more obvious to me than --lint.  (My guess was that not all users will be
familiar with using the word 'lint' to mean 'check for errors'.  It is a
kind of niche term).

Cheers
  Nick

Re: [PATCH] readelf: Warn zero-sized relocation sections

[H.J. Lu]

On Sun, Apr 26, 2020 at 8:28 AM Nick Clifton via Binutils
<binutils@sourceware.org> wrote:
>
> Hi Fangrui,
>
> > It is convenient to write a general linker script with lots of output
> > section descriptions but many omitted in some cases. So empty sections
> > may be common, especially in embedded systems. Not warning by default
> > seems sensible.
>
> Right - this seems to be the opinion of several other contributors as well.
>
> > Took a rough look at the patch. Looks great! Just one question, -L's
> > mnemonic is *L*int. Why is the long option --enable-checks, instead of
> > --lint?
>
> Eh - no good reason - and I am happy to change the option name if people
> think that --lint is better.  I used -L as it was reminiscent of "lint"
> (and "l" was already taken).  But I picked --enable-checks as this seemed
> more obvious to me than --lint.  (My guess was that not all users will be
> familiar with using the word 'lint' to mean 'check for errors'.  It is a
> kind of niche term).
>

In my opinion, zero-sized non SHT_REL/SHT_RELA sections are very common.
But zero-sized SHT_REL/SHT_RELA section is an indication of not well
implemented tools.

-- 
H.J.

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Jan Beulich]

On 24.04.2020 16:04, Nick Clifton wrote:
> Hi Guys,
> 
>   OK, here is a first draft of a patch to implement a checking mode
>   for readelf.  It incorporates H.J.'s zero-size section test, although
>   I have extended it to cover any SHT_PROGBITS section as well.  (Mainly
>   because it makes writing a test case much easier).
> 
>   One thing that the patch does not do, but which maybe it should, is
>   disable the normal non-warning output when the checking mode is enabled.
>   The effect can be achieved by redirecting stdout to the null device
>   of course, but I appreciate that this is not always possible/easy to do.
> 
>   Anyway - comments ?

Well, as said before, and as now supported by another reply
you've got, I'm unconvinced warning about empty .text or
.data in particular, but perhaps @progbits / @qnobits in
general is helpful behavior. Especially not as long as gas
created such files without there being a way to prevent it
doing so.

Apart from that, besides noticing a few seemingly unrelated
changes, is __attribute__((weak)) portable enough to be used
in binutils? (Sorry, didn't check if there are pre-existing
instances.)

Jan

Re: [PATCH] readelf: Warn zero-sized relocation sections

[Nick Clifton]

Hi Jan,

> Apart from that, besides noticing a few seemingly unrelated
> changes, is __attribute__((weak)) portable enough to be used
> in binutils? (Sorry, didn't check if there are pre-existing
> instances.)

Ah yes - I meant to mention that.  I am not sure that it is used elsewhere.
The only other places where I could find it being used were in testsuite
cases.  Also there was no define of ATTRIBUTE_WEAK in include/ansidecl.h 
which surprised me.  So possible it is not as portable as I hoped.

The change is not unrelated however, as I wanted to override the warn()
function provided by elf-comm.c and instead use one supplied in readelf.c.
Doing this allowed me to avoid changing lots of code in the files in the
binutils/ sub-directory that use warn() normally.

So I am still undecided as to whether using this attribute is the best
thing to do.

Cheers
  Nick