public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: binutils@sourceware.org
Subject: objcopy section alignment
Date: Sat, 6 Aug 2022 18:41:49 +0930	[thread overview]
Message-ID: <Yu4wVc+CbZeWWrGg@squeak.grove.modra.org> (raw)

bfd_set_section_alignment currently always returns true.  This patch
changes it to return false on silly alignment values, avoiding yet
another way to trigger ubsan errors like coffcode.h:3192:12: runtime
error: shift exponent 299 is too large for 32-bit type 'int'.  We'll
catch that one in objcopy.c:setup_sections.  However, setup_sections
gives up on other setup operations that are necessary even after an
error of some sort.  Change that to keep going, which might change the
error message but that shouldn't matter in the least.

bfd/
	* section.c (bfd_set_section_alignment): Return false and
	don't set alignment_power for stupidly large alignments.
	* bfd-in2.h: Regenerate.
	* coffcode.h (coff_compute_section_file_positions): Don't use
	an int constant when calculating alignment.
binutils/
	* objcopy.c (setup_section): Keep on going after hitting
	non-fatal errors.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 0d2e915b413..4ab7e2d6934 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1201,6 +1201,8 @@ bfd_set_section_lma (asection *sec, bfd_vma val)
 static inline bool
 bfd_set_section_alignment (asection *sec, unsigned int val)
 {
+  if (val >= sizeof (bfd_vma) * 8 - 1)
+    return false;
   sec->alignment_power = val;
   return true;
 }
diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index 0dc68a9a25f..798b9f249b5 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -3189,7 +3189,7 @@ coff_compute_section_file_positions (bfd * abfd)
 #ifdef COFF_IMAGE_WITH_PE
 	  sofar = BFD_ALIGN (sofar, page_size);
 #else
-	  sofar = BFD_ALIGN (sofar, 1 << current->alignment_power);
+	  sofar = BFD_ALIGN (sofar, (bfd_vma) 1 << current->alignment_power);
 #endif
 
 #ifdef RS6000COFF_C
@@ -3259,7 +3259,7 @@ coff_compute_section_file_positions (bfd * abfd)
 
 	  old_size = current->size;
 	  current->size = BFD_ALIGN (current->size,
-				     1 << current->alignment_power);
+				     (bfd_vma) 1 << current->alignment_power);
 	  align_adjust = current->size != old_size;
 	  sofar += current->size - old_size;
 	}
@@ -3269,7 +3269,7 @@ coff_compute_section_file_positions (bfd * abfd)
 #ifdef COFF_IMAGE_WITH_PE
 	  sofar = BFD_ALIGN (sofar, page_size);
 #else
-	  sofar = BFD_ALIGN (sofar, 1 << current->alignment_power);
+	  sofar = BFD_ALIGN (sofar, (bfd_vma) 1 << current->alignment_power);
 #endif
 	  align_adjust = sofar != old_sofar;
 	  current->size += sofar - old_sofar;
@@ -3315,7 +3315,8 @@ coff_compute_section_file_positions (bfd * abfd)
   /* Make sure the relocations are aligned.  We don't need to make
      sure that this byte exists, because it will only matter if there
      really are relocs.  */
-  sofar = BFD_ALIGN (sofar, 1 << COFF_DEFAULT_SECTION_ALIGNMENT_POWER);
+  sofar = BFD_ALIGN (sofar,
+		     (bfd_vma) 1 << COFF_DEFAULT_SECTION_ALIGNMENT_POWER);
 
   obj_relocbase (abfd) = sofar;
   abfd->output_has_begun = true;
diff --git a/bfd/section.c b/bfd/section.c
index 5a487ce6c6f..c7a02d729f2 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -631,6 +631,8 @@ CODE_FRAGMENT
 .static inline bool
 .bfd_set_section_alignment (asection *sec, unsigned int val)
 .{
+.  if (val >= sizeof (bfd_vma) * 8 - 1)
+.    return false;
 .  sec->alignment_power = val;
 .  return true;
 .}
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 21c3a7127c8..b907b02d5e7 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -4014,7 +4014,7 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   bfd_vma vma;
   bfd_vma lma;
   flagword flags;
-  const char *err;
+  const char *err = NULL;
   const char * name;
   const char * new_name;
   char *prefix = NULL;
@@ -4097,10 +4097,7 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   else if (extract_symbol)
     size = 0;
   if (!bfd_set_section_size (osection, size))
-    {
-      err = _("failed to set size");
-      goto loser;
-    }
+    err = _("failed to set size");
 
   vma = bfd_section_vma (isection);
   p = find_section_list (bfd_section_name (isection), false,
@@ -4116,10 +4113,7 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
     vma += change_section_address;
 
   if (!bfd_set_section_vma (osection, vma))
-    {
-      err = _("failed to set vma");
-      goto loser;
-    }
+    err = _("failed to set vma");
 
   lma = isection->lma;
   p = find_section_list (bfd_section_name (isection), false,
@@ -4146,10 +4140,7 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   /* FIXME: This is probably not enough.  If we change the LMA we
      may have to recompute the header for the file as well.  */
   if (!bfd_set_section_alignment (osection, alignment))
-    {
-      err = _("failed to set alignment");
-      goto loser;
-    }
+    err = _("failed to set alignment");
 
   /* Copy merge entity size.  */
   osection->entsize = isection->entsize;
@@ -4178,16 +4169,13 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   /* Allow the BFD backend to copy any private data it understands
      from the input section to the output section.  */
   if (!bfd_copy_private_section_data (ibfd, isection, obfd, osection))
-    {
-      err = _("failed to copy private data");
-      goto loser;
-    }
+    err = _("failed to copy private data");
 
   if (make_nobits)
     elf_section_type (osection) = SHT_NOBITS;
 
-  /* All went well.  */
-  return;
+  if (!err)
+    return;
 
  loser:
   status = 1;

-- 
Alan Modra
Australia Development Lab, IBM

                 reply	other threads:[~2022-08-06  9:11 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=Yu4wVc+CbZeWWrGg@squeak.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).