public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [gold patch] incremental 5/18: support --start-lib/--end-lib
@ 2011-03-28 23:00 Cary Coutant
  2011-03-30  0:24 ` Ian Lance Taylor
  0 siblings, 1 reply; 3+ messages in thread
From: Cary Coutant @ 2011-03-28 23:00 UTC (permalink / raw)
  To: Ian Lance Taylor, Binutils

[-- Attachment #1: Type: text/plain, Size: 3723 bytes --]

Ian,

I've got a series of 14 more patches lined up now for incremental
linking support, including the four that are still pending from last
September. I've updated the pending patches a bit and rebased them to
the current head. I'll send them to you a few at a time.

This patch adds support for tracking virtual libraries defined by
--start-lib/--end-lib. I moved class Archive and class Lib_group under
a new common base class, and replaced the old iterator over unused
symbols with an iterator function that takes a visitor class. (As it
turns out, this is storing way too many symbols in the incremental
info. I think that all we really need to store is a list of unused
symbols that actually got defined later in the link. I still have a
to-do to fix this, and another to-do to implement the check that none
of the unused symbols would have been referenced by any changed input
files up to the point of searching the library.)

-cary


2011-03-28 Cary Coutant  <ccoutant@google.com>

	* archive.cc (Library_base::should_include_member): Move
	method here from class Archive.
	(Archive::Archive): Initialize base class.
	(Archive::should_include_member): Move to base class.
	(Archive::do_for_all_unused_symbols): New function.
	(Add_archive_symbols::run): Remove redundant access to
	incremental_inputs.
	(Lib_group::Lib_group): Initialize base class.
	(Lib_group::do_filename): New function.
	(Lib_group::include_member): Pass pointer to Lib_group to
	report_object.
	(Lib_group::do_for_all_unused_symbols): New function.
	(Add_lib_group_symbols::run): Report archive information for
	incremental links.
	* archive.h (class Library_base): New base class.
	(class Archive): Derive from Library_base.
	(Archive::filename): Move to base class.
	(Archive::set_incremental_info): Likewise.
	(Archive::incremental_info): Likewise.
	(Archive::Should_include): Likewise.
	(Archive::should_include_member): Likewise.
	(Archive::Armap_entry): Remove.
	(Archive::Unused_symbol_iterator): Remove.
	(Archive::unused_symbols_begin): Remove.
	(Archive::unused_symbols_end): Remove.
	(Archive::do_filename): New function.
	(Archive::do_get_mtime): New function.
	(Archive::do_for_all_unused_symbols): New function.
	(Archive::task_): Move to base class.
	(Archive::incremental_info_): Likewise.
	(class Lib_group): Derive from Library_base.
	(Lib_group::do_filename): New function.
	(Lib_group::do_get_mtime): New function.
	(Lib_group::do_for_all_unused_symbols): New function.
	(Lib_group::task_): Move to base class.
	* dynobj.cc (Sized_dynobj::do_for_all_global_symbols): New
	function.
	* dynobj.h (Sized_dynobj::do_for_all_global_symbols): New
	function.
	* incremental.cc (Incremental_inputs::report_archive_begin):
	Use Library_base; call library's get_mtime; add incremental inputs
	entry before members.
	(class Unused_symbol_visitor): New class.
	(Incremental_inputs::report_archive_end): Use Library_base; use
	visitor class to record unused symbols; don't add incremental inputs
	entry after members.
	(Incremental_inputs::report_object): Use Library_base.
	* incremental.h
	(Incremental_archive_entry::Incremental_archive_entry): Remove
	unused Archive parameter.
	(Incremental_inputs::report_archive_begin): Use Library_base.
	(Incremental_inputs::report_archive_end): Likewise.
	(Incremental_inputs::report_object): Likewise.
	* object.cc (Sized_relobj::do_for_all_global_symbols): New
	function.
	* object.h (Object::for_all_global_symbols): New function.
	(Object::do_for_all_global_symbols): New function.
	(Sized_relobj::do_for_all_global_symbols): New function.
	* plugin.cc (Sized_pluginobj::do_for_all_global_symbols):  New
	function.
	* plugin.h (Sized_pluginobj::do_for_all_global_symbols):  New
	function.

[-- Attachment #2: incr-patch-5.txt --]
[-- Type: text/plain, Size: 30552 bytes --]

Add incremental linking support for --start-lib/--end-lib.


2011-03-28 Cary Coutant  <ccoutant@google.com>

	* archive.cc (Library_base::should_include_member): Move
	method here from class Archive.
	(Archive::Archive): Initialize base class.
	(Archive::should_include_member): Move to base class.
	(Archive::do_for_all_unused_symbols): New function.
	(Add_archive_symbols::run): Remove redundant access to
	incremental_inputs.
	(Lib_group::Lib_group): Initialize base class.
	(Lib_group::do_filename): New function.
	(Lib_group::include_member): Pass pointer to Lib_group to
	report_object.
	(Lib_group::do_for_all_unused_symbols): New function.
	(Add_lib_group_symbols::run): Report archive information for
	incremental links.
	* archive.h (class Library_base): New base class.
	(class Archive): Derive from Library_base.
	(Archive::filename): Move to base class.
	(Archive::set_incremental_info): Likewise.
	(Archive::incremental_info): Likewise.
	(Archive::Should_include): Likewise.
	(Archive::should_include_member): Likewise.
	(Archive::Armap_entry): Remove.
	(Archive::Unused_symbol_iterator): Remove.
	(Archive::unused_symbols_begin): Remove.
	(Archive::unused_symbols_end): Remove.
	(Archive::do_filename): New function.
	(Archive::do_get_mtime): New function.
	(Archive::do_for_all_unused_symbols): New function.
	(Archive::task_): Move to base class.
	(Archive::incremental_info_): Likewise.
	(class Lib_group): Derive from Library_base.
	(Lib_group::do_filename): New function.
	(Lib_group::do_get_mtime): New function.
	(Lib_group::do_for_all_unused_symbols): New function.
	(Lib_group::task_): Move to base class.
	* dynobj.cc (Sized_dynobj::do_for_all_global_symbols): New
	function.
	* dynobj.h (Sized_dynobj::do_for_all_global_symbols): New
	function.
	* incremental.cc (Incremental_inputs::report_archive_begin):
	Use Library_base; call library's get_mtime; add incremental inputs
	entry before members.
	(class Unused_symbol_visitor): New class.
	(Incremental_inputs::report_archive_end): Use Library_base; use
	visitor class to record unused symbols; don't add incremental inputs
	entry after members.
	(Incremental_inputs::report_object): Use Library_base.
	* incremental.h
	(Incremental_archive_entry::Incremental_archive_entry): Remove
	unused Archive parameter.
	(Incremental_inputs::report_archive_begin): Use Library_base.
	(Incremental_inputs::report_archive_end): Likewise.
	(Incremental_inputs::report_object): Likewise.
	* object.cc (Sized_relobj::do_for_all_global_symbols): New
	function.
	* object.h (Object::for_all_global_symbols): New function.
	(Object::do_for_all_global_symbols): New function.
	(Sized_relobj::do_for_all_global_symbols): New function.
	* plugin.cc (Sized_pluginobj::do_for_all_global_symbols):  New
	function.
	* plugin.h (Sized_pluginobj::do_for_all_global_symbols):  New
	function.


diff --git a/gold/archive.cc b/gold/archive.cc
index 89fc422..5edcb47 100644
--- a/gold/archive.cc
+++ b/gold/archive.cc
@@ -44,6 +44,90 @@
 namespace gold
 {
 
+// Library_base methods.
+
+// Determine whether a definition of SYM_NAME should cause an archive
+// library member to be included in the link.  Returns SHOULD_INCLUDE_YES
+// if the symbol is referenced but not defined, SHOULD_INCLUDE_NO if the
+// symbol is already defined, and SHOULD_INCLUDE_UNKNOWN if the symbol is
+// neither referenced nor defined.
+
+Library_base::Should_include
+Library_base::should_include_member(Symbol_table* symtab, Layout* layout,
+				    const char* sym_name, Symbol** symp,
+				    std::string* why, char** tmpbufp,
+				    size_t* tmpbuflen)
+{
+  // In an object file, and therefore in an archive map, an
+  // '@' in the name separates the symbol name from the
+  // version name.  If there are two '@' characters, this is
+  // the default version.
+  char* tmpbuf = *tmpbufp;
+  const char* ver = strchr(sym_name, '@');
+  bool def = false;
+  if (ver != NULL)
+    {
+      size_t symlen = ver - sym_name;
+      if (symlen + 1 > *tmpbuflen)
+        {
+          tmpbuf = static_cast<char*>(xrealloc(tmpbuf, symlen + 1));
+          *tmpbufp = tmpbuf;
+          *tmpbuflen = symlen + 1;
+        }
+      memcpy(tmpbuf, sym_name, symlen);
+      tmpbuf[symlen] = '\0';
+      sym_name = tmpbuf;
+
+      ++ver;
+      if (*ver == '@')
+        {
+          ++ver;
+          def = true;
+        }
+    }
+
+  Symbol* sym = symtab->lookup(sym_name, ver);
+  if (def
+      && ver != NULL
+      && (sym == NULL
+          || !sym->is_undefined()
+          || sym->binding() == elfcpp::STB_WEAK))
+    sym = symtab->lookup(sym_name, NULL);
+
+  *symp = sym;
+
+  if (sym == NULL)
+    {
+      // Check whether the symbol was named in a -u option.
+      if (parameters->options().is_undefined(sym_name))
+        {
+          *why = "-u ";
+          *why += sym_name;
+        }
+      else if (layout->script_options()->is_referenced(sym_name))
+	{
+	  size_t alc = 100 + strlen(sym_name);
+	  char* buf = new char[alc];
+	  snprintf(buf, alc, _("script or expression reference to %s"),
+		   sym_name);
+	  *why = buf;
+	  delete[] buf;
+	}
+      else
+	return Library_base::SHOULD_INCLUDE_UNKNOWN;
+    }
+  else if (!sym->is_undefined())
+    return Library_base::SHOULD_INCLUDE_NO;
+  // PR 12001: Do not include an archive when the undefined
+  // symbol has actually been defined on the command line.
+  else if (layout->script_options()->is_pending_assignment(sym_name))
+    return Library_base::SHOULD_INCLUDE_NO;
+  else if (sym->binding() == elfcpp::STB_WEAK)
+    return Library_base::SHOULD_INCLUDE_UNKNOWN;
+
+  return Library_base::SHOULD_INCLUDE_YES;
+}
+
 // The header of an entry in the archive.  This is all readable text,
 // padded with spaces where necessary.  If the contents of an archive
 // are all text file, the entire archive is readable.
@@ -87,11 +171,10 @@ const char Archive::arfmag[2] = { '`', '\n' };
 
 Archive::Archive(const std::string& name, Input_file* input_file,
                  bool is_thin_archive, Dirsearch* dirpath, Task* task)
-  : name_(name), input_file_(input_file), armap_(), armap_names_(),
-    extended_names_(), armap_checked_(), seen_offsets_(), members_(),
-    is_thin_archive_(is_thin_archive), included_member_(false),
-    nested_archives_(), dirpath_(dirpath), task_(task), num_members_(0),
-    incremental_info_(NULL)
+  : Library_base(task), name_(name), input_file_(input_file), armap_(),
+    armap_names_(), extended_names_(), armap_checked_(), seen_offsets_(),
+    members_(), is_thin_archive_(is_thin_archive), included_member_(false),
+    nested_archives_(), dirpath_(dirpath), num_members_(0)
 {
   this->no_export_ =
     parameters->options().check_excluded_libs(input_file->found_name());
@@ -605,82 +688,6 @@ Archive::read_symbols(off_t off)
   this->members_[off] = member;
 }
 
-Archive::Should_include
-Archive::should_include_member(Symbol_table* symtab, Layout* layout,
-			       const char* sym_name, Symbol** symp,
-			       std::string* why, char** tmpbufp,
-                               size_t* tmpbuflen)
-{
-  // In an object file, and therefore in an archive map, an
-  // '@' in the name separates the symbol name from the
-  // version name.  If there are two '@' characters, this is
-  // the default version.
-  char* tmpbuf = *tmpbufp;
-  const char* ver = strchr(sym_name, '@');
-  bool def = false;
-  if (ver != NULL)
-    {
-      size_t symlen = ver - sym_name;
-      if (symlen + 1 > *tmpbuflen)
-        {
-          tmpbuf = static_cast<char*>(xrealloc(tmpbuf, symlen + 1));
-          *tmpbufp = tmpbuf;
-          *tmpbuflen = symlen + 1;
-        }
-      memcpy(tmpbuf, sym_name, symlen);
-      tmpbuf[symlen] = '\0';
-      sym_name = tmpbuf;
-
-      ++ver;
-      if (*ver == '@')
-        {
-          ++ver;
-          def = true;
-        }
-    }
-
-  Symbol* sym = symtab->lookup(sym_name, ver);
-  if (def
-      && ver != NULL
-      && (sym == NULL
-          || !sym->is_undefined()
-          || sym->binding() == elfcpp::STB_WEAK))
-    sym = symtab->lookup(sym_name, NULL);
-
-  *symp = sym;
-
-  if (sym == NULL)
-    {
-      // Check whether the symbol was named in a -u option.
-      if (parameters->options().is_undefined(sym_name))
-        {
-          *why = "-u ";
-          *why += sym_name;
-        }
-      else if (layout->script_options()->is_referenced(sym_name))
-	{
-	  size_t alc = 100 + strlen(sym_name);
-	  char* buf = new char[alc];
-	  snprintf(buf, alc, _("script or expression reference to %s"),
-		   sym_name);
-	  *why = buf;
-	  delete[] buf;
-	}
-      else
-	return Archive::SHOULD_INCLUDE_UNKNOWN;
-    }
-  else if (!sym->is_undefined())
-    return Archive::SHOULD_INCLUDE_NO;
-  // PR 12001: Do not include an archive when the undefined
-  // symbol has actually been defined on the command line.
-  else if (layout->script_options()->is_pending_assignment(sym_name))
-    return Archive::SHOULD_INCLUDE_NO;
-  else if (sym->binding() == elfcpp::STB_WEAK)
-    return Archive::SHOULD_INCLUDE_UNKNOWN;
-
-  return Archive::SHOULD_INCLUDE_YES;
-}
-
 // Select members from the archive and add them to the link.  We walk
 // through the elements in the archive map, and look each one up in
 // the symbol table.  If it exists as a strong undefined symbol, we
@@ -957,6 +964,21 @@ Archive::include_member(Symbol_table* symtab, Layout* layout,
   return true;
 }
 
+// Iterate over all unused symbols, and call the visitor class V for each.
+
+void
+Archive::do_for_all_unused_symbols(Symbol_visitor_base& v) const
+{
+  for (std::vector<Armap_entry>::const_iterator p = this->armap_.begin();
+       p != this->armap_.end();
+       ++p)
+    {
+      if (this->seen_offsets_.find(p->file_offset)
+          == this->seen_offsets_.end())
+        v.visit(this->armap_names_.data() + p->name_offset);
+    }
+}
+
 // Print statistical information to stderr.  This is used for --stats.
 
 void
@@ -1033,7 +1055,6 @@ Add_archive_symbols::run(Workqueue* workqueue)
   else
     {
       // For an incremental link, finish recording the layout information.
-      Incremental_inputs* incremental_inputs = this->layout_->incremental_inputs();
       if (incremental_inputs != NULL)
 	incremental_inputs->report_archive_end(this->archive_);
 
@@ -1059,11 +1080,18 @@ unsigned int Lib_group::total_members;
 unsigned int Lib_group::total_members_loaded;
 
 Lib_group::Lib_group(const Input_file_lib* lib, Task* task)
-  : lib_(lib), task_(task), members_()
+  : Library_base(task), lib_(lib), members_()
 {
   this->members_.resize(lib->size());
 }
 
+const std::string&
+Lib_group::do_filename() const
+{
+  std::string *filename = new std::string("<group>");
+  return *filename;
+}
+
 // Select members from the lib group and add them to the link.  We walk
 // through the members, and check if each one up should be included.
 // If the object says it should be included, we do so.  We have to do
@@ -1153,9 +1181,8 @@ Lib_group::include_member(Symbol_table* symtab, Layout* layout,
   obj->lock(this->task_);
   if (input_objects->add_object(obj))
     {
-      // FIXME: Record incremental link info for --start-lib/--end-lib.
       if (layout->incremental_inputs() != NULL)
-	layout->incremental_inputs()->report_object(obj, NULL);
+	layout->incremental_inputs()->report_object(obj, this);
       obj->layout(symtab, layout, sd);
       obj->add_symbols(symtab, sd, layout);
     }
@@ -1164,6 +1191,22 @@ Lib_group::include_member(Symbol_table* symtab, Layout* layout,
   obj->unlock(this->task_);
 }
 
+// Iterate over all unused symbols, and call the visitor class V for each.
+
+void
+Lib_group::do_for_all_unused_symbols(Symbol_visitor_base& v) const
+{
+  // Files are removed from the members list when used, so all the
+  // files remaining on the list are unused.
+  for (std::vector<Archive_member>::const_iterator p = this->members_.begin();
+       p != this->members_.end();
+       ++p)
+    {
+      Object* obj = p->obj_;
+      obj->for_all_global_symbols(p->sd_, v);
+    }
+}
+
 // Print statistical information to stderr.  This is used for --stats.
 
 void
@@ -1196,9 +1239,15 @@ Add_lib_group_symbols::locks(Task_locker* tl)
 void
 Add_lib_group_symbols::run(Workqueue*)
 {
+  // For an incremental link, begin recording layout information.
+  Incremental_inputs* incremental_inputs = this->layout_->incremental_inputs();
+  if (incremental_inputs != NULL)
+    incremental_inputs->report_archive_begin(this->lib_);
+
   this->lib_->add_symbols(this->symtab_, this->layout_, this->input_objects_);
 
-  // FIXME: Record incremental link info for --start_lib/--end_lib.
+  if (incremental_inputs != NULL)
+    incremental_inputs->report_archive_end(this->lib_);
 }
 
 Add_lib_group_symbols::~Add_lib_group_symbols()
diff --git a/gold/archive.h b/gold/archive.h
index c5ba114..2e57076 100644
--- a/gold/archive.h
+++ b/gold/archive.h
@@ -59,10 +59,100 @@ struct Archive_member
   Read_symbols_data* sd_;
 };
 
+// This class serves as a base class for Archive and Lib_group objects.
+
+class Library_base
+{
+ public:
+  Library_base(Task* task)
+    : task_(task), incremental_info_(NULL)
+  { }
+
+  virtual
+  ~Library_base()
+  { }
+
+  // The file name.
+  const std::string&
+  filename() const
+  { return this->do_filename(); }
+
+  // The modification time of the archive file.
+  Timespec
+  get_mtime()
+  { return this->do_get_mtime(); }
+
+  // When we see a symbol in an archive we might decide to include the member,
+  // not include the member or be undecided. This enum represents these
+  // possibilities.
+
+  enum Should_include
+  {
+    SHOULD_INCLUDE_NO,
+    SHOULD_INCLUDE_YES,
+    SHOULD_INCLUDE_UNKNOWN
+  };
+
+  static Should_include
+  should_include_member(Symbol_table* symtab, Layout*, const char* sym_name,
+                        Symbol** symp, std::string* why, char** tmpbufp,
+                        size_t* tmpbuflen);
+
+  // Store a pointer to the incremental link info for the library.
+  void
+  set_incremental_info(Incremental_archive_entry* info)
+  { this->incremental_info_ = info; }
+
+  // Return the pointer to the incremental link info for the library.
+  Incremental_archive_entry*
+  incremental_info() const
+  { return this->incremental_info_; }
+
+  // Abstract base class for processing unused symbols.
+  class Symbol_visitor_base
+  {
+   public:
+    Symbol_visitor_base()
+    { }
+
+    virtual
+    ~Symbol_visitor_base()
+    { }
+
+    virtual void
+    visit(const char*) = 0;
+  };
+
+  // Iterator for unused global symbols in the library.
+  void
+  for_all_unused_symbols(Symbol_visitor_base& v) const
+  { this->do_for_all_unused_symbols(v); }
+
+ protected:
+  // The task reading this archive.
+  Task *task_;
+
+ private:
+  // The file name.
+  virtual const std::string&
+  do_filename() const = 0;
+
+  // Return the modification time of the archive file.
+  virtual Timespec
+  do_get_mtime() = 0;
+
+  // Iterator for unused global symbols in the library.
+  virtual void
+  do_for_all_unused_symbols(Symbol_visitor_base& v) const = 0;
+
+  // The incremental link information for this archive.
+  Incremental_archive_entry* incremental_info_;
+};
+
 // This class represents an archive--generally a libNAME.a file.
 // Archives have a symbol table and a list of objects.
 
-class Archive
+class Archive : public Library_base
 {
  public:
   Archive(const std::string& name, Input_file* input_file,
@@ -90,11 +180,6 @@ class Archive
   input_file() const
   { return this->input_file_; }
 
-  // The file name.
-  const std::string&
-  filename() const
-  { return this->input_file_->filename(); }
-
   // Set up the archive: read the symbol map.
   void
   setup();
@@ -169,99 +254,20 @@ class Archive
   no_export()
   { return this->no_export_; }
 
-  // Store a pointer to the incremental link info for the archive.
-  void
-  set_incremental_info(Incremental_archive_entry* info)
-  { this->incremental_info_ = info; }
-
-  // Return the pointer to the incremental link info for the archive.
-  Incremental_archive_entry*
-  incremental_info() const
-  { return this->incremental_info_; }
-
-  // When we see a symbol in an archive we might decide to include the member,
-  // not include the member or be undecided. This enum represents these
-  // possibilities.
-
-  enum Should_include
-  {
-    SHOULD_INCLUDE_NO,
-    SHOULD_INCLUDE_YES,
-    SHOULD_INCLUDE_UNKNOWN
-  };
-
-  static Should_include
-  should_include_member(Symbol_table* symtab, Layout*, const char* sym_name,
-                        Symbol** symp, std::string* why, char** tmpbufp,
-                        size_t* tmpbuflen);
-
- private:
-  struct Armap_entry;
-
- public:
-  // Iterator class for unused global symbols.  This iterator is used
-  // for incremental links.
-
-  class Unused_symbol_iterator
-  {
-   public:
-    Unused_symbol_iterator(Archive* arch,
-                           std::vector<Armap_entry>::const_iterator it)
-      : arch_(arch), it_(it)
-    { this->skip_used_symbols(); }
-
-    const char*
-    operator*() const
-    { return this->arch_->armap_names_.data() + this->it_->name_offset; }
-
-    Unused_symbol_iterator&
-    operator++()
-    {
-      ++this->it_;
-      this->skip_used_symbols();
-      return *this;
-    }
-
-    bool
-    operator==(const Unused_symbol_iterator p) const
-    { return this->it_ == p.it_; }
-
-    bool
-    operator!=(const Unused_symbol_iterator p) const
-    { return this->it_ != p.it_; }
-
-   private:
-    // Skip over symbols defined by members that have been included.
-    void
-    skip_used_symbols()
-    {
-      while (this->it_ != this->arch_->armap_.end()
-	     && (this->arch_->seen_offsets_.find(this->it_->file_offset)
-		 != this->arch_->seen_offsets_.end()))
-	++it_;
-    }
-
-    // The underlying archive.
-    Archive* arch_;
-
-    // The underlying iterator over all entries in the archive map.
-    std::vector<Armap_entry>::const_iterator it_;
-  };
-
-  // Return an iterator referring to the first unused symbol.
-  Unused_symbol_iterator
-  unused_symbols_begin()
-  { return Unused_symbol_iterator(this, this->armap_.begin()); }
-
-  // Return an iterator referring to the end of the unused symbols.
-  Unused_symbol_iterator
-  unused_symbols_end()
-  { return Unused_symbol_iterator(this, this->armap_.end()); }
-
  private:
   Archive(const Archive&);
   Archive& operator=(const Archive&);
 
+  // The file name.
+  const std::string&
+  do_filename() const
+  { return this->input_file_->filename(); }
+
+  // The modification time of the archive file.
+  Timespec
+  do_get_mtime()
+  { return this->file().get_mtime(); }
+
   struct Archive_header;
 
   // Total number of archives seen.
@@ -339,6 +345,10 @@ class Archive
 
   friend class const_iterator;
 
+  // Iterator for unused global symbols in the library.
+  void
+  do_for_all_unused_symbols(Symbol_visitor_base& v) const;
+
   // An entry in the archive map of symbols to object files.
   struct Armap_entry
   {
@@ -384,14 +394,10 @@ class Archive
   Nested_archive_table nested_archives_;
   // The directory search path.
   Dirsearch* dirpath_;
-  // The task reading this archive.
-  Task* task_;
   // Number of members in this archive;
   unsigned int num_members_;
   // True if we exclude this library archive from automatic export.
   bool no_export_;
-  // The incremental link information for this archive.
-  Incremental_archive_entry* incremental_info_;
 };
 
 // This class is used to read an archive and pick out the desired
@@ -451,7 +457,7 @@ class Add_archive_symbols : public Task
 
 // This class represents the files surrounded by a --start-lib ... --end-lib.
 
-class Lib_group
+class Lib_group : public Library_base
 {
  public:
   Lib_group(const Input_file_lib* lib, Task* task);
@@ -470,10 +476,6 @@ class Lib_group
     return &this->members_[i];
   }
 
-  // Dump statistical information to stderr.
-  static void
-  print_stats();
-
   // Total number of archives seen.
   static unsigned int total_lib_groups;
   // Total number of archive members seen.
@@ -481,11 +483,27 @@ class Lib_group
   // Number of archive members loaded.
   static unsigned int total_members_loaded;
 
+  // Dump statistical information to stderr.
+  static void
+  print_stats();
+
  private:
+  // The file name.
+  const std::string&
+  do_filename() const;
+
+  // A Lib_group does not have a modification time, since there is no
+  // real library file.
+  Timespec
+  do_get_mtime()
+  { return Timespec(0, 0); }
+
+  // Iterator for unused global symbols in the library.
+  void
+  do_for_all_unused_symbols(Symbol_visitor_base&) const;
+
   // For reading the files.
   const Input_file_lib* lib_;
-  // The task reading this lib group.
-  Task* task_;
   // Table of the objects in the group.
   std::vector<Archive_member> members_;
 };
diff --git a/gold/dynobj.cc b/gold/dynobj.cc
index d2ba8ae..face503 100644
--- a/gold/dynobj.cc
+++ b/gold/dynobj.cc
@@ -761,6 +761,32 @@ Sized_dynobj<size, big_endian>::do_should_include_member(Symbol_table*,
   return Archive::SHOULD_INCLUDE_YES;
 }
 
+// Iterate over global symbols, calling a visitor class V for each.
+
+template<int size, bool big_endian>
+void
+Sized_dynobj<size, big_endian>::do_for_all_global_symbols(
+    Read_symbols_data* sd,
+    Library_base::Symbol_visitor_base& v)
+{
+  const char* sym_names =
+      reinterpret_cast<const char*>(sd->symbol_names->data());
+  const unsigned char* syms =
+      sd->symbols->data() + sd->external_symbols_offset;
+  const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
+  size_t symcount = ((sd->symbols_size - sd->external_symbols_offset)
+                     / sym_size);
+  const unsigned char* p = syms;
+
+  for (size_t i = 0; i < symcount; ++i, p += sym_size)
+    {
+      elfcpp::Sym<size, big_endian> sym(p);
+      if (sym.get_st_shndx() != elfcpp::SHN_UNDEF
+	  && sym.get_st_bind() != elfcpp::STB_LOCAL)
+	v.visit(sym_names + sym.get_st_name());
+    }
+}
+
 // Get symbol counts.
 
 template<int size, bool big_endian>
diff --git a/gold/dynobj.h b/gold/dynobj.h
index 8787ada..637081c 100644
--- a/gold/dynobj.h
+++ b/gold/dynobj.h
@@ -181,6 +181,11 @@ class Sized_dynobj : public Dynobj
   do_should_include_member(Symbol_table* symtab, Layout*, Read_symbols_data*,
                            std::string* why);
 
+  // Iterate over global symbols, calling a visitor class V for each.
+  void
+  do_for_all_global_symbols(Read_symbols_data* sd,
+			    Library_base::Symbol_visitor_base& v);
+
   // Get the size of a section.
   uint64_t
   do_section_size(unsigned int shndx)
diff --git a/gold/incremental.cc b/gold/incremental.cc
index aa9d7d3..017ee7d 100644
--- a/gold/incremental.cc
+++ b/gold/incremental.cc
@@ -436,38 +436,54 @@ Incremental_inputs::report_command_line(int argc, const char* const* argv)
 // input objects until report_archive_end is called.
 
 void
-Incremental_inputs::report_archive_begin(Archive* arch)
+Incremental_inputs::report_archive_begin(Library_base* arch)
 {
   Stringpool::Key filename_key;
-  Timespec mtime = arch->file().get_mtime();
+  Timespec mtime = arch->get_mtime();
 
   this->strtab_->add(arch->filename().c_str(), false, &filename_key);
   Incremental_archive_entry* entry =
-      new Incremental_archive_entry(filename_key, arch, mtime);
+      new Incremental_archive_entry(filename_key, mtime);
   arch->set_incremental_info(entry);
+  this->inputs_.push_back(entry);
 }
 
+// Visitor class for processing the unused global symbols in a library.
+
+class Unused_symbol_visitor : public Library_base::Symbol_visitor_base
+{
+ public:
+  Unused_symbol_visitor(Incremental_archive_entry* entry, Stringpool* strtab)
+    : entry_(entry), strtab_(strtab)
+  { }
+
+  void
+  visit(const char* sym)
+  {
+    Stringpool::Key symbol_key;
+    this->strtab_->add(sym, true, &symbol_key);
+    this->entry_->add_unused_global_symbol(symbol_key);
+  }
+
+ private:
+  Incremental_archive_entry* entry_;
+  Stringpool* strtab_;
+};
+
 // Finish recording the input archive file ARCHIVE.  This is called by the
 // Add_archive_symbols task after determining which archive members
 // to include.
 
 void
-Incremental_inputs::report_archive_end(Archive* arch)
+Incremental_inputs::report_archive_end(Library_base* arch)
 {
   Incremental_archive_entry* entry = arch->incremental_info();
 
   gold_assert(entry != NULL);
 
   // Collect unused global symbols.
-  for (Archive::Unused_symbol_iterator p = arch->unused_symbols_begin();
-       p != arch->unused_symbols_end();
-       ++p)
-    {
-      Stringpool::Key symbol_key;
-      this->strtab_->add(*p, true, &symbol_key);
-      entry->add_unused_global_symbol(symbol_key);
-    }
-  this->inputs_.push_back(entry);
+  Unused_symbol_visitor v(entry, this->strtab_);
+  arch->for_all_unused_symbols(v);
 }
 
 // Record the input object file OBJ.  If ARCH is not NULL, attach
@@ -475,7 +491,7 @@ Incremental_inputs::report_archive_end(Archive* arch)
 // Add_symbols task after finding out the type of the file.
 
 void
-Incremental_inputs::report_object(Object* obj, Archive* arch)
+Incremental_inputs::report_object(Object* obj, Library_base* arch)
 {
   Stringpool::Key filename_key;
   Timespec mtime = obj->input_file()->file().get_mtime();
diff --git a/gold/incremental.h b/gold/incremental.h
index cc0e839..b55aa71 100644
--- a/gold/incremental.h
+++ b/gold/incremental.h
@@ -451,8 +451,7 @@ class Incremental_object_entry : public Incremental_input_entry
 class Incremental_archive_entry : public Incremental_input_entry
 {
  public:
-  Incremental_archive_entry(Stringpool::Key filename_key, Archive*,
-			    Timespec mtime)
+  Incremental_archive_entry(Stringpool::Key filename_key, Timespec mtime)
     : Incremental_input_entry(filename_key, mtime), members_(), unused_syms_()
   { }
 
@@ -531,16 +530,16 @@ class Incremental_inputs
 
   // Record the initial info for archive file ARCHIVE.
   void
-  report_archive_begin(Archive* arch);
+  report_archive_begin(Library_base* arch);
 
   // Record the final info for archive file ARCHIVE.
   void
-  report_archive_end(Archive* arch);
+  report_archive_end(Library_base* arch);
 
   // Record the info for object file OBJ.  If ARCH is not NULL,
   // attach the object file to the archive.
   void
-  report_object(Object* obj, Archive* arch);
+  report_object(Object* obj, Library_base* arch);
 
   // Record an input section belonging to object file OBJ.
   void
diff --git a/gold/object.cc b/gold/object.cc
index e2c0113..954961a 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -1691,6 +1691,31 @@ Sized_relobj<size, big_endian>::do_should_include_member(Symbol_table* symtab,
   return Archive::SHOULD_INCLUDE_UNKNOWN;
 }
 
+// Iterate over global symbols, calling a visitor class V for each.
+
+template<int size, bool big_endian>
+void
+Sized_relobj<size, big_endian>::do_for_all_global_symbols(
+    Read_symbols_data* sd,
+    Library_base::Symbol_visitor_base& v)
+{
+  const char* sym_names =
+      reinterpret_cast<const char*>(sd->symbol_names->data());
+  const unsigned char* syms =
+      sd->symbols->data() + sd->external_symbols_offset;
+  const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
+  size_t symcount = ((sd->symbols_size - sd->external_symbols_offset)
+                     / sym_size);
+  const unsigned char* p = syms;
+
+  for (size_t i = 0; i < symcount; ++i, p += sym_size)
+    {
+      elfcpp::Sym<size, big_endian> sym(p);
+      if (sym.get_st_shndx() != elfcpp::SHN_UNDEF)
+	v.visit(sym_names + sym.get_st_name());
+    }
+}
+
 // Return whether the local symbol SYMNDX has a PLT offset.
 
 template<int size, bool big_endian>
diff --git a/gold/object.h b/gold/object.h
index 8ee03a7..f48e664 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -422,6 +422,12 @@ class Object
 			Read_symbols_data* sd, std::string* why)
   { return this->do_should_include_member(symtab, layout, sd, why); }
 
+  // Iterate over global symbols, calling a visitor class V for each.
+  void
+  for_all_global_symbols(Read_symbols_data* sd,
+			 Library_base::Symbol_visitor_base& v)
+  { return this->do_for_all_global_symbols(sd, v); }
+
   // Functions and types for the elfcpp::Elf_file interface.  This
   // permit us to use Object as the File template parameter for
   // elfcpp::Elf_file.
@@ -572,6 +578,11 @@ class Object
   do_should_include_member(Symbol_table* symtab, Layout*, Read_symbols_data*,
                            std::string* why) = 0;
 
+  // Iterate over global symbols, calling a visitor class V for each.
+  virtual void
+  do_for_all_global_symbols(Read_symbols_data* sd,
+			    Library_base::Symbol_visitor_base& v) = 0;
+
   // Return the location of the contents of a section.  Implemented by
   // child class.
   virtual Location
@@ -1810,6 +1821,11 @@ class Sized_relobj : public Relobj
   do_should_include_member(Symbol_table* symtab, Layout*, Read_symbols_data*,
                            std::string* why);
 
+  // Iterate over global symbols, calling a visitor class V for each.
+  void
+  do_for_all_global_symbols(Read_symbols_data* sd,
+			    Library_base::Symbol_visitor_base& v);
+
   // Read the relocs.
   void
   do_read_relocs(Read_relocs_data*);
diff --git a/gold/plugin.cc b/gold/plugin.cc
index 7e259e0..0cd76cb 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -985,6 +985,22 @@ Sized_pluginobj<size, big_endian>::do_should_include_member(
   return Archive::SHOULD_INCLUDE_UNKNOWN;
 }
 
+// Iterate over global symbols, calling a visitor class V for each.
+
+template<int size, bool big_endian>
+void
+Sized_pluginobj<size, big_endian>::do_for_all_global_symbols(
+    Read_symbols_data*,
+    Library_base::Symbol_visitor_base& v)
+{
+  for (int i = 0; i < this->nsyms_; ++i)
+    {
+      const struct ld_plugin_symbol& sym = this->syms_[i];
+      if (sym.def != LDPK_UNDEF)
+	v.visit(sym.name);
+    }
+}
+
 // Get the size of a section.  Not used for plugin objects.
 
 template<int size, bool big_endian>
diff --git a/gold/plugin.h b/gold/plugin.h
index 87747bf..b9a0718 100644
--- a/gold/plugin.h
+++ b/gold/plugin.h
@@ -449,6 +449,11 @@ class Sized_pluginobj : public Pluginobj
   do_should_include_member(Symbol_table* symtab, Layout*, Read_symbols_data*,
                            std::string* why);
 
+  // Iterate over global symbols, calling a visitor class V for each.
+  void
+  do_for_all_global_symbols(Read_symbols_data* sd,
+			    Library_base::Symbol_visitor_base& v);
+
   // Get the size of a section.
   uint64_t
   do_section_size(unsigned int shndx);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [gold patch] incremental 5/18: support --start-lib/--end-lib
  2011-03-28 23:00 [gold patch] incremental 5/18: support --start-lib/--end-lib Cary Coutant
@ 2011-03-30  0:24 ` Ian Lance Taylor
  2011-03-30  0:36   ` Cary Coutant
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Lance Taylor @ 2011-03-30  0:24 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

Cary Coutant <ccoutant@google.com> writes:

> I've got a series of 14 more patches lined up now for incremental
> linking support, including the four that are still pending from last
> September. I've updated the pending patches a bit and rebased them to
> the current head. I'll send them to you a few at a time.

Just checking--are you going to resend the patches from last September?
(I think there are only three, as one was approved.)

> 2011-03-28 Cary Coutant  <ccoutant@google.com>
>
> 	* archive.cc (Library_base::should_include_member): Move
> 	method here from class Archive.
> 	(Archive::Archive): Initialize base class.
> 	(Archive::should_include_member): Move to base class.
> 	(Archive::do_for_all_unused_symbols): New function.
> 	(Add_archive_symbols::run): Remove redundant access to
> 	incremental_inputs.
> 	(Lib_group::Lib_group): Initialize base class.
> 	(Lib_group::do_filename): New function.
> 	(Lib_group::include_member): Pass pointer to Lib_group to
> 	report_object.
> 	(Lib_group::do_for_all_unused_symbols): New function.
> 	(Add_lib_group_symbols::run): Report archive information for
> 	incremental links.
> 	* archive.h (class Library_base): New base class.
> 	(class Archive): Derive from Library_base.
> 	(Archive::filename): Move to base class.
> 	(Archive::set_incremental_info): Likewise.
> 	(Archive::incremental_info): Likewise.
> 	(Archive::Should_include): Likewise.
> 	(Archive::should_include_member): Likewise.
> 	(Archive::Armap_entry): Remove.
> 	(Archive::Unused_symbol_iterator): Remove.
> 	(Archive::unused_symbols_begin): Remove.
> 	(Archive::unused_symbols_end): Remove.
> 	(Archive::do_filename): New function.
> 	(Archive::do_get_mtime): New function.
> 	(Archive::do_for_all_unused_symbols): New function.
> 	(Archive::task_): Move to base class.
> 	(Archive::incremental_info_): Likewise.
> 	(class Lib_group): Derive from Library_base.
> 	(Lib_group::do_filename): New function.
> 	(Lib_group::do_get_mtime): New function.
> 	(Lib_group::do_for_all_unused_symbols): New function.
> 	(Lib_group::task_): Move to base class.
> 	* dynobj.cc (Sized_dynobj::do_for_all_global_symbols): New
> 	function.
> 	* dynobj.h (Sized_dynobj::do_for_all_global_symbols): New
> 	function.
> 	* incremental.cc (Incremental_inputs::report_archive_begin):
> 	Use Library_base; call library's get_mtime; add incremental inputs
> 	entry before members.
> 	(class Unused_symbol_visitor): New class.
> 	(Incremental_inputs::report_archive_end): Use Library_base; use
> 	visitor class to record unused symbols; don't add incremental inputs
> 	entry after members.
> 	(Incremental_inputs::report_object): Use Library_base.
> 	* incremental.h
> 	(Incremental_archive_entry::Incremental_archive_entry): Remove
> 	unused Archive parameter.
> 	(Incremental_inputs::report_archive_begin): Use Library_base.
> 	(Incremental_inputs::report_archive_end): Likewise.
> 	(Incremental_inputs::report_object): Likewise.
> 	* object.cc (Sized_relobj::do_for_all_global_symbols): New
> 	function.
> 	* object.h (Object::for_all_global_symbols): New function.
> 	(Object::do_for_all_global_symbols): New function.
> 	(Sized_relobj::do_for_all_global_symbols): New function.
> 	* plugin.cc (Sized_pluginobj::do_for_all_global_symbols):  New
> 	function.
> 	* plugin.h (Sized_pluginobj::do_for_all_global_symbols):  New
> 	function.

> +// Iterate over all unused symbols, and call the visitor class V for each.
> +
> +void
> +Archive::do_for_all_unused_symbols(Symbol_visitor_base& v) const
> +{
> +  for (std::vector<Armap_entry>::const_iterator p = this->armap_.begin();
> +       p != this->armap_.end();
> +       ++p)
> +    {
> +      if (this->seen_offsets_.find(p->file_offset)
> +          == this->seen_offsets_.end())
> +        v.visit(this->armap_names_.data() + p->name_offset);
> +    }
> +}

In gold we should avoid non-const references.  Make v a pointer instead.

> +  // Abstract base class for processing unused symbols.
> +  class Symbol_visitor_base
> +  {
> +   public:
> +    Symbol_visitor_base()
> +    { }
> +
> +    virtual
> +    ~Symbol_visitor_base()
> +    { }
> +
> +    virtual void
> +    visit(const char*) = 0;
> +  };

The visit member function should have a comment saying that the
parameter is.

> +  // Iterator for unused global symbols in the library.
> +  void
> +  for_all_unused_symbols(Symbol_visitor_base& v) const
> +  { this->do_for_all_unused_symbols(v); }

Clarify: unused symbol definitions or unused symbol references?

> +// Iterate over global symbols, calling a visitor class V for each.

s/global symbol/global defined symbols/

> +// Visitor class for processing the unused global symbols in a library.
> +
> +class Unused_symbol_visitor : public Library_base::Symbol_visitor_base
> +{

Expand comment: what sort of processing?

> diff --git a/gold/object.cc b/gold/object.cc
> index e2c0113..954961a 100644
> --- a/gold/object.cc
> +++ b/gold/object.cc
> @@ -1691,6 +1691,31 @@ Sized_relobj<size, big_endian>::do_should_include_member(Symbol_table* symtab,
>    return Archive::SHOULD_INCLUDE_UNKNOWN;
>  }
>  
> +// Iterate over global symbols, calling a visitor class V for each.
> +
> +template<int size, bool big_endian>
> +void
> +Sized_relobj<size, big_endian>::do_for_all_global_symbols(
> +    Read_symbols_data* sd,
> +    Library_base::Symbol_visitor_base& v)
> +{

s/global symbols/global defined symbols/


This is OK with those changes.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [gold patch] incremental 5/18: support --start-lib/--end-lib
  2011-03-30  0:24 ` Ian Lance Taylor
@ 2011-03-30  0:36   ` Cary Coutant
  0 siblings, 0 replies; 3+ messages in thread
From: Cary Coutant @ 2011-03-30  0:36 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Binutils

> Just checking--are you going to resend the patches from last September?
> (I think there are only three, as one was approved.)

Yes. This one was the first of the unreviewed patches from September,
and is largely unchanged from then, except for rebasing it to the
current head. The other two I posted yesterday are also pretty close
to the ones from September. The next one, which I'm about to post, has
been updated substantially from the September version (that's the big
one that adds the bulk of the incremental update support).

Thanks for reviewing it!

-cary

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-03-30  0:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-28 23:00 [gold patch] incremental 5/18: support --start-lib/--end-lib Cary Coutant
2011-03-30  0:24 ` Ian Lance Taylor
2011-03-30  0:36   ` Cary Coutant

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).