This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
gold patch committed: More thread safety patches
- From: Ian Lance Taylor <iant at google dot com>
- To: binutils at sourceware dot org
- Date: Thu, 11 Feb 2010 19:30:49 -0800
- Subject: gold patch committed: More thread safety patches
I committed this set of patches to make gold --threads clean with
respect to Thread Sanitizer
(http://code.google.com/p/data-race-test/wiki/ThreadSanitizer).
* The code to set the target_ field in the Parameters structure was
invoked by multiple threads. This was not a race on typical
machines since the field was always set to the same value, but I
generalized the lock initializer support to avoid the race.
* The code to instantiate the selected target used double checked
locking, and was probably fine, but I used the extended initializer
support for that as well.
* At the start of a group the symbol reading thread fetched the number
of undefined symbols. This was a race because that number could be
updated simulataneously by the current symbol adding thread. It was
not very serious since the count was only used to decide whether to
go around the group again. However, the count fetched at that time
was reliably wrong, which caused the linker to process groups twice
in cases where that was unnecessary. I introduced a Start_group
task to fetch the number of undefined symbols just before entering
the group, which should reliably permit the linker to only scan the
group once if scanning the group did not add any undefined symbols.
I also changed the undefined symbol count from int to size_t.
* The byte counts for file I/O maintained for --stats output were
simply incremented by each thread, so they could race each other and
lose increments. I added a lock, and changed the code to only keep
the counts when --stats is used.
* Clearing the empty string actually writes a zero in the empty string
representation, which looks like a race condition. This is not a
real problem, of course, but it's easy to avoid.
Ian
2010-02-11 Ian Lance Taylor <iant@google.com>
* gold-threads.h (class Once): Define.
(class Initialize_lock): Rewrite as child of Once.
* gold-threads.cc (class Once_initialize): Define.
(once_pointer_control): New static variable.
(once_pointer, once_arg): New static variables.
(c_run_once): New static function.
(Once::Once, Once::run_once, Once::internal_run): New functions.
(class Initialize_lock_once): Remove.
(initialize_lock_control): Remove.
(initialize_lock_pointer): Remove.
(initialize_lock_once): Remove.
(Initialize_lock::Initialize_lock): Move to gold-threads.h.
(Initialize_lock::initialize): Rewrite.
(Initialize_lock::do_run_once): New function.
* archive.cc (Archive::interpret_header): Only clear name if it is
not already empty.
* fileread.cc: Include "gold-threads.h"
(file_counts_lock): New static variable.
(file_counts_initialize_lock): Likewise.
(File_read::release): Only increment counts when using --stats.
Use a lock around the increment.
* parameters.cc (class Set_parameters_target_once): Define.
(set_parameters_target_once): New static variable.
(Parameters::Parameters): Move here from parameters.h.
(Parameters::set_target): Rewrite.
(Parameters::set_target_once): New function.
(Parameters::clear_target): Move here and rewrite.
* parameters.h (class Parameters): Update declarations. Add
set_parameters_target_once_ field.
(Parameters::Parameters): Move to parameters.cc.
(Parameters::clear_target): Likewise.
* readsyms.cc (Read_symbols::do_group): Create a Start_group
task.
(Start_group::~Start_group): New function.
(Start_group::is_runnable): New function.
(Start_group::locks, Start_group::run): New functions.
(Finish_group::run): Change saw_undefined to size_t.
* readsyms.h (class Start_group): Define.
(class Finish_group): Change saw_undefined_ field to size_t.
(Finish_group::Finish_group): Remove saw_undefined and
this_blocker parameters. Change all callers.
(Finish_group::set_saw_undefined): New function.
(Finish_group::set_blocker): New function.
* symtab.h (class Symbol_table): Change saw_undefined to return
size_t. Change saw_undefined_ field to size_t.
* target-select.cc (Set_target_once::do_run_once): New function.
(Target_selector::Target_selector): Initialize set_target_once_
field. Don't initialize lock_ and initialize_lock_ fields.
(Target_selector::instantiate_target): Rewrite.
(Target_selector::set_target): New function.
* target-select.h (class Set_target_once): Define.
(class Target_selector): Update declarations. Make
Set_target_once a friend. Remove lock_ and initialize_lock_
fields. Add set_target_once_ field.
Index: archive.cc
===================================================================
RCS file: /cvs/src/src/gold/archive.cc,v
retrieving revision 1.49
diff -u -p -u -r1.49 archive.cc
--- archive.cc 15 Dec 2009 22:05:22 -0000 1.49
+++ archive.cc 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// archive.cc -- archive support 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.
@@ -275,7 +275,8 @@ Archive::interpret_header(const Archive_
else if (hdr->ar_name[1] == ' ')
{
// This is the symbol table.
- pname->clear();
+ if (!pname->empty())
+ pname->clear();
}
else if (hdr->ar_name[1] == '/')
{
Index: fileread.cc
===================================================================
RCS file: /cvs/src/src/gold/fileread.cc,v
retrieving revision 1.59
diff -u -p -u -r1.59 fileread.cc
--- fileread.cc 14 Dec 2009 19:53:04 -0000 1.59
+++ fileread.cc 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// fileread.cc -- read 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.
@@ -38,6 +38,7 @@
#include "target.h"
#include "binary.h"
#include "descriptors.h"
+#include "gold-threads.h"
#include "fileread.h"
#ifndef HAVE_READV
@@ -95,6 +96,10 @@ File_read::View::is_locked()
// Class File_read.
+// A lock for the File_read static variables.
+static Lock* file_counts_lock = NULL;
+static Initialize_lock file_counts_initialize_lock(&file_counts_lock);
+
// The File_read static variables.
unsigned long long File_read::total_mapped_bytes;
unsigned long long File_read::current_mapped_bytes;
@@ -204,11 +209,17 @@ File_read::release()
{
gold_assert(this->is_locked());
- File_read::total_mapped_bytes += this->mapped_bytes_;
- File_read::current_mapped_bytes += this->mapped_bytes_;
+ if (!parameters->options_valid() || parameters->options().stats())
+ {
+ file_counts_initialize_lock.initialize();
+ Hold_optional_lock hl(file_counts_lock);
+ File_read::total_mapped_bytes += this->mapped_bytes_;
+ File_read::current_mapped_bytes += this->mapped_bytes_;
+ if (File_read::current_mapped_bytes > File_read::maximum_mapped_bytes)
+ File_read::maximum_mapped_bytes = File_read::current_mapped_bytes;
+ }
+
this->mapped_bytes_ = 0;
- if (File_read::current_mapped_bytes > File_read::maximum_mapped_bytes)
- File_read::maximum_mapped_bytes = File_read::current_mapped_bytes;
// Only clear views if there is only one attached object. Otherwise
// we waste time trying to clear cached archive views. Similarly
Index: fileread.h
===================================================================
RCS file: /cvs/src/src/gold/fileread.h,v
retrieving revision 1.40
diff -u -p -u -r1.40 fileread.h
--- fileread.h 14 Dec 2009 19:53:04 -0000 1.40
+++ fileread.h 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// fileread.h -- read 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.
@@ -218,16 +218,15 @@ class File_read
File_read(const File_read&);
File_read& operator=(const File_read&);
- // Total bytes mapped into memory during the link. This variable
- // may not be accurate when running multi-threaded.
+ // Total bytes mapped into memory during the link if --stats.
static unsigned long long total_mapped_bytes;
- // Current number of bytes mapped into memory during the link. This
- // variable may not be accurate when running multi-threaded.
+ // Current number of bytes mapped into memory during the link if
+ // --stats.
static unsigned long long current_mapped_bytes;
- // High water mark of bytes mapped into memory during the link.
- // This variable may not be accurate when running multi-threaded.
+ // High water mark of bytes mapped into memory during the link if
+ // --stats.
static unsigned long long maximum_mapped_bytes;
// A view into the file.
Index: gold-threads.cc
===================================================================
RCS file: /cvs/src/src/gold/gold-threads.cc,v
retrieving revision 1.9
diff -u -p -u -r1.9 gold-threads.cc
--- gold-threads.cc 24 Mar 2009 17:32:43 -0000 1.9
+++ gold-threads.cc 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// gold-threads.cc -- thread support 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.
@@ -278,13 +278,13 @@ Condvar::~Condvar()
#ifdef ENABLE_THREADS
-// Class Initialize_lock_once. This exists to hold a pthread_once_t
-// structure for Initialize_lock.
+// Class Once_initialize. This exists to hold a pthread_once_t
+// structure for Once.
-class Initialize_lock_once
+class Once_initialize
{
public:
- Initialize_lock_once()
+ Once_initialize()
: once_(PTHREAD_ONCE_INIT)
{ }
@@ -297,107 +297,151 @@ class Initialize_lock_once
pthread_once_t once_;
};
-#endif // !defined(ENABLE_THREADS)
+#endif // defined(ENABLE_THREADS)
#ifdef ENABLE_THREADS
-// A single lock which controls access to initialize_lock_pointer.
-// This is used because we can't pass parameters to functions passed
-// to pthread_once.
+// A single lock which controls access to once_pointer. This is used
+// because we can't pass parameters to functions passed to
+// pthread_once.
+
+static pthread_mutex_t once_pointer_control = PTHREAD_MUTEX_INITIALIZER;
-static pthread_mutex_t initialize_lock_control = PTHREAD_MUTEX_INITIALIZER;
+// A pointer to Once structure we want to run. Access to this is
+// controlled by once_pointer_control.
-// A pointer to a pointer to the lock which we need to initialize
-// once. Access to this is controlled by initialize_lock_control.
+static Once* once_pointer;
-static Lock** initialize_lock_pointer;
+// The argument to pass to the Once structure. Access to this is
+// controlled by once_pointer_control.
-// A routine passed to pthread_once which initializes the lock which
-// initialize_lock_pointer points to.
+static void* once_arg;
+
+// A routine passed to pthread_once which runs the Once pointer.
extern "C"
{
static void
-initialize_lock_once()
+c_run_once(void)
{
- *initialize_lock_pointer = new Lock();
+ once_pointer->internal_run(once_arg);
}
}
-#endif // !defined(ENABLE_THREADS)
+#endif // defined(ENABLE_THREADS)
-// Class Initialize_lock.
+// Class Once.
-Initialize_lock::Initialize_lock(Lock** pplock)
- : pplock_(pplock)
+Once::Once()
+ : was_run_(false), was_run_lock_(0)
{
#ifndef ENABLE_THREADS
this->once_ = NULL;
#else
- this->once_ = new Initialize_lock_once();
+ this->once_ = new Once_initialize();
#endif
}
-// Initialize the lock.
+// Run the function once.
-bool
-Initialize_lock::initialize()
+void
+Once::run_once(void* arg)
{
- // If the lock has already been initialized, we don't need to do
- // anything. Note that this assumes that the pointer value will be
- // set completely or not at all. I hope this is always safe. We
- // want to do this for efficiency.
- if (*this->pplock_ != NULL)
- return true;
+#ifndef ENABLE_THREADS
- // We can't initialize the lock until we have read the options.
- if (!parameters->options_valid())
- return false;
+ // If there is no threads support, we don't need to use pthread_once.
+ if (!this->was_run_)
+ this->internal_run(arg);
- // If the user did not use --threads, then we can initialize
- // directly.
- if (!parameters->options().threads())
+#else // defined(ENABLE_THREADS)
+
+ if (parameters->options_valid() && !parameters->options().threads())
{
- *this->pplock_ = new Lock();
- return true;
+ // If we are not using threads, we don't need to lock.
+ if (!this->was_run_)
+ this->internal_run(arg);
+ return;
}
-#ifndef ENABLE_THREADS
-
- // If there is no threads support, we don't need to use
- // pthread_once.
- *this->pplock_ = new Lock();
-
-#else // !defined(ENABLE_THREADS)
+ // If we have the sync builtins, use them to skip the lock if the
+ // value has already been initialized.
+#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+ while (true)
+ {
+ if (__sync_bool_compare_and_swap(&this->was_run_lock_, 0, 1))
+ break;
+ }
+ bool was_run = this->was_run_;
+ while (true)
+ {
+ if (__sync_bool_compare_and_swap(&this->was_run_lock_, 1, 0))
+ break;
+ }
+ if (was_run)
+ return;
+#endif
// Since we can't pass parameters to routines called by
- // pthread_once, we use a static variable: initialize_lock_pointer.
- // That in turns means that we need to use a mutex to control access
- // to initialize_lock_pointer.
+ // pthread_once, we use a static variable: once_pointer. This in
+ // turns means that we need to use a mutex to control access to
+ // once_pointer.
- int err = pthread_mutex_lock(&initialize_lock_control);
+ int err = pthread_mutex_lock(&once_pointer_control);
if (err != 0)
gold_fatal(_("pthread_mutex_lock failed: %s"), strerror(err));
- initialize_lock_pointer = this->pplock_;
+ once_pointer = this;
+ once_arg = arg;
- err = pthread_once(this->once_->once_control(), initialize_lock_once);
+ err = pthread_once(this->once_->once_control(), c_run_once);
if (err != 0)
gold_fatal(_("pthread_once failed: %s"), strerror(err));
- initialize_lock_pointer = NULL;
+ once_pointer = NULL;
+ once_arg = NULL;
- err = pthread_mutex_unlock(&initialize_lock_control);
+ err = pthread_mutex_unlock(&once_pointer_control);
if (err != 0)
- gold_fatal(_("pthread_mutex_unlock failed: %s"), strerror(err));
+ gold_fatal(_("pthread_mutex_unlock falied: %s"), strerror(err));
- gold_assert(*this->pplock_ != NULL);
+#endif // defined(ENABLE_THREADS)
+}
+
+// Actually run the function in the child class. This function will
+// be run only once.
+
+void
+Once::internal_run(void* arg)
+{
+ this->do_run_once(arg);
+ this->was_run_ = true;
+}
+
+// Class Initialize_lock.
+
+// Initialize the lock.
+
+bool
+Initialize_lock::initialize()
+{
+ // We can't initialize the lock until we have read the options.
+ if (!parameters->options_valid())
+ return false;
+ else
+ {
+ this->run_once(NULL);
+ return true;
+ }
+}
-#endif // !defined(ENABLE_THREADS)
+// Initialize the lock exactly once.
- return true;
+void
+Initialize_lock::do_run_once(void*)
+{
+ *this->pplock_ = new Lock();
}
} // End namespace gold.
Index: gold-threads.h
===================================================================
RCS file: /cvs/src/src/gold/gold-threads.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 gold-threads.h
--- gold-threads.h 24 Mar 2009 04:50:32 -0000 1.6
+++ gold-threads.h 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// gold-threads.h -- thread support for gold -*- C++ -*-
-// 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.
@@ -35,6 +35,7 @@ namespace gold
{
class Condvar;
+class Once_initialize;
class Initialize_lock_once;
// The interface for the implementation of a Lock.
@@ -191,6 +192,39 @@ class Condvar
Condvar_impl* condvar_;
};
+// A class used to do something once. This is an abstract parent
+// class; any actual use will involve a child of this.
+
+class Once
+{
+ public:
+ Once();
+
+ // Call this function to do whatever it is. We pass an argument
+ // even though you have to use a child class because in some uses
+ // setting the argument would itself require a Once class.
+ void
+ run_once(void* arg);
+
+ // This is an internal function, which must be public because it is
+ // run by an extern "C" function called via pthread_once.
+ void
+ internal_run(void* arg);
+
+ protected:
+ // This must be implemented by the child class.
+ virtual void
+ do_run_once(void* arg) = 0;
+
+ private:
+ // True if we have already run the function.
+ bool was_run_;
+ // Internal compare-and-swap lock on was_run_;
+ uint32_t was_run_lock_;
+ // The lock to run the function only once.
+ Once_initialize* once_;
+};
+
// A class used to initialize a lock exactly once, after the options
// have been read. This is needed because the implementation of locks
// depends on whether we've seen the --threads option. Before the
@@ -199,23 +233,27 @@ class Condvar
// variable of the class which has a lock which needs to be
// initialized.
-class Initialize_lock
+class Initialize_lock : public Once
{
public:
// The class which uses this will have a pointer to a lock. This
// must be constructed with a pointer to that pointer.
- Initialize_lock(Lock** pplock);
+ Initialize_lock(Lock** pplock)
+ : pplock_(pplock)
+ { }
// Initialize the lock. Return true if the lock is now initialized,
// false if it is not (because the options have not yet been read).
bool
initialize();
+ protected:
+ void
+ do_run_once(void*);
+
private:
// A pointer to the lock pointer which must be initialized.
Lock** const pplock_;
- // If needed, a pointer to a pthread_once_t structure.
- Initialize_lock_once* once_;
};
} // End namespace gold.
Index: parameters.cc
===================================================================
RCS file: /cvs/src/src/gold/parameters.cc,v
retrieving revision 1.31
diff -u -p -u -r1.31 parameters.cc
--- parameters.cc 3 Feb 2010 05:36:55 -0000 1.31
+++ parameters.cc 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// parameters.cc -- general parameters for a link using 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.
@@ -30,6 +30,47 @@
namespace gold
{
+// Our local version of the variable, which is not const.
+
+static Parameters static_parameters;
+
+// The global variable.
+
+const Parameters* parameters = &static_parameters;
+
+// A helper class to set the target once.
+
+class Set_parameters_target_once : public Once
+{
+ public:
+ Set_parameters_target_once(Parameters* parameters)
+ : parameters_(parameters)
+ { }
+
+ protected:
+ void
+ do_run_once(void* arg)
+ { this->parameters_->set_target_once(static_cast<Target*>(arg)); }
+
+ private:
+ Parameters* parameters_;
+};
+
+// We only need one Set_parameters_target_once.
+
+static
+Set_parameters_target_once set_parameters_target_once(&static_parameters);
+
+// Class Parameters.
+
+Parameters::Parameters()
+ : errors_(NULL), options_(NULL), target_(NULL),
+ doing_static_link_valid_(false), doing_static_link_(false),
+ debug_(0),
+ set_parameters_target_once_(&set_parameters_target_once)
+ {
+ }
+
void
Parameters::set_errors(Errors* errors)
{
@@ -61,10 +102,28 @@ Parameters::set_doing_static_link(bool d
void
Parameters::set_target(Target* target)
{
- if (!this->target_valid())
- this->target_ = target;
- else
- gold_assert(target == this->target_);
+ this->set_parameters_target_once_->run_once(static_cast<void*>(target));
+ gold_assert(target == this->target_);
+}
+
+// This is called at most once.
+
+void
+Parameters::set_target_once(Target* target)
+{
+ gold_assert(this->target_ == NULL);
+ this->target_ = target;
+}
+
+// Clear the target, for testing.
+
+void
+Parameters::clear_target()
+{
+ this->target_ = NULL;
+ // We need a new Set_parameters_target_once so that we can set the
+ // target again.
+ this->set_parameters_target_once_ = new Set_parameters_target_once(this);
}
// Return whether TARGET is compatible with the target we are using.
@@ -122,15 +181,6 @@ Parameters::size_and_endianness() const
gold_unreachable();
}
-
-// Our local version of the variable, which is not const.
-
-static Parameters static_parameters;
-
-// The global variable.
-
-const Parameters* parameters = &static_parameters;
-
void
set_parameters_errors(Errors* errors)
{ static_parameters.set_errors(errors); }
Index: parameters.h
===================================================================
RCS file: /cvs/src/src/gold/parameters.h,v
retrieving revision 1.27
diff -u -p -u -r1.27 parameters.h
--- parameters.h 30 Sep 2009 22:21:13 -0000 1.27
+++ parameters.h 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// parameters.h -- general parameters for a link using gold -*- C++ -*-
-// 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.
@@ -31,6 +31,7 @@ class Errors;
class Target;
template<int size, bool big_endian>
class Sized_target;
+class Set_parameters_target_once;
// Here we define the Parameters class which simply holds simple
// general parameters which apply to the entire link. We use a global
@@ -49,11 +50,7 @@ class Sized_target;
class Parameters
{
public:
- Parameters()
- : errors_(NULL), options_(NULL), target_(NULL),
- doing_static_link_valid_(false), doing_static_link_(false),
- debug_(0)
- { }
+ Parameters();
// These should be called as soon as they are known.
void
@@ -112,8 +109,7 @@ class Parameters
// Clear the target, for testing.
void
- clear_target()
- { this->target_ = NULL; }
+ clear_target();
// Return true if TARGET is compatible with the current target.
bool
@@ -150,12 +146,18 @@ class Parameters
private:
+ void
+ set_target_once(Target*);
+
+ friend class Set_parameters_target_once;
+
Errors* errors_;
const General_options* options_;
Target* target_;
bool doing_static_link_valid_;
bool doing_static_link_;
int debug_;
+ Set_parameters_target_once* set_parameters_target_once_;
};
// This is a global variable.
Index: readsyms.cc
===================================================================
RCS file: /cvs/src/src/gold/readsyms.cc,v
retrieving revision 1.39
diff -u -p -u -r1.39 readsyms.cc
--- readsyms.cc 10 Oct 2009 07:39:04 -0000 1.39
+++ readsyms.cc 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// readsyms.cc -- read input file symbols 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.
@@ -360,6 +360,19 @@ Read_symbols::do_group(Workqueue* workqu
const Input_file_group* group = this->input_argument_->group();
Task_token* this_blocker = this->this_blocker_;
+ Finish_group* finish_group = new Finish_group(this->input_objects_,
+ this->symtab_,
+ this->layout_,
+ this->mapfile_,
+ input_group,
+ this->next_blocker_);
+
+ Task_token* next_blocker = new Task_token(true);
+ next_blocker->add_blocker();
+ workqueue->queue_soon(new Start_group(this->symtab_, finish_group,
+ this_blocker, next_blocker));
+ this_blocker = next_blocker;
+
for (Input_file_group::const_iterator p = group->begin();
p != group->end();
++p)
@@ -367,7 +380,7 @@ Read_symbols::do_group(Workqueue* workqu
const Input_argument* arg = &*p;
gold_assert(arg->is_file());
- Task_token* next_blocker = new Task_token(true);
+ next_blocker = new Task_token(true);
next_blocker->add_blocker();
workqueue->queue_soon(new Read_symbols(this->input_objects_,
this->symtab_, this->layout_,
@@ -377,15 +390,9 @@ Read_symbols::do_group(Workqueue* workqu
this_blocker = next_blocker;
}
- const int saw_undefined = this->symtab_->saw_undefined();
- workqueue->queue_soon(new Finish_group(this->input_objects_,
- this->symtab_,
- this->layout_,
- this->mapfile_,
- input_group,
- saw_undefined,
- this_blocker,
- this->next_blocker_));
+ finish_group->set_blocker(this_blocker);
+
+ workqueue->queue_soon(finish_group);
}
// Return a debugging name for a Read_symbols task.
@@ -476,6 +483,40 @@ Add_symbols::run(Workqueue*)
this->sd_ = NULL;
}
+// Class Start_group.
+
+Start_group::~Start_group()
+{
+ if (this->this_blocker_ != NULL)
+ delete this->this_blocker_;
+ // next_blocker_ is deleted by the task associated with the first
+ // file in the group.
+}
+
+// We need to wait for THIS_BLOCKER_ and unblock NEXT_BLOCKER_.
+
+Task_token*
+Start_group::is_runnable()
+{
+ if (this->this_blocker_ != NULL && this->this_blocker_->is_blocked())
+ return this->this_blocker_;
+ return NULL;
+}
+
+void
+Start_group::locks(Task_locker* tl)
+{
+ tl->add(this, this->next_blocker_);
+}
+
+// Store the number of undefined symbols we see now.
+
+void
+Start_group::run(Workqueue*)
+{
+ this->finish_group_->set_saw_undefined(this->symtab_->saw_undefined());
+}
+
// Class Finish_group.
Finish_group::~Finish_group()
@@ -507,7 +548,7 @@ Finish_group::locks(Task_locker* tl)
void
Finish_group::run(Workqueue*)
{
- int saw_undefined = this->saw_undefined_;
+ size_t saw_undefined = this->saw_undefined_;
while (saw_undefined != this->symtab_->saw_undefined())
{
saw_undefined = this->symtab_->saw_undefined();
Index: readsyms.h
===================================================================
RCS file: /cvs/src/src/gold/readsyms.h,v
retrieving revision 1.18
diff -u -p -u -r1.18 readsyms.h
--- readsyms.h 14 Mar 2009 05:56:46 -0000 1.18
+++ readsyms.h 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// readsyms.h -- read input file symbols for gold -*- C++ -*-
-// 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.
@@ -35,6 +35,7 @@ class Input_objects;
class Symbol_table;
class Input_group;
class Archive;
+class Finish_group;
// This Task is responsible for reading the symbols from an input
// file. This also includes reading the relocations so that we can
@@ -200,6 +201,43 @@ class Input_group
Archives archives_;
};
+// This class starts the handling of a group. It exists only to pick
+// up the number of undefined symbols at that point, so that we only
+// run back through the group if we saw a new undefined symbol.
+
+class Start_group : public Task
+{
+ public:
+ Start_group(Symbol_table* symtab, Finish_group* finish_group,
+ Task_token* this_blocker, Task_token* next_blocker)
+ : symtab_(symtab), finish_group_(finish_group),
+ this_blocker_(this_blocker), next_blocker_(next_blocker)
+ { }
+
+ ~Start_group();
+
+ // The standard Task methods.
+
+ Task_token*
+ is_runnable();
+
+ void
+ locks(Task_locker*);
+
+ void
+ run(Workqueue*);
+
+ std::string
+ get_name() const
+ { return "Start_group"; }
+
+ private:
+ Symbol_table* symtab_;
+ Finish_group* finish_group_;
+ Task_token* this_blocker_;
+ Task_token* next_blocker_;
+};
+
// This class is used to finish up handling a group. It is just a
// closure.
@@ -208,16 +246,28 @@ class Finish_group : public Task
public:
Finish_group(Input_objects* input_objects, Symbol_table* symtab,
Layout* layout, Mapfile* mapfile, Input_group* input_group,
- int saw_undefined, Task_token* this_blocker,
Task_token* next_blocker)
: input_objects_(input_objects), symtab_(symtab),
layout_(layout), mapfile_(mapfile), input_group_(input_group),
- saw_undefined_(saw_undefined), this_blocker_(this_blocker),
- next_blocker_(next_blocker)
+ saw_undefined_(0), this_blocker_(NULL), next_blocker_(next_blocker)
{ }
~Finish_group();
+ // Set the number of undefined symbols when we start processing the
+ // group. This is called by the Start_group task.
+ void
+ set_saw_undefined(size_t saw_undefined)
+ { this->saw_undefined_ = saw_undefined; }
+
+ // Set the blocker to use for this task.
+ void
+ set_blocker(Task_token* this_blocker)
+ {
+ gold_assert(this->this_blocker_ == NULL);
+ this->this_blocker_ = this_blocker;
+ }
+
// The standard Task methods.
Task_token*
@@ -239,7 +289,7 @@ class Finish_group : public Task
Layout* layout_;
Mapfile* mapfile_;
Input_group* input_group_;
- int saw_undefined_;
+ size_t saw_undefined_;
Task_token* this_blocker_;
Task_token* next_blocker_;
};
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.104
diff -u -p -u -r1.104 symtab.h
--- symtab.h 9 Jan 2010 00:13:48 -0000 1.104
+++ symtab.h 12 Feb 2010 02:23:20 -0000
@@ -1354,7 +1354,7 @@ class Symbol_table
get_sized_symbol(const Symbol*) const;
// Return the count of undefined symbols seen.
- int
+ size_t
saw_undefined() const
{ return this->saw_undefined_; }
@@ -1709,7 +1709,7 @@ class Symbol_table
// We increment this every time we see a new undefined symbol, for
// use in archive groups.
- int saw_undefined_;
+ size_t saw_undefined_;
// The index of the first global symbol in the output file.
unsigned int first_global_index_;
// The file offset within the output symtab section where we should
Index: target-select.cc
===================================================================
RCS file: /cvs/src/src/gold/target-select.cc,v
retrieving revision 1.12
diff -u -p -u -r1.12 target-select.cc
--- target-select.cc 14 Dec 2009 19:53:05 -0000 1.12
+++ target-select.cc 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// target-select.cc -- select a target for an object file
-// 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.
@@ -39,6 +39,14 @@ gold::Target_selector* target_selectors;
namespace gold
{
+// Class Set_target_once.
+
+void
+Set_target_once::do_run_once(void*)
+{
+ this->target_selector_->set_target();
+}
+
// Construct a Target_selector, which means adding it to the linked
// list. This runs at global constructor time, so we want it to be
// fast.
@@ -46,32 +54,31 @@ namespace gold
Target_selector::Target_selector(int machine, int size, bool is_big_endian,
const char* bfd_name)
: machine_(machine), size_(size), is_big_endian_(is_big_endian),
- bfd_name_(bfd_name), instantiated_target_(NULL), lock_(NULL),
- initialize_lock_(&this->lock_)
-
+ bfd_name_(bfd_name), instantiated_target_(NULL), set_target_once_(this)
{
this->next_ = target_selectors;
target_selectors = this;
}
-// Instantiate the target and return it. Use a lock to avoid
-// instantiating two instances of the same target.
+// Instantiate the target and return it. Use SET_TARGET_ONCE_ to
+// avoid instantiating two instances of the same target.
Target*
Target_selector::instantiate_target()
{
- // We assume that the pointer will either be written entirely or not
- // at all.
- if (this->instantiated_target_ == NULL)
- {
- this->initialize_lock_.initialize();
- Hold_optional_lock hl(this->lock_);
- if (this->instantiated_target_ == NULL)
- this->instantiated_target_ = this->do_instantiate_target();
- }
+ this->set_target_once_.run_once(NULL);
return this->instantiated_target_;
}
+// Instantiate the target. This is called at most once.
+
+void
+Target_selector::set_target()
+{
+ gold_assert(this->instantiated_target_ == NULL);
+ this->instantiated_target_ = this->do_instantiate_target();
+}
+
// Find the target for an ELF file.
Target*
Index: target-select.h
===================================================================
RCS file: /cvs/src/src/gold/target-select.h,v
retrieving revision 1.11
diff -u -p -u -r1.11 target-select.h
--- target-select.h 14 Dec 2009 19:53:05 -0000 1.11
+++ target-select.h 12 Feb 2010 02:23:20 -0000
@@ -1,6 +1,6 @@
// target-select.h -- select a target for an object file -*- C++ -*-
-// 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.
@@ -31,6 +31,24 @@ namespace gold
{
class Target;
+class Target_selector;
+
+// Used to set the target only once.
+
+class Set_target_once : public Once
+{
+ public:
+ Set_target_once(Target_selector* target_selector)
+ : target_selector_(target_selector)
+ { }
+
+ protected:
+ void
+ do_run_once(void*);
+
+ private:
+ Target_selector* target_selector_;
+};
// We want to avoid a master list of targets, which implies using a
// global constructor. And we also want the program to start up as
@@ -141,6 +159,12 @@ class Target_selector
instantiate_target();
private:
+ // Set the target.
+ void
+ set_target();
+
+ friend class Set_target_once;
+
// ELF machine code.
const int machine_;
// Target size--32 or 64.
@@ -154,11 +178,8 @@ class Target_selector
// The singleton Target structure--this points to an instance of the
// real implementation.
Target* instantiated_target_;
- // Lock to make sure that we don't instantiate the target more than
- // once.
- Lock* lock_;
- // We only want to initialize the lock_ pointer once.
- Initialize_lock initialize_lock_;
+ // Used to set the target only once.
+ Set_target_once set_target_once_;
};
// Select the target for an ELF file.