[PATCH] strip: Don't treat fat IR objects as plugin object
H.J. Lu
hjl.tools@gmail.com
Tue Aug 5 15:06:36 GMT 2025
On Tue, Aug 5, 2025 at 7:42 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Aug 04, 2025 at 07:59:55PM -0700, H.J. Lu wrote:
> > (bfd_check_format_matches_lto): Add a bool argument to indicate
> > called from strip.
>
> This seems like a poor idea to me. I'd rather not see special cases
> like this in bfd_check_format. I think it would be better to keep the
> logic in objcopy.c, even if it means closing then reopening a bfd with
> plugins disabled via bfd_plugin_no in bfd->plugin_format.
>
> The following is something I worked on today, but haven't yet
> committed.
>
> From 8dd8dafe1603e2fe2cc968424a7634283da778bd Mon Sep 17 00:00:00 2001
> From: Alan Modra <amodra@gmail.com>
> Date: Tue, 5 Aug 2025 21:18:44 +0930
> Subject: Remove bfd_check_format_lto
>
> Tidy changes to bfd_check_format_matches made by commit 9b854f169df9
> which added a bfd_plugin_specified_p test and commit f752be8f916e
> which added an lto_sections_removed arg. Both of these changes are
> unnecessary if plugin_format is set to bfd_plugin_no before calling
> bfd_check_format. bfd_plugin_no will prevent the plugin object_p
> function from returning a match (and in the first case from a segfault
> when loading plugins while a plugin is running). The plugin object_p
> function already protected itself from recursive calls by setting
> bfd_plugin_no before loading a plugin, but commit 9b854f169df9 opened
> new bfds so they were unprotected.
>
> It isn't strictly necessary to test for bfd_plugin_no in
> bfd_check_format_matches but I kept the check to minimise functional
> changes. Close inspection of the patch will notice I've added an
> is_linker_input test too. That also isn't strictly necessary, I
> think, but the match_count test was for the linker. See commit
> 999d6dff80fa.
>
> PR 12291
> PR 12430
> PR 13298
> PR 33198
> bfd/
> * format.c (bfd_check_format_lto): Revert to bfd_check_format.
> (bfd_check_format_matches_lto): Revert to bfd_check_format_matches.
> Correct comments. Manage both the lto_sections_removed and
> bfd_plugin_specified_p cases by testing for bfd_plugin_no.
> * plugin.c (bfd_plugin_get_symbols_in_object_only): Set
> plugin_format to bfd_plugin_no before checking new bfds.
> * bfd-in2.h: Regenerate.
> binutils/
> * objcopy.c (copy_archive): Replace bfd_check_format_lto calls
> with bfd_check_format using plugin_format set to bfd_plugin_no.
> (check_format_object): New function.
> (copy_file): Use it.
>
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 2ff3e930bfa..b013ef954da 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -2903,15 +2903,8 @@ bool generic_core_file_matches_executable_p
> (bfd *core_bfd, bfd *exec_bfd);
>
> /* Extracted from format.c. */
> -bool bfd_check_format_lto (bfd *abfd, bfd_format format,
> - bool lto_sections_removed);
> -
> bool bfd_check_format (bfd *abfd, bfd_format format);
>
> -bool bfd_check_format_matches_lto
> - (bfd *abfd, bfd_format format, char ***matching,
> - bool lto_sections_removed);
> -
> bool bfd_check_format_matches
> (bfd *abfd, bfd_format format, char ***matching);
>
> diff --git a/bfd/format.c b/bfd/format.c
> index f3a0774af08..dc05880873c 100644
> --- a/bfd/format.c
> +++ b/bfd/format.c
> @@ -56,19 +56,16 @@ extern const size_t _bfd_target_vector_entries;
>
> /*
> FUNCTION
> - bfd_check_format_lto
> + bfd_check_format
>
> SYNOPSIS
> - bool bfd_check_format_lto (bfd *abfd, bfd_format format,
> - bool lto_sections_removed);
> + bool bfd_check_format (bfd *abfd, bfd_format format);
>
> DESCRIPTION
> Verify if the file attached to the BFD @var{abfd} is compatible
> with the format @var{format} (i.e., one of <<bfd_object>>,
> <<bfd_archive>> or <<bfd_core>>).
>
> - If LTO_SECTION_REMOVED is true, ignore plugin target.
> -
> If the BFD has been set to a specific target before the
> call, only the named target and format combination is
> checked. If the target has not been set, or has been set to
> @@ -102,31 +99,10 @@ DESCRIPTION
> itself.
> */
>
> -bool
> -bfd_check_format_lto (bfd *abfd, bfd_format format,
> - bool lto_sections_removed)
> -{
> - return bfd_check_format_matches_lto (abfd, format, NULL,
> - lto_sections_removed);
> -}
> -
> -
> -/*
> -FUNCTION
> - bfd_check_format
> -
> -SYNOPSIS
> - bool bfd_check_format (bfd *abfd, bfd_format format);
> -
> -DESCRIPTION
> - Similar to bfd_check_format_plugin, except plugin target isn't
> - ignored.
> -*/
> -
> bool
> bfd_check_format (bfd *abfd, bfd_format format)
> {
> - return bfd_check_format_matches_lto (abfd, format, NULL, false);
> + return bfd_check_format_matches (abfd, format, NULL);
> }
>
> struct bfd_preserve
> @@ -431,36 +407,32 @@ bfd_set_lto_type (bfd *abfd ATTRIBUTE_UNUSED)
>
> /*
> FUNCTION
> - bfd_check_format_matches_lto
> + bfd_check_format_matches
>
> SYNOPSIS
> - bool bfd_check_format_matches_lto
> - (bfd *abfd, bfd_format format, char ***matching,
> - bool lto_sections_removed);
> + bool bfd_check_format_matches
> + (bfd *abfd, bfd_format format, char ***matching);
>
> DESCRIPTION
> Like <<bfd_check_format>>, except when it returns FALSE with
> - <<bfd_errno>> set to <<bfd_error_file_ambiguously_recognized>>. In that
> - case, if @var{matching} is not NULL, it will be filled in with
> - a NULL-terminated list of the names of the formats that matched,
> - allocated with <<malloc>>.
> + <<bfd_errno>> set to <<bfd_error_file_ambiguously_recognized>>.
> + In that case, if @var{matching} is not NULL, it will be filled
> + in with a NULL-terminated list of the names of the formats
> + that matched, allocated with <<malloc>>.
> Then the user may choose a format and try again.
>
> When done with the list that @var{matching} points to, the caller
> should free it.
> -
> - If LTO_SECTION_REMOVED is true, ignore plugin target.
> */
>
> bool
> -bfd_check_format_matches_lto (bfd *abfd, bfd_format format,
> - char ***matching,
> - bool lto_sections_removed ATTRIBUTE_UNUSED)
> +bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
> {
> extern const bfd_target binary_vec;
> const bfd_target * const *target;
> const bfd_target **matching_vector = NULL;
> const bfd_target *save_targ, *right_targ, *ar_right_targ, *match_targ;
> + const bfd_target *fail_targ;
> int match_count, best_count, best_match;
> int ar_match_index;
> unsigned int initial_section_id = _bfd_section_id;
> @@ -524,13 +496,14 @@ bfd_check_format_matches_lto (bfd *abfd, bfd_format format,
> if (!bfd_preserve_save (abfd, &preserve, NULL))
> goto err_ret;
>
> - /* If the target type was explicitly specified, just check that target.
> - If LTO_SECTION_REMOVED is true, don't match the plugin target. */
> + /* If the target type was explicitly specified, just check that target. */
> + fail_targ = NULL;
> if (!abfd->target_defaulted
> #if BFD_SUPPORTS_PLUGINS
> - && (!lto_sections_removed || !bfd_plugin_target_p (abfd->xvec))
> + && !(abfd->plugin_format == bfd_plugin_no
> + && bfd_plugin_target_p (save_targ))
> #endif
> - )
> + )
> {
> if (bfd_seek (abfd, 0, SEEK_SET) != 0) /* rewind! */
> goto err_ret;
> @@ -554,10 +527,10 @@ bfd_check_format_matches_lto (bfd *abfd, bfd_format format,
> target. */
> if (format == bfd_archive && save_targ == &binary_vec)
> goto err_unrecog;
> + fail_targ = save_targ;
> }
>
> - /* Since the target type was defaulted, check them all in the hope
> - that one will be uniquely recognized. */
> + /* Check all targets in the hope that one will be recognized. */
> right_targ = NULL;
> ar_right_targ = NULL;
> match_targ = NULL;
> @@ -571,25 +544,25 @@ bfd_check_format_matches_lto (bfd *abfd, bfd_format format,
> void **high_water;
>
> /* The binary target matches anything, so don't return it when
> - searching. Don't match the plugin target if we have another
> - alternative since we want to properly set the input format
> - before allowing a plugin to claim the file. Also, don't
> - check the default target twice. If LTO_SECTION_REMOVED is
> - true, don't match the plugin target. */
> + searching. Also, don't check the current target twice when
> + it has failed already.
> + Don't match the plugin target during linking if we have
> + another alternative since we want to properly set the input
> + format before allowing a plugin to claim the file.
> + Also as an optimisation don't match the plugin target when
> + abfd->plugin_format is set to bfd_plugin_no. (This occurs
> + when LTO sections have been stripped or when we have a
> + recursive call here from the plugin object_p via
> + bfd_plugin_get_symbols_in_object_only.) */
> if (*target == &binary_vec
> + || *target == fail_targ
> #if BFD_SUPPORTS_PLUGINS
> - || ((lto_sections_removed || match_count != 0)
> + || (((abfd->is_linker_input && match_count != 0)
> + || abfd->plugin_format == bfd_plugin_no)
> && bfd_plugin_target_p (*target))
> #endif
> - || (!abfd->target_defaulted && *target == save_targ))
> - continue;
> -
> -#if BFD_SUPPORTS_PLUGINS
> - /* If the plugin target is explicitly specified when a BFD file
> - is opened, don't check it twice. */
> - if (bfd_plugin_specified_p () && bfd_plugin_target_p (*target))
> + )
> continue;
> -#endif
>
> /* If we already tried a match, the bfd is modified and may
> have sections attached, which will confuse the next
> @@ -831,32 +804,6 @@ bfd_check_format_matches_lto (bfd *abfd, bfd_format format,
> return false;
> }
>
> -/*
> -FUNCTION
> - bfd_check_format_matches
> -
> -SYNOPSIS
> - bool bfd_check_format_matches
> - (bfd *abfd, bfd_format format, char ***matching);
> -
> -DESCRIPTION
> - Like <<bfd_check_format>>, except when it returns FALSE with
> - <<bfd_errno>> set to <<bfd_error_file_ambiguously_recognized>>. In that
> - case, if @var{matching} is not NULL, it will be filled in with
> - a NULL-terminated list of the names of the formats that matched,
> - allocated with <<malloc>>.
> - Then the user may choose a format and try again.
> -
> - When done with the list that @var{matching} points to, the caller
> - should free it.
> -*/
> -
> -bool
> -bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
> -{
> - return bfd_check_format_matches_lto (abfd, format, matching, false);
> -}
> -
> /*
> FUNCTION
> bfd_set_format
> diff --git a/bfd/plugin.c b/bfd/plugin.c
> index 1c72b748a8f..f78974861c6 100644
> --- a/bfd/plugin.c
> +++ b/bfd/plugin.c
> @@ -196,7 +196,8 @@ bfd_plugin_get_symbols_in_object_only (bfd *abfd)
> abfd->filename, bfd_errmsg (bfd_get_error ()));
> return;
> }
> - else if (!bfd_check_format (nbfd, bfd_object))
> + nbfd->plugin_format = bfd_plugin_no;
> + if (!bfd_check_format (nbfd, bfd_object))
> {
> /* There is no object only section if it isn't a bfd_object
> file. */
> @@ -239,6 +240,7 @@ bfd_plugin_get_symbols_in_object_only (bfd *abfd)
>
> /* Open the file containing object only section. */
> nbfd = bfd_openr (object_only_file, NULL);
> + nbfd->plugin_format = bfd_plugin_no;
> if (!bfd_check_format (nbfd, bfd_object))
> {
> (*_bfd_error_handler)
> diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> index 5774711abe6..8bd523aba13 100644
> --- a/binutils/objcopy.c
> +++ b/binutils/objcopy.c
> @@ -170,6 +170,8 @@ static bool sections_removed;
> #if BFD_SUPPORTS_PLUGINS
> /* TRUE if all GCC LTO sections are to be removed. */
> static bool lto_sections_removed;
> +#else
> +#define lto_sections_removed false
> #endif
>
> /* TRUE if only some sections are to be copied. */
> @@ -3745,11 +3747,10 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
>
> #if BFD_SUPPORTS_PLUGINS
> /* Ignore plugin target if all LTO sections should be removed. */
> - ok_object = bfd_check_format_lto (this_element, bfd_object,
> - lto_sections_removed);
> -#else
> - ok_object = bfd_check_format (this_element, bfd_object);
> + if (lto_sections_removed)
> + this_element->plugin_format = bfd_plugin_no;
> #endif
> + ok_object = bfd_check_format (this_element, bfd_object);
>
> /* PR binutils/3110: Cope with archives
> containing multiple target types. */
> @@ -3863,6 +3864,25 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
> return ok;
> }
>
> +static bool
> +check_format_object (bfd *ibfd, char ***obj_matching,
> + bool no_plugins ATTRIBUTE_UNUSED)
> +{
> +#if BFD_SUPPORTS_PLUGINS
> + /* Ignore plugin target first if all LTO sections should be
> + removed. Try with plugin target next if ignoring plugin
> + target fails to match the format. */
> + if (no_plugins && ibfd->plugin_format == bfd_plugin_unknown)
> + {
> + ibfd->plugin_format = bfd_plugin_no;
> + if (bfd_check_format_matches (ibfd, bfd_object, obj_matching))
> + return true;
> + ibfd->plugin_format = bfd_plugin_unknown;
> + }
> +#endif
> + return bfd_check_format_matches (ibfd, bfd_object, obj_matching);
> +}
> +
> /* The top-level control. */
>
> static void
> @@ -3988,21 +4008,7 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
> input_arch, target_defaulted))
> status = 1;
> }
> - else if (
> -#if BFD_SUPPORTS_PLUGINS
> - /* Ignore plugin target first if all LTO sections should be
> - removed. Try with plugin target next if ignoring plugin
> - target fails to match the format. */
> - bfd_check_format_matches_lto (ibfd, bfd_object, &obj_matching,
> - lto_sections_removed)
> - || (lto_sections_removed
> - && bfd_check_format_matches_lto (ibfd, bfd_object,
> - &obj_matching, false))
> -#else
> - bfd_check_format_matches_lto (ibfd, bfd_object, &obj_matching,
> - false)
> -#endif
> - )
> + else if (check_format_object (ibfd, &obj_matching, lto_sections_removed))
> {
> bfd *obfd;
> do_copy:
>
This failed PR 33246 tests in my patch:
FAIL: pr33246-fat with --strip-debug --enable-deterministic-archives
(strip tmpdir/pr33246-fat.o)
FAIL: pr33246-llvm-fat with --strip-debug
--enable-deterministic-archives (strip tmpdir/pr33246-llvm-fat.o)
--
H.J.
More information about the Binutils
mailing list