commit 2cfbf2fece582c29df348104b28677c38a8301f4 Author: Cary Coutant Date: Tue Feb 3 19:54:57 2015 -0800 Fix a file descriptor leak in gold. When an LTO linker plugin claims an external member of a thin archive, gold does not properly unlock the file and make its file descriptor available for reuse. This patch fixes the problem by modifying Archive::include_member to unlock the object file via an RAII class instance, ensuring that it will be unlocked no matter what path is taken through the function. gold/ PR gold/15660 * archive.cc (Thin_archive_object_unlocker): New class. (Archive::include_member): Unlock external members of thin archives. * testsuite/Makefile.am (plugin_test_1): Rename .syms files. (plugin_test_2): Likewise. (plugin_test_3): Likewise. (plugin_test_4): Likewise. (plugin_test_5): Likewise. (plugin_test_6): Likewise. (plugin_test_7): Likewise. (plugin_test_8): Likewise. (plugin_test_9): Likewise. (plugin_test_10): Likewise. (plugin_test_11): New test case. * testsuite/Makefile.in: Regenerate. * testsuite/plugin_test.c (claim_file_hook): Check for parallel .syms file to decide whether to claim file. (all_symbols_read_hook): Likewise. * testsuite/plugin_test_1.sh: Adjust expected output. * testsuite/plugin_test_2.sh: Likewise. * testsuite/plugin_test_3.sh: Likewise. * testsuite/plugin_test_6.sh: Likewise. * testsuite/plugin_test_tls.sh: Likewise. * testsuite/plugin_test_11.sh: New testcase. diff --git a/gold/archive.cc b/gold/archive.cc index 69107f5..6d25980 100644 --- a/gold/archive.cc +++ b/gold/archive.cc @@ -930,6 +930,32 @@ Archive::count_members() return ret; } +// RAII class to ensure we unlock the object if it's a member of a +// thin archive. We can't use Task_lock_obj in Archive::include_member +// because the object file is already locked when it's opened by +// get_elf_object_for_member. + +class Thin_archive_object_unlocker +{ + public: + Thin_archive_object_unlocker(const Task *task, Object* obj) + : task_(task), obj_(obj) + { } + + ~Thin_archive_object_unlocker() + { + if (this->obj_->offset() == 0) + this->obj_->unlock(this->task_); + } + + private: + Thin_archive_object_unlocker(const Thin_archive_object_unlocker&); + Thin_archive_object_unlocker& operator=(const Thin_archive_object_unlocker&); + + const Task* task_; + Object* obj_; +}; + // Include an archive member in the link. OFF is the file offset of // the member header. WHY is the reason we are including this member. // Return true if we added the member or if we had an error, return @@ -978,6 +1004,10 @@ Archive::include_member(Symbol_table* symtab, Layout* layout, return unconfigured ? false : true; } + // If the object is an external member of a thin archive, + // unlock it when we're done here. + Thin_archive_object_unlocker unlocker(this->task_, obj); + if (mapfile != NULL) mapfile->report_include_archive_member(obj->name(), sym, why); @@ -991,31 +1021,21 @@ Archive::include_member(Symbol_table* symtab, Layout* layout, if (!input_objects->add_object(obj)) { - // If this is an external member of a thin archive, unlock the - // file. - if (obj->offset() == 0) - obj->unlock(this->task_); delete obj; + return true; } - else - { - { - if (layout->incremental_inputs() != NULL) - layout->incremental_inputs()->report_object(obj, 0, this, NULL); - 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. - if (obj->offset() == 0) - obj->unlock(this->task_); - this->included_member_ = true; - } + if (layout->incremental_inputs() != NULL) + layout->incremental_inputs()->report_object(obj, 0, this, NULL); + + { + Read_symbols_data sd; + obj->read_symbols(&sd); + obj->layout(symtab, layout, &sd); + obj->add_symbols(symtab, &sd, layout); + } + this->included_member_ = true; return true; }