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