From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id 41C8838582AD for ; Sat, 6 Aug 2022 09:11:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 41C8838582AD Received: by mail-pl1-x633.google.com with SMTP id iw1so4524786plb.6 for ; Sat, 06 Aug 2022 02:11:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:subject:message-id:mime-version :content-disposition; bh=RCJyyIZn8OPsnhgEdg4IajgSqSD5Lp5YnJborvlO974=; b=QPv2T15CWJIl2Z78RdQnXLyAdRo+kjA273Kk+xEXHejFV6tNRhjsXyUmm9hek2n+8k sjc7LAvoi2uItipSJb5y4Ce/rOU4xRQ5gPMmbtWDGX8pqACVdVWJC7K+BLHa1h3XFVla FTC/wFompJauhf1nDplk1aly1Zs+5IT/Cgk8BKf4rKQsJiX2Q/MH1Igzs0X328XNpsF3 j0j5oJwcIUBehB/ZV9FddH1F3Z18jKg/yUw2A/9j0mMVD+u4PuIsYJMRIWI2rzesTmCa IExRSoGpE1uVX65HvONuB14bLACIt9LZ6uf7KdDcM6yqPQsfaIsK3yq5GiO6uRCX1SPu O2VA== X-Gm-Message-State: ACgBeo0FFrPtNcHcsnDSX5THt5in/Pz8FRWIN8+C5NYiYWMUzryIpc3P X7n+eILBJNFfFjDTfuCuizXp9GoPaQ0= X-Google-Smtp-Source: AA6agR5gf5ZnUH7WB9jzkajmkgDRENXl8qEs+uYifmVGyUTRhPcLkHiCaywkpnpD1ItHOo87Z7TbQQ== X-Received: by 2002:a17:902:b945:b0:16e:e702:bbb4 with SMTP id h5-20020a170902b94500b0016ee702bbb4mr10417851pls.25.1659777112982; Sat, 06 Aug 2022 02:11:52 -0700 (PDT) Received: from squeak.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id ix2-20020a170902f80200b0016ee328fd61sm4431919plb.198.2022.08.06.02.11.52 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 06 Aug 2022 02:11:52 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id D9F4E1142EB7; Sat, 6 Aug 2022 18:41:49 +0930 (ACST) Date: Sat, 6 Aug 2022 18:41:49 +0930 From: Alan Modra To: binutils@sourceware.org Subject: objcopy section alignment Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3036.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Aug 2022 09:11:55 -0000 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