public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Get rid of SEC_ELF_RENAME
@ 2022-12-06  5:03 Alan Modra
  2022-12-06 21:01 ` [PATCH] bfd: Avoid signed overflow for new_size adjustment H.J. Lu
  0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2022-12-06  5:03 UTC (permalink / raw)
  To: binutils

SEC_ELF_RENAME is a flag used to effect section name changes when
compressing/decompressing zlib-gnu debug sections.  This can be
accomplished more directly in one of the objcopy specific bfd
functions.  Renaming for ld input is simplified too.  Ld input object
files always have BFD_DECOMPRESS set.

bfd/
	* compress.c (bfd_convert_section_size): Rename to..
	(bfd_convert_section_setup): ..this.  Handle objcopy renaming
	of compressed/decompressed debug sections.
	* elf.c (_bfd_elf_make_section_from_shdr): Only rename zdebug
	input for linker.
	(elf_fake_sections): Don't handle renaming of debug sections for
	objcopy here.
	* section.c (SEC_ELF_RENAME): Delete.
	* bfd-in2.h: Regenerate.
binutils/
	* objcopy.c (setup_section): Call bfd_convert_section_setup.
	Don't call bfd_convert_section_size.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 24f9305c47c..d983268563d 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -932,10 +932,6 @@ typedef struct bfd_section
      TMS320C54X only.  */
 #define SEC_TIC54X_BLOCK           0x10000000
 
-  /* This section should be renamed.  This is for ELF linker
-     internal use only.  */
-#define SEC_ELF_RENAME             0x10000000
-
   /* Conditionally link this section; do not link if there are no
      references found to any symbol in the section.  This is for TI
      TMS320C54X only.  */
@@ -7982,8 +7978,9 @@ void bfd_update_compression_header
 
 int bfd_get_compression_header_size (bfd *abfd, asection *sec);
 
-bfd_size_type bfd_convert_section_size
-   (bfd *ibfd, asection *isec, bfd *obfd, bfd_size_type size);
+bool bfd_convert_section_setup
+   (bfd *ibfd, asection *isec, bfd *obfd,
+    const char **new_name, bfd_size_type *new_size);
 
 bool bfd_convert_section_contents
    (bfd *ibfd, asection *isec, bfd *obfd,
diff --git a/bfd/compress.c b/bfd/compress.c
index a4e6a8ee7b5..bb55a6ec0ac 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -225,53 +225,89 @@ bfd_get_compression_header_size (bfd *abfd, asection *sec)
 
 /*
 FUNCTION
-	bfd_convert_section_size
+	bfd_convert_section_setup
 
 SYNOPSIS
-	bfd_size_type bfd_convert_section_size
-	  (bfd *ibfd, asection *isec, bfd *obfd, bfd_size_type size);
+	bool bfd_convert_section_setup
+	  (bfd *ibfd, asection *isec, bfd *obfd,
+	   const char **new_name, bfd_size_type *new_size);
 
 DESCRIPTION
-	Convert the size @var{size} of the section @var{isec} in input
-	BFD @var{ibfd} to the section size in output BFD @var{obfd}.
+	Do early setup for objcopy, when copying @var{isec} in input
+	BFD @var{ibfd} to output BFD @var{obfd}.  Returns the name and
+	size of the output section.
 */
 
-bfd_size_type
-bfd_convert_section_size (bfd *ibfd, sec_ptr isec, bfd *obfd,
-			  bfd_size_type size)
+bool
+bfd_convert_section_setup (bfd *ibfd, asection *isec, bfd *obfd,
+			   const char **new_name, bfd_size_type *new_size)
 {
   bfd_size_type hdr_size;
 
+  if ((isec->flags & SEC_DEBUGGING) != 0
+      && (isec->flags & SEC_HAS_CONTENTS) != 0)
+    {
+      const char *name = *new_name;
+
+      if ((ibfd->flags & (BFD_DECOMPRESS | BFD_COMPRESS_GABI)) != 0)
+	{
+	  /* When we decompress or compress with SHF_COMPRESSED,
+	     convert section name from .zdebug_* to .debug_*.  */
+	  if (startswith (name, ".zdebug_"))
+	    {
+	      name = bfd_zdebug_name_to_debug (obfd, name);
+	      if (name == NULL)
+		return false;
+	    }
+	}
+
+      /* PR binutils/18087: Compression does not always make a
+	 section smaller.  So only rename the section when
+	 compression has actually taken place.  If input section
+	 name is .zdebug_*, we should never compress it again.  */
+      else if (isec->compress_status == COMPRESS_SECTION_DONE
+	       && startswith (name, ".debug_"))
+	{
+	  name = bfd_debug_name_to_zdebug (obfd, name);
+	  if (name == NULL)
+	    return false;
+	}
+      *new_name = name;
+    }
+  *new_size = bfd_section_size (isec);
+
   /* Do nothing if either input or output aren't ELF.  */
   if (bfd_get_flavour (ibfd) != bfd_target_elf_flavour
       || bfd_get_flavour (obfd) != bfd_target_elf_flavour)
-    return size;
+    return true;
 
   /* Do nothing if ELF classes of input and output are the same. */
   if (get_elf_backend_data (ibfd)->s->elfclass
       == get_elf_backend_data (obfd)->s->elfclass)
-    return size;
+    return true;
 
   /* Convert GNU property size.  */
   if (startswith (isec->name, NOTE_GNU_PROPERTY_SECTION_NAME))
-    return _bfd_elf_convert_gnu_property_size (ibfd, obfd);
+    {
+      *new_size = _bfd_elf_convert_gnu_property_size (ibfd, obfd);
+      return true;
+    }
 
   /* Do nothing if input file will be decompressed.  */
   if ((ibfd->flags & BFD_DECOMPRESS))
-    return size;
+    return true;
 
   /* Do nothing if the input section isn't a SHF_COMPRESSED section. */
   hdr_size = bfd_get_compression_header_size (ibfd, isec);
   if (hdr_size == 0)
-    return size;
+    return true;
 
   /* Adjust the size of the output SHF_COMPRESSED section.  */
   if (hdr_size == sizeof (Elf32_External_Chdr))
-    return (size - sizeof (Elf32_External_Chdr)
-	    + sizeof (Elf64_External_Chdr));
+    *new_size += sizeof (Elf64_External_Chdr) - sizeof (Elf32_External_Chdr);
   else
-    return (size - sizeof (Elf64_External_Chdr)
-	    + sizeof (Elf32_External_Chdr));
+    *new_size += sizeof (Elf32_External_Chdr) - sizeof (Elf64_External_Chdr);
+  return true;
 }
 
 /*
diff --git a/bfd/elf.c b/bfd/elf.c
index a013f8885c7..61058deaea1 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -1246,30 +1246,16 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
 	      return false;
 	    }
 #endif
-	}
-
-      if (action != nothing)
-	{
-	  if (abfd->is_linker_input)
+	  if (abfd->is_linker_input
+	      && name[1] == 'z')
 	    {
-	      if (name[1] == 'z'
-		  && (action == decompress
-		      || (action == compress
-			  && (abfd->flags & BFD_COMPRESS_GABI) != 0)))
-		{
-		  /* Convert section name from .zdebug_* to .debug_* so
-		     that linker will consider this section as a debug
-		     section.  */
-		  char *new_name = bfd_zdebug_name_to_debug (abfd, name);
-		  if (new_name == NULL)
-		    return false;
-		  bfd_rename_section (newsect, new_name);
-		}
+	      /* Rename section from .zdebug_* to .debug_* so that ld
+		 scripts will see this section as a debug section.  */
+	      char *new_name = bfd_zdebug_name_to_debug (abfd, name);
+	      if (new_name == NULL)
+		return false;
+	      bfd_rename_section (newsect, new_name);
 	    }
-	  else
-	    /* For objdump, don't rename the section.  For objcopy, delay
-	       section rename to elf_fake_sections.  */
-	    newsect->flags |= SEC_ELF_RENAME;
 	}
     }
 
@@ -3181,57 +3167,20 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
 
   this_hdr = &esd->this_hdr;
 
-  if (arg->link_info)
-    {
-      /* ld: compress DWARF debug sections with names: .debug_*.  */
-      if ((arg->link_info->compress_debug & COMPRESS_DEBUG)
-	  && (asect->flags & SEC_DEBUGGING)
-	  && name[1] == 'd'
-	  && name[6] == '_')
-	{
-	  /* Set SEC_ELF_COMPRESS to indicate this section should be
-	     compressed.  */
-	  asect->flags |= SEC_ELF_COMPRESS;
-	  /* If this section will be compressed, delay adding section
-	     name to section name section after it is compressed in
-	     _bfd_elf_assign_file_positions_for_non_load.  */
-	  delay_st_name_p = true;
-	}
-    }
-  else if ((asect->flags & SEC_ELF_RENAME))
-    {
-      /* objcopy: rename output DWARF debug section.  */
-      if ((abfd->flags & (BFD_DECOMPRESS | BFD_COMPRESS_GABI)))
-	{
-	  /* When we decompress or compress with SHF_COMPRESSED,
-	     convert section name from .zdebug_* to .debug_* if
-	     needed.  */
-	  if (name[1] == 'z')
-	    {
-	      char *new_name = bfd_zdebug_name_to_debug (abfd, name);
-	      if (new_name == NULL)
-		{
-		  arg->failed = true;
-		  return;
-		}
-	      name = new_name;
-	    }
-	}
-      else if (asect->compress_status == COMPRESS_SECTION_DONE
-	       && name[1] == 'd')
-	{
-	  /* PR binutils/18087: Compression does not always make a
-	     section smaller.  So only rename the section when
-	     compression has actually taken place.  If input section
-	     name is .zdebug_*, we should never compress it again.  */
-	  char *new_name = bfd_debug_name_to_zdebug (abfd, name);
-	  if (new_name == NULL)
-	    {
-	      arg->failed = true;
-	      return;
-	    }
-	  name = new_name;
-	}
+  /* ld: compress DWARF debug sections with names: .debug_*.  */
+  if (arg->link_info
+      && (arg->link_info->compress_debug & COMPRESS_DEBUG) != 0
+      && (asect->flags & SEC_DEBUGGING)
+      && name[1] == 'd'
+      && name[6] == '_')
+    {
+      /* Set SEC_ELF_COMPRESS to indicate this section should be
+	 compressed.  */
+      asect->flags |= SEC_ELF_COMPRESS;
+      /* If this section will be compressed, delay adding section
+	 name to section name section after it is compressed in
+	 _bfd_elf_assign_file_positions_for_non_load.  */
+      delay_st_name_p = true;
     }
 
   if (delay_st_name_p)
diff --git a/bfd/section.c b/bfd/section.c
index f73e0345e15..30ab6a7d338 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -347,10 +347,6 @@ CODE_FRAGMENT
 .     TMS320C54X only.  *}
 .#define SEC_TIC54X_BLOCK           0x10000000
 .
-.  {* This section should be renamed.  This is for ELF linker
-.     internal use only.  *}
-.#define SEC_ELF_RENAME             0x10000000
-.
 .  {* Conditionally link this section; do not link if there are no
 .     references found to any symbol in the section.  This is for TI
 .     TMS320C54X only.  *}
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 6814e20a2fc..19dbb50d3e6 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -4118,6 +4118,13 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
       flags &= ~clr;
     }
 
+  if (!bfd_convert_section_setup (ibfd, isection, obfd, &name, &size))
+    {
+      osection = NULL;
+      err = _("failed to create output section");
+      goto loser;
+    }
+
   osection = bfd_make_section_anyway_with_flags (obfd, name, flags);
 
   if (osection == NULL)
@@ -4126,8 +4133,6 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
       goto loser;
     }
 
-  size = bfd_section_size (isection);
-  size = bfd_convert_section_size (ibfd, isection, obfd, size);
   if (copy_byte >= 0)
     size = (size + interleave - 1) / interleave * copy_width;
   else if (extract_symbol)

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] bfd: Avoid signed overflow for new_size adjustment
  2022-12-06  5:03 Get rid of SEC_ELF_RENAME Alan Modra
@ 2022-12-06 21:01 ` H.J. Lu
  2022-12-06 22:46   ` Alan Modra
  0 siblings, 1 reply; 3+ messages in thread
From: H.J. Lu @ 2022-12-06 21:01 UTC (permalink / raw)
  To: Alan Modra; +Cc: Binutils

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

On Mon, Dec 5, 2022 at 9:04 PM Alan Modra via Binutils
<binutils@sourceware.org> wrote:
>
> SEC_ELF_RENAME is a flag used to effect section name changes when
> compressing/decompressing zlib-gnu debug sections.  This can be
> accomplished more directly in one of the objcopy specific bfd
> functions.  Renaming for ld input is simplified too.  Ld input object
> files always have BFD_DECOMPRESS set.
>
> bfd/
>         * compress.c (bfd_convert_section_size): Rename to..
>         (bfd_convert_section_setup): ..this.  Handle objcopy renaming
>         of compressed/decompressed debug sections.
>         * elf.c (_bfd_elf_make_section_from_shdr): Only rename zdebug
>         input for linker.
>         (elf_fake_sections): Don't handle renaming of debug sections for
>         objcopy here.
>         * section.c (SEC_ELF_RENAME): Delete.
>         * bfd-in2.h: Regenerate.
> binutils/
>         * objcopy.c (setup_section): Call bfd_convert_section_setup.
>         Don't call bfd_convert_section_size.
>
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 24f9305c47c..d983268563d 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -932,10 +932,6 @@ typedef struct bfd_section
>       TMS320C54X only.  */
>  #define SEC_TIC54X_BLOCK           0x10000000
>
> -  /* This section should be renamed.  This is for ELF linker
> -     internal use only.  */
> -#define SEC_ELF_RENAME             0x10000000
> -
>    /* Conditionally link this section; do not link if there are no
>       references found to any symbol in the section.  This is for TI
>       TMS320C54X only.  */
> @@ -7982,8 +7978,9 @@ void bfd_update_compression_header
>
>  int bfd_get_compression_header_size (bfd *abfd, asection *sec);
>
> -bfd_size_type bfd_convert_section_size
> -   (bfd *ibfd, asection *isec, bfd *obfd, bfd_size_type size);
> +bool bfd_convert_section_setup
> +   (bfd *ibfd, asection *isec, bfd *obfd,
> +    const char **new_name, bfd_size_type *new_size);
>
>  bool bfd_convert_section_contents
>     (bfd *ibfd, asection *isec, bfd *obfd,
> diff --git a/bfd/compress.c b/bfd/compress.c
> index a4e6a8ee7b5..bb55a6ec0ac 100644
> --- a/bfd/compress.c
> +++ b/bfd/compress.c
> @@ -225,53 +225,89 @@ bfd_get_compression_header_size (bfd *abfd, asection *sec)
>
>  /*
>  FUNCTION
> -       bfd_convert_section_size
> +       bfd_convert_section_setup
>
>  SYNOPSIS
> -       bfd_size_type bfd_convert_section_size
> -         (bfd *ibfd, asection *isec, bfd *obfd, bfd_size_type size);
> +       bool bfd_convert_section_setup
> +         (bfd *ibfd, asection *isec, bfd *obfd,
> +          const char **new_name, bfd_size_type *new_size);
>
>  DESCRIPTION
> -       Convert the size @var{size} of the section @var{isec} in input
> -       BFD @var{ibfd} to the section size in output BFD @var{obfd}.
> +       Do early setup for objcopy, when copying @var{isec} in input
> +       BFD @var{ibfd} to output BFD @var{obfd}.  Returns the name and
> +       size of the output section.
>  */
>
> -bfd_size_type
> -bfd_convert_section_size (bfd *ibfd, sec_ptr isec, bfd *obfd,
> -                         bfd_size_type size)
> +bool
> +bfd_convert_section_setup (bfd *ibfd, asection *isec, bfd *obfd,
> +                          const char **new_name, bfd_size_type *new_size)
>  {
>    bfd_size_type hdr_size;
>
> +  if ((isec->flags & SEC_DEBUGGING) != 0
> +      && (isec->flags & SEC_HAS_CONTENTS) != 0)
> +    {
> +      const char *name = *new_name;
> +
> +      if ((ibfd->flags & (BFD_DECOMPRESS | BFD_COMPRESS_GABI)) != 0)
> +       {
> +         /* When we decompress or compress with SHF_COMPRESSED,
> +            convert section name from .zdebug_* to .debug_*.  */
> +         if (startswith (name, ".zdebug_"))
> +           {
> +             name = bfd_zdebug_name_to_debug (obfd, name);
> +             if (name == NULL)
> +               return false;
> +           }
> +       }
> +
> +      /* PR binutils/18087: Compression does not always make a
> +        section smaller.  So only rename the section when
> +        compression has actually taken place.  If input section
> +        name is .zdebug_*, we should never compress it again.  */
> +      else if (isec->compress_status == COMPRESS_SECTION_DONE
> +              && startswith (name, ".debug_"))
> +       {
> +         name = bfd_debug_name_to_zdebug (obfd, name);
> +         if (name == NULL)
> +           return false;
> +       }
> +      *new_name = name;
> +    }
> +  *new_size = bfd_section_size (isec);
> +
>    /* Do nothing if either input or output aren't ELF.  */
>    if (bfd_get_flavour (ibfd) != bfd_target_elf_flavour
>        || bfd_get_flavour (obfd) != bfd_target_elf_flavour)
> -    return size;
> +    return true;
>
>    /* Do nothing if ELF classes of input and output are the same. */
>    if (get_elf_backend_data (ibfd)->s->elfclass
>        == get_elf_backend_data (obfd)->s->elfclass)
> -    return size;
> +    return true;
>
>    /* Convert GNU property size.  */
>    if (startswith (isec->name, NOTE_GNU_PROPERTY_SECTION_NAME))
> -    return _bfd_elf_convert_gnu_property_size (ibfd, obfd);
> +    {
> +      *new_size = _bfd_elf_convert_gnu_property_size (ibfd, obfd);
> +      return true;
> +    }
>
>    /* Do nothing if input file will be decompressed.  */
>    if ((ibfd->flags & BFD_DECOMPRESS))
> -    return size;
> +    return true;
>
>    /* Do nothing if the input section isn't a SHF_COMPRESSED section. */
>    hdr_size = bfd_get_compression_header_size (ibfd, isec);
>    if (hdr_size == 0)
> -    return size;
> +    return true;
>
>    /* Adjust the size of the output SHF_COMPRESSED section.  */
>    if (hdr_size == sizeof (Elf32_External_Chdr))
> -    return (size - sizeof (Elf32_External_Chdr)
> -           + sizeof (Elf64_External_Chdr));
> +    *new_size += sizeof (Elf64_External_Chdr) - sizeof (Elf32_External_Chdr);
>    else
> -    return (size - sizeof (Elf64_External_Chdr)
> -           + sizeof (Elf32_External_Chdr));
> +    *new_size += sizeof (Elf32_External_Chdr) - sizeof (Elf64_External_Chdr);

This doesn't work for 32-bit program since

 sizeof (Elf32_External_Chdr) - sizeof (Elf64_External_Chdr);

is negative and will overflow.

We should use

*new_size -= sizeof (Elf64_External_Chdr) - sizeof (Elf32_External_Chdr);

instead.

OK for master?

-- 
H.J.

[-- Attachment #2: 0001-bfd-Avoid-signed-overflow-for-new_size-adjustment.patch --]
[-- Type: application/x-patch, Size: 1229 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] bfd: Avoid signed overflow for new_size adjustment
  2022-12-06 21:01 ` [PATCH] bfd: Avoid signed overflow for new_size adjustment H.J. Lu
@ 2022-12-06 22:46   ` Alan Modra
  0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2022-12-06 22:46 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Tue, Dec 06, 2022 at 01:01:42PM -0800, H.J. Lu wrote:
> This doesn't work for 32-bit program since
> 
>  sizeof (Elf32_External_Chdr) - sizeof (Elf64_External_Chdr);
> 
> is negative and will overflow.
> 
> We should use
> 
> *new_size -= sizeof (Elf64_External_Chdr) - sizeof (Elf32_External_Chdr);
> 
> instead.
> 
> OK for master?

Yes thanks.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-06 22:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06  5:03 Get rid of SEC_ELF_RENAME Alan Modra
2022-12-06 21:01 ` [PATCH] bfd: Avoid signed overflow for new_size adjustment H.J. Lu
2022-12-06 22:46   ` Alan Modra

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).