Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1134)

Unified Diff: third_party/binutils/unlock-thin.patch

Issue 899783003: Gold: Patch linker to fix LTO related bugs. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « third_party/binutils/plugin-dso-fix.patch ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/binutils/unlock-thin.patch
diff --git a/third_party/binutils/unlock-thin.patch b/third_party/binutils/unlock-thin.patch
new file mode 100644
index 0000000000000000000000000000000000000000..6a4b6a7c98426044151c3fc9926315cf789db6b7
--- /dev/null
+++ b/third_party/binutils/unlock-thin.patch
@@ -0,0 +1,129 @@
+commit 2cfbf2fece582c29df348104b28677c38a8301f4
+Author: Cary Coutant <ccoutant@google.com>
+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;
+ }
+
« no previous file with comments | « third_party/binutils/plugin-dso-fix.patch ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698