From: Alan Modra Date: Wed, 14 Jun 2023 04:54:50 +0000 (+0930) Subject: asprintf memory leaks X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=6f860418d556d4e5492b3da9e1a52e4b85a85f3e;p=binutils-gdb.git asprintf memory leaks A number of backends want to return bfd_reloc_dangerous messaqes from relocation special_function, and construct the message using asprintf. Such messages are not freed anywhere, leading to small memory leaks inside libbfd. To limit the leaks, I'd implemented a static buffer in the ppc backends that was freed before use in asprintf output. This patch extends that scheme to other backends using a shared static buffer and goes further in freeing the buffer on any bfd_close. The patch also fixes a few other cases where asprintf output was not freed after use. bfd/ * bfd.c (_input_error_msg): Make global and rename to.. (_bfd_error_buf): ..this. (bfd_asprintf): New function. (bfd_errmsg): Use bfd_asprintf. * opncls.c (bfd_close_all_done): Free _buf_error_buf. * elf32-arm.c (find_thumb_glue, find_arm_glue): Use bfd_asprintf. * elf32-nios2.c (nios2_elf32_relocate_section): Likewise. * elf32-ppc.c (ppc_elf_unhandled_reloc): Likewise. * elf64-ppc.c (ppc64_elf_unhandled_reloc): Likewise. * elfnn-riscv.c (riscv_resolve_pcrel_lo_relocs): Likewise. (riscv_elf_relocate_section): Likewise. * libbfd.h: Regenerate. gas/ * read.c (read_end): Free current_name and current_label. (do_s_func): Likewise on error path. strdup label. ld/ * pe-dll.c (make_head, make_tail, make_one), (make_singleton_name_thunk, make_import_fixup_entry), (make_runtime_pseudo_reloc), (pe_create_runtime_relocator_reference: Free oname after use. --- diff --git a/bfd/bfd.c b/bfd/bfd.c index 4ae73701ce1..804acabf621 100644 --- a/bfd/bfd.c +++ b/bfd/bfd.c @@ -700,12 +700,16 @@ CODE_FRAGMENT .} .bfd_error_type; . +INTERNAL +.{* A buffer that is freed on bfd_close. *} +.extern char *_bfd_error_buf; +. */ static bfd_error_type bfd_error; static bfd_error_type input_error; static bfd *input_bfd; -static char *input_error_msg; +char *_bfd_error_buf; const char *const bfd_errmsgs[] = { @@ -793,8 +797,8 @@ bfd_set_input_error (bfd *input, bfd_error_type error_tag) /* This is an error that occurred during bfd_close when writing an archive, but on one of the input files. */ bfd_error = bfd_error_on_input; - free (input_error_msg); - input_error_msg = NULL; + free (_bfd_error_buf); + _bfd_error_buf = NULL; input_bfd = input; input_error = error_tag; if (input_error >= bfd_error_on_input) @@ -822,12 +826,10 @@ bfd_errmsg (bfd_error_type error_tag) if (error_tag == bfd_error_on_input) { const char *msg = bfd_errmsg (input_error); - - free (input_error_msg); - input_error_msg = NULL; - if (asprintf (&input_error_msg, _(bfd_errmsgs [error_tag]), - bfd_get_filename (input_bfd), msg) != -1) - return input_error_msg; + char *ret = bfd_asprintf (_(bfd_errmsgs[error_tag]), + bfd_get_filename (input_bfd), msg); + if (ret) + return ret; /* Ick, what to do on out of memory? */ return msg; @@ -839,7 +841,7 @@ bfd_errmsg (bfd_error_type error_tag) if (error_tag > bfd_error_invalid_error_code) error_tag = bfd_error_invalid_error_code; /* sanity check */ - return _(bfd_errmsgs [error_tag]); + return _(bfd_errmsgs[error_tag]); } /* @@ -868,6 +870,40 @@ bfd_perror (const char *message) fflush (stderr); } +/* +INTERNAL_FUNCTION + bfd_asprintf + +SYNOPSIS + char *bfd_asprintf (const char *fmt, ...); + +DESCRIPTION + Primarily for error reporting, this function is like + libiberty's xasprintf except that it can return NULL on no + memory and the returned string should not be freed. Uses a + single malloc'd buffer managed by libbfd, _bfd_error_buf. + Be aware that a call to this function frees the result of any + previous call. bfd_errmsg (bfd_error_on_input) also calls + this function. +*/ + +char * +bfd_asprintf (const char *fmt, ...) +{ + free (_bfd_error_buf); + _bfd_error_buf = NULL; + va_list ap; + va_start (ap, fmt); + int count = vasprintf (&_bfd_error_buf, fmt, ap); + va_end (ap); + if (count == -1) + { + bfd_set_error (bfd_error_no_memory); + _bfd_error_buf = NULL; + } + return _bfd_error_buf; +} + /* SUBSECTION BFD error handler @@ -1663,8 +1699,8 @@ bfd_init (void) { bfd_error = bfd_error_no_error; input_bfd = NULL; - free (input_error_msg); - input_error_msg = NULL; + free (_bfd_error_buf); + _bfd_error_buf = NULL; input_error = bfd_error_no_error; _bfd_error_program_name = NULL; _bfd_error_internal = error_handler_fprintf; diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c index cb22989f6f1..2afe67abdf1 100644 --- a/bfd/elf32-arm.c +++ b/bfd/elf32-arm.c @@ -7113,10 +7113,13 @@ find_thumb_glue (struct bfd_link_info *link_info, hash = elf_link_hash_lookup (&(hash_table)->root, tmp_name, false, false, true); - if (hash == NULL - && asprintf (error_message, _("unable to find %s glue '%s' for '%s'"), - "Thumb", tmp_name, name) == -1) - *error_message = (char *) bfd_errmsg (bfd_error_system_call); + if (hash == NULL) + { + *error_message = bfd_asprintf (_("unable to find %s glue '%s' for '%s'"), + "Thumb", tmp_name, name); + if (*error_message == NULL) + *error_message = (char *) bfd_errmsg (bfd_error_system_call); + } free (tmp_name); @@ -7148,11 +7151,13 @@ find_arm_glue (struct bfd_link_info *link_info, myh = elf_link_hash_lookup (&(hash_table)->root, tmp_name, false, false, true); - if (myh == NULL - && asprintf (error_message, _("unable to find %s glue '%s' for '%s'"), - "ARM", tmp_name, name) == -1) - *error_message = (char *) bfd_errmsg (bfd_error_system_call); - + if (myh == NULL) + { + *error_message = bfd_asprintf (_("unable to find %s glue '%s' for '%s'"), + "ARM", tmp_name, name); + if (*error_message == NULL) + *error_message = (char *) bfd_errmsg (bfd_error_system_call); + } free (tmp_name); return myh; diff --git a/bfd/elf32-nios2.c b/bfd/elf32-nios2.c index a699b0cfa9d..b02de7417a2 100644 --- a/bfd/elf32-nios2.c +++ b/bfd/elf32-nios2.c @@ -3724,7 +3724,6 @@ nios2_elf32_relocate_section (bfd *output_bfd, const char *name = NULL; int r_type; const char *format; - char *msgbuf = NULL; char *msg = NULL; bool unresolved_reloc; bfd_vma off; @@ -3825,10 +3824,7 @@ nios2_elf32_relocate_section (bfd *output_bfd, format = _("global pointer relative relocation at address " "%#" PRIx64 " when _gp not defined\n"); - if (asprintf (&msgbuf, format, - (uint64_t) reloc_address) == -1) - msgbuf = NULL; - msg = msgbuf; + msg = bfd_asprintf (format, (uint64_t) reloc_address); r = bfd_reloc_dangerous; } else @@ -3857,11 +3853,10 @@ nios2_elf32_relocate_section (bfd *output_bfd, "the global pointer (at %#" PRIx64 ") " "because the offset (%" PRId64 ") is out of " "the allowed range, -32678 to 32767\n" ); - if (asprintf (&msgbuf, format, name, - (uint64_t) symbol_address, (uint64_t) gp, - (int64_t) relocation) == -1) - msgbuf = NULL; - msg = msgbuf; + msg = bfd_asprintf (format, name, + (uint64_t) symbol_address, + (uint64_t) gp, + (int64_t) relocation); r = bfd_reloc_outofrange; } else @@ -4531,7 +4526,6 @@ nios2_elf32_relocate_section (bfd *output_bfd, { (*info->callbacks->warning) (info, msg, name, input_bfd, input_section, rel->r_offset); - free (msgbuf); return false; } } diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c index 2cff158a5f5..2c544b11de5 100644 --- a/bfd/elf32-ppc.c +++ b/bfd/elf32-ppc.c @@ -987,14 +987,8 @@ ppc_elf_unhandled_reloc (bfd *abfd, input_section, output_bfd, error_message); if (error_message != NULL) - { - static char *message; - free (message); - if (asprintf (&message, _("generic linker can't handle %s"), - reloc_entry->howto->name) < 0) - message = NULL; - *error_message = message; - } + *error_message = bfd_asprintf (_("generic linker can't handle %s"), + reloc_entry->howto->name); return bfd_reloc_dangerous; } diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c index 0e9a7ff96c3..dea9408ca49 100644 --- a/bfd/elf64-ppc.c +++ b/bfd/elf64-ppc.c @@ -1750,14 +1750,8 @@ ppc64_elf_unhandled_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol, input_section, output_bfd, error_message); if (error_message != NULL) - { - static char *message; - free (message); - if (asprintf (&message, _("generic linker can't handle %s"), - reloc_entry->howto->name) < 0) - message = NULL; - *error_message = message; - } + *error_message = bfd_asprintf (_("generic linker can't handle %s"), + reloc_entry->howto->name); return bfd_reloc_dangerous; } diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c index 30d2faa405d..09aa7be225e 100644 --- a/bfd/elfnn-riscv.c +++ b/bfd/elfnn-riscv.c @@ -2090,14 +2090,14 @@ riscv_resolve_pcrel_lo_relocs (riscv_pcrel_relocs *p) != RISCV_CONST_HIGH_PART (entry->value + r->reloc->r_addend)) { /* Check the overflow when adding reloc addend. */ - if (asprintf (&string, - _("%%pcrel_lo overflow with an addend, the " - "value of %%pcrel_hi is 0x%" PRIx64 " without " - "any addend, but may be 0x%" PRIx64 " after " - "adding the %%pcrel_lo addend"), - (int64_t) RISCV_CONST_HIGH_PART (entry->value), - (int64_t) RISCV_CONST_HIGH_PART - (entry->value + r->reloc->r_addend)) == -1) + string = bfd_asprintf (_("%%pcrel_lo overflow with an addend," + " the value of %%pcrel_hi is 0x%" PRIx64 + " without any addend, but may be 0x%" PRIx64 + " after adding the %%pcrel_lo addend"), + (int64_t) RISCV_CONST_HIGH_PART (entry->value), + (int64_t) RISCV_CONST_HIGH_PART + (entry->value + r->reloc->r_addend)); + if (string == NULL) string = _("%pcrel_lo overflow with an addend"); } @@ -2184,7 +2184,6 @@ riscv_elf_relocate_section (bfd *output_bfd, int r_type = ELFNN_R_TYPE (rel->r_info), tls_type; reloc_howto_type *howto = riscv_elf_rtype_to_howto (input_bfd, r_type); const char *msg = NULL; - char *msg_buf = NULL; bool resolved_to_zero; if (howto == NULL) @@ -2705,14 +2704,12 @@ riscv_elf_relocate_section (bfd *output_bfd, Perhaps we also need the similar checks for the R_RISCV_BRANCH and R_RISCV_RVC_BRANCH relocations. */ - if (asprintf (&msg_buf, - _("%%X%%P: relocation %s against `%s' which " - "may bind externally can not be used when " - "making a shared object; recompile " - "with -fPIC\n"), - howto->name, h->root.root.string) == -1) - msg_buf = NULL; - msg = msg_buf; + msg = bfd_asprintf (_("%%X%%P: relocation %s against `%s'" + " which may bind externally" + " can not be used" + " when making a shared object;" + " recompile with -fPIC\n"), + howto->name, h->root.root.string); r = bfd_reloc_notsupported; } } @@ -2999,13 +2996,10 @@ riscv_elf_relocate_section (bfd *output_bfd, && _bfd_elf_section_offset (output_bfd, info, input_section, rel->r_offset) != (bfd_vma) -1) { - if (asprintf (&msg_buf, - _("%%X%%P: unresolvable %s relocation against " - "symbol `%s'\n"), - howto->name, - h->root.root.string) == -1) - msg_buf = NULL; - msg = msg_buf; + msg = bfd_asprintf (_("%%X%%P: unresolvable %s relocation against " + "symbol `%s'\n"), + howto->name, + h->root.root.string); r = bfd_reloc_notsupported; } @@ -3062,9 +3056,6 @@ riscv_elf_relocate_section (bfd *output_bfd, if (msg && r != bfd_reloc_dangerous) info->callbacks->einfo (msg); - /* Free the unused `msg_buf`. */ - free (msg_buf); - /* We already reported the error via a callback, so don't try to report it again by returning false. That leads to spurious errors. */ ret = true; diff --git a/bfd/libbfd.h b/bfd/libbfd.h index a9fa1111a3d..55aa8f91cbd 100644 --- a/bfd/libbfd.h +++ b/bfd/libbfd.h @@ -933,6 +933,11 @@ bool bfd_write_bigendian_4byte_int (bfd *, unsigned int) ATTRIBUTE_HIDDEN; unsigned int bfd_log2 (bfd_vma x) ATTRIBUTE_HIDDEN; /* Extracted from bfd.c. */ +/* A buffer that is freed on bfd_close. */ +extern char *_bfd_error_buf; + +char *bfd_asprintf (const char *fmt, ...) ATTRIBUTE_HIDDEN; + bfd_error_handler_type _bfd_set_error_handler_caching (bfd *) ATTRIBUTE_HIDDEN; const char *_bfd_get_error_program_name (void) ATTRIBUTE_HIDDEN; diff --git a/bfd/opncls.c b/bfd/opncls.c index 7cb09a108e5..ee34a1231d7 100644 --- a/bfd/opncls.c +++ b/bfd/opncls.c @@ -929,6 +929,8 @@ bfd_close_all_done (bfd *abfd) } _bfd_delete_bfd (abfd); + free (_bfd_error_buf); + _bfd_error_buf = NULL; return ret; } diff --git a/gas/read.c b/gas/read.c index b4b628f2e5c..0cceca8a802 100644 --- a/gas/read.c +++ b/gas/read.c @@ -322,6 +322,8 @@ read_end (void) stabs_end (); poend (); _obstack_free (&cond_obstack, NULL); + free (current_name); + free (current_label); } #ifndef TC_ADDRESS_BYTES @@ -6048,6 +6050,8 @@ do_s_func (int end_p, const char *default_prefix) if (debug_type == DEBUG_STABS) stabs_generate_asm_endfunc (current_name, current_label); + free (current_name); + free (current_label); current_name = current_label = NULL; } else /* ! end_p */ @@ -6084,7 +6088,7 @@ do_s_func (int end_p, const char *default_prefix) as_fatal ("%s", xstrerror (errno)); } else - label = name; + label = xstrdup (name); } } else diff --git a/ld/pe-dll.c b/ld/pe-dll.c index e45ae104265..371915ac1cb 100644 --- a/ld/pe-dll.c +++ b/ld/pe-dll.c @@ -2107,6 +2107,7 @@ make_head (bfd *parent) tmp_seq++; abfd = bfd_create (oname, parent); + free (oname); bfd_find_target (pe_details->object_target, abfd); bfd_make_writable (abfd); @@ -2200,6 +2201,7 @@ make_tail (bfd *parent) tmp_seq++; abfd = bfd_create (oname, parent); + free (oname); bfd_find_target (pe_details->object_target, abfd); bfd_make_writable (abfd); @@ -2392,6 +2394,7 @@ make_one (def_file_export *exp, bfd *parent, bool include_jmp_stub) tmp_seq++; abfd = bfd_create (oname, parent); + free (oname); bfd_find_target (pe_details->object_target, abfd); bfd_make_writable (abfd); @@ -2586,6 +2589,7 @@ make_singleton_name_thunk (const char *import, bfd *parent) tmp_seq++; abfd = bfd_create (oname, parent); + free (oname); bfd_find_target (pe_details->object_target, abfd); bfd_make_writable (abfd); @@ -2666,6 +2670,7 @@ make_import_fixup_entry (const char *name, tmp_seq++; abfd = bfd_create (oname, parent); + free (oname); bfd_find_target (pe_details->object_target, abfd); bfd_make_writable (abfd); @@ -2724,6 +2729,7 @@ make_runtime_pseudo_reloc (const char *name ATTRIBUTE_UNUSED, tmp_seq++; abfd = bfd_create (oname, parent); + free (oname); bfd_find_target (pe_details->object_target, abfd); bfd_make_writable (abfd); @@ -2816,6 +2822,7 @@ pe_create_runtime_relocator_reference (bfd *parent) tmp_seq++; abfd = bfd_create (oname, parent); + free (oname); bfd_find_target (pe_details->object_target, abfd); bfd_make_writable (abfd);