James Robinson | 7b766f4 | 2015-02-06 15:14:04 -0800 | [diff] [blame] | 1 | commit 2cfbf2fece582c29df348104b28677c38a8301f4 |
| 2 | Author: Cary Coutant <ccoutant@google.com> |
| 3 | Date: Tue Feb 3 19:54:57 2015 -0800 |
| 4 | |
| 5 | Fix a file descriptor leak in gold. |
| 6 | |
| 7 | When an LTO linker plugin claims an external member of a thin archive, gold |
| 8 | does not properly unlock the file and make its file descriptor available for |
| 9 | reuse. This patch fixes the problem by modifying Archive::include_member to |
| 10 | unlock the object file via an RAII class instance, ensuring that it will be |
| 11 | unlocked no matter what path is taken through the function. |
| 12 | |
| 13 | gold/ |
| 14 | PR gold/15660 |
| 15 | * archive.cc (Thin_archive_object_unlocker): New class. |
| 16 | (Archive::include_member): Unlock external members of thin archives. |
| 17 | * testsuite/Makefile.am (plugin_test_1): Rename .syms files. |
| 18 | (plugin_test_2): Likewise. |
| 19 | (plugin_test_3): Likewise. |
| 20 | (plugin_test_4): Likewise. |
| 21 | (plugin_test_5): Likewise. |
| 22 | (plugin_test_6): Likewise. |
| 23 | (plugin_test_7): Likewise. |
| 24 | (plugin_test_8): Likewise. |
| 25 | (plugin_test_9): Likewise. |
| 26 | (plugin_test_10): Likewise. |
| 27 | (plugin_test_11): New test case. |
| 28 | * testsuite/Makefile.in: Regenerate. |
| 29 | * testsuite/plugin_test.c (claim_file_hook): Check for parallel .syms |
| 30 | file to decide whether to claim file. |
| 31 | (all_symbols_read_hook): Likewise. |
| 32 | * testsuite/plugin_test_1.sh: Adjust expected output. |
| 33 | * testsuite/plugin_test_2.sh: Likewise. |
| 34 | * testsuite/plugin_test_3.sh: Likewise. |
| 35 | * testsuite/plugin_test_6.sh: Likewise. |
| 36 | * testsuite/plugin_test_tls.sh: Likewise. |
| 37 | * testsuite/plugin_test_11.sh: New testcase. |
| 38 | |
| 39 | diff --git a/gold/archive.cc b/gold/archive.cc |
| 40 | index 69107f5..6d25980 100644 |
| 41 | --- a/gold/archive.cc |
| 42 | +++ b/gold/archive.cc |
| 43 | @@ -930,6 +930,32 @@ Archive::count_members() |
| 44 | return ret; |
| 45 | } |
| 46 | |
| 47 | +// RAII class to ensure we unlock the object if it's a member of a |
| 48 | +// thin archive. We can't use Task_lock_obj in Archive::include_member |
| 49 | +// because the object file is already locked when it's opened by |
| 50 | +// get_elf_object_for_member. |
| 51 | + |
| 52 | +class Thin_archive_object_unlocker |
| 53 | +{ |
| 54 | + public: |
| 55 | + Thin_archive_object_unlocker(const Task *task, Object* obj) |
| 56 | + : task_(task), obj_(obj) |
| 57 | + { } |
| 58 | + |
| 59 | + ~Thin_archive_object_unlocker() |
| 60 | + { |
| 61 | + if (this->obj_->offset() == 0) |
| 62 | + this->obj_->unlock(this->task_); |
| 63 | + } |
| 64 | + |
| 65 | + private: |
| 66 | + Thin_archive_object_unlocker(const Thin_archive_object_unlocker&); |
| 67 | + Thin_archive_object_unlocker& operator=(const Thin_archive_object_unlocker&); |
| 68 | + |
| 69 | + const Task* task_; |
| 70 | + Object* obj_; |
| 71 | +}; |
| 72 | + |
| 73 | // Include an archive member in the link. OFF is the file offset of |
| 74 | // the member header. WHY is the reason we are including this member. |
| 75 | // Return true if we added the member or if we had an error, return |
| 76 | @@ -978,6 +1004,10 @@ Archive::include_member(Symbol_table* symtab, Layout* layout, |
| 77 | return unconfigured ? false : true; |
| 78 | } |
| 79 | |
| 80 | + // If the object is an external member of a thin archive, |
| 81 | + // unlock it when we're done here. |
| 82 | + Thin_archive_object_unlocker unlocker(this->task_, obj); |
| 83 | + |
| 84 | if (mapfile != NULL) |
| 85 | mapfile->report_include_archive_member(obj->name(), sym, why); |
| 86 | |
| 87 | @@ -991,31 +1021,21 @@ Archive::include_member(Symbol_table* symtab, Layout* layout, |
| 88 | |
| 89 | if (!input_objects->add_object(obj)) |
| 90 | { |
| 91 | - // If this is an external member of a thin archive, unlock the |
| 92 | - // file. |
| 93 | - if (obj->offset() == 0) |
| 94 | - obj->unlock(this->task_); |
| 95 | delete obj; |
| 96 | + return true; |
| 97 | } |
| 98 | - else |
| 99 | - { |
| 100 | - { |
| 101 | - if (layout->incremental_inputs() != NULL) |
| 102 | - layout->incremental_inputs()->report_object(obj, 0, this, NULL); |
| 103 | - Read_symbols_data sd; |
| 104 | - obj->read_symbols(&sd); |
| 105 | - obj->layout(symtab, layout, &sd); |
| 106 | - obj->add_symbols(symtab, &sd, layout); |
| 107 | - } |
| 108 | - |
| 109 | - // If this is an external member of a thin archive, unlock the file |
| 110 | - // for the next task. |
| 111 | - if (obj->offset() == 0) |
| 112 | - obj->unlock(this->task_); |
| 113 | |
| 114 | - this->included_member_ = true; |
| 115 | - } |
| 116 | + if (layout->incremental_inputs() != NULL) |
| 117 | + layout->incremental_inputs()->report_object(obj, 0, this, NULL); |
| 118 | + |
| 119 | + { |
| 120 | + Read_symbols_data sd; |
| 121 | + obj->read_symbols(&sd); |
| 122 | + obj->layout(symtab, layout, &sd); |
| 123 | + obj->add_symbols(symtab, &sd, layout); |
| 124 | + } |
| 125 | |
| 126 | + this->included_member_ = true; |
| 127 | return true; |
| 128 | } |
| 129 | |