[PATCH] strip: Don't treat fat IR objects as plugin object
H.J. Lu
hjl.tools@gmail.com
Tue Aug 5 15:50:59 GMT 2025
On Tue, Aug 5, 2025 at 8:06 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> 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)
>
This works on top of yours.
--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: b.diff
Type: text/x-patch
Size: 6353 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20250805/029427d2/attachment-0001.bin>
More information about the Binutils
mailing list