Re: bfd_cleanup for object_p

["H.J. Lu" <hjl.tools@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6369 bytes --]

On Mon, Mar 2, 2020 at 12:59 AM Alan Modra <amodra@gmail.com> wrote:
>
> The object_p (and archive_p, core_file_p) functions are not supposed
> to have any target specific malloc'd memory attached to the bfd on
> their return.  This should be obvious on a failure return, but it's
> also true for a successful return.  The reason is that even though the
> object_p recognises the file, that particular target may not be used
> and thus the bfd won't be closed calling close_and_cleanup for the
> target that allocated the memory.
>
> It turns out that the object_p bfd_target* return value isn't needed.
> In all cases except ld/plugin.c the target is abfd->xvec and with
> ld/plugin.c the target isn't used.  So this patch returns a cleanup
> function from object_p instead, called in bfd_check_format_matches to
> tidy the bfd before trying a different target match.  The only cleanup
> that does anything at this stage is the alpha-vms one.
>
> bfd/
>         * targets.c (bfd_cleanup): New typedef.
>         (struct bfd <_bfd_check_format>): Return a bfd_cleanup.
>         * libbfd-in.h (_bfd_no_cleanup): Define.
>         * format.c (bfd_reinit): Add cleanup parameter, call it.
>         (bfd_check_format_matches): Set cleanup from _bfd_check_format
>         call and pass to bfd_reinit.  Delete temp, use abfd->xvec instead.
>         * aout-target.h (callback, object_p): Return bfd_cleanup.
>         * aout-tic30.c (tic30_aout_callback, tic30_aout_object_p): Likewise.
>         * archive.c (bfd_generic_archive_p): Likewise.
>         * binary.c (binary_object_p): Likewise.
>         * coff-alpha.c (alpha_ecoff_object_p): Likewise.
>         * coff-ia64.c (ia64coff_object_p): Likewise.
>         * coff-rs6000.c (_bfd_xcoff_archive_p, rs6000coff_core_p): Likewise.
>         * coff-sh.c (coff_small_object_p): Likewise.
>         * coff-stgo32.c (go32_check_format): Likewise.
>         * coff64-rs6000.c (xcoff64_archive_p, rs6000coff_core_p),
>         (xcoff64_core_p): Likewise.
>         * coffgen.c (coff_real_object_p, coff_object_p): Likewise.
>         * elf-bfd.h (bfd_elf32_object_p, bfd_elf32_core_file_p),
>         (bfd_elf64_object_p, bfd_elf64_core_file_p): Likewise.
>         * elfcode.h (elf_object_p): Likewise.
>         * elfcore.h (elf_core_file_p): Likewise.
>         * i386msdos.c (msdos_object_p): Likewise.
>         * ihex.c (ihex_object_p): Likewise.
>         * libaout.h (some_aout_object_p): Likewise.
>         * libbfd-in.h (bfd_generic_archive_p, _bfd_dummy_target),
>         (_bfd_vms_lib_alpha_archive_p, _bfd_vms_lib_ia64_archive_p): Likewise.
>         * libbfd.c (_bfd_dummy_target): Likewise.
>         * libcoff-in.h (coff_object_p): Likewise.
>         * mach-o-aarch64.c (bfd_mach_o_arm64_object_p),
>         (bfd_mach_o_arm64_core_p): Likewise.
>         * mach-o-arm.c (bfd_mach_o_arm_object_p),
>         (bfd_mach_o_arm_core_p): Likewise.
>         * mach-o-i386.c (bfd_mach_o_i386_object_p),
>         (bfd_mach_o_i386_core_p): Likewise.
>         * mach-o-x86-64.c (bfd_mach_o_x86_64_object_p),
>         (bfd_mach_o_x86_64_core_p): Likewise.
>         * mach-o.c (bfd_mach_o_header_p, bfd_mach_o_gen_object_p),
>         (bfd_mach_o_gen_core_p, bfd_mach_o_fat_archive_p): Likewise.
>         * mach-o.h (bfd_mach_o_object_p, bfd_mach_o_core_p),
>         (bfd_mach_o_fat_archive_p, bfd_mach_o_header_p): Likewise.
>         * mmo.c (mmo_object_p): Likewise.
>         * pef.c (bfd_pef_object_p, bfd_pef_xlib_object_p): Likewise.
>         * peicode.h (coff_real_object_p, pe_ILF_object_p),
>         (pe_bfd_object_p): Likewise.
>         * plugin.c (ld_plugin_object_p, bfd_plugin_object_p): Likewise.
>         * ppcboot.c (ppcboot_object_p): Likewise.
>         * rs6000-core.c (rs6000coff_core_p): Likewise.
>         * som.c (som_object_setup, som_object_p): Likewise.
>         * srec.c (srec_object_p, symbolsrec_object_p): Likewise.
>         * tekhex.c (tekhex_object_p): Likewise.
>         * vms-alpha.c (alpha_vms_object_p): Likewise.
>         * vms-lib.c (_bfd_vms_lib_archive_p, _bfd_vms_lib_alpha_archive_p),
>         (_bfd_vms_lib_ia64_archive_p, _bfd_vms_lib_txt_archive_p): Likewise.
>         * wasm-module.c (wasm_object_p): Likewise.
>         * xsym.c (bfd_sym_object_p): Likewise.
>         * xsym.h (bfd_sym_object_p): Likewise.
>         * aoutx.h (some_aout_object_p): Likewise, and callback parameter
>         return type.
>         * pdp11.c (some_aout_object_p): Likewise.
>         * plugin.c (register_ld_plugin_object_p): Update object_p
>         parameter type.
>         * plugin.h (register_ld_plugin_object_p): Likewise.
>         * bfd-in2.h: Regenerate.
>         * libbfd.h: Regenerate.
>         * libcoff.h: Regenerate.
> ld/
>         * plugin.c (plugin_object_p): Return a bfd_cleanup.
>         (plugin_cleanup): New function.

I am checking in this to fix Linux/i386 build:

libtool: compile:  /usr/gcc-9.2.1-32bit/bin/gcc -m32 -DHAVE_CONFIG_H
-I. -I/export/gnu/import/git/sources/binutils-gdb/bfd
-DBINDIR=\"/usr/local/bin\" -DLIBDIR=\"/usr/local/lib\" -DTRAD_CORE
-I. -I/export/gnu/import/git/sources/binutils-gdb/bfd
-I/export/gnu/import/git/sources/binutils-gdb/bfd/../include
-DHAVE_i386_elf32_vec -DHAVE_iamcu_elf32_vec -DHAVE_i386_pei_vec
-DHAVE_elf32_le_vec -DHAVE_elf32_be_vec -W -Wall -Wstrict-prototypes
-Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror
-I/export/gnu/import/git/sources/binutils-gdb/bfd/../zlib -g -O2 -MT
trad-core.lo -MD -MP -MF .deps/trad-core.Tpo -c
/export/gnu/import/git/sources/binutils-gdb/bfd/trad-core.c -o
trad-core.o
/export/gnu/import/git/sources/binutils-gdb/bfd/trad-core.c:293:7:
error: initialization of ‘void (* (*)(bfd *))(bfd *)’ {aka ‘void (*
(*)(struct bfd *))(struct bfd *)’} from incompatible pointer type
‘const bfd_target * (*)(bfd *)’ {aka ‘const struct bfd_target *
(*)(struct bfd *)’} [-Werror=incompatible-pointer-types]
  293 |       trad_unix_core_file_p  /* a core file */
      |       ^~~~~~~~~~~~~~~~~~~~~
/export/gnu/import/git/sources/binutils-gdb/bfd/trad-core.c:293:7:
note: (near initialization for ‘core_trad_vec._bfd_check_format[3]’)
mv -f .deps/cofflink.Tpo .deps/cofflink.Plo
cc1: all warnings being treated as errors

-- 
H.J.

[-- Attachment #2: 0001-trad_unix_core_file_p-Return-bfd_cleanup.patch --]
[-- Type: text/x-patch, Size: 1303 bytes --]

From 728d32c496435cbd2529f7de9f5277d88c9c04e2 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 2 Mar 2020 04:35:23 -0800
Subject: [PATCH] trad_unix_core_file_p: Return bfd_cleanup

	* trad-core.c (trad_unix_core_file_p): Return bfd_cleanup.
---
 bfd/ChangeLog   | 4 ++++
 bfd/trad-core.c | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index a917631b38..505da06add 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,7 @@
+2020-03-02  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* trad-core.c (trad_unix_core_file_p): Return bfd_cleanup.
+
 2020-03-02  Alan Modra  <amodra@gmail.com>
 
 	* targets.c (bfd_cleanup): New typedef.
diff --git a/bfd/trad-core.c b/bfd/trad-core.c
index 10ab252134..1b2477a4c5 100644
--- a/bfd/trad-core.c
+++ b/bfd/trad-core.c
@@ -71,7 +71,7 @@ struct trad_core_struct
 
 /* Handle 4.2-style (and perhaps also sysV-style) core dump file.  */
 
-static const bfd_target *
+static bfd_cleanup
 trad_unix_core_file_p (bfd *abfd)
 {
   int val;
@@ -220,7 +220,7 @@ trad_unix_core_file_p (bfd *abfd)
   core_datasec (abfd)->alignment_power = 2;
   core_regsec (abfd)->alignment_power = 2;
 
-  return abfd->xvec;
+  return _bfd_no_cleanup;
 
  fail:
   bfd_release (abfd, abfd->tdata.any);
-- 
2.24.1