Home Original page

objcopy/strip of IR files and is_strip_input

H.J. Lu hjl.tools@gmail.com
Mon Aug 11 19:58:51 GMT 2025 
On Mon, Aug 11, 2025 at 3:23 AM Alan Modra <amodra@gmail.com> wrote:
>
> This tidies objcopy/strip handling of IR objects, in the process of
> removing the unnecessary is_strip_input flag.
>
> The first thing I noticed when looking at is_strip_input code was that
> the abfd->my_archive test in bfd_check_format_matches meant that
> plugins were disabled when reading archive elements.  We can instead
> disable plugins by setting bfd_no_plugin, so there doesn't seem to be
> a need for is_strip_input in objcopy.c:copy_archive.  This isn't
> exactly the same, because bfd_no_plugin prevents the plugin target
> recognising archive elements in the bfd_check_format_matches loop over
> all targets as well as just the first !target_defaulted test.  But
> that turns out to be fine.  IR code is handled in copy_archive as for
> other unknown format files.  In fact, the only need for the plugin
> target when copying archives is when reading symbols for the archive
> map.  I've made that plain by moving the plugin target override and
> commenting on why it is really needed.
>
> So on to plain object files.  Here, IR code is also copied unchanged,
> so there doesn't seem a need for the plugin target there either.  It
> isn't quite so simple though, because the objcopy/strip code handling
> object files wants to verify the format of the object file.  Allowing
> objcopy/strip to copy unknown format files would be a change in
> behaviour (and results in mmix testsuite fails, ld-mmix/b-badfil1 and
> others).  However, always excluding the plugin target results in a
> fail of tests added in commit c2729c37f10a.  So I've enabled a plugin
> format check only for files that are otherwise unrecognised, and
> commented why this is done.  I question the need to objcopy LLVM
> bytecode files.

Will the new strip work on archives which contain regular objects
as well as LLVM slim IR objects?

> Another thing (not done in this patch): I think that both places where
> we do something special with lto_slim_ir_object can be removed too,
> but that causes
> FAIL: pr33246 with --strip-debug --enable-deterministic-archives (strip tmpdir/pr33246.o)
> due to strip-debug removing the FILE symbol but otherwise leaving the
> object file good.  ie. the test is too restrictive IMO.  Is it true
> that no one will ever want to manipulate slim IR files?  (Apart from
> the currently allowed removal of all IR sections.)

Binutils recogizes and treats LLVM slim and GCC slim IR objects differently.
I think it is better to treat them the same.  Slim IR objects should be
always copied as unknown objects.

> bfd/
>         * bfd.c (struct bfd<is_strip_input>): Delete.
>         * format.c (bfd_check_format_matches): Delete is_strip_input
>         special case code.
>         * bfd-in2.h: Regenerate.
> binutils/
>         * objcopy.c (copy_archive): Don't set is_strip_input.  Always
>         set bfd_plugin_no when reading elements.  Enable plugins when
>         opening copied elements.
>         (check_format_object): Delete.
>         (copy_file): Don't enable plugin target here.  Don't set
>         is_strip_input.  Set bfd_plugin_no.  Move bfd_core handling
>         code earlier to remove goto.  Enable plugin for llvm bytecode.
>         Copy slim IR files as unknown objects.
>
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 12512a3962c..b013ef954da 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -2131,9 +2131,6 @@ struct bfd
>    /* Set if this is the linker input BFD.  */
>    unsigned int is_linker_input : 1;
>
> -  /* Set if this is the strip input BFD.  */
> -  unsigned int is_strip_input : 1;
> -
>    /* If this is an input for a compiler plug-in library.  */
>    ENUM_BITFIELD (bfd_plugin_format) plugin_format : 2;
>
> diff --git a/bfd/bfd.c b/bfd/bfd.c
> index 4aded6809bb..858ab5ce017 100644
> --- a/bfd/bfd.c
> +++ b/bfd/bfd.c
> @@ -296,9 +296,6 @@ CODE_FRAGMENT
>  .  {* Set if this is the linker input BFD.  *}
>  .  unsigned int is_linker_input : 1;
>  .
> -.  {* Set if this is the strip input BFD.  *}
> -.  unsigned int is_strip_input : 1;
> -.
>  .  {* If this is an input for a compiler plug-in library.  *}
>  .  ENUM_BITFIELD (bfd_plugin_format) plugin_format : 2;
>  .
> diff --git a/bfd/format.c b/bfd/format.c
> index fba8d2a3d37..25feb5266df 100644
> --- a/bfd/format.c
> +++ b/bfd/format.c
> @@ -511,20 +511,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
>
>        cleanup = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
>
> -      /* When called from strip, don't treat archive member nor
> -        standalone fat IR object as an IR object.  For archive
> -        member, it will be copied as an unknown object if the
> -        plugin target is in use or it is a slim IR object.  For
> -        standalone fat IR object, it will be copied as non-IR
> -        object.  */
> -      if (cleanup
> -#if BFD_SUPPORTS_PLUGINS
> -         && (!abfd->is_strip_input
> -             || !bfd_plugin_target_p (abfd->xvec)
> -             || (abfd->lto_type != lto_fat_ir_object
> -                 && abfd->my_archive == NULL))
> -#endif
> -         )
> +      if (cleanup)
>         goto ok_ret;
>
>        /* For a long time the code has dropped through to check all
> diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> index 654d2b9b44a..bc07176799a 100644
> --- a/binutils/objcopy.c
> +++ b/binutils/objcopy.c
> @@ -3690,8 +3690,6 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
>        bool ok_object;
>        const char *element_name;
>
> -      this_element->is_strip_input = 1;
> -
>        element_name = bfd_get_filename (this_element);
>        /* PR binutils/17533: Do not allow directory traversal
>          outside of the current directory tree by archive members.  */
> @@ -3748,11 +3746,7 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
>        l->obfd = NULL;
>        list = l;
>
> -#if BFD_SUPPORTS_PLUGINS
> -      /* Ignore plugin target if all LTO sections should be removed.  */
> -      if (lto_sections_removed)
> -       this_element->plugin_format = bfd_plugin_no;
> -#endif
> +      this_element->plugin_format = bfd_plugin_no;
>        ok_object = bfd_check_format (this_element, bfd_object);
>
>        /* PR binutils/3110: Cope with archives
> @@ -3772,11 +3766,9 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
>
>  #if BFD_SUPPORTS_PLUGINS
>        /* Copy LTO IR file as unknown object.  */
> -      if ((!lto_sections_removed
> -          && this_element->lto_type == lto_slim_ir_object)
> -         || bfd_plugin_target_p (this_element->xvec))
> +      if (!lto_sections_removed
> +         && this_element->lto_type == lto_slim_ir_object)
>         ok_object = false;
> -      else
>  #endif
>        if (ok_object)
>         {
> @@ -3811,8 +3803,15 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
>           if (preserve_dates && stat_status == 0)
>             set_times (output_name, &buf);
>
> -         /* Open the newly created output file and attach to our list.  */
> -         output_element = bfd_openr (output_name, output_target);
> +         /* Open the newly created output file and attach to our
> +            list.  We must enable the plugin target here in order to
> +            read IR symbols for the archive map.  */
> +         const char *targ = output_target;
> +#if BFD_SUPPORTS_PLUGINS
> +         if (!force_output_target)
> +           targ = "plugin";
> +#endif
> +         output_element = bfd_openr (output_name, targ);
>
>           list->obfd = output_element;
>
> @@ -3869,25 +3868,6 @@ 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
> @@ -3912,12 +3892,6 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
>        return;
>      }
>
> -#if BFD_SUPPORTS_PLUGINS
> -  /* Enable LTO plugin in strip.  */
> -  if (is_strip && !target)
> -    target = "plugin";
> -#endif
> -
>    /* To allow us to do "strip *" without dying on the first
>       non-object file, failures are nonfatal.  */
>    ibfd = bfd_openr (input_filename, target);
> @@ -3971,7 +3945,7 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
>        break;
>      }
>
> -  ibfd->is_strip_input = 1;
> +  ibfd->plugin_format = bfd_plugin_no;
>
>    if (bfd_check_format (ibfd, bfd_archive))
>      {
> @@ -4014,16 +3988,66 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
>        if (!copy_archive (ibfd, obfd, output_target, force_output_target,
>                          input_arch, target_defaulted))
>         status = 1;
> +      return;
> +    }
> +
> +  bool ok_plugin = false;
> +  bool ok_object = bfd_check_format_matches (ibfd, bfd_object, &obj_matching);
> +  bfd_error_type obj_error = bfd_get_error ();
> +  bfd_error_type core_error = bfd_error_no_error;
> +  if (!ok_object)
> +    {
> +      ok_object = bfd_check_format_matches (ibfd, bfd_core, &core_matching);
> +      core_error = bfd_get_error ();
> +      if (ok_object)
> +       {
> +         if (obj_error == bfd_error_file_ambiguously_recognized)
> +           free (obj_matching);
> +         obj_error = bfd_error_no_error;
> +       }
> +#if BFD_SUPPORTS_PLUGINS
> +      else
> +       {
> +         /* This is for LLVM bytecode files, which are not ELF objects.
> +            Since objcopy/strip does nothing with these files except
> +            copy them whole perhaps we ought to just reject them?  */
> +         bfd_find_target ("plugin", ibfd);
> +         ibfd->plugin_format = bfd_plugin_unknown;
> +         ok_plugin = bfd_check_format (ibfd, bfd_object);
> +       }
> +#endif
> +    }
> +
> +  if (obj_error == bfd_error_file_ambiguously_recognized)
> +    {
> +      if (core_error == bfd_error_file_ambiguously_recognized)
> +       free (core_matching);
> +      bfd_set_error (obj_error);
> +      status = 1;
> +      bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
> +      list_matching_formats (obj_matching);
>      }
> -  else if (check_format_object (ibfd, &obj_matching, lto_sections_removed))
> +  else if (core_error == bfd_error_file_ambiguously_recognized)
> +    {
> +      status = 1;
> +      bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
> +      list_matching_formats (core_matching);
> +    }
> +  else if (!ok_object && !ok_plugin)
> +    {
> +      status = 1;
> +      bfd_set_error (obj_error);
> +      bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
> +    }
> +  else
>      {
>        bfd *obfd;
> -    do_copy:
>
>        /* bfd_get_target does not return the correct value until
>          bfd_check_format succeeds.  */
> -      if (output_target == NULL
> -         || strcmp (output_target, "default") == 0)
> +      if (ok_object
> +         && (output_target == NULL
> +             || strcmp (output_target, "default") == 0))
>         output_target = bfd_get_target (ibfd);
>
>        if (ofd >= 0)
> @@ -4042,65 +4066,32 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
>         }
>
>  #if BFD_SUPPORTS_PLUGINS
> -      if (bfd_plugin_target_p (ibfd->xvec))
> -       {
> -         /* Copy LTO IR file as unknown file.  */
> -         if (!copy_unknown_file (ibfd, obfd, in_stat->st_size,
> -                                 in_stat->st_mode))
> -           status = 1;
> -         else if (!bfd_close_all_done (obfd))
> -           status = 1;
> -       }
> -      else
> +      /* Copy LTO IR file as unknown object.  */
> +      if (!lto_sections_removed
> +         && ibfd->lto_type == lto_slim_ir_object)
> +       ok_object = false;
>  #endif
> -       {
> -         if (! copy_object (ibfd, obfd, input_arch, target_defaulted))
> -           status = 1;
> -
> -         /* PR 17512: file: 0f15796a.
> -            If the file could not be copied it may not be in a writeable
> -            state.  So use bfd_close_all_done to avoid the possibility of
> -            writing uninitialised data into the file.  */
> -         if (! (status ? bfd_close_all_done (obfd) : bfd_close (obfd)))
> -           {
> -             status = 1;
> -             bfd_nonfatal_message (output_filename, NULL, NULL, NULL);
> -           }
> -       }
> +      if (ok_object
> +         ? !copy_object (ibfd, obfd, input_arch, target_defaulted)
> +         : !copy_unknown_file (ibfd, obfd,
> +                               in_stat->st_size, in_stat->st_mode))
> +       status = 1;
>
> -      if (!bfd_close (ibfd))
> +      /* PR 17512: file: 0f15796a.
> +        If the file could not be copied it may not be in a writeable
> +        state.  So use bfd_close_all_done to avoid the possibility of
> +        writing uninitialised data into the file.  */
> +      if (!(ok_object && !status ? bfd_close : bfd_close_all_done) (obfd))
>         {
>           status = 1;
> -         bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
> +         bfd_nonfatal_message (output_filename, NULL, NULL, NULL);
>         }
>      }
> -  else
> -    {
> -      bfd_error_type obj_error = bfd_get_error ();
> -      bfd_error_type core_error;
> -
> -      if (bfd_check_format_matches (ibfd, bfd_core, &core_matching))
> -       {
> -         /* This probably can't happen..  */
> -         if (obj_error == bfd_error_file_ambiguously_recognized)
> -           free (obj_matching);
> -         goto do_copy;
> -       }
>
> -      core_error = bfd_get_error ();
> -      /* Report the object error in preference to the core error.  */
> -      if (obj_error != core_error)
> -       bfd_set_error (obj_error);
> -
> -      bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
> -
> -      if (obj_error == bfd_error_file_ambiguously_recognized)
> -       list_matching_formats (obj_matching);
> -      if (core_error == bfd_error_file_ambiguously_recognized)
> -       list_matching_formats (core_matching);
> -
> -      bfd_close (ibfd);
> +  if (!bfd_close (ibfd))
> +    {
>        status = 1;
> +      bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
>      }
>  }
>
>
> --
> Alan Modra



-- 
H.J.
More information about the Binutils mailing list