* Re: PATCH: binutils/2467: "ar q" / ranlib has large memory use (linear in archive size)
2006-03-24 7:27 ` H. J. Lu
@ 2006-04-07 1:08 ` H. J. Lu
2006-04-25 17:22 ` Nick Clifton
0 siblings, 1 reply; 5+ messages in thread
From: H. J. Lu @ 2006-04-07 1:08 UTC (permalink / raw)
To: binutils, dcoutts
On Thu, Mar 23, 2006 at 09:17:35PM -0800, H. J. Lu wrote:
> On Fri, Mar 24, 2006 at 11:41:39AM +1030, Alan Modra wrote:
> > On Mon, Mar 20, 2006 at 06:23:03PM -0800, H. J. Lu wrote:
> > > Will this patch break anything?
> >
> > I don't know the answer to that, but really, it's something that you as
> > patch submitter should answer. You could gain a reasonable level of
>
> I think it is safe. _bfd_generic_bfd_free_cached_info is called from
> _bfd_compute_and_write_armap only. It has no problems with gcc build.
>
> > confidence by allocating and scribbling on some memory after you've
> > freed the objalloc. Test it with a coff an aout target too.
> >
>
> I added
>
> abfd->sections = NULL;
> abfd->section_last = NULL;
> abfd->outsymbols = NULL;
> abfd->tdata.any = NULL;
> abfd->usrdata = NULL;
> abfd->memory = NULL;
>
> It doesn't cause any trouble. I checked i386-coff and i386-aout.
>
It breaks "strip -g" on archive. The problem is
1. bfd_close on archive member calls _bfd_write_archive_contents.
2. It calls _bfd_compute_and_write_armap.
3. It calls bfd_free_cached_info on archive member.
4. Then bfd_close calls _bfd_delete_bfd on archive member, which tries
to free the same memory.
I rewrote the patch to limit it to ELF and check memory before free
it.
H.J.
----
bfd/
2006-04-06 H.J. Lu <hongjiu.lu@intel.com>
PR binutils/2467
* elf.c (_bfd_elf_close_and_cleanup): Check elf_tdata (abfd)
is NULL first.
* elf32-arm.c (elf32_arm_close_and_cleanup): Check if
abfd->sections is NULL.
(elf32_arm_bfd_free_cached_info): New.
(bfd_elf32_bfd_free_cached_info): Defined.
* elfxx-target.h (bfd_elfNN_bfd_free_cached_info): Default it
to _bfd_free_cached_info.
* libbfd-in.h (_bfd_free_cached_info): New.
* libbfd: Regenerated.
* opncls.c (_bfd_delete_bfd): Check if abfd->memory is NULL.
(_bfd_free_cached_info): New.
binutils/testsuite/
2006-04-06 H.J. Lu <hongjiu.lu@intel.com>
PR binutils/2467
* binutils-all/objcopy.exp (strip_test): Also test "strip -g"
on archive.
--- binutils/bfd/elf.c.cached 2006-03-16 12:37:42.000000000 -0800
+++ binutils/bfd/elf.c 2006-04-06 16:25:59.000000000 -0700
@@ -7171,7 +7171,7 @@ _bfd_elf_close_and_cleanup (bfd *abfd)
{
if (bfd_get_format (abfd) == bfd_object)
{
- if (elf_shstrtab (abfd) != NULL)
+ if (elf_tdata (abfd) != NULL && elf_shstrtab (abfd) != NULL)
_bfd_elf_strtab_free (elf_shstrtab (abfd));
_bfd_dwarf2_cleanup_debug_info (abfd);
}
--- binutils/bfd/elf32-arm.c.cached 2006-03-16 12:37:42.000000000 -0800
+++ binutils/bfd/elf32-arm.c 2006-04-06 16:27:29.000000000 -0700
@@ -7936,11 +7936,25 @@ unrecord_section_via_map_over_sections (
static bfd_boolean
elf32_arm_close_and_cleanup (bfd * abfd)
{
- bfd_map_over_sections (abfd, unrecord_section_via_map_over_sections, NULL);
+ if (abfd->sections)
+ bfd_map_over_sections (abfd,
+ unrecord_section_via_map_over_sections,
+ NULL);
return _bfd_elf_close_and_cleanup (abfd);
}
+static bfd_boolean
+elf32_arm_bfd_free_cached_info (bfd * abfd)
+{
+ if (abfd->sections)
+ bfd_map_over_sections (abfd,
+ unrecord_section_via_map_over_sections,
+ NULL);
+
+ return _bfd_free_cached_info (abfd);
+}
+
/* Display STT_ARM_TFUNC symbols as functions. */
static void
@@ -8100,6 +8114,7 @@ const struct elf_size_info elf32_arm_siz
#define bfd_elf32_new_section_hook elf32_arm_new_section_hook
#define bfd_elf32_bfd_is_target_special_symbol elf32_arm_is_target_special_symbol
#define bfd_elf32_close_and_cleanup elf32_arm_close_and_cleanup
+#define bfd_elf32_bfd_free_cached_info elf32_arm_bfd_free_cached_info
#define bfd_elf32_bfd_final_link elf32_arm_bfd_final_link
#define elf_backend_get_symbol_type elf32_arm_get_symbol_type
--- binutils/bfd/elfxx-target.h.cached 2006-02-27 15:50:54.000000000 -0800
+++ binutils/bfd/elfxx-target.h 2006-04-06 16:16:35.000000000 -0700
@@ -29,7 +29,9 @@
#ifndef bfd_elfNN_close_and_cleanup
#define bfd_elfNN_close_and_cleanup _bfd_elf_close_and_cleanup
#endif
-#define bfd_elfNN_bfd_free_cached_info _bfd_generic_bfd_free_cached_info
+#ifndef bfd_elfNN_bfd_free_cached_info
+#define bfd_elfNN_bfd_free_cached_info _bfd_free_cached_info
+#endif
#ifndef bfd_elfNN_get_section_contents
#define bfd_elfNN_get_section_contents _bfd_generic_get_section_contents
#endif
--- binutils/bfd/libbfd-in.h.cached 2006-03-16 12:37:43.000000000 -0800
+++ binutils/bfd/libbfd-in.h 2006-04-06 16:14:01.000000000 -0700
@@ -155,6 +155,8 @@ bfd * _bfd_new_bfd
(void);
void _bfd_delete_bfd
(bfd *);
+bfd_boolean _bfd_free_cached_info
+ (bfd *);
bfd_boolean bfd_false
(bfd *ignore);
--- binutils/bfd/opncls.c.cached 2006-03-16 12:37:43.000000000 -0800
+++ binutils/bfd/opncls.c 2006-04-06 16:12:31.000000000 -0700
@@ -115,11 +115,35 @@ _bfd_new_bfd_contained_in (bfd *obfd)
void
_bfd_delete_bfd (bfd *abfd)
{
- bfd_hash_table_free (&abfd->section_htab);
- objalloc_free ((struct objalloc *) abfd->memory);
+ if (abfd->memory)
+ {
+ bfd_hash_table_free (&abfd->section_htab);
+ objalloc_free ((struct objalloc *) abfd->memory);
+ }
free (abfd);
}
+/* Free objalloc memory. */
+
+bfd_boolean
+_bfd_free_cached_info (bfd *abfd)
+{
+ if (abfd->memory)
+ {
+ bfd_hash_table_free (&abfd->section_htab);
+ objalloc_free ((struct objalloc *) abfd->memory);
+
+ abfd->sections = NULL;
+ abfd->section_last = NULL;
+ abfd->outsymbols = NULL;
+ abfd->tdata.any = NULL;
+ abfd->usrdata = NULL;
+ abfd->memory = NULL;
+ }
+
+ return TRUE;
+}
+
/*
SECTION
Opening and closing BFDs
--- binutils/binutils/testsuite/binutils-all/objcopy.exp.cached 2005-10-20 10:06:40.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/objcopy.exp 2006-04-04 07:32:21.000000000 -0700
@@ -359,6 +359,12 @@ proc strip_test { } {
return
}
+ set exec_output [binutils_run $STRIP "-g $archive"]
+ if ![string match "" $exec_output] {
+ fail $test
+ return
+ }
+
set exec_output [binutils_run $STRIP "$STRIPFLAGS $archive"]
if ![string match "" $exec_output] {
fail $test
^ permalink raw reply [flat|nested] 5+ messages in thread