This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
gold patch committed: Fix race condition with --threads
- From: Ian Lance Taylor <iant at google dot com>
- To: binutils at sourceware dot org
- Cc: ppluzhnikov at google dot com
- Date: Wed, 10 Feb 2010 23:44:19 -0800
- Subject: gold patch committed: Fix race condition with --threads
Paul Pluzhnikov, with some effort, managed to produce a repeatable
race condition when using gold --threads. After poring over it for a
while, I realized that I had idiotically assumed that I could
increment the number of blockers on a token even after queuing jobs to
start. The race occurred when the jobs finished while I was still
incrementing the number of blockers. I committed this patch to fix
the problem.
Ian
2010-02-10 Ian Lance Taylor <iant@google.com>
* dirsearch.cc (Dirsearch::initialize): Add all blockers before
queueing any tasks.
* gold.cc (queue_middle_gc_tasks): Likewise. Fix final blocker.
(queue_middle_tasks): Add all blockers before queueing any tasks.
(queue_final_tasks): Likewise.
* token.h (Task_token::add_blockers): New function.
* object.h (Input_objects::number_of_relobjs): New function.
? autom4te.cache
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gold/ChangeLog,v
retrieving revision 1.494
diff -p -u -r1.494 ChangeLog
--- ChangeLog 11 Feb 2010 07:21:44 -0000 1.494
+++ ChangeLog 11 Feb 2010 07:42:07 -0000
@@ -1,5 +1,15 @@
2010-02-10 Ian Lance Taylor <iant@google.com>
+ * dirsearch.cc (Dirsearch::initialize): Add all blockers before
+ queueing any tasks.
+ * gold.cc (queue_middle_gc_tasks): Likewise. Fix final blocker.
+ (queue_middle_tasks): Add all blockers before queueing any tasks.
+ (queue_final_tasks): Likewise.
+ * token.h (Task_token::add_blockers): New function.
+ * object.h (Input_objects::number_of_relobjs): New function.
+
+2010-02-10 Ian Lance Taylor <iant@google.com>
+
* i386.cc (Relocate::relocate_tls): A local symbol is final if not
shared, not if not position independent.
* x86_64.cc (Relocate::relocate_tls): Likewise.
Index: dirsearch.cc
===================================================================
RCS file: /cvs/src/src/gold/dirsearch.cc,v
retrieving revision 1.11
diff -p -u -r1.11 dirsearch.cc
--- dirsearch.cc 14 Mar 2009 05:56:46 -0000 1.11
+++ dirsearch.cc 11 Feb 2010 07:42:07 -0000
@@ -1,6 +1,6 @@
// dirsearch.cc -- directory searching for gold
-// Copyright 2006, 2007, 2008 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
// Written by Ian Lance Taylor <iant@google.com>.
// This file is part of gold.
@@ -229,13 +229,11 @@ Dirsearch::initialize(Workqueue* workque
gold_assert(caches == NULL);
caches = new Dir_caches;
this->directories_ = directories;
+ this->token_.add_blockers(directories->size());
for (General_options::Dir_list::const_iterator p = directories->begin();
p != directories->end();
++p)
- {
- this->token_.add_blocker();
- workqueue->queue(new Dir_cache_task(p->name().c_str(), this->token_));
- }
+ workqueue->queue(new Dir_cache_task(p->name().c_str(), this->token_));
}
// Search for a file. NOTE: we only log failed file-lookup attempts
Index: gold.cc
===================================================================
RCS file: /cvs/src/src/gold/gold.cc,v
retrieving revision 1.76
diff -p -u -r1.76 gold.cc
--- gold.cc 6 Jan 2010 05:30:24 -0000 1.76
+++ gold.cc 11 Feb 2010 07:42:08 -0000
@@ -266,24 +266,20 @@ queue_middle_gc_tasks(const General_opti
// Read_relocs for all the objects must be done and processed to find
// unused sections before any scanning of the relocs can take place.
Task_token* blocker = new Task_token(true);
+ blocker->add_blockers(input_objects->number_of_relobjs());
Task_token* symtab_lock = new Task_token(false);
for (Input_objects::Relobj_iterator p = input_objects->relobj_begin();
p != input_objects->relobj_end();
++p)
- {
- // We can read and process the relocations in any order.
- blocker->add_blocker();
- workqueue->queue(new Read_relocs(symtab, layout, *p, symtab_lock,
- blocker));
- }
+ workqueue->queue(new Read_relocs(symtab, layout, *p, symtab_lock,
+ blocker));
- Task_token* this_blocker = new Task_token(true);
workqueue->queue(new Task_function(new Middle_runner(options,
input_objects,
symtab,
layout,
mapfile),
- this_blocker,
+ blocker,
"Task_function Middle_runner"));
}
@@ -480,6 +476,10 @@ queue_middle_tasks(const General_options
layout->define_group_signatures(symtab);
Task_token* blocker = new Task_token(true);
+ blocker->add_blockers(input_objects->number_of_relobjs());
+ if (parameters->options().define_common())
+ blocker->add_blocker();
+
Task_token* symtab_lock = new Task_token(false);
// If doing garbage collection, the relocations have already been read.
@@ -490,12 +490,9 @@ queue_middle_tasks(const General_options
for (Input_objects::Relobj_iterator p = input_objects->relobj_begin();
p != input_objects->relobj_end();
++p)
- {
- blocker->add_blocker();
- workqueue->queue(new Scan_relocs(symtab, layout, *p,
- (*p)->get_relocs_data(),
- symtab_lock, blocker));
- }
+ workqueue->queue(new Scan_relocs(symtab, layout, *p,
+ (*p)->get_relocs_data(),
+ symtab_lock, blocker));
}
else
{
@@ -519,7 +516,6 @@ queue_middle_tasks(const General_options
// So we queue up a task for each object to read the
// relocations. That task will in turn queue a task to wait
// until it can write to the symbol table.
- blocker->add_blocker();
workqueue->queue(new Read_relocs(symtab, layout, *p, symtab_lock,
blocker));
}
@@ -528,11 +524,8 @@ queue_middle_tasks(const General_options
// Allocate common symbols. This requires write access to the
// symbol table, but is independent of the relocation processing.
if (parameters->options().define_common())
- {
- blocker->add_blocker();
- workqueue->queue(new Allocate_commons_task(symtab, layout, mapfile,
- symtab_lock, blocker));
- }
+ workqueue->queue(new Allocate_commons_task(symtab, layout, mapfile,
+ symtab_lock, blocker));
// When all those tasks are complete, we can start laying out the
// output file.
@@ -570,17 +563,26 @@ queue_final_tasks(const General_options&
// written out.
Task_token* input_sections_blocker = NULL;
if (!any_postprocessing_sections)
- input_sections_blocker = new Task_token(true);
+ {
+ input_sections_blocker = new Task_token(true);
+ input_sections_blocker->add_blockers(input_objects->number_of_relobjs());
+ }
// Use a blocker to block any objects which have to wait for the
// output sections to complete before they can apply relocations.
Task_token* output_sections_blocker = new Task_token(true);
+ output_sections_blocker->add_blocker();
// Use a blocker to block the final cleanup task.
Task_token* final_blocker = new Task_token(true);
+ // Write_symbols_task, Write_sections_task, Write_data_task,
+ // Relocate_tasks.
+ final_blocker->add_blockers(3);
+ final_blocker->add_blockers(input_objects->number_of_relobjs());
+ if (!any_postprocessing_sections)
+ final_blocker->add_blocker();
// Queue a task to write out the symbol table.
- final_blocker->add_blocker();
workqueue->queue(new Write_symbols_task(layout,
symtab,
input_objects,
@@ -590,13 +592,10 @@ queue_final_tasks(const General_options&
final_blocker));
// Queue a task to write out the output sections.
- output_sections_blocker->add_blocker();
- final_blocker->add_blocker();
workqueue->queue(new Write_sections_task(layout, of, output_sections_blocker,
final_blocker));
// Queue a task to write out everything else.
- final_blocker->add_blocker();
workqueue->queue(new Write_data_task(layout, symtab, of, final_blocker));
// Queue a task for each input object to relocate the sections and
@@ -604,15 +603,10 @@ queue_final_tasks(const General_options&
for (Input_objects::Relobj_iterator p = input_objects->relobj_begin();
p != input_objects->relobj_end();
++p)
- {
- if (input_sections_blocker != NULL)
- input_sections_blocker->add_blocker();
- final_blocker->add_blocker();
- workqueue->queue(new Relocate_task(symtab, layout, *p, of,
- input_sections_blocker,
- output_sections_blocker,
- final_blocker));
- }
+ workqueue->queue(new Relocate_task(symtab, layout, *p, of,
+ input_sections_blocker,
+ output_sections_blocker,
+ final_blocker));
// Queue a task to write out the output sections which depend on
// input sections. If there are any sections which require
@@ -620,7 +614,6 @@ queue_final_tasks(const General_options&
// the output file.
if (!any_postprocessing_sections)
{
- final_blocker->add_blocker();
Task* t = new Write_after_input_sections_task(layout, of,
input_sections_blocker,
final_blocker);
Index: object.h
===================================================================
RCS file: /cvs/src/src/gold/object.h,v
retrieving revision 1.91
diff -p -u -r1.91 object.h
--- object.h 29 Jan 2010 18:23:18 -0000 1.91
+++ object.h 11 Feb 2010 07:42:08 -0000
@@ -2009,6 +2009,11 @@ class Input_objects
any_dynamic() const
{ return !this->dynobj_list_.empty(); }
+ // Return the number of non dynamic objects.
+ int
+ number_of_relobjs() const
+ { return this->relobj_list_.size(); }
+
// Return the number of input objects.
int
number_of_input_objects() const
Index: token.h
===================================================================
RCS file: /cvs/src/src/gold/token.h,v
retrieving revision 1.5
diff -p -u -r1.5 token.h
--- token.h 14 Dec 2009 19:53:05 -0000 1.5
+++ token.h 11 Feb 2010 07:42:08 -0000
@@ -1,6 +1,6 @@
// token.h -- lock tokens for gold -*- C++ -*-
-// Copyright 2006, 2007, 2008 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2010 Free Software Foundation, Inc.
// Written by Ian Lance Taylor <iant@google.com>.
// This file is part of gold.
@@ -144,6 +144,15 @@ class Task_token
this->writer_ = NULL;
}
+ // Add some number of blockers to the token.
+ void
+ add_blockers(int c)
+ {
+ gold_assert(this->is_blocker_);
+ this->blockers_ += c;
+ this->writer_ = NULL;
+ }
+
// Remove a blocker from the token. Returns true if block count
// drops to zero.
bool