This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
gold patch committed: Serialize Scan_relocs
- From: Ian Lance Taylor <iant at google dot com>
- To: binutils at sourceware dot org
- Date: Thu, 11 Feb 2010 20:35:29 -0800
- Subject: gold patch committed: Serialize Scan_relocs
I noticed that when using threads the output is nondeterministic.
This is because I assumed that there was no ordering requirement on
Scan_relocs tasks. However, Scan_relocs tasks can add PLT and GOT
entries, and the order in which they are added does matter. So they
need to be serialized. The Read_relocs tasks can continue to run in
parallel. I committed this patch to fix the problem.
Ian
2010-02-11 Ian Lance Taylor <iant@google.com>
* gold.cc (queue_middle_gc_tasks): Use a separate blocker for each
Read_relocs task.
(queue_middle_tasks): Likewise, and also for Scan_relocs. Run
Allocate_commons_task first.
* reloc.cc (Read_relocs::run): Pass next_blocker_ down to next
task, rather than symtab_lock_.
(Gc_process_relocs::~Gc_process_relocs): New function.
(Gc_process_relocs::is_runnable): Check this_blocker_.
(Gc_process_relocs::locks): Use next_blocker_ rather than
blocker_.
(Scan_relocs::~Scan_relocs): New function.
(Scan_relocs::is_runnable): Check this_blocker_ rather than
symtab_lock_.
(Scan_relocs::locks): Drop symtab_lock_ and blocker_. Add
next_blocker_.
* reloc.h (class Read_relocs): Drop symtab_lock_ and blocker_
fields. Add this_blocker_ and next_blocker_ fields. Adjust
constructor accordingly.
(class Gc_process_relocs): Likewise.
(class Scan_relocs): Likewise.
* common.h (class Allocate_commons_task): Remove symtab_lock_
field, and corresponding constructor parameter.
* common.cc (Allocate_commons_tasK::is_runnable): Remove use of
symtab_lock_.
(Allocate_commons_task::locks): Likewise.
Index: common.cc
===================================================================
RCS file: /cvs/src/src/gold/common.cc,v
retrieving revision 1.22
diff -p -u -r1.22 common.cc
--- common.cc 31 Dec 2009 05:07:21 -0000 1.22
+++ common.cc 12 Feb 2010 04:25:35 -0000
@@ -1,6 +1,6 @@
// common.cc -- handle common symbols 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.
@@ -36,24 +36,21 @@ namespace gold
// Allocate_commons_task methods.
-// This task allocates the common symbols. We need a lock on the
-// symbol table.
+// This task allocates the common symbols. We arrange to run it
+// before anything else which needs to access the symbol table.
Task_token*
Allocate_commons_task::is_runnable()
{
- if (!this->symtab_lock_->is_writable())
- return this->symtab_lock_;
return NULL;
}
-// Return the locks we hold: one on the symbol table, and one blocker.
+// Release a blocker.
void
Allocate_commons_task::locks(Task_locker* tl)
{
tl->add(this, this->blocker_);
- tl->add(this, this->symtab_lock_);
}
// Allocate the common symbols.
Index: common.h
===================================================================
RCS file: /cvs/src/src/gold/common.h,v
retrieving revision 1.7
diff -p -u -r1.7 common.h
--- common.h 21 May 2008 21:37:44 -0000 1.7
+++ common.h 12 Feb 2010 04:25:35 -0000
@@ -1,6 +1,6 @@
// common.h -- handle common symbols 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.
@@ -36,9 +36,8 @@ class Allocate_commons_task : public Tas
{
public:
Allocate_commons_task(Symbol_table* symtab, Layout* layout, Mapfile* mapfile,
- Task_token* symtab_lock, Task_token* blocker)
- : symtab_(symtab), layout_(layout), mapfile_(mapfile),
- symtab_lock_(symtab_lock), blocker_(blocker)
+ Task_token* blocker)
+ : symtab_(symtab), layout_(layout), mapfile_(mapfile), blocker_(blocker)
{ }
// The standard Task methods.
@@ -60,7 +59,6 @@ class Allocate_commons_task : public Tas
Symbol_table* symtab_;
Layout* layout_;
Mapfile* mapfile_;
- Task_token* symtab_lock_;
Task_token* blocker_;
};
Index: gold.cc
===================================================================
RCS file: /cvs/src/src/gold/gold.cc,v
retrieving revision 1.77
diff -p -u -r1.77 gold.cc
--- gold.cc 11 Feb 2010 07:42:17 -0000 1.77
+++ gold.cc 12 Feb 2010 04:25:35 -0000
@@ -265,21 +265,23 @@ 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);
+ Task_token* this_blocker = NULL;
for (Input_objects::Relobj_iterator p = input_objects->relobj_begin();
p != input_objects->relobj_end();
++p)
- workqueue->queue(new Read_relocs(symtab, layout, *p, symtab_lock,
- blocker));
-
+ {
+ Task_token* next_blocker = new Task_token(true);
+ next_blocker->add_blocker();
+ workqueue->queue(new Read_relocs(symtab, layout, *p, this_blocker,
+ next_blocker));
+ this_blocker = next_blocker;
+ }
workqueue->queue(new Task_function(new Middle_runner(options,
input_objects,
symtab,
layout,
mapfile),
- blocker,
+ this_blocker,
"Task_function Middle_runner"));
}
@@ -475,12 +477,18 @@ queue_middle_tasks(const General_options
// Make sure we have symbols for any required group signatures.
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* this_blocker = NULL;
- Task_token* symtab_lock = new Task_token(false);
+ // Allocate common symbols. We use a blocker to run this before the
+ // Scan_relocs tasks, because it writes to the symbol table just as
+ // they do.
+ if (parameters->options().define_common())
+ {
+ this_blocker = new Task_token(true);
+ this_blocker->add_blocker();
+ workqueue->queue(new Allocate_commons_task(symtab, layout, mapfile,
+ this_blocker));
+ }
// If doing garbage collection, the relocations have already been read.
// Otherwise, read and scan the relocations.
@@ -490,9 +498,14 @@ queue_middle_tasks(const General_options
for (Input_objects::Relobj_iterator p = input_objects->relobj_begin();
p != input_objects->relobj_end();
++p)
- workqueue->queue(new Scan_relocs(symtab, layout, *p,
- (*p)->get_relocs_data(),
- symtab_lock, blocker));
+ {
+ Task_token* next_blocker = new Task_token(true);
+ next_blocker->add_blocker();
+ workqueue->queue(new Scan_relocs(symtab, layout, *p,
+ (*p)->get_relocs_data(),
+ this_blocker, next_blocker));
+ this_blocker = next_blocker;
+ }
}
else
{
@@ -511,22 +524,14 @@ queue_middle_tasks(const General_options
p != input_objects->relobj_end();
++p)
{
- // We can read and process the relocations in any order. But we
- // only want one task to write to the symbol table at a time.
- // 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.
- workqueue->queue(new Read_relocs(symtab, layout, *p, symtab_lock,
- blocker));
+ Task_token* next_blocker = new Task_token(true);
+ next_blocker->add_blocker();
+ workqueue->queue(new Read_relocs(symtab, layout, *p, this_blocker,
+ next_blocker));
+ this_blocker = next_blocker;
}
}
- // Allocate common symbols. This requires write access to the
- // symbol table, but is independent of the relocation processing.
- if (parameters->options().define_common())
- 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.
// TODO(csilvers): figure out a more principled way to get the target
@@ -537,7 +542,7 @@ queue_middle_tasks(const General_options
target,
layout,
mapfile),
- blocker,
+ this_blocker,
"Task_function Layout_task_runner"));
}
Index: reloc.cc
===================================================================
RCS file: /cvs/src/src/gold/reloc.cc,v
retrieving revision 1.53
diff -p -u -r1.53 reloc.cc
--- reloc.cc 14 Dec 2009 19:53:05 -0000 1.53
+++ reloc.cc 12 Feb 2010 04:25:35 -0000
@@ -1,6 +1,6 @@
// reloc.cc -- relocate input files for gold.
-// Copyright 2006, 2007, 2008, 2009 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.
@@ -75,15 +75,15 @@ Read_relocs::run(Workqueue* workqueue)
workqueue->queue_next(new Gc_process_relocs(this->symtab_,
this->layout_,
this->object_, rd,
- this->symtab_lock_,
- this->blocker_));
+ this->this_blocker_,
+ this->next_blocker_));
}
else
{
workqueue->queue_next(new Scan_relocs(this->symtab_, this->layout_,
this->object_, rd,
- this->symtab_lock_,
- this->blocker_));
+ this->this_blocker_,
+ this->next_blocker_));
}
}
@@ -97,13 +97,22 @@ Read_relocs::get_name() const
// Gc_process_relocs methods.
-// These tasks process the relocations read by Read_relocs and
+Gc_process_relocs::~Gc_process_relocs()
+{
+ if (this->this_blocker_ != NULL)
+ delete this->this_blocker_;
+}
+
+// These tasks process the relocations read by Read_relocs and
// determine which sections are referenced and which are garbage.
-// This task is done only when --gc-sections is used.
+// This task is done only when --gc-sections is used. This is blocked
+// by THIS_BLOCKER_. It unblocks NEXT_BLOCKER_.
Task_token*
Gc_process_relocs::is_runnable()
{
+ if (this->this_blocker_ != NULL && this->this_blocker_->is_blocked())
+ return this->this_blocker_;
if (this->object_->is_locked())
return this->object_->token();
return NULL;
@@ -113,7 +122,7 @@ void
Gc_process_relocs::locks(Task_locker* tl)
{
tl->add(this, this->object_->token());
- tl->add(this, this->blocker_);
+ tl->add(this, this->next_blocker_);
}
void
@@ -133,6 +142,12 @@ Gc_process_relocs::get_name() const
// Scan_relocs methods.
+Scan_relocs::~Scan_relocs()
+{
+ if (this->this_blocker_ != NULL)
+ delete this->this_blocker_;
+}
+
// These tasks scan the relocations read by Read_relocs and mark up
// the symbol table to indicate which relocations are required. We
// use a lock on the symbol table to keep them from interfering with
@@ -141,8 +156,8 @@ Gc_process_relocs::get_name() const
Task_token*
Scan_relocs::is_runnable()
{
- if (!this->symtab_lock_->is_writable())
- return this->symtab_lock_;
+ if (this->this_blocker_ != NULL && this->this_blocker_->is_blocked())
+ return this->this_blocker_;
if (this->object_->is_locked())
return this->object_->token();
return NULL;
@@ -155,8 +170,7 @@ void
Scan_relocs::locks(Task_locker* tl)
{
tl->add(this, this->object_->token());
- tl->add(this, this->symtab_lock_);
- tl->add(this, this->blocker_);
+ tl->add(this, this->next_blocker_);
}
// Scan the relocs.
Index: reloc.h
===================================================================
RCS file: /cvs/src/src/gold/reloc.h,v
retrieving revision 1.28
diff -p -u -r1.28 reloc.h
--- reloc.h 14 Dec 2009 19:53:05 -0000 1.28
+++ reloc.h 12 Feb 2010 04:25:35 -0000
@@ -1,6 +1,6 @@
// reloc.h -- relocate input files for gold -*- C++ -*-
-// Copyright 2006, 2007, 2008, 2009 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.
@@ -62,12 +62,13 @@ class Output_data_reloc;
class Read_relocs : public Task
{
public:
- // SYMTAB_LOCK is used to lock the symbol table. BLOCKER should be
- // unblocked when the Scan_relocs task completes.
+ // THIS_BLOCKER and NEXT_BLOCKER are passed along to a Scan_relocs
+ // or Gc_process_relocs task, so that they run in a deterministic
+ // order.
Read_relocs(Symbol_table* symtab, Layout* layout, Relobj* object,
- Task_token* symtab_lock, Task_token* blocker)
+ Task_token* this_blocker, Task_token* next_blocker)
: symtab_(symtab), layout_(layout), object_(object),
- symtab_lock_(symtab_lock), blocker_(blocker)
+ this_blocker_(this_blocker), next_blocker_(next_blocker)
{ }
// The standard Task methods.
@@ -88,8 +89,8 @@ class Read_relocs : public Task
Symbol_table* symtab_;
Layout* layout_;
Relobj* object_;
- Task_token* symtab_lock_;
- Task_token* blocker_;
+ Task_token* this_blocker_;
+ Task_token* next_blocker_;
};
// Process the relocs to figure out which sections are garbage.
@@ -98,15 +99,18 @@ class Read_relocs : public Task
class Gc_process_relocs : public Task
{
public:
- // SYMTAB_LOCK is used to lock the symbol table. BLOCKER should be
- // unblocked when the task completes.
+ // THIS_BLOCKER prevents this task from running until the previous
+ // one is finished. NEXT_BLOCKER prevents the next task from
+ // running.
Gc_process_relocs(Symbol_table* symtab, Layout* layout, Relobj* object,
- Read_relocs_data* rd, Task_token* symtab_lock,
- Task_token* blocker)
+ Read_relocs_data* rd, Task_token* this_blocker,
+ Task_token* next_blocker)
: symtab_(symtab), layout_(layout), object_(object), rd_(rd),
- symtab_lock_(symtab_lock), blocker_(blocker)
+ this_blocker_(this_blocker), next_blocker_(next_blocker)
{ }
+ ~Gc_process_relocs();
+
// The standard Task methods.
Task_token*
@@ -126,8 +130,8 @@ class Gc_process_relocs : public Task
Layout* layout_;
Relobj* object_;
Read_relocs_data* rd_;
- Task_token* symtab_lock_;
- Task_token* blocker_;
+ Task_token* this_blocker_;
+ Task_token* next_blocker_;
};
// Scan the relocations for an object to see if they require any
@@ -136,15 +140,18 @@ class Gc_process_relocs : public Task
class Scan_relocs : public Task
{
public:
- // SYMTAB_LOCK is used to lock the symbol table. BLOCKER should be
- // unblocked when the task completes.
+ // THIS_BLOCKER prevents this task from running until the previous
+ // one is finished. NEXT_BLOCKER prevents the next task from
+ // running.
Scan_relocs(Symbol_table* symtab, Layout* layout, Relobj* object,
- Read_relocs_data* rd, Task_token* symtab_lock,
- Task_token* blocker)
+ Read_relocs_data* rd, Task_token* this_blocker,
+ Task_token* next_blocker)
: symtab_(symtab), layout_(layout), object_(object), rd_(rd),
- symtab_lock_(symtab_lock), blocker_(blocker)
+ this_blocker_(this_blocker), next_blocker_(next_blocker)
{ }
+ ~Scan_relocs();
+
// The standard Task methods.
Task_token*
@@ -164,8 +171,8 @@ class Scan_relocs : public Task
Layout* layout_;
Relobj* object_;
Read_relocs_data* rd_;
- Task_token* symtab_lock_;
- Task_token* blocker_;
+ Task_token* this_blocker_;
+ Task_token* next_blocker_;
};
// A class to perform all the relocations for an object file.