This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Correct constructor priority ordering


After a lot of thought, I still managed to misread the GNU linker file
and mess up the ordering of constructors with priorities compared to
constructors without priorities.  This patch fixes that and updates
the test case.  This makes the code a little bit simpler.

Ian


2008-03-29  Ian Lance Taylor  <iant@google.com>

	* output.cc
	(Output_section::Input_section_sort_entry::has_priority): New
	function.
	(Output_section::Input_section_sort_entry::match_file_name): New
	function.
	(Output_section::Input_section_sort_entry::match_section_name):
	Remove.
	(Output_section::Input_section_sort_entry::match_section_name_prefix):
	Remove.
	(Output_section::Input_section_sort_entry::match_section_file):
	Remove.
	(Output_section::Input_section_sort_compare::operator()): Rewrite
	using new Input_section_sort_entry functions.  Sort crtbegin and
	crtend first.  Sort sections with no priority before sections with
	a priority.
	* testsuite/initpri1.c (d3): Check j != 4.
	(cd5): New constructor/destructor function.
	(main): Check j != 2.


Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.67
diff -u -p -r1.67 output.cc
--- output.cc	28 Mar 2008 22:42:34 -0000	1.67
+++ output.cc	29 Mar 2008 08:05:31 -0000
@@ -2055,42 +2055,25 @@ class Output_section::Input_section_sort
     return this->section_name_;
   }
 
-  // Return true if the section name is either SECTION_NAME1 or
-  // SECTION_NAME2.
+  // Return true if the section name has a priority.  This is assumed
+  // to be true if it has a dot after the initial dot.
   bool
-  match_section_name(const char* section_name1, const char* section_name2) const
+  has_priority() const
   {
     gold_assert(this->section_has_name_);
-    return (this->section_name_ == section_name1
-	    || this->section_name_ == section_name2);
+    return this->section_name_.find('.', 1);
   }
 
-  // Return true if PREFIX1 or PREFIX2 is a prefix of the section
-  // name.
+  // Return true if this an input file whose base name matches
+  // FILE_NAME.  The base name must have an extension of ".o", and
+  // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
+  // This is to match crtbegin.o as well as crtbeginS.o without
+  // getting confused by other possibilities.  Overall matching the
+  // file name this way is a dreadful hack, but the GNU linker does it
+  // in order to better support gcc, and we need to be compatible.
   bool
-  match_section_name_prefix(const char* prefix1, const char* prefix2) const
+  match_file_name(const char* match_file_name) const
   {
-    gold_assert(this->section_has_name_);
-    return (this->section_name_.compare(0, strlen(prefix1), prefix1) == 0
-	    || this->section_name_.compare(0, strlen(prefix2), prefix2) == 0);
-  }
-
-  // Return true if this is for a section named SECTION_NAME1 or
-  // SECTION_NAME2 in an input file whose base name matches FILE_NAME.
-  // The base name must have an extension of ".o", and must be exactly
-  // FILE_NAME.o or FILE_NAME, one character, ".o".  This is to match
-  // crtbegin.o as well as crtbeginS.o without getting confused by
-  // other possibilities.  Overall matching the file name this way is
-  // a dreadful hack, but the GNU linker does it in order to better
-  // support gcc, and we need to be compatible.
-  bool
-  match_section_file(const char* section_name1, const char* section_name2,
-		     const char* match_file_name) const
-  {
-    gold_assert(this->section_has_name_);
-    if (this->section_name_ != section_name1
-	&& this->section_name_ != section_name2)
-      return false;
     const std::string& file_name(this->input_section_.relobj()->name());
     const char* base_name = lbasename(file_name.c_str());
     size_t match_len = strlen(match_file_name);
@@ -2121,20 +2104,9 @@ Output_section::Input_section_sort_compa
     const Output_section::Input_section_sort_entry& s1,
     const Output_section::Input_section_sort_entry& s2) const
 {
-  // We sort all the sections with no names to the end.
-  if (!s1.section_has_name() || !s2.section_has_name())
-    {
-      if (s1.section_has_name())
-	return true;
-      if (s2.section_has_name())
-	return false;
-      return s1.index() < s2.index();
-    }
-
-  // A .ctors or .dtors section from crtbegin.o must come before any
-  // other .ctors* or .dtors* section.
-  bool s1_begin = s1.match_section_file(".ctors", ".dtors", "crtbegin");
-  bool s2_begin = s2.match_section_file(".ctors", ".dtors", "crtbegin");
+  // crtbegin.o must come first.
+  bool s1_begin = s1.match_file_name("crtbegin");
+  bool s2_begin = s2.match_file_name("crtbegin");
   if (s1_begin || s2_begin)
     {
       if (!s1_begin)
@@ -2144,10 +2116,9 @@ Output_section::Input_section_sort_compa
       return s1.index() < s2.index();
     }
 
-  // A .ctors or .dtors section from crtend.o must come after any
-  // other .ctors* or .dtors* section.
-  bool s1_end = s1.match_section_file(".ctors", ".dtors", "crtend");
-  bool s2_end = s2.match_section_file(".ctors", ".dtors", "crtend");
+  // crtend.o must come last.
+  bool s1_end = s1.match_file_name("crtend");
+  bool s2_end = s2.match_file_name("crtend");
   if (s1_end || s2_end)
     {
       if (!s1_end)
@@ -2157,22 +2128,24 @@ Output_section::Input_section_sort_compa
       return s1.index() < s2.index();
     }
 
-  // A .ctors or .init_array section with a priority precedes a .ctors
-  // or .init_array section without a priority.
-  if (s1.match_section_name_prefix(".ctors.", ".init_array.")
-      && s2.match_section_name(".ctors", ".init_array"))
-    return true;
-  if (s2.match_section_name_prefix(".ctors.", ".init_array.")
-      && s1.match_section_name(".ctors", ".init_array"))
-    return false;
+  // We sort all the sections with no names to the end.
+  if (!s1.section_has_name() || !s2.section_has_name())
+    {
+      if (s1.section_has_name())
+	return true;
+      if (s2.section_has_name())
+	return false;
+      return s1.index() < s2.index();
+    }
 
-  // A .dtors or .fini_array section with a priority follows a .dtors
-  // or .fini_array section without a priority.
-  if (s1.match_section_name_prefix(".dtors.", ".fini_array.")
-      && s2.match_section_name(".dtors", ".fini_array"))
+  // A section with a priority follows a section without a priority.
+  // The GNU linker does this for all but .init_array sections; until
+  // further notice we'll assume that that is an mistake.
+  bool s1_has_priority = s1.has_priority();
+  bool s2_has_priority = s2.has_priority();
+  if (s1_has_priority && !s2_has_priority)
     return false;
-  if (s2.match_section_name_prefix(".dtors.", ".fini_array.")
-      && s1.match_section_name(".dtors", ".fini_array"))
+  if (!s1_has_priority && s2_has_priority)
     return true;
 
   // Otherwise we sort by name.
Index: testsuite/initpri1.c
===================================================================
RCS file: /cvs/src/src/gold/testsuite/initpri1.c,v
retrieving revision 1.1
diff -u -p -r1.1 initpri1.c
--- testsuite/initpri1.c	28 Mar 2008 22:42:34 -0000	1.1
+++ testsuite/initpri1.c	29 Mar 2008 08:05:31 -0000
@@ -64,7 +64,7 @@ void d2() {
 }
 
 void d3() {
-  if (j != 2)
+  if (j != 4)
     abort ();
   if (--i != 1)
     abort ();
@@ -78,10 +78,18 @@ void cd4() {
   ++j;
 }
 
+void cd5() __attribute__((constructor, destructor));
+
+void cd5() {
+  if (i != 3)
+    abort();
+  ++j;
+}
+
 int main () {
   if (i != 3)
     return 1;
-  if (j != 1)
+  if (j != 2)
     abort ();
   return 0;
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]