[gold][patch] Cleanup more mmap leakage

Cary Coutant ccoutant@google.com
Sat Mar 27 01:30:00 GMT 2010


This patch fixes some more mmap leakage that I've found in processing
thin archives, shared libraries, and during garbage collection. As far
as I can tell now, --no-keep-files-mapped now releases all the memory
it should between passes. These fixes reduce gold's maximum memory
usage slightly, but didn't make any significant performance difference
on the tests I ran.

In most cases, gold is careful to explicitly delete File_view objects
as soon as they are no longer needed, but there were unusual code
paths (e.g., when a shared library is added a second time) that were
missing those explicit delete statements. I chose to add destructors
to make sure that all File_view members are deleted when
Read_symbols_data and Read_relocs_data objects are destroyed, and to
make sure that the Read_symbols_data and Read_relocs_data objects are
destroyed before releasing the associated file (so that
File_read::clear_views would unmap the regions). I didn't bother to
remove the places where the File_view objects were explicitly deleted,
since in those places it's still better to delete them as early as
possible.

No regressions on x86_64. OK to checkin?

-cary

        * archive.cc (include_member): Destroy Read_symbols_data object before
        releasing file.
        * object.cc (Read_symbols_data::~Read_symbols_data) New destructor.
        * object.h (Read_symbols_data::Read_symbols_data) New constructor.
        (Read_symbols_data::~Read_symbols_data) New destructor.
        (Section_relocs::Section_relocs) New constructor.
        (Section_relocs::~Section_relocs) New destructor.
        (Read_relocs_data::Read_relocs_data) New constructor.
        (Read_relocs_data::~Read_relocs_data) New destructor.
        * testsuite/binary_unittest.cc (Sized_binary_test): Set sd member
        pointers to NULL after deleting.
-------------- next part --------------
Index: archive.cc
===================================================================
RCS file: /cvs/src/src/gold/archive.cc,v
retrieving revision 1.52
diff -u -p -r1.52 archive.cc
--- archive.cc	22 Mar 2010 14:18:24 -0000	1.52
+++ archive.cc	26 Mar 2010 22:56:22 -0000
@@ -877,10 +877,12 @@ Archive::include_member(Symbol_table* sy
     delete obj;
   else
     {
-      Read_symbols_data sd;
-      obj->read_symbols(&sd);
-      obj->layout(symtab, layout, &sd);
-      obj->add_symbols(symtab, &sd, layout);
+      {
+	Read_symbols_data sd;
+	obj->read_symbols(&sd);
+	obj->layout(symtab, layout, &sd);
+	obj->add_symbols(symtab, &sd, layout);
+      }
 
       // If this is an external member of a thin archive, unlock the file
       // for the next task.
Index: object.cc
===================================================================
RCS file: /cvs/src/src/gold/object.cc,v
retrieving revision 1.121
diff -u -p -r1.121 object.cc
--- object.cc	22 Mar 2010 14:18:24 -0000	1.121
+++ object.cc	26 Mar 2010 22:56:22 -0000
@@ -43,6 +43,28 @@
 namespace gold
 {
 
+// Struct Read_symbols_data.
+
+// Destroy any remaining File_view objects.
+
+Read_symbols_data::~Read_symbols_data()
+{
+  if (this->section_headers != NULL)
+    delete this->section_headers;
+  if (this->section_names != NULL)
+    delete this->section_names;
+  if (this->symbols != NULL)
+    delete this->symbols;
+  if (this->symbol_names != NULL)
+    delete this->symbol_names;
+  if (this->versym != NULL)
+    delete this->versym;
+  if (this->verdef != NULL)
+    delete this->verdef;
+  if (this->verneed != NULL)
+    delete this->verneed;
+}
+
 // Class Xindex.
 
 // Initialize the symtab_xindex_ array.  Find the SHT_SYMTAB_SHNDX
Index: object.h
===================================================================
RCS file: /cvs/src/src/gold/object.h,v
retrieving revision 1.94
diff -u -p -r1.94 object.h
--- object.h	22 Mar 2010 14:18:24 -0000	1.94
+++ object.h	26 Mar 2010 22:56:22 -0000
@@ -55,6 +55,13 @@ class Stringpool_template;
 
 struct Read_symbols_data
 {
+  Read_symbols_data()
+    : section_headers(NULL), section_names(NULL), symbols(NULL),
+      symbol_names(NULL), versym(NULL), verdef(NULL), verneed(NULL)
+  { }
+
+  ~Read_symbols_data();
+
   // Section headers.
   File_view* section_headers;
   // Section names.
@@ -102,6 +109,13 @@ struct Symbol_location_info
 
 struct Section_relocs
 {
+  Section_relocs()
+    : contents(NULL)
+  { }
+
+  ~Section_relocs()
+  { delete this->contents; }
+
   // Index of reloc section.
   unsigned int reloc_shndx;
   // Index of section that relocs apply to.
@@ -125,6 +139,13 @@ struct Section_relocs
 
 struct Read_relocs_data
 {
+  Read_relocs_data()
+    : local_symbols(NULL)
+  { }
+
+  ~Read_relocs_data()
+  { delete this->local_symbols; }
+
   typedef std::vector<Section_relocs> Relocs_list;
   // The relocations.
   Relocs_list relocs;
Index: testsuite/binary_unittest.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/binary_unittest.cc,v
retrieving revision 1.7
diff -u -p -r1.7 binary_unittest.cc
--- testsuite/binary_unittest.cc	30 Sep 2009 22:21:13 -0000	1.7
+++ testsuite/binary_unittest.cc	26 Mar 2010 22:56:22 -0000
@@ -87,9 +87,13 @@ Sized_binary_test()
   Read_symbols_data sd;
   object->read_symbols(&sd);
   delete sd.section_headers;
+  sd.section_headers = NULL;
   delete sd.section_names;
+  sd.section_names = NULL;
   delete sd.symbols;
+  sd.symbols = NULL;
   delete sd.symbol_names;
+  sd.symbol_names = NULL;
 
   Sized_relobj<size, big_endian>* relobj =
     static_cast<Sized_relobj<size, big_endian>*>(object);


More information about the Binutils mailing list