public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Patch to do reorder text and data sections according to a user   specified sequence.
@ 2010-02-11  2:20 Sriraman Tallam
  2010-02-12  5:29 ` Ian Lance Taylor
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-02-11  2:20 UTC (permalink / raw)
  To: binutils, Ian Lance Taylor, Cary Coutant, Xinliang David Li

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

Hi,

   I have attached a patch to reorder text and data sections in the
linker according to a user specified sequence. I have added a new
option --final-layout to do this. Currently, this could be done using
linker scripts but this patch makes it really easy to do this. Let me
explain using a simple example.

test.cc

void foo()
{ }

void bar()
{ }

int main()
{
  return 0;
}

$ g++ -ffunction-sections test.cc
$ nm -n a.out
...
000000000040038c T _Z3foov
0000000000400392 T _Z3barv
....

foo is laid to be before bar. Now, I can change this order as follows.

$ (echo "_Z3barv" && echo "_Z3foov") > sequence.txt
$ g++ -ffunction-sections test.cc -Wl,--final-layout,sequence.txt
$ nm -n a.out
...
0000000000400658 T _Z3barv
000000000040065e T _Z3foov
...

The order is changed.

This can be done for text or data sections.

I am planning to write a linker plugin to be able to do a feed-back
directed function/data layout according to the Pettis-Hansen code
positioning algorithm. The feed-back information will be supplied by
the compiler, FDO in the case of gcc. This patch is the first in the
series to do that.

Comments please.

	* layout.cc (Layout::read_layout_from_file): New method.
	(Layout::section_reorder): New method.
	(Layout::finalize): Call section_reorder when --final-layout is
	passed.
	* layout.h (Layout::read_layout_from_file): New method.
	(Layout::section_reorder): New method.
	(Layout::input_section_order): New method.
	(Layout::input_section_order_): New private member.
	* options.h (--final-layout): New option.
	* output.cc (Output_section::add_input_section): Keep input sections
	when --final-layout is used.
	(Output_section::reorder_layout): New method.
	* output.h (Output_section::reorder_layout): New method.
	(Input_section::Input_section): Initialize secname_.
	(Input_section::section_name): New method.
	(Input_section::set_section_name): New method.
	(Input_section::secname_): New member.
	* testsuite/Makefile.am (final_layout): New test case.
	* testsuite.Makefile.in: Regenerate.
	* testsuite/final_layout.cc: New file.
	* testsuite/final_layout.sh: New file.



Thanks,
-Sriraman.

[-- Attachment #2: final_layout_patch.txt --]
[-- Type: text/plain, Size: 18840 bytes --]

Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.164
diff -u -u -p -r1.164 layout.cc
--- layout.cc	23 Jan 2010 01:07:59 -0000	1.164
+++ layout.cc	11 Feb 2010 02:06:25 -0000
@@ -1572,6 +1572,56 @@ Layout::relaxation_loop_body(
   return off;
 }
 
+// Read the sequence of input sections from the file specified with
+// --final-layout.
+
+bool
+Layout::read_layout_from_file()
+{
+  const char* filename = parameters->options().final_layout();
+  char *buf = NULL;
+  size_t len = 0;
+  FILE* fp = fopen(filename, "r");
+
+  if (fp == NULL)
+    {
+      gold_error(_("Error opening layout file : %s\n"), filename);
+      gold_exit(false);
+    }
+
+  while (getline(&buf, &len, fp) != -1)
+    {
+      buf[strlen(buf) - 1] = 0;
+      this->input_section_order_.push_back(std::string(buf));
+    }
+
+  if (buf != NULL)
+    free(buf);
+
+  fclose(fp);
+  return true;
+}
+
+// If --final-layout option is used, reorder the input sections in
+// .text, .data, .bss and .rodata according to the specified sequence.
+
+void
+Layout::section_reorder()
+{
+  this->read_layout_from_file();
+
+  for (Section_list::iterator p = this->section_list_.begin();
+       p != this->section_list_.end();
+       ++p)
+    {
+      if (strcmp(".text", (*p)->name()) == 0
+          || strcmp(".data", (*p)->name()) == 0
+          || strcmp(".bss", (*p)->name()) == 0
+          || strcmp(".rodata", (*p)->name()) == 0)
+        (*p)->reorder_layout(this);
+    }
+}
+
 // Finalize the layout.  When this is called, we have created all the
 // output sections and all the output segments which are based on
 // input sections.  We have several things to do, and we have to do
@@ -1608,6 +1658,10 @@ off_t
 Layout::finalize(const Input_objects* input_objects, Symbol_table* symtab,
 		 Target* target, const Task* task)
 {
+  // Check if the input sections need to be reordered.
+  if (parameters->options().final_layout())
+    this->section_reorder();
+
   target->finalize_sections(this, input_objects, symtab);
 
   this->count_local_symbols(task, input_objects);
Index: layout.h
===================================================================
RCS file: /cvs/src/src/gold/layout.h,v
retrieving revision 1.78
diff -u -u -p -r1.78 layout.h
--- layout.h	7 Jan 2010 21:09:31 -0000	1.78
+++ layout.h	11 Feb 2010 02:06:25 -0000
@@ -308,6 +308,13 @@ class Layout
 	 const char* name, const elfcpp::Shdr<size, big_endian>& shdr,
 	 unsigned int reloc_shndx, unsigned int reloc_type, off_t* offset);
 
+  bool
+  read_layout_from_file();
+
+  std::vector<std::string>&
+  input_section_order()
+  { return this->input_section_order_; }
+
   // Layout an input reloc section when doing a relocatable link.  The
   // section is RELOC_SHNDX in OBJECT, with data in SHDR.
   // DATA_SECTION is the reloc section to which it refers.  RR is the
@@ -463,6 +470,8 @@ class Layout
 			   unsigned int shndx, bool is_comdat,
 			   bool is_group_name, Kept_section** kept_section);
 
+  void section_reorder();
+
   // Finalize the layout after all the input sections have been added.
   off_t
   finalize(const Input_objects*, Symbol_table*, Target*, const Task*);
@@ -1036,6 +1045,8 @@ class Layout
   Segment_states* segment_states_;
   // A relaxation debug checker.  We only create one when in debugging mode.
   Relaxation_debug_check* relaxation_debug_check_;
+  // Vector to store the desired sequence of input_sections.
+  std::vector<std::string> input_section_order_;
 };
 
 // This task handles writing out data in output sections which is not
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.139
diff -u -u -p -r1.139 options.h
--- options.h	22 Jan 2010 19:43:00 -0000	1.139
+++ options.h	11 Feb 2010 02:06:25 -0000
@@ -719,6 +719,10 @@ class General_options
 	      N_("Treat warnings as errors"),
 	      N_("Do not treat warnings as errors"));
 
+  DEFINE_string(final_layout, options::TWO_DASHES, '\0', NULL,
+		N_("Layout functions and data in the order specified."),
+		N_("FILENAME"));
+
   DEFINE_string(fini, options::ONE_DASH, '\0', "_fini",
                 N_("Call SYMBOL at unload-time"), N_("SYMBOL"));
 
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.118
diff -u -u -p -r1.118 output.cc
--- output.cc	23 Jan 2010 01:07:59 -0000	1.118
+++ output.cc	11 Feb 2010 02:06:25 -0000
@@ -2032,16 +2032,21 @@ Output_section::add_input_section(Sized_
   // We need to keep track of this section if we are already keeping
   // track of sections, or if we are relaxing.  Also, if this is a
   // section which requires sorting, or which may require sorting in
-  // the future, we keep track of the sections.
+  // the future, we keep track of the sections.  If the --final-layout
+  // option is used to specify the order of sections, we need to keep
+  // track of sections.
   if (have_sections_script
       || !this->input_sections_.empty()
       || this->may_sort_attached_input_sections()
       || this->must_sort_attached_input_sections()
       || parameters->options().user_set_Map()
-      || parameters->target().may_relax())
-    this->input_sections_.push_back(Input_section(object, shndx,
-						  shdr.get_sh_size(),
-						  addralign));
+      || parameters->target().may_relax()
+      || parameters->options().final_layout())
+    {
+      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
+      isecn.set_section_name(secname);
+      this->input_sections_.push_back(isecn);
+    }
 
   return aligned_offset_in_section;
 }
@@ -2604,6 +2609,83 @@ Output_section::do_set_tls_offset(uint64
   this->tls_offset_ = this->address() - tls_base;
 }
 
+void
+Output_section::reorder_layout(Layout* layout)
+{
+  std::vector<Input_section> dont_care_list;
+  std::vector<Input_section> care_list;
+  std::map<std::string, Input_section*> reorder_input_section_map;
+  // Length of the section prefix, eg : ".rodata.".
+  const unsigned int secn_prefix_len = strlen(this->name()) + 1;
+
+  // Go through the input section sequence and add them to the reorder map.
+  for (std::vector<std::string>::iterator it_v
+         = layout->input_section_order().begin();
+       it_v != layout->input_section_order().end();
+       ++it_v)
+      reorder_input_section_map[*it_v] = NULL;
+
+  unsigned int num_sections = 0;
+  // Update the map with the address of the input sections to be reordered.
+  for (unsigned int i = 0; i < this->input_sections_.size(); ++i)
+    {
+      Input_section& isecn = this->input_sections_[i];
+      if (isecn.is_input_section()
+          && isecn.section_name().length() > secn_prefix_len)
+        {
+          // Search for the symbol name which follows the section prefix.
+          std::map<std::string, Input_section*>::iterator p
+            = reorder_input_section_map.find(
+                isecn.section_name().substr(secn_prefix_len));
+          if (p == reorder_input_section_map.end())
+            {
+              dont_care_list.push_back(isecn);
+            }
+          else
+            {
+              num_sections++;
+              p->second = &isecn;
+            }
+        }
+      else
+        {
+          dont_care_list.push_back(isecn);
+        }
+    }
+
+  // There is nothing to reorder when the number of candidates is less than 2.
+  if (num_sections < 2)
+      return;
+
+  // Reorder the sections.
+  for (std::vector<std::string>::iterator it_v
+         = layout->input_section_order().begin();
+       it_v != layout->input_section_order().end();
+       ++it_v)
+    {
+      std::map<std::string, Input_section*>::iterator p
+        = reorder_input_section_map.find(*it_v);
+      gold_assert (p != reorder_input_section_map.end());
+      if (p->second == NULL)
+        continue;
+      care_list.push_back(*(p->second));
+    }
+
+  this->input_sections_.clear();
+  // Push back the re-ordered sections from the lists.
+  for (std::vector<Input_section>::iterator it = dont_care_list.begin();
+       it != dont_care_list.end();
+       ++it)
+    this->input_sections_.push_back(*it);
+
+  for (std::vector<Input_section>::iterator it = care_list.begin();
+       it != care_list.end();
+       ++it)
+    this->input_sections_.push_back(*it);
+
+  return;
+}
+
 // In a few cases we need to sort the input sections attached to an
 // output section.  This is used to implement the type of constructor
 // priority ordering implemented by the GNU linker, in which the
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.99
diff -u -u -p -r1.99 output.h
--- output.h	23 Jan 2010 01:07:59 -0000	1.99
+++ output.h	11 Feb 2010 02:06:25 -0000
@@ -2203,6 +2203,9 @@ class Output_section : public Output_dat
   Output_section(const char* name, elfcpp::Elf_Word, elfcpp::Elf_Xword);
   virtual ~Output_section();
 
+  void
+  reorder_layout(Layout* layout);
+
   // Add a new input section SHNDX, named NAME, with header SHDR, from
   // object OBJECT.  RELOC_SHNDX is the index of a relocation section
   // which applies to this section, or 0 if none, or -1 if more than
@@ -2916,7 +2919,8 @@ class Output_section : public Output_dat
     Input_section(Relobj* object, unsigned int shndx, off_t data_size,
 		  uint64_t addralign)
       : shndx_(shndx),
-	p2align_(ffsll(static_cast<long long>(addralign)))
+	p2align_(ffsll(static_cast<long long>(addralign))),
+        secname_("")
     {
       gold_assert(shndx != OUTPUT_SECTION_CODE
 		  && shndx != MERGE_DATA_SECTION_CODE
@@ -2928,7 +2932,7 @@ class Output_section : public Output_dat
 
     // For a non-merge output section.
     Input_section(Output_section_data* posd)
-      : shndx_(OUTPUT_SECTION_CODE), p2align_(0)
+      : shndx_(OUTPUT_SECTION_CODE), p2align_(0), secname_("")
     {
       this->u1_.data_size = 0;
       this->u2_.posd = posd;
@@ -2939,7 +2943,8 @@ class Output_section : public Output_dat
       : shndx_(is_string
 	       ? MERGE_STRING_SECTION_CODE
 	       : MERGE_DATA_SECTION_CODE),
-	p2align_(0)
+	p2align_(0),
+        secname_("")
     {
       this->u1_.entsize = entsize;
       this->u2_.posd = posd;
@@ -2947,12 +2952,24 @@ class Output_section : public Output_dat
 
     // For a relaxed input section.
     Input_section(Output_relaxed_input_section *psection)
-      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0)
+      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0), secname_("")
     {
       this->u1_.data_size = 0;
       this->u2_.poris = psection;
     }
 
+    std::string&
+    section_name()
+    {
+      return secname_;
+    }
+
+    void
+    set_section_name(const char* name)
+    {
+      this->secname_ = std::string(name);
+    }
+
     // The required alignment.
     uint64_t
     addralign() const
@@ -3161,6 +3178,7 @@ class Output_section : public Output_dat
       // For RELAXED_INPUT_SECTION_CODE, the data.
       Output_relaxed_input_section* poris;
     } u2_;
+    std::string secname_;
   };
 
   typedef std::vector<Input_section> Input_section_list;
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.118
diff -u -u -p -r1.118 Makefile.am
--- testsuite/Makefile.am	12 Jan 2010 19:12:40 -0000	1.118
+++ testsuite/Makefile.am	11 Feb 2010 02:06:25 -0000
@@ -179,6 +179,18 @@ icf_safe_test: icf_safe_test.o gcctestdi
 icf_safe_test.stdout: icf_safe_test
 	$(TEST_NM) icf_safe_test > icf_safe_test.stdout
 
+check_SCRIPTS += final_layout.sh
+check_DATA += final_layout.stdout
+MOSTLYCLEANFILES += final_layout
+final_layout.o: final_layout.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections  -fdata-sections -g -o $@ $<
+final_layout_sequence.txt:
+	(echo "_Z3barv" && echo "_Z3bazv" && echo "_Z3foov" && echo "global_varb" && echo "global_vara" && echo "global_varc") > final_layout_sequence.txt
+final_layout: final_layout.o final_layout_sequence.txt gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,--final-layout,final_layout_sequence.txt final_layout.o
+final_layout.stdout: final_layout
+	$(TEST_NM) final_layout > final_layout.stdout
+
 check_PROGRAMS += basic_test
 check_PROGRAMS += basic_static_test
 check_PROGRAMS += basic_pic_test
Index: testsuite/Makefile.in
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.in,v
retrieving revision 1.124
diff -u -u -p -r1.124 Makefile.in
--- testsuite/Makefile.in	12 Jan 2010 19:12:40 -0000	1.124
+++ testsuite/Makefile.in	11 Feb 2010 02:06:25 -0000
@@ -60,7 +60,7 @@ check_PROGRAMS = object_unittest$(EXEEXT
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	gc_orphan_section_test.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_test.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_keep_unique_test.sh \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test.sh final_layout.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared.sh weak_plt.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	debug_msg.sh undef_symbol.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_1.sh ver_test_2.sh \
@@ -87,6 +87,7 @@ check_PROGRAMS = object_unittest$(EXEEXT
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_test.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_keep_unique_test.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test.stdout \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	final_layout.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared.dbg \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	weak_plt_shared.so debug_msg.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	debug_msg_so.err \
@@ -107,7 +108,7 @@ check_PROGRAMS = object_unittest$(EXEEXT
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	gc_comdat_test gc_tls_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	gc_orphan_section_test icf_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_keep_unique_test \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test final_layout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared.dbg \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	alt/weak_undef_lib.so
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_4 = basic_test \
@@ -2609,6 +2610,14 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_test.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_test.stdout: icf_safe_test
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_NM) icf_safe_test > icf_safe_test.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@final_layout.o: final_layout.cc 
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXCOMPILE) -O0 -c -ffunction-sections  -fdata-sections -g -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@final_layout_sequence.txt:
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	(echo "_Z3barv" && echo "_Z3bazv" && echo "_Z3foov" && echo "global_varb" && echo "global_vara" && echo "global_varc") > final_layout_sequence.txt
+@GCC_TRUE@@NATIVE_LINKER_TRUE@final_layout: final_layout.o final_layout_sequence.txt gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,--final-layout,final_layout_sequence.txt final_layout.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@final_layout.stdout: final_layout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_NM) final_layout > final_layout.stdout
 @GCC_TRUE@@NATIVE_LINKER_TRUE@basic_test.o: basic_test.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXCOMPILE) -O0 -c -o $@ $<
 @GCC_TRUE@@NATIVE_LINKER_TRUE@basic_test: basic_test.o gcctestdir/ld
Index: testsuite/final_layout.cc
===================================================================
RCS file: testsuite/final_layout.cc
diff -N testsuite/final_layout.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.cc	11 Feb 2010 02:06:25 -0000
@@ -0,0 +1,48 @@
+// final_layout.cc -- a test case for gold
+
+// Copyright 2010 Free Software Foundation, Inc.
+// Written by Sriraman Tallam <tmsriram@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify if --final-layout orders the .text
+// and .data sections correctly as specified.
+
+int global_vara;
+int global_varb;
+int global_varc;
+
+int foo()
+{
+  return 1;
+}
+
+int bar()
+{
+  return 1;
+}
+
+int baz()
+{
+  return 1;
+}
+
+int main()
+{
+  return 1;
+}
Index: testsuite/final_layout.sh
===================================================================
RCS file: testsuite/final_layout.sh
diff -N testsuite/final_layout.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.sh	11 Feb 2010 02:06:25 -0000
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+# final_layout.sh -- test --final-layout
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Sriraman Tallam <tmsriram@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# The goal of this program is to verify if --final-layoyt icf works as
+# intended.  File final_layout.cc is in this test.
+
+check()
+{
+    func_addr_1=$((16#`grep $2 $1 | awk '{print $1}'`))
+    func_addr_2=$((16#`grep $3 $1 | awk '{print $1}'`))
+    if [ $func_addr_1 -gt $func_addr_2 ]
+    then
+        echo "final layout of" $2 "and" $3 "is not right."
+	exit 1
+    fi
+}
+
+check final_layout.stdout "_Z3barv" "_Z3bazv"
+check final_layout.stdout "_Z3bazv" "_Z3foov"
+check final_layout.stdout "global_varb" "global_vara"
+check final_layout.stdout "global_vara" "global_varc"

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-02-11  2:20 Patch to do reorder text and data sections according to a user specified sequence Sriraman Tallam
@ 2010-02-12  5:29 ` Ian Lance Taylor
  2010-03-03  2:43   ` Sriraman Tallam
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Lance Taylor @ 2010-02-12  5:29 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: binutils, Cary Coutant, Xinliang David Li

Sriraman Tallam <tmsriram@google.com> writes:

>    I have attached a patch to reorder text and data sections in the
> linker according to a user specified sequence. I have added a new
> option --final-layout to do this. Currently, this could be done using
> linker scripts but this patch makes it really easy to do this. Let me
> explain using a simple example.
>
> test.cc
>
> void foo()
> { }
>
> void bar()
> { }
>
> int main()
> {
>   return 0;
> }
>
> $ g++ -ffunction-sections test.cc
> $ nm -n a.out
> ...
> 000000000040038c T _Z3foov
> 0000000000400392 T _Z3barv
> ....
>
> foo is laid to be before bar. Now, I can change this order as follows.
>
> $ (echo "_Z3barv" && echo "_Z3foov") > sequence.txt
> $ g++ -ffunction-sections test.cc -Wl,--final-layout,sequence.txt
> $ nm -n a.out
> ...
> 0000000000400658 T _Z3barv
> 000000000040065e T _Z3foov
> ...
>
> The order is changed.
>
> This can be done for text or data sections.


As I understand it, in linker terms, you are sorting the sections by
suffixes.  When two input sections are in the same output section, and
both input sections have suffixes which appear in the file, then the
input sections are sorted in the order in which the suffixes appear in
the file.

I think it would be more natural to sort the input sections by name
rather than by suffix.  Since you don't want to fuss with writing
".text." all the time, suppose we say that we sort the input sections
by name, and we match the input section names using glob patterns.  We
already use glob patterns in linker scripts, so that is not a big
stretch.

Just a few comments on the rest of the patch.



> +// Read the sequence of input sections from the file specified with
> +// --final-layout.
> +
> +bool
> +Layout::read_layout_from_file()
> +{
> +  const char* filename = parameters->options().final_layout();
> +  char *buf = NULL;
> +  size_t len = 0;
> +  FILE* fp = fopen(filename, "r");
> +
> +  if (fp == NULL)
> +    {
> +      gold_error(_("Error opening layout file : %s\n"), filename);
> +      gold_exit(false);
> +    }
> +
> +  while (getline(&buf, &len, fp) != -1)
> +    {
> +      buf[strlen(buf) - 1] = 0;
> +      this->input_section_order_.push_back(std::string(buf));
> +    }
> +
> +  if (buf != NULL)
> +    free(buf);
> +
> +  fclose(fp);
> +  return true;
> +}

The getline function is insufficient portable for use in gold.  Search
for std::getline in options.cc for an alternate approach you can use.
Emulate the error message style you see there too--no capital letter,
name the file, no space before colon, no \n.  And if you really want
to exit on failure, call gold_fatal.


> +// If --final-layout option is used, reorder the input sections in
> +// .text, .data, .bss and .rodata according to the specified sequence.
> +
> +void
> +Layout::section_reorder()
> +{
> +  this->read_layout_from_file();
> +
> +  for (Section_list::iterator p = this->section_list_.begin();
> +       p != this->section_list_.end();
> +       ++p)
> +    {
> +      if (strcmp(".text", (*p)->name()) == 0
> +          || strcmp(".data", (*p)->name()) == 0
> +          || strcmp(".bss", (*p)->name()) == 0
> +          || strcmp(".rodata", (*p)->name()) == 0)
> +        (*p)->reorder_layout(this);
> +    }
> +}

Why restrict this to those output sections?  Why not sort input
sections in any output section?


> +  DEFINE_string(final_layout, options::TWO_DASHES, '\0', NULL,
> +		N_("Layout functions and data in the order specified."),
> +		N_("FILENAME"));

I'm not sure I care for --final-layout as the option name.  Perhaps
--section-ordering-file?  Perhaps somebody else has a better idea.


> +  // the future, we keep track of the sections.  If the --final-layout
> +  // option is used to specify the order of sections, we need to keep
> +  // track of sections.
>    if (have_sections_script
>        || !this->input_sections_.empty()
>        || this->may_sort_attached_input_sections()
>        || this->must_sort_attached_input_sections()
>        || parameters->options().user_set_Map()
> -      || parameters->target().may_relax())
> -    this->input_sections_.push_back(Input_section(object, shndx,
> -						  shdr.get_sh_size(),
> -						  addralign));
> +      || parameters->target().may_relax()
> +      || parameters->options().final_layout())
> +    {
> +      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
> +      isecn.set_section_name(secname);
> +      this->input_sections_.push_back(isecn);
> +    }

Don't save the string here, that's just going to bloat memory usage.
Instead, when you read the file, give each line a number.  Then match
the section name which you have here against the list of patterns.  If
you find a match, store the number in the Input_section structure.
Also, if you find a match, set a flag in the output section.  Then
sort the sections, by number, in a function called from
set_final_data_size.

You will see that there is already some section ordering in that
function, which is used to implement constructor/destructor priority
ordering.  I guess the priority ordering should take precedence.
Maybe.

Ian

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

* Re: Patch to do reorder text and data sections according to a user   specified sequence.
  2010-02-12  5:29 ` Ian Lance Taylor
@ 2010-03-03  2:43   ` Sriraman Tallam
  2010-03-05 23:11     ` Taras Glek
  2010-05-14  0:58     ` Sriraman Tallam
  0 siblings, 2 replies; 24+ messages in thread
From: Sriraman Tallam @ 2010-03-03  2:43 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: binutils, Cary Coutant, Xinliang David Li

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

Hi Ian,

    I finally got around to making the changes you specified. Please
take a look when you get a chance.

	* layout.cc (Layout::read_layout_from_file): New method.
	(Layout::layout): Pass this pointer to add_input_section.
	(Layout::layout_eh_frame): Ditto.
	* layout.h (Layout::read_layout_from_file): New method.
	(Layout::input_section_order): New method.
	(Layout::input_section_order_): New private member.
	* main.cc (main): Call read_layout_from_file here.
	* options.h (--section-ordering-file): New option.
	* output.cc (Output_section::input_section_order_specified_): New
	member.
	(Output_section::add_input_section): Add new parameter.
	Keep input sections when --section-ordering-file is used.
	(Output_section::set_final_data_size): Sort input sections when
	section ordering file is specified.
	(Output_section::Input_section_sort_entry::section_order_index_): New
	member.
	(Output_section::Input_section_sort_entry::section_order_index): New
	method.
	(Output_section::Input_section_sort_compare::operator()): Change to
	consider section_order_index.
	(Output_section::Input_section_sort_init_fini_compare::operator()):
	Change to consider section_order_index.
	(Output_section::Input_section_sort_section_order_index_compare
	::operator()): New method.
	(Output_section::sort_attached_input_sections): Change to sort
	according to section order when specified.
	* output.h (Output_section::input_section_order_specified): New
	method.
	(Output_section::set_input_section_order_specified): New method.
	(Input_section::glob_pattern_number): New method.
	(Input_section::set_glob_pattern_number): New method.
	(Input_section::glob_pattern_number_): New member.
	(Input_section::Input_section_sort_section_order_index_compare): New
	struct.
	(Output_section::input_section_order_specified_): New member.
	* testsuite/Makefile.am (final_layout): New test case.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/final_layout.cc: New file.
	* testsuite/final_layout.sh: New file


Thanks,
-Sriraman.


On Thu, Feb 11, 2010 at 9:29 PM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>>    I have attached a patch to reorder text and data sections in the
>> linker according to a user specified sequence. I have added a new
>> option --final-layout to do this. Currently, this could be done using
>> linker scripts but this patch makes it really easy to do this. Let me
>> explain using a simple example.
>>
>> test.cc
>>
>> void foo()
>> { }
>>
>> void bar()
>> { }
>>
>> int main()
>> {
>>   return 0;
>> }
>>
>> $ g++ -ffunction-sections test.cc
>> $ nm -n a.out
>> ...
>> 000000000040038c T _Z3foov
>> 0000000000400392 T _Z3barv
>> ....
>>
>> foo is laid to be before bar. Now, I can change this order as follows.
>>
>> $ (echo "_Z3barv" && echo "_Z3foov") > sequence.txt
>> $ g++ -ffunction-sections test.cc -Wl,--final-layout,sequence.txt
>> $ nm -n a.out
>> ...
>> 0000000000400658 T _Z3barv
>> 000000000040065e T _Z3foov
>> ...
>>
>> The order is changed.
>>
>> This can be done for text or data sections.
>
>
> As I understand it, in linker terms, you are sorting the sections by
> suffixes.  When two input sections are in the same output section, and
> both input sections have suffixes which appear in the file, then the
> input sections are sorted in the order in which the suffixes appear in
> the file.
>
> I think it would be more natural to sort the input sections by name
> rather than by suffix.  Since you don't want to fuss with writing
> ".text." all the time, suppose we say that we sort the input sections
> by name, and we match the input section names using glob patterns.  We
> already use glob patterns in linker scripts, so that is not a big
> stretch.
>
> Just a few comments on the rest of the patch.
>
>
>
>> +// Read the sequence of input sections from the file specified with
>> +// --final-layout.
>> +
>> +bool
>> +Layout::read_layout_from_file()
>> +{
>> +  const char* filename = parameters->options().final_layout();
>> +  char *buf = NULL;
>> +  size_t len = 0;
>> +  FILE* fp = fopen(filename, "r");
>> +
>> +  if (fp == NULL)
>> +    {
>> +      gold_error(_("Error opening layout file : %s\n"), filename);
>> +      gold_exit(false);
>> +    }
>> +
>> +  while (getline(&buf, &len, fp) != -1)
>> +    {
>> +      buf[strlen(buf) - 1] = 0;
>> +      this->input_section_order_.push_back(std::string(buf));
>> +    }
>> +
>> +  if (buf != NULL)
>> +    free(buf);
>> +
>> +  fclose(fp);
>> +  return true;
>> +}
>
> The getline function is insufficient portable for use in gold.  Search
> for std::getline in options.cc for an alternate approach you can use.
> Emulate the error message style you see there too--no capital letter,
> name the file, no space before colon, no \n.  And if you really want
> to exit on failure, call gold_fatal.
>
>
>> +// If --final-layout option is used, reorder the input sections in
>> +// .text, .data, .bss and .rodata according to the specified sequence.
>> +
>> +void
>> +Layout::section_reorder()
>> +{
>> +  this->read_layout_from_file();
>> +
>> +  for (Section_list::iterator p = this->section_list_.begin();
>> +       p != this->section_list_.end();
>> +       ++p)
>> +    {
>> +      if (strcmp(".text", (*p)->name()) == 0
>> +          || strcmp(".data", (*p)->name()) == 0
>> +          || strcmp(".bss", (*p)->name()) == 0
>> +          || strcmp(".rodata", (*p)->name()) == 0)
>> +        (*p)->reorder_layout(this);
>> +    }
>> +}
>
> Why restrict this to those output sections?  Why not sort input
> sections in any output section?
>
>
>> +  DEFINE_string(final_layout, options::TWO_DASHES, '\0', NULL,
>> +             N_("Layout functions and data in the order specified."),
>> +             N_("FILENAME"));
>
> I'm not sure I care for --final-layout as the option name.  Perhaps
> --section-ordering-file?  Perhaps somebody else has a better idea.
>
>
>> +  // the future, we keep track of the sections.  If the --final-layout
>> +  // option is used to specify the order of sections, we need to keep
>> +  // track of sections.
>>    if (have_sections_script
>>        || !this->input_sections_.empty()
>>        || this->may_sort_attached_input_sections()
>>        || this->must_sort_attached_input_sections()
>>        || parameters->options().user_set_Map()
>> -      || parameters->target().may_relax())
>> -    this->input_sections_.push_back(Input_section(object, shndx,
>> -                                               shdr.get_sh_size(),
>> -                                               addralign));
>> +      || parameters->target().may_relax()
>> +      || parameters->options().final_layout())
>> +    {
>> +      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
>> +      isecn.set_section_name(secname);
>> +      this->input_sections_.push_back(isecn);
>> +    }
>
> Don't save the string here, that's just going to bloat memory usage.
> Instead, when you read the file, give each line a number.  Then match
> the section name which you have here against the list of patterns.  If
> you find a match, store the number in the Input_section structure.
> Also, if you find a match, set a flag in the output section.  Then
> sort the sections, by number, in a function called from
> set_final_data_size.
>
> You will see that there is already some section ordering in that
> function, which is used to implement constructor/destructor priority
> ordering.  I guess the priority ordering should take precedence.
> Maybe.
>
> Ian
>

[-- Attachment #2: final_layout_patch.txt --]
[-- Type: text/plain, Size: 23085 bytes --]

Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.167
diff -u -u -p -r1.167 layout.cc
--- layout.cc	1 Mar 2010 21:43:49 -0000	1.167
+++ layout.cc	3 Mar 2010 02:03:36 -0000
@@ -26,6 +26,7 @@
 #include <cstring>
 #include <algorithm>
 #include <iostream>
+#include <fstream>
 #include <utility>
 #include <fcntl.h>
 #include <unistd.h>
@@ -636,7 +637,8 @@ Layout::layout(Sized_relobj<size, big_en
   // FIXME: Handle SHF_LINK_ORDER somewhere.
 
   *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
-			       this->script_options_->saw_sections_clause());
+			       this->script_options_->saw_sections_clause(),
+                               this);
   this->have_added_input_section_ = true;
 
   return os;
@@ -845,7 +847,7 @@ Layout::layout_eh_frame(Sized_relobj<siz
       // Add it as a normal section.
       bool saw_sections_clause = this->script_options_->saw_sections_clause();
       *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
-				   saw_sections_clause);
+				   saw_sections_clause, this);
       this->have_added_input_section_ = true;
     }
 
@@ -1593,6 +1595,31 @@ Layout::relaxation_loop_body(
   return off;
 }
 
+// Read the sequence of input sections from the file specified with
+// --section-ordering-file.
+
+void
+Layout::read_layout_from_file()
+{
+  const char* filename = parameters->options().section_ordering_file();
+  std::ifstream in;
+  std::string line;
+
+  in.open(filename);
+  std::getline(in, line);   // this chops off the trailing \n, if any
+  if (!in)
+    gold_fatal(_("unable to open --section-ordering-file file %s: %s"),
+               filename, strerror(errno));
+
+  while (in)
+    {
+      if (!line.empty() && line[line.length() - 1] == '\r')   // Windows
+        line.resize(line.length() - 1);
+      this->input_section_order_.push_back(line);
+      std::getline(in, line);
+    }
+}
+
 // Finalize the layout.  When this is called, we have created all the
 // output sections and all the output segments which are based on
 // input sections.  We have several things to do, and we have to do
Index: layout.h
===================================================================
RCS file: /cvs/src/src/gold/layout.h,v
retrieving revision 1.79
diff -u -u -p -r1.79 layout.h
--- layout.h	9 Feb 2010 20:29:44 -0000	1.79
+++ layout.h	3 Mar 2010 02:03:36 -0000
@@ -308,6 +308,13 @@ class Layout
 	 const char* name, const elfcpp::Shdr<size, big_endian>& shdr,
 	 unsigned int reloc_shndx, unsigned int reloc_type, off_t* offset);
 
+  void
+  read_layout_from_file();
+
+  std::vector<std::string>&
+  input_section_order()
+  { return this->input_section_order_; }
+
   // Layout an input reloc section when doing a relocatable link.  The
   // section is RELOC_SHNDX in OBJECT, with data in SHDR.
   // DATA_SECTION is the reloc section to which it refers.  RR is the
@@ -1036,6 +1043,8 @@ class Layout
   Segment_states* segment_states_;
   // A relaxation debug checker.  We only create one when in debugging mode.
   Relaxation_debug_check* relaxation_debug_check_;
+  // Vector to store the desired sequence of input_sections.
+  std::vector<std::string> input_section_order_;
 };
 
 // This task handles writing out data in output sections which is not
Index: main.cc
===================================================================
RCS file: /cvs/src/src/gold/main.cc,v
retrieving revision 1.37
diff -u -u -p -r1.37 main.cc
--- main.cc	5 Jan 2010 21:52:51 -0000	1.37
+++ main.cc	3 Mar 2010 02:03:36 -0000
@@ -234,6 +234,9 @@ main(int argc, char** argv)
       layout.incremental_inputs()->report_inputs(command_line.inputs());
     }
 
+  if (parameters->options().section_ordering_file())
+    layout.read_layout_from_file();
+
   // Get the search path from the -L options.
   Dirsearch search_path;
   search_path.initialize(&workqueue, &command_line.options().library_path());
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.140
diff -u -u -p -r1.140 options.h
--- options.h	13 Feb 2010 02:04:20 -0000	1.140
+++ options.h	3 Mar 2010 02:03:36 -0000
@@ -868,6 +868,10 @@ class General_options
                  N_("Add DIR to link time shared library search path"),
                  N_("DIR"));
 
+  DEFINE_string(section_ordering_file, options::TWO_DASHES, '\0', NULL,
+		N_("Layout functions and data in the order specified."),
+		N_("FILENAME"));
+
   DEFINE_special(section_start, options::TWO_DASHES, '\0',
 		 N_("Set address of section"), N_("SECTION=ADDRESS"));
 
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.121
diff -u -u -p -r1.121 output.cc
--- output.cc	1 Mar 2010 21:43:49 -0000	1.121
+++ output.cc	3 Mar 2010 02:03:36 -0000
@@ -29,6 +29,7 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
+#include <fnmatch.h>
 #include <algorithm>
 #include "libiberty.h"
 
@@ -1893,6 +1894,7 @@ Output_section::Output_section(const cha
     found_in_sections_clause_(false),
     has_load_address_(false),
     info_uses_section_index_(false),
+    input_section_order_specified_(false),
     may_sort_attached_input_sections_(false),
     must_sort_attached_input_sections_(false),
     attached_input_sections_are_sorted_(false),
@@ -1961,7 +1963,8 @@ Output_section::add_input_section(Sized_
 				  const char* secname,
 				  const elfcpp::Shdr<size, big_endian>& shdr,
 				  unsigned int reloc_shndx,
-				  bool have_sections_script)
+				  bool have_sections_script,
+				  Layout* layout)
 {
   elfcpp::Elf_Xword addralign = shdr.get_sh_addralign();
   if ((addralign & (addralign - 1)) != 0)
@@ -2048,16 +2051,38 @@ Output_section::add_input_section(Sized_
   // We need to keep track of this section if we are already keeping
   // track of sections, or if we are relaxing.  Also, if this is a
   // section which requires sorting, or which may require sorting in
-  // the future, we keep track of the sections.
+  // the future, we keep track of the sections.  If the
+  // --section-ordering-file option is used to specify the order of
+  // sections, we need to keep track of sections.
   if (have_sections_script
       || !this->input_sections_.empty()
       || this->may_sort_attached_input_sections()
       || this->must_sort_attached_input_sections()
       || parameters->options().user_set_Map()
-      || parameters->target().may_relax())
-    this->input_sections_.push_back(Input_section(object, shndx,
-						  shdr.get_sh_size(),
-						  addralign));
+      || parameters->target().may_relax()
+      || parameters->options().section_ordering_file())
+    {
+      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
+      if (parameters->options().section_ordering_file())
+        {
+          // Check if this section name matches any of the input glob
+          // patterns.
+          unsigned int glob_pattern_number = 1;
+          for (std::vector<std::string>::iterator it_v
+               = layout->input_section_order().begin();
+               it_v != layout->input_section_order().end();
+               ++it_v, ++glob_pattern_number) 
+            {
+              if (fnmatch((*it_v).c_str(),secname, FNM_NOESCAPE) == 0)
+	        {
+                  isecn.set_glob_pattern_number(glob_pattern_number);
+		  this->set_input_section_order_specified();
+                  break;
+                }
+            }
+        }
+      this->input_sections_.push_back(isecn);
+    }
 
   return aligned_offset_in_section;
 }
@@ -2561,7 +2586,8 @@ Output_section::set_final_data_size()
       return;
     }
 
-  if (this->must_sort_attached_input_sections())
+  if (this->must_sort_attached_input_sections()
+      || this->input_section_order_specified())
     this->sort_attached_input_sections();
 
   uint64_t address = this->address();
@@ -2633,16 +2659,20 @@ class Output_section::Input_section_sort
  public:
   Input_section_sort_entry()
     : input_section_(), index_(-1U), section_has_name_(false),
-      section_name_()
+      section_name_(), section_order_index_(0)
   { }
 
   Input_section_sort_entry(const Input_section& input_section,
-			   unsigned int index)
+			   unsigned int index,
+			   unsigned int section_order_index,
+			   bool must_sort_attached_input_sections)
     : input_section_(input_section), index_(index),
       section_has_name_(input_section.is_input_section()
-			|| input_section.is_relaxed_input_section())
+			|| input_section.is_relaxed_input_section()),
+      section_order_index_(section_order_index)
   {
-    if (this->section_has_name_)
+    if (this->section_has_name_
+        && must_sort_attached_input_sections)
       {
 	// This is only called single-threaded from Layout::finalize,
 	// so it is OK to lock.  Unfortunately we have no way to pass
@@ -2719,6 +2749,10 @@ class Output_section::Input_section_sort
     return memcmp(base_name + base_len - 2, ".o", 2) == 0;
   }
 
+  unsigned int
+  section_order_index() const
+  { return section_order_index_; }
+
  private:
   // The Input_section we are sorting.
   Input_section input_section_;
@@ -2729,6 +2763,7 @@ class Output_section::Input_section_sort
   bool section_has_name_;
   // The section name if there is one.
   std::string section_name_;
+  unsigned int section_order_index_;
 };
 
 // Return true if S1 should come before S2 in the output section.
@@ -2780,6 +2815,11 @@ Output_section::Input_section_sort_compa
   if (!s1_has_priority && s2_has_priority)
     return true;
 
+  // Check if a input sequence has been specified.
+  if (s1.section_order_index() > 0
+      || s2.section_order_index() > 0)
+    return (s1.section_order_index() < s2.section_order_index());
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2816,6 +2856,11 @@ Output_section::Input_section_sort_init_
   if (!s1_has_priority && s2_has_priority)
     return false;
 
+  // Check if a input sequence has been specified.
+  if (s1.section_order_index() > 0
+      || s2.section_order_index() > 0)
+    return (s1.section_order_index() < s2.section_order_index());
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2825,6 +2870,19 @@ Output_section::Input_section_sort_init_
   return s1.index() < s2.index();
 }
 
+// Return true if S1 should come before S2.
+bool
+Output_section::Input_section_sort_section_order_index_compare::operator()(
+    const Output_section::Input_section_sort_entry& s1,
+    const Output_section::Input_section_sort_entry& s2) const
+{
+  if (s1.section_order_index() > 0
+      || s2.section_order_index() > 0)
+    return (s1.section_order_index() < s2.section_order_index());
+  // Otherwise we keep the input order.
+  return s1.index() < s2.index();
+}
+
 // Sort the input sections attached to an output section.
 
 void
@@ -2850,17 +2908,28 @@ Output_section::sort_attached_input_sect
   for (Input_section_list::iterator p = this->input_sections_.begin();
        p != this->input_sections_.end();
        ++p, ++i)
-    sort_list.push_back(Input_section_sort_entry(*p, i));
+    sort_list.push_back(Input_section_sort_entry(*p, i,
+                          (*p).glob_pattern_number(),
+			  this->must_sort_attached_input_sections()));
 
   // Sort the input sections.
-  if (this->type() == elfcpp::SHT_PREINIT_ARRAY
-      || this->type() == elfcpp::SHT_INIT_ARRAY
-      || this->type() == elfcpp::SHT_FINI_ARRAY)
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_init_fini_compare());
+  if (this->must_sort_attached_input_sections())
+    {
+      if (this->type() == elfcpp::SHT_PREINIT_ARRAY
+          || this->type() == elfcpp::SHT_INIT_ARRAY
+          || this->type() == elfcpp::SHT_FINI_ARRAY)
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_init_fini_compare());
+      else
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_compare());
+    }
   else
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_compare());
+    {
+      gold_assert(parameters->options().section_ordering_file());
+      std::sort(sort_list.begin(), sort_list.end(),
+	        Input_section_sort_section_order_index_compare());
+    }
 
   // Copy the sorted input sections back to our list.
   this->input_sections_.clear();
@@ -4387,7 +4456,8 @@ Output_section::add_input_section<32, fa
     const char* secname,
     const elfcpp::Shdr<32, false>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -4399,7 +4469,8 @@ Output_section::add_input_section<32, tr
     const char* secname,
     const elfcpp::Shdr<32, true>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -4411,7 +4482,8 @@ Output_section::add_input_section<64, fa
     const char* secname,
     const elfcpp::Shdr<64, false>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -4423,7 +4495,8 @@ Output_section::add_input_section<64, tr
     const char* secname,
     const elfcpp::Shdr<64, true>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_32_LITTLE
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.102
diff -u -u -p -r1.102 output.h
--- output.h	1 Mar 2010 21:43:50 -0000	1.102
+++ output.h	3 Mar 2010 02:03:36 -0000
@@ -2336,7 +2336,8 @@ class Output_section : public Output_dat
   add_input_section(Sized_relobj<size, big_endian>* object, unsigned int shndx,
 		    const char *name,
 		    const elfcpp::Shdr<size, big_endian>& shdr,
-		    unsigned int reloc_shndx, bool have_sections_script);
+		    unsigned int reloc_shndx, bool have_sections_script,
+		    Layout* layout);
 
   // Add generated data POSD to this output section.
   void
@@ -2554,6 +2555,18 @@ class Output_section : public Output_dat
   set_may_sort_attached_input_sections()
   { this->may_sort_attached_input_sections_ = true; }
 
+  // Returns true if one or more input section names match the given
+  // glob patterns through --section-order.
+  bool
+  input_section_order_specified()
+  { return this->input_section_order_specified_; }
+
+  // Record that input sections must be sorted according to the order
+  // specified.
+  void
+  set_input_section_order_specified()
+  { this->input_section_order_specified_ = true; }
+
   // Return whether the input sections attached to this output section
   // require sorting.  This is used to handle constructor priorities
   // compatibly with GNU ld.
@@ -3037,7 +3050,8 @@ class Output_section : public Output_dat
     Input_section(Relobj* object, unsigned int shndx, off_t data_size,
 		  uint64_t addralign)
       : shndx_(shndx),
-	p2align_(ffsll(static_cast<long long>(addralign)))
+	p2align_(ffsll(static_cast<long long>(addralign))),
+        glob_pattern_number_(0)
     {
       gold_assert(shndx != OUTPUT_SECTION_CODE
 		  && shndx != MERGE_DATA_SECTION_CODE
@@ -3049,7 +3063,8 @@ class Output_section : public Output_dat
 
     // For a non-merge output section.
     Input_section(Output_section_data* posd)
-      : shndx_(OUTPUT_SECTION_CODE), p2align_(0)
+      : shndx_(OUTPUT_SECTION_CODE), p2align_(0),
+        glob_pattern_number_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.posd = posd;
@@ -3060,7 +3075,8 @@ class Output_section : public Output_dat
       : shndx_(is_string
 	       ? MERGE_STRING_SECTION_CODE
 	       : MERGE_DATA_SECTION_CODE),
-	p2align_(0)
+	p2align_(0),
+        glob_pattern_number_(0)
     {
       this->u1_.entsize = entsize;
       this->u2_.posd = posd;
@@ -3068,12 +3084,25 @@ class Output_section : public Output_dat
 
     // For a relaxed input section.
     Input_section(Output_relaxed_input_section *psection)
-      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0)
+      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0),
+        glob_pattern_number_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.poris = psection;
     }
 
+    unsigned int
+    glob_pattern_number()
+    {
+      return this->glob_pattern_number_;
+    }
+
+    void
+    set_glob_pattern_number(unsigned int number)
+    {
+      this->glob_pattern_number_ = number;  
+    }
+
     // The required alignment.
     uint64_t
     addralign() const
@@ -3282,6 +3311,9 @@ class Output_section : public Output_dat
       // For RELAXED_INPUT_SECTION_CODE, the data.
       Output_relaxed_input_section* poris;
     } u2_;
+    // The line number of the glob pattern it matches in the --section-order
+    // file.  It is 0 if does not match any pattern.
+    unsigned int glob_pattern_number_;
   };
 
   typedef std::vector<Input_section> Input_section_list;
@@ -3394,6 +3426,15 @@ class Output_section : public Output_dat
 	       const Input_section_sort_entry&) const;
   };
 
+  // This is the sort comparison function when a section order is specified
+  // from an input file.
+  struct Input_section_sort_section_order_index_compare
+  {
+    bool
+    operator()(const Input_section_sort_entry&,
+	       const Input_section_sort_entry&) const;
+  };
+
   // Fill data.  This is used to fill in data between input sections.
   // It is also used for data statements (BYTE, WORD, etc.) in linker
   // scripts.  When we have to keep track of the input sections, we
@@ -3627,6 +3668,9 @@ class Output_section : public Output_dat
   // section, false if it means the symbol index of the corresponding
   // section symbol.
   bool info_uses_section_index_ : 1;
+  // True if input sections attached to this output section have to be
+  // sorted according to a specified order.
+  bool input_section_order_specified_ : 1;
   // True if the input sections attached to this output section may
   // need sorting.
   bool may_sort_attached_input_sections_ : 1;
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.124
diff -u -u -p -r1.124 Makefile.am
--- testsuite/Makefile.am	27 Feb 2010 00:36:49 -0000	1.124
+++ testsuite/Makefile.am	3 Mar 2010 02:03:36 -0000
@@ -193,6 +193,18 @@ icf_safe_so_test_1.stdout: icf_safe_so_t
 icf_safe_so_test_2.stdout: icf_safe_so_test
 	$(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
 
+check_SCRIPTS += final_layout.sh
+check_DATA += final_layout.stdout
+MOSTLYCLEANFILES += final_layout
+final_layout.o: final_layout.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections  -fdata-sections -g -o $@ $<
+final_layout_sequence.txt:
+	(echo "*_Z3barv*" && echo "*_Z3bazv*" && echo "*_Z3foov*" && echo "*global_varb*" && echo "*global_vara*" && echo "*global_varc*") > final_layout_sequence.txt
+final_layout: final_layout.o final_layout_sequence.txt gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,--section-ordering-file,final_layout_sequence.txt final_layout.o
+final_layout.stdout: final_layout
+	$(TEST_NM) final_layout > final_layout.stdout
+
 check_PROGRAMS += basic_test
 check_PROGRAMS += basic_static_test
 check_PROGRAMS += basic_pic_test
Index: testsuite/final_layout.cc
===================================================================
RCS file: testsuite/final_layout.cc
diff -N testsuite/final_layout.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.cc	3 Mar 2010 02:03:36 -0000
@@ -0,0 +1,48 @@
+// final_layout.cc -- a test case for gold
+
+// Copyright 2010 Free Software Foundation, Inc.
+// Written by Sriraman Tallam <tmsriram@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify if --section-ordering-file orders
+// the .text and .data sections correctly as specified.
+
+int global_vara;
+int global_varb;
+int global_varc;
+
+int foo()
+{
+  return 1;
+}
+
+int bar()
+{
+  return 1;
+}
+
+int baz()
+{
+  return 1;
+}
+
+int main()
+{
+  return 1;
+}
Index: testsuite/final_layout.sh
===================================================================
RCS file: testsuite/final_layout.sh
diff -N testsuite/final_layout.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.sh	3 Mar 2010 02:03:36 -0000
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+# final_layout.sh -- test --final-layout
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Sriraman Tallam <tmsriram@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# The goal of this program is to verify if --section-ordering-file works as
+# intended.  File final_layout.cc is in this test.
+
+check()
+{
+    func_addr_1=$((16#`grep $2 $1 | awk '{print $1}'`))
+    func_addr_2=$((16#`grep $3 $1 | awk '{print $1}'`))
+    if [ $func_addr_1 -gt $func_addr_2 ]
+    then
+        echo "final layout of" $2 "and" $3 "is not right."
+	exit 1
+    fi
+}
+
+check final_layout.stdout "_Z3barv" "_Z3bazv"
+check final_layout.stdout "_Z3bazv" "_Z3foov"
+check final_layout.stdout "global_varb" "global_vara"
+check final_layout.stdout "global_vara" "global_varc"

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

* Re: Patch to do reorder text and data sections according to a user    specified sequence.
  2010-03-03  2:43   ` Sriraman Tallam
@ 2010-03-05 23:11     ` Taras Glek
  2010-03-06  2:35       ` Taras Glek
  2010-03-08  8:28       ` Sriraman Tallam
  2010-05-14  0:58     ` Sriraman Tallam
  1 sibling, 2 replies; 24+ messages in thread
From: Taras Glek @ 2010-03-05 23:11 UTC (permalink / raw)
  To: Sriraman Tallam
  Cc: Ian Lance Taylor, binutils, Cary Coutant, Xinliang David Li

On 03/02/2010 06:43 PM, Sriraman Tallam wrote:
> Hi Ian,
>
>      I finally got around to making the changes you specified. Please
> take a look when you get a chance.

Hi Sriraman,
Ian pointed me at your patch when he realized I was trying to do the 
same thing with linker scripts. This is a big improvement over hacking 
linker scripts in terms of usability.

I had two issues with the patch:
a) It would be nice to specify the sections by symbol names alone. So 
instead of bss.globalvar one could say "globalvar". Ian pointed out that 
I solve this with a *.globalvar regexp since you use fnmatch(). Which 
lead me to my next problem.
b) This patch appears to have N^2 behavior since you are doing fnmatch() 
through the entire input_section_order list.

My link time for a large mozilla binary went from under 3s to over 
10minutes when I specified the order(24K entries).

I think adding a second argument to -section-ordering-file could help. 
The second argument could be 
-section-ordering-match=<symbol|section|regexp> where symbol/section 
could use a more efficient search.

Taras

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

* Re: Patch to do reorder text and data sections according to a user    specified sequence.
  2010-03-05 23:11     ` Taras Glek
@ 2010-03-06  2:35       ` Taras Glek
  2010-03-08  8:28       ` Sriraman Tallam
  1 sibling, 0 replies; 24+ messages in thread
From: Taras Glek @ 2010-03-06  2:35 UTC (permalink / raw)
  To: binutils; +Cc: Ian Lance Taylor, binutils, Cary Coutant, Xinliang David Li

On 03/02/2010 06:43 PM, Sriraman Tallam wrote:
> Hi Ian,
>
>      I finally got around to making the changes you specified. Please
> take a look when you get a chance.

Hi Sriraman,
Ian pointed me at your patch when he realized I was trying to do the 
same thing with linker scripts. This is a big improvement over hacking 
linker scripts in terms of usability.

I had two issues with the patch:
a) It would be nice to specify the sections by symbol names alone. So 
instead of bss.globalvar one could say "globalvar". Ian pointed out that 
I solve this with a *.globalvar regexp since you use fnmatch(). Which 
lead me to my next problem.
b) This patch appears to have N^2 behavior since you are doing fnmatch() 
through the entire input_section_order list.

My link time for a large mozilla binary went from under 3s to over 
10minutes when I specified the order(24K entries).

I think adding a second argument to -section-ordering-file could help. 
The second argument could be 
-section-ordering-match=<symbol|section|regexp> where symbol/section 
could use a more efficient search.

Taras

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

* Re: Patch to do reorder text and data sections according to a user   specified sequence.
  2010-03-05 23:11     ` Taras Glek
  2010-03-06  2:35       ` Taras Glek
@ 2010-03-08  8:28       ` Sriraman Tallam
  2010-03-08 19:48         ` Sriraman Tallam
  1 sibling, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-03-08  8:28 UTC (permalink / raw)
  To: Taras Glek; +Cc: Ian Lance Taylor, binutils, Cary Coutant, Xinliang David Li

Hi Taras,

On Fri, Mar 5, 2010 at 3:11 PM, Taras Glek <tglek@mozilla.com> wrote:
> On 03/02/2010 06:43 PM, Sriraman Tallam wrote:
>>
>> Hi Ian,
>>
>>     I finally got around to making the changes you specified. Please
>> take a look when you get a chance.
>
> Hi Sriraman,
> Ian pointed me at your patch when he realized I was trying to do the same
> thing with linker scripts. This is a big improvement over hacking linker
> scripts in terms of usability.
>
> I had two issues with the patch:
> a) It would be nice to specify the sections by symbol names alone. So
> instead of bss.globalvar one could say "globalvar". Ian pointed out that I
> solve this with a *.globalvar regexp since you use fnmatch(). Which lead me
> to my next problem.
> b) This patch appears to have N^2 behavior since you are doing fnmatch()
> through the entire input_section_order list.
>
> My link time for a large mozilla binary went from under 3s to over 10minutes
> when I specified the order(24K entries).

This was the thing that bothered me a lot when I was writing this
patch. I just did not think somebody would face it this soon. I was
thinking about how I could incorporate some kind of binary search.

>
> I think adding a second argument to -section-ordering-file could help. The
> second argument could be -section-ordering-match=<symbol|section|regexp>
> where symbol/section could use a more efficient search.

Maybe for symbol and section, I could provide a binary search option
if you could pass the symbol and section names in sorted order. That
would bring it down to NlogN. Did you have anything else in mind ?

Thanks,
-Sriraman.

>
> Taras
>

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

* Re: Patch to do reorder text and data sections according to a user   specified sequence.
  2010-03-08  8:28       ` Sriraman Tallam
@ 2010-03-08 19:48         ` Sriraman Tallam
  2010-03-08 20:08           ` Taras Glek
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-03-08 19:48 UTC (permalink / raw)
  To: Taras Glek; +Cc: Ian Lance Taylor, binutils, Cary Coutant, Xinliang David Li

On Sat, Mar 6, 2010 at 1:53 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Hi Taras,
>
> On Fri, Mar 5, 2010 at 3:11 PM, Taras Glek <tglek@mozilla.com> wrote:
>> On 03/02/2010 06:43 PM, Sriraman Tallam wrote:
>>>
>>> Hi Ian,
>>>
>>>     I finally got around to making the changes you specified. Please
>>> take a look when you get a chance.
>>
>> Hi Sriraman,
>> Ian pointed me at your patch when he realized I was trying to do the same
>> thing with linker scripts. This is a big improvement over hacking linker
>> scripts in terms of usability.
>>
>> I had two issues with the patch:
>> a) It would be nice to specify the sections by symbol names alone. So
>> instead of bss.globalvar one could say "globalvar". Ian pointed out that I
>> solve this with a *.globalvar regexp since you use fnmatch(). Which lead me
>> to my next problem.
>> b) This patch appears to have N^2 behavior since you are doing fnmatch()
>> through the entire input_section_order list.
>>
>> My link time for a large mozilla binary went from under 3s to over 10minutes
>> when I specified the order(24K entries).
>
> This was the thing that bothered me a lot when I was writing this
> patch. I just did not think somebody would face it this soon. I was
> thinking about how I could incorporate some kind of binary search.
>
>>
>> I think adding a second argument to -section-ordering-file could help. The
>> second argument could be -section-ordering-match=<symbol|section|regexp>
>> where symbol/section could use a more efficient search.
>
> Maybe for symbol and section, I could provide a binary search option
> if you could pass the symbol and section names in sorted order. That
> would bring it down to NlogN. Did you have anything else in mind ?

I do not know what I was drinking when I wrote this as this does not
make much sense.  You obviously cannot sort the section names in the
input file as the order is fixed. However, I thought about this a
little bit more. If you can provide the integer index of the position
next to the symbol name in the input file then you can sort the names
in the input file. A binary search is then possible. Thoughts ?

>
> Thanks,
> -Sriraman.
>
>>
>> Taras
>>
>

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

* Re: Patch to do reorder text and data sections according to a user   specified sequence.
  2010-03-08 19:48         ` Sriraman Tallam
@ 2010-03-08 20:08           ` Taras Glek
  2010-03-08 21:41             ` Ian Lance Taylor
  0 siblings, 1 reply; 24+ messages in thread
From: Taras Glek @ 2010-03-08 20:08 UTC (permalink / raw)
  To: Sriraman Tallam
  Cc: Ian Lance Taylor, binutils, Cary Coutant, Xinliang David Li

On 03/08/2010 10:55 AM, Sriraman Tallam wrote:
> On Sat, Mar 6, 2010 at 1:53 PM, Sriraman Tallam<tmsriram@google.com>  wrote:
>    
>> Hi Taras,
>>
>> On Fri, Mar 5, 2010 at 3:11 PM, Taras Glek<tglek@mozilla.com>  wrote:
>>      
>>> On 03/02/2010 06:43 PM, Sriraman Tallam wrote:
>>>        
>>>> Hi Ian,
>>>>
>>>>      I finally got around to making the changes you specified. Please
>>>> take a look when you get a chance.
>>>>          
>>> Hi Sriraman,
>>> Ian pointed me at your patch when he realized I was trying to do the same
>>> thing with linker scripts. This is a big improvement over hacking linker
>>> scripts in terms of usability.
>>>
>>> I had two issues with the patch:
>>> a) It would be nice to specify the sections by symbol names alone. So
>>> instead of bss.globalvar one could say "globalvar". Ian pointed out that I
>>> solve this with a *.globalvar regexp since you use fnmatch(). Which lead me
>>> to my next problem.
>>> b) This patch appears to have N^2 behavior since you are doing fnmatch()
>>> through the entire input_section_order list.
>>>
>>> My link time for a large mozilla binary went from under 3s to over 10minutes
>>> when I specified the order(24K entries).
>>>        
>> This was the thing that bothered me a lot when I was writing this
>> patch. I just did not think somebody would face it this soon. I was
>> thinking about how I could incorporate some kind of binary search.
>>
>>      
>>> I think adding a second argument to -section-ordering-file could help. The
>>> second argument could be -section-ordering-match=<symbol|section|regexp>
>>> where symbol/section could use a more efficient search.
>>>        
>> Maybe for symbol and section, I could provide a binary search option
>> if you could pass the symbol and section names in sorted order. That
>> would bring it down to NlogN. Did you have anything else in mind ?
>>      
> I do not know what I was drinking when I wrote this as this does not
> make much sense.  You obviously cannot sort the section names in the
> input file as the order is fixed. However, I thought about this a
> little bit more. If you can provide the integer index of the position
> next to the symbol name in the input file then you can sort the names
> in the input file. A binary search is then possible. Thoughts ?
>    
That sounds like a pretty severe hack. You can just use an stl 
map<filename, position in order file> instead of the vector to achieve a 
similar effect.
  Seems like matching symbols enmass should be a relatively common 
problem. Perhaps Ian can suggest the best way to do such an operation 
efficiently?

An easy solution for me would be to get rid of wildcards in the input 
file and instead of do partial matches based on the ending of the 
string. Ie sort the section-ordering-file back-to-front(like you 
suggested in another email), then you could match ".text.main" and 
"main" equally effectively. I think nlogn behavior is ok for now.


Taras

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

* Re: Patch to do reorder text and data sections according to a user specified sequence.
  2010-03-08 20:08           ` Taras Glek
@ 2010-03-08 21:41             ` Ian Lance Taylor
  0 siblings, 0 replies; 24+ messages in thread
From: Ian Lance Taylor @ 2010-03-08 21:41 UTC (permalink / raw)
  To: Taras Glek; +Cc: Sriraman Tallam, binutils, Cary Coutant, Xinliang David Li

Taras Glek <tglek@mozilla.com> writes:

>  Seems like matching symbols enmass should be a relatively common
> problem. Perhaps Ian can suggest the best way to do such an operation
> efficiently?

Perhaps someone can, but I can't.  Linker scripts face a similar
issue.  There I separate the patterns which contain wildcards from the
ones which do not, and create a hash table for the patterns which
don't have wildcards.  For the others I just call fnmatch one at a
time.

It would be possible to convert a set of glob expressions into a
single regular expression.  However, I can't see any reason why
matching that regular expression would be any faster than calling
fnmatch several times.

Ian

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-03-03  2:43   ` Sriraman Tallam
  2010-03-05 23:11     ` Taras Glek
@ 2010-05-14  0:58     ` Sriraman Tallam
  2010-05-22  1:42       ` Sriraman Tallam
  1 sibling, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-05-14  0:58 UTC (permalink / raw)
  To: Ian Lance Taylor, Taras Glek; +Cc: binutils, Cary Coutant, Xinliang David Li

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

Hi Ian and Taras,

     Sorry for taking this long to get back. I have modified the patch
to address Taras' timing issue. Basically, I split patterns specified
into glob and non-glob by just looking for wild-card characters in the
pattern. Non-glob look-ups are through a hash table and fast. Glob
patterns are be searched linearly. So, if you specify non-glob symbol
names in the section ordering file, the link should be faster than
before.

     Further, I have prioritized non-glob patters over glob patterns.
Let me explain with an example :

If object main.o has sections .text._foov, .text._barv, and
.text._zapv and the seection ordering file is :
===============================
*bar*
.text.*
.text._barv
===============================

Then, .text._barv will be the last section in .text even though it
matches the first glob pattern.


The changes are :

	* gold.h (is_wildcard_string): New function.
	* layout.cc (Layout::layout): Pass this pointer to add_input_section.
	(Layout::layout_eh_frame): Ditto.
	(Layout::find_section_order_index): New method.
	(Layout::read_layout_from_file): New method.
	* layout.h (Layout::find_section_order_index): New method.
	(Layout::read_layout_from_file): New method.
	(Layout::input_section_position_): New private member.
	(Layout::input_section_glob_): New private member.
	* main.cc (main): Call read_layout_from_file here.
	* options.h (--section-ordering-file): New option.
	* output.cc (Output_section::input_section_order_specified_): New
	member.
	(Output_section::Output_section): Initialize new member.
	(Output_section::add_input_section): Add new parameter.
	Keep input sections when --section-ordering-file is used.
	(Output_section::set_final_data_size): Sort input sections when
	section ordering file is specified.
	(Output_section::Input_section_sort_entry): Add new parameter.
	Check sorting type.
	(Output_section::Input_section_sort_entry::is_before_in_sequence):
	New method.
	(Output_section::Input_section_sort_compare::operator()): Change to
	consider section_order_index.
	(Output_section::Input_section_sort_init_fini_compare::operator()):
	Change to consider section_order_index.
	(Output_section::Input_section_sort_section_order_index_compare
	::operator()): New method.
	(Output_section::sort_attached_input_sections): Change to sort
	according to section order when specified.
	(Output_section::add_input_section<32, true>): Add new parameter.
	(Output_section::add_input_section<64, true>): Add new parameter.
	(Output_section::add_input_section<32, false>): Add new parameter.
	(Output_section::add_input_section<64, false>): Add new parameter.
	* output.h (Output_section::add_input_section): Add new parameter.
	(Output_section::input_section_order_specified): New
	method.
	(Output_section::set_input_section_order_specified): New method.
	(Input_section::Input_section): Initialize section_order_index_.
	(Input_section::section_order_index): New method.
	(Input_section::set_section_order_index): New method.
	(Input_section::section_order_index_): New member.
	(Input_section::Input_section_sort_section_order_index_compare): New
	struct.
	(Output_section::input_section_order_specified_): New member.
	* script-sections.cc (is_wildcard_string): Delete and move modified
	method to gold.h.
	(Output_section_element_input::Output_section_element_input): Modify
	call to is_wildcard_string.
	(Output_section_element_input::Input_section_pattern
	::Input_section_pattern): Ditto.
	(Output_section_element_input::Output_section_element_input): Ditto.
	* testsuite/Makefile.am (final_layout): New test case.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/final_layout.cc: New file.
	* testsuite/final_layout.sh: New file.

Please let me know what you think.

Thanks,
-Sri.



On Tue, Mar 2, 2010 at 7:43 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Hi Ian,
>
>    I finally got around to making the changes you specified. Please
> take a look when you get a chance.
>
>        * layout.cc (Layout::read_layout_from_file): New method.
>        (Layout::layout): Pass this pointer to add_input_section.
>        (Layout::layout_eh_frame): Ditto.
>        * layout.h (Layout::read_layout_from_file): New method.
>        (Layout::input_section_order): New method.
>        (Layout::input_section_order_): New private member.
>        * main.cc (main): Call read_layout_from_file here.
>        * options.h (--section-ordering-file): New option.
>        * output.cc (Output_section::input_section_order_specified_): New
>        member.
>        (Output_section::add_input_section): Add new parameter.
>        Keep input sections when --section-ordering-file is used.
>        (Output_section::set_final_data_size): Sort input sections when
>        section ordering file is specified.
>        (Output_section::Input_section_sort_entry::section_order_index_): New
>        member.
>        (Output_section::Input_section_sort_entry::section_order_index): New
>        method.
>        (Output_section::Input_section_sort_compare::operator()): Change to
>        consider section_order_index.
>        (Output_section::Input_section_sort_init_fini_compare::operator()):
>        Change to consider section_order_index.
>        (Output_section::Input_section_sort_section_order_index_compare
>        ::operator()): New method.
>        (Output_section::sort_attached_input_sections): Change to sort
>        according to section order when specified.
>        * output.h (Output_section::input_section_order_specified): New
>        method.
>        (Output_section::set_input_section_order_specified): New method.
>        (Input_section::glob_pattern_number): New method.
>        (Input_section::set_glob_pattern_number): New method.
>        (Input_section::glob_pattern_number_): New member.
>        (Input_section::Input_section_sort_section_order_index_compare): New
>        struct.
>        (Output_section::input_section_order_specified_): New member.
>        * testsuite/Makefile.am (final_layout): New test case.
>        * testsuite/Makefile.in: Regenerate.
>        * testsuite/final_layout.cc: New file.
>        * testsuite/final_layout.sh: New file
>
>
> Thanks,
> -Sriraman.
>
>
> On Thu, Feb 11, 2010 at 9:29 PM, Ian Lance Taylor <iant@google.com> wrote:
>> Sriraman Tallam <tmsriram@google.com> writes:
>>
>>>    I have attached a patch to reorder text and data sections in the
>>> linker according to a user specified sequence. I have added a new
>>> option --final-layout to do this. Currently, this could be done using
>>> linker scripts but this patch makes it really easy to do this. Let me
>>> explain using a simple example.
>>>
>>> test.cc
>>>
>>> void foo()
>>> { }
>>>
>>> void bar()
>>> { }
>>>
>>> int main()
>>> {
>>>   return 0;
>>> }
>>>
>>> $ g++ -ffunction-sections test.cc
>>> $ nm -n a.out
>>> ...
>>> 000000000040038c T _Z3foov
>>> 0000000000400392 T _Z3barv
>>> ....
>>>
>>> foo is laid to be before bar. Now, I can change this order as follows.
>>>
>>> $ (echo "_Z3barv" && echo "_Z3foov") > sequence.txt
>>> $ g++ -ffunction-sections test.cc -Wl,--final-layout,sequence.txt
>>> $ nm -n a.out
>>> ...
>>> 0000000000400658 T _Z3barv
>>> 000000000040065e T _Z3foov
>>> ...
>>>
>>> The order is changed.
>>>
>>> This can be done for text or data sections.
>>
>>
>> As I understand it, in linker terms, you are sorting the sections by
>> suffixes.  When two input sections are in the same output section, and
>> both input sections have suffixes which appear in the file, then the
>> input sections are sorted in the order in which the suffixes appear in
>> the file.
>>
>> I think it would be more natural to sort the input sections by name
>> rather than by suffix.  Since you don't want to fuss with writing
>> ".text." all the time, suppose we say that we sort the input sections
>> by name, and we match the input section names using glob patterns.  We
>> already use glob patterns in linker scripts, so that is not a big
>> stretch.
>>
>> Just a few comments on the rest of the patch.
>>
>>
>>
>>> +// Read the sequence of input sections from the file specified with
>>> +// --final-layout.
>>> +
>>> +bool
>>> +Layout::read_layout_from_file()
>>> +{
>>> +  const char* filename = parameters->options().final_layout();
>>> +  char *buf = NULL;
>>> +  size_t len = 0;
>>> +  FILE* fp = fopen(filename, "r");
>>> +
>>> +  if (fp == NULL)
>>> +    {
>>> +      gold_error(_("Error opening layout file : %s\n"), filename);
>>> +      gold_exit(false);
>>> +    }
>>> +
>>> +  while (getline(&buf, &len, fp) != -1)
>>> +    {
>>> +      buf[strlen(buf) - 1] = 0;
>>> +      this->input_section_order_.push_back(std::string(buf));
>>> +    }
>>> +
>>> +  if (buf != NULL)
>>> +    free(buf);
>>> +
>>> +  fclose(fp);
>>> +  return true;
>>> +}
>>
>> The getline function is insufficient portable for use in gold.  Search
>> for std::getline in options.cc for an alternate approach you can use.
>> Emulate the error message style you see there too--no capital letter,
>> name the file, no space before colon, no \n.  And if you really want
>> to exit on failure, call gold_fatal.
>>
>>
>>> +// If --final-layout option is used, reorder the input sections in
>>> +// .text, .data, .bss and .rodata according to the specified sequence.
>>> +
>>> +void
>>> +Layout::section_reorder()
>>> +{
>>> +  this->read_layout_from_file();
>>> +
>>> +  for (Section_list::iterator p = this->section_list_.begin();
>>> +       p != this->section_list_.end();
>>> +       ++p)
>>> +    {
>>> +      if (strcmp(".text", (*p)->name()) == 0
>>> +          || strcmp(".data", (*p)->name()) == 0
>>> +          || strcmp(".bss", (*p)->name()) == 0
>>> +          || strcmp(".rodata", (*p)->name()) == 0)
>>> +        (*p)->reorder_layout(this);
>>> +    }
>>> +}
>>
>> Why restrict this to those output sections?  Why not sort input
>> sections in any output section?
>>
>>
>>> +  DEFINE_string(final_layout, options::TWO_DASHES, '\0', NULL,
>>> +             N_("Layout functions and data in the order specified."),
>>> +             N_("FILENAME"));
>>
>> I'm not sure I care for --final-layout as the option name.  Perhaps
>> --section-ordering-file?  Perhaps somebody else has a better idea.
>>
>>
>>> +  // the future, we keep track of the sections.  If the --final-layout
>>> +  // option is used to specify the order of sections, we need to keep
>>> +  // track of sections.
>>>    if (have_sections_script
>>>        || !this->input_sections_.empty()
>>>        || this->may_sort_attached_input_sections()
>>>        || this->must_sort_attached_input_sections()
>>>        || parameters->options().user_set_Map()
>>> -      || parameters->target().may_relax())
>>> -    this->input_sections_.push_back(Input_section(object, shndx,
>>> -                                               shdr.get_sh_size(),
>>> -                                               addralign));
>>> +      || parameters->target().may_relax()
>>> +      || parameters->options().final_layout())
>>> +    {
>>> +      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
>>> +      isecn.set_section_name(secname);
>>> +      this->input_sections_.push_back(isecn);
>>> +    }
>>
>> Don't save the string here, that's just going to bloat memory usage.
>> Instead, when you read the file, give each line a number.  Then match
>> the section name which you have here against the list of patterns.  If
>> you find a match, store the number in the Input_section structure.
>> Also, if you find a match, set a flag in the output section.  Then
>> sort the sections, by number, in a function called from
>> set_final_data_size.
>>
>> You will see that there is already some section ordering in that
>> function, which is used to implement constructor/destructor priority
>> ordering.  I guess the priority ordering should take precedence.
>> Maybe.
>>
>> Ian
>>
>

[-- Attachment #2: final_layout_patch.txt --]
[-- Type: text/plain, Size: 26498 bytes --]

Index: gold.h
===================================================================
RCS file: /cvs/src/src/gold/gold.h,v
retrieving revision 1.42
diff -u -u -p -r1.42 gold.h
--- gold.h	7 Jan 2010 07:14:29 -0000	1.42
+++ gold.h	14 May 2010 00:48:50 -0000
@@ -401,6 +401,15 @@ string_hash(const Char_type* s)
   return h;
 }
 
+// Return whether STRING contains a wildcard character.  This is used
+// to speed up matching.
+
+inline bool
+is_wildcard_string(const char* s)
+{
+  return strpbrk(s, "?*[") != NULL;
+}
+
 } // End namespace gold.
 
 #endif // !defined(GOLD_GOLD_H)
Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.169
diff -u -u -p -r1.169 layout.cc
--- layout.cc	24 Apr 2010 14:32:23 -0000	1.169
+++ layout.cc	14 May 2010 00:48:50 -0000
@@ -26,8 +26,10 @@
 #include <cstring>
 #include <algorithm>
 #include <iostream>
+#include <fstream>
 #include <utility>
 #include <fcntl.h>
+#include <fnmatch.h>
 #include <unistd.h>
 #include "libiberty.h"
 #include "md5.h"
@@ -669,7 +671,8 @@ Layout::layout(Sized_relobj<size, big_en
   // FIXME: Handle SHF_LINK_ORDER somewhere.
 
   *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
-			       this->script_options_->saw_sections_clause());
+			       this->script_options_->saw_sections_clause(),
+                               this);
   this->have_added_input_section_ = true;
 
   return os;
@@ -887,7 +890,7 @@ Layout::layout_eh_frame(Sized_relobj<siz
       // Add it as a normal section.
       bool saw_sections_clause = this->script_options_->saw_sections_clause();
       *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
-				   saw_sections_clause);
+				   saw_sections_clause, this);
       this->have_added_input_section_ = true;
     }
 
@@ -1642,6 +1645,65 @@ Layout::relaxation_loop_body(
   return off;
 }
 
+// Search the list of patterns and find the postion of the given section
+// name in the output section.  If the section name matches a glob
+// pattern and a non-glob name, then the non-glob position takes
+// precedence.  Return 0 if no match is found.
+
+unsigned int
+Layout::find_section_order_index(const std::string& section_name)
+{
+  std::map<std::string, unsigned int>::iterator map_it;
+  map_it = this->input_section_position_.find(section_name);
+  if (map_it != this->input_section_position_.end())
+    return map_it->second;
+  
+  // Absolute match failed.  Linear search the glob patterns.   
+  std::vector<std::string>::iterator it;
+  for (it = this->input_section_glob_.begin();
+       it != this->input_section_glob_.end();
+       ++it)
+    {
+       if (fnmatch((*it).c_str(), section_name.c_str(), FNM_NOESCAPE) == 0)
+         {
+           map_it = this->input_section_position_.find(*it);
+           gold_assert(map_it != this->input_section_position_.end());
+           return map_it->second;
+         }
+    }
+  return 0;
+}
+
+// Read the sequence of input sections from the file specified with
+// --section-ordering-file.
+
+void
+Layout::read_layout_from_file()
+{
+  const char* filename = parameters->options().section_ordering_file();
+  std::ifstream in;
+  std::string line;
+
+  in.open(filename);
+  std::getline(in, line);   // this chops off the trailing \n, if any
+  if (!in)
+    gold_fatal(_("unable to open --section-ordering-file file %s: %s"),
+               filename, strerror(errno));
+
+  unsigned int position = 1;
+  while (in)
+    {
+      if (!line.empty() && line[line.length() - 1] == '\r')   // Windows
+        line.resize(line.length() - 1);
+      this->input_section_position_[line] = position;
+      // Store all glob patterns in a vector.
+      if (is_wildcard_string(line.c_str()))
+        this->input_section_glob_.push_back(line);
+      position++;
+      std::getline(in, line);
+    }
+}
+
 // Finalize the layout.  When this is called, we have created all the
 // output sections and all the output segments which are based on
 // input sections.  We have several things to do, and we have to do
Index: layout.h
===================================================================
RCS file: /cvs/src/src/gold/layout.h,v
retrieving revision 1.80
diff -u -u -p -r1.80 layout.h
--- layout.h	9 Apr 2010 17:32:58 -0000	1.80
+++ layout.h	14 May 2010 00:48:50 -0000
@@ -308,6 +308,12 @@ class Layout
 	 const char* name, const elfcpp::Shdr<size, big_endian>& shdr,
 	 unsigned int reloc_shndx, unsigned int reloc_type, off_t* offset);
 
+  unsigned int
+  find_section_order_index(const std::string&);
+
+  void
+  read_layout_from_file();
+
   // Layout an input reloc section when doing a relocatable link.  The
   // section is RELOC_SHNDX in OBJECT, with data in SHDR.
   // DATA_SECTION is the reloc section to which it refers.  RR is the
@@ -1037,6 +1043,10 @@ class Layout
   Segment_states* segment_states_;
   // A relaxation debug checker.  We only create one when in debugging mode.
   Relaxation_debug_check* relaxation_debug_check_;
+  // Hash a pattern to its position in the section ordering file.
+  std::map<std::string, unsigned int> input_section_position_;
+  // Vector of glob only patterns in the section_ordering file.
+  std::vector<std::string> input_section_glob_;
 };
 
 // This task handles writing out data in output sections which is not
Index: main.cc
===================================================================
RCS file: /cvs/src/src/gold/main.cc,v
retrieving revision 1.38
diff -u -u -p -r1.38 main.cc
--- main.cc	22 Mar 2010 14:18:24 -0000	1.38
+++ main.cc	14 May 2010 00:48:50 -0000
@@ -234,6 +234,9 @@ main(int argc, char** argv)
       layout.incremental_inputs()->report_inputs(command_line.inputs());
     }
 
+  if (parameters->options().section_ordering_file())
+    layout.read_layout_from_file();
+
   // Get the search path from the -L options.
   Dirsearch search_path;
   search_path.initialize(&workqueue, &command_line.options().library_path());
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.145
diff -u -u -p -r1.145 options.h
--- options.h	21 Apr 2010 16:32:28 -0000	1.145
+++ options.h	14 May 2010 00:48:51 -0000
@@ -888,6 +888,10 @@ class General_options
                  N_("Add DIR to link time shared library search path"),
                  N_("DIR"));
 
+  DEFINE_string(section_ordering_file, options::TWO_DASHES, '\0', NULL,
+		N_("Layout functions and data in the order specified."),
+		N_("FILENAME"));
+
   DEFINE_special(section_start, options::TWO_DASHES, '\0',
 		 N_("Set address of section"), N_("SECTION=ADDRESS"));
 
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.124
diff -u -u -p -r1.124 output.cc
--- output.cc	9 Apr 2010 17:32:58 -0000	1.124
+++ output.cc	14 May 2010 00:48:51 -0000
@@ -29,6 +29,7 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
+#include <fnmatch.h>
 #include <algorithm>
 #include "libiberty.h"
 
@@ -1897,6 +1898,7 @@ Output_section::Output_section(const cha
     found_in_sections_clause_(false),
     has_load_address_(false),
     info_uses_section_index_(false),
+    input_section_order_specified_(false),
     may_sort_attached_input_sections_(false),
     must_sort_attached_input_sections_(false),
     attached_input_sections_are_sorted_(false),
@@ -1966,7 +1968,8 @@ Output_section::add_input_section(Sized_
 				  const char* secname,
 				  const elfcpp::Shdr<size, big_endian>& shdr,
 				  unsigned int reloc_shndx,
-				  bool have_sections_script)
+				  bool have_sections_script,
+				  Layout* layout)
 {
   elfcpp::Elf_Xword addralign = shdr.get_sh_addralign();
   if ((addralign & (addralign - 1)) != 0)
@@ -2053,16 +2056,30 @@ Output_section::add_input_section(Sized_
   // We need to keep track of this section if we are already keeping
   // track of sections, or if we are relaxing.  Also, if this is a
   // section which requires sorting, or which may require sorting in
-  // the future, we keep track of the sections.
+  // the future, we keep track of the sections.  If the
+  // --section-ordering-file option is used to specify the order of
+  // sections, we need to keep track of sections.
   if (have_sections_script
       || !this->input_sections_.empty()
       || this->may_sort_attached_input_sections()
       || this->must_sort_attached_input_sections()
       || parameters->options().user_set_Map()
-      || parameters->target().may_relax())
-    this->input_sections_.push_back(Input_section(object, shndx,
-						  shdr.get_sh_size(),
-						  addralign));
+      || parameters->target().may_relax()
+      || parameters->options().section_ordering_file())
+    {
+      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
+      if (parameters->options().section_ordering_file())
+        {
+          unsigned int section_order_index =
+            layout->find_section_order_index(std::string(secname));
+	  if (section_order_index)
+            {
+              isecn.set_section_order_index(section_order_index);
+              this->set_input_section_order_specified();
+            }
+        }
+      this->input_sections_.push_back(isecn);
+    }
 
   return aligned_offset_in_section;
 }
@@ -2563,7 +2580,8 @@ Output_section::set_final_data_size()
       return;
     }
 
-  if (this->must_sort_attached_input_sections())
+  if (this->must_sort_attached_input_sections()
+      || this->input_section_order_specified())
     this->sort_attached_input_sections();
 
   uint64_t address = this->address();
@@ -2640,12 +2658,14 @@ class Output_section::Input_section_sort
   { }
 
   Input_section_sort_entry(const Input_section& input_section,
-			   unsigned int index)
+			   unsigned int index,
+			   bool must_sort_attached_input_sections)
     : input_section_(input_section), index_(index),
       section_has_name_(input_section.is_input_section()
 			|| input_section.is_relaxed_input_section())
   {
-    if (this->section_has_name_)
+    if (this->section_has_name_
+        && must_sort_attached_input_sections)
       {
 	// This is only called single-threaded from Layout::finalize,
 	// so it is OK to lock.  Unfortunately we have no way to pass
@@ -2722,6 +2742,18 @@ class Output_section::Input_section_sort
     return memcmp(base_name + base_len - 2, ".o", 2) == 0;
   }
 
+  // Returns 0 if no order is specified. Returns 1 if "this" is the
+  // first section in order (is_before), returns 2 for s.
+  unsigned int
+  is_before_in_sequence(const Input_section_sort_entry& s) const
+  {
+    if (this->input_section_.section_order_index() == 0
+        && s.input_section().section_order_index() == 0)
+      return 0;
+    return (this->input_section_.section_order_index()
+            <= s.input_section().section_order_index());
+  }
+
  private:
   // The Input_section we are sorting.
   Input_section input_section_;
@@ -2783,6 +2815,11 @@ Output_section::Input_section_sort_compa
   if (!s1_has_priority && s2_has_priority)
     return true;
 
+  // Check if a input sequence has been specified.
+  unsigned int sequence_num = s1.is_before_in_sequence(s2);
+  if (sequence_num)
+    return sequence_num == 1;
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2819,6 +2856,11 @@ Output_section::Input_section_sort_init_
   if (!s1_has_priority && s2_has_priority)
     return false;
 
+  // Check if a input sequence has been specified.
+  unsigned int sequence_num = s1.is_before_in_sequence(s2);
+  if (sequence_num)
+    return sequence_num == 1;
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2828,6 +2870,21 @@ Output_section::Input_section_sort_init_
   return s1.index() < s2.index();
 }
 
+// Return true if S1 should come before S2.
+bool
+Output_section::Input_section_sort_section_order_index_compare::operator()(
+    const Output_section::Input_section_sort_entry& s1,
+    const Output_section::Input_section_sort_entry& s2) const
+{  
+  // Check if a input sequence has been specified.
+  unsigned int sequence_num = s1.is_before_in_sequence(s2);
+  if (sequence_num)
+    return sequence_num == 1;
+
+  // Otherwise we keep the input order.
+  return s1.index() < s2.index();
+}
+
 // Sort the input sections attached to an output section.
 
 void
@@ -2853,17 +2910,27 @@ Output_section::sort_attached_input_sect
   for (Input_section_list::iterator p = this->input_sections_.begin();
        p != this->input_sections_.end();
        ++p, ++i)
-    sort_list.push_back(Input_section_sort_entry(*p, i));
+    sort_list.push_back(Input_section_sort_entry(*p, i,
+			  this->must_sort_attached_input_sections()));
 
   // Sort the input sections.
-  if (this->type() == elfcpp::SHT_PREINIT_ARRAY
-      || this->type() == elfcpp::SHT_INIT_ARRAY
-      || this->type() == elfcpp::SHT_FINI_ARRAY)
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_init_fini_compare());
+  if (this->must_sort_attached_input_sections())
+    {
+      if (this->type() == elfcpp::SHT_PREINIT_ARRAY
+          || this->type() == elfcpp::SHT_INIT_ARRAY
+          || this->type() == elfcpp::SHT_FINI_ARRAY)
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_init_fini_compare());
+      else
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_compare());
+    }
   else
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_compare());
+    {
+      gold_assert(parameters->options().section_ordering_file());
+      std::sort(sort_list.begin(), sort_list.end(),
+	        Input_section_sort_section_order_index_compare());
+    }
 
   // Copy the sorted input sections back to our list.
   this->input_sections_.clear();
@@ -4390,7 +4457,8 @@ Output_section::add_input_section<32, fa
     const char* secname,
     const elfcpp::Shdr<32, false>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -4402,7 +4470,8 @@ Output_section::add_input_section<32, tr
     const char* secname,
     const elfcpp::Shdr<32, true>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -4414,7 +4483,8 @@ Output_section::add_input_section<64, fa
     const char* secname,
     const elfcpp::Shdr<64, false>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -4426,7 +4496,8 @@ Output_section::add_input_section<64, tr
     const char* secname,
     const elfcpp::Shdr<64, true>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_32_LITTLE
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.104
diff -u -u -p -r1.104 output.h
--- output.h	23 Apr 2010 04:47:32 -0000	1.104
+++ output.h	14 May 2010 00:48:51 -0000
@@ -2336,7 +2336,8 @@ class Output_section : public Output_dat
   add_input_section(Sized_relobj<size, big_endian>* object, unsigned int shndx,
 		    const char *name,
 		    const elfcpp::Shdr<size, big_endian>& shdr,
-		    unsigned int reloc_shndx, bool have_sections_script);
+		    unsigned int reloc_shndx, bool have_sections_script,
+		    Layout* layout);
 
   // Add generated data POSD to this output section.
   void
@@ -2554,6 +2555,18 @@ class Output_section : public Output_dat
   set_may_sort_attached_input_sections()
   { this->may_sort_attached_input_sections_ = true; }
 
+  // Returns true if input sections must be sorted according to the
+  // order in which their name appear in the --section-ordering-file.
+  bool
+  input_section_order_specified()
+  { return this->input_section_order_specified_; }
+
+  // Record that input sections must be sorted as some of their names
+  // match the patterns specified through --section-ordering-file.
+  void
+  set_input_section_order_specified()
+  { this->input_section_order_specified_ = true; }
+
   // Return whether the input sections attached to this output section
   // require sorting.  This is used to handle constructor priorities
   // compatibly with GNU ld.
@@ -3047,7 +3060,8 @@ class Output_section : public Output_dat
     Input_section(Relobj* object, unsigned int shndx, off_t data_size,
 		  uint64_t addralign)
       : shndx_(shndx),
-	p2align_(ffsll(static_cast<long long>(addralign)))
+	p2align_(ffsll(static_cast<long long>(addralign))),
+        section_order_index_(0)
     {
       gold_assert(shndx != OUTPUT_SECTION_CODE
 		  && shndx != MERGE_DATA_SECTION_CODE
@@ -3059,7 +3073,8 @@ class Output_section : public Output_dat
 
     // For a non-merge output section.
     Input_section(Output_section_data* posd)
-      : shndx_(OUTPUT_SECTION_CODE), p2align_(0)
+      : shndx_(OUTPUT_SECTION_CODE), p2align_(0),
+        section_order_index_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.posd = posd;
@@ -3070,7 +3085,8 @@ class Output_section : public Output_dat
       : shndx_(is_string
 	       ? MERGE_STRING_SECTION_CODE
 	       : MERGE_DATA_SECTION_CODE),
-	p2align_(0)
+	p2align_(0),
+        section_order_index_(0)
     {
       this->u1_.entsize = entsize;
       this->u2_.posd = posd;
@@ -3078,12 +3094,25 @@ class Output_section : public Output_dat
 
     // For a relaxed input section.
     Input_section(Output_relaxed_input_section *psection)
-      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0)
+      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0),
+        section_order_index_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.poris = psection;
     }
 
+    unsigned int
+    section_order_index() const
+    {
+      return this->section_order_index_;
+    }
+
+    void
+    set_section_order_index(unsigned int number)
+    {
+      this->section_order_index_ = number;  
+    }
+
     // The required alignment.
     uint64_t
     addralign() const
@@ -3292,6 +3321,9 @@ class Output_section : public Output_dat
       // For RELAXED_INPUT_SECTION_CODE, the data.
       Output_relaxed_input_section* poris;
     } u2_;
+    // The line number of the pattern it matches in the --section-ordering-file
+    // file.  It is 0 if does not match any pattern.
+    unsigned int section_order_index_;
   };
 
   typedef std::vector<Input_section> Input_section_list;
@@ -3404,6 +3436,15 @@ class Output_section : public Output_dat
 	       const Input_section_sort_entry&) const;
   };
 
+  // This is the sort comparison function when a section order is specified
+  // from an input file.
+  struct Input_section_sort_section_order_index_compare
+  {
+    bool
+    operator()(const Input_section_sort_entry&,
+	       const Input_section_sort_entry&) const;
+  };
+
   // Fill data.  This is used to fill in data between input sections.
   // It is also used for data statements (BYTE, WORD, etc.) in linker
   // scripts.  When we have to keep track of the input sections, we
@@ -3637,6 +3678,9 @@ class Output_section : public Output_dat
   // section, false if it means the symbol index of the corresponding
   // section symbol.
   bool info_uses_section_index_ : 1;
+  // True if input sections attached to this output section have to be
+  // sorted according to a specified order.
+  bool input_section_order_specified_ : 1;
   // True if the input sections attached to this output section may
   // need sorting.
   bool may_sort_attached_input_sections_ : 1;
Index: script-sections.cc
===================================================================
RCS file: /cvs/src/src/gold/script-sections.cc,v
retrieving revision 1.33
diff -u -u -p -r1.33 script-sections.cc
--- script-sections.cc	23 Apr 2010 04:47:33 -0000	1.33
+++ script-sections.cc	14 May 2010 00:48:51 -0000
@@ -983,15 +983,6 @@ class Output_section_element_fill : publ
   Expression* val_;
 };
 
-// Return whether STRING contains a wildcard character.  This is used
-// to speed up matching.
-
-static inline bool
-is_wildcard_string(const std::string& s)
-{
-  return strpbrk(s.c_str(), "?*[") != NULL;
-}
-
 // An input section specification in an output section
 
 class Output_section_element_input : public Output_section_element
@@ -1035,7 +1026,7 @@ class Output_section_element_input : pub
     Input_section_pattern(const char* patterna, size_t patternlena,
 			  Sort_wildcard sorta)
       : pattern(patterna, patternlena),
-	pattern_is_wildcard(is_wildcard_string(this->pattern)),
+	pattern_is_wildcard(is_wildcard_string(this->pattern.c_str())),
 	sort(sorta)
     { }
   };
@@ -1102,7 +1093,7 @@ Output_section_element_input::Output_sec
   if (spec->file.name.length != 1 || spec->file.name.value[0] != '*')
     this->filename_pattern_.assign(spec->file.name.value,
 				   spec->file.name.length);
-  this->filename_is_wildcard_ = is_wildcard_string(this->filename_pattern_);
+  this->filename_is_wildcard_ = is_wildcard_string(this->filename_pattern_.c_str());
 
   if (spec->input_sections.exclude != NULL)
     {
@@ -1111,7 +1102,7 @@ Output_section_element_input::Output_sec
 	   p != spec->input_sections.exclude->end();
 	   ++p)
 	{
-	  bool is_wildcard = is_wildcard_string(*p);
+	  bool is_wildcard = is_wildcard_string((*p).c_str());
 	  this->filename_exclusions_.push_back(std::make_pair(*p,
 							      is_wildcard));
 	}
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.133
diff -u -u -p -r1.133 Makefile.am
--- testsuite/Makefile.am	13 May 2010 02:41:15 -0000	1.133
+++ testsuite/Makefile.am	14 May 2010 00:48:51 -0000
@@ -193,6 +193,18 @@ icf_safe_so_test_1.stdout: icf_safe_so_t
 icf_safe_so_test_2.stdout: icf_safe_so_test
 	$(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
 
+check_SCRIPTS += final_layout.sh
+check_DATA += final_layout.stdout
+MOSTLYCLEANFILES += final_layout
+final_layout.o: final_layout.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections  -fdata-sections -g -o $@ $<
+final_layout_sequence.txt:
+	(echo "*_Z3barv*" && echo ".text._Z3bazv" && echo "*_Z3foov*" && echo "*global_varb*" && echo "*global_vara*" && echo "*global_varc*") > final_layout_sequence.txt
+final_layout: final_layout.o final_layout_sequence.txt gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,--section-ordering-file,final_layout_sequence.txt final_layout.o
+final_layout.stdout: final_layout
+	$(TEST_NM) final_layout > final_layout.stdout
+
 check_PROGRAMS += icf_virtual_function_folding_test
 MOSTLYCLEANFILES += icf_virtual_function_folding_test
 icf_virtual_function_folding_test.o: icf_virtual_function_folding_test.cc
Index: testsuite/final_layout.cc
===================================================================
RCS file: testsuite/final_layout.cc
diff -N testsuite/final_layout.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.cc	14 May 2010 00:48:51 -0000
@@ -0,0 +1,48 @@
+// final_layout.cc -- a test case for gold
+
+// Copyright 2010 Free Software Foundation, Inc.
+// Written by Sriraman Tallam <tmsriram@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify if --section-ordering-file orders
+// the .text and .data sections correctly as specified.
+
+int global_vara;
+int global_varb;
+int global_varc;
+
+int foo()
+{
+  return 1;
+}
+
+int bar()
+{
+  return 1;
+}
+
+int baz()
+{
+  return 1;
+}
+
+int main()
+{
+  return 1;
+}
Index: testsuite/final_layout.sh
===================================================================
RCS file: testsuite/final_layout.sh
diff -N testsuite/final_layout.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.sh	14 May 2010 00:48:51 -0000
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+# final_layout.sh -- test --final-layout
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Sriraman Tallam <tmsriram@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# The goal of this program is to verify if --section-ordering-file works as
+# intended.  File final_layout.cc is in this test.
+
+check()
+{
+    func_addr_1=$((16#`grep $2 $1 | awk '{print $1}'`))
+    func_addr_2=$((16#`grep $3 $1 | awk '{print $1}'`))
+    if [ $func_addr_1 -gt $func_addr_2 ]
+    then
+        echo "final layout of" $2 "and" $3 "is not right."
+	exit 1
+    fi
+}
+
+check final_layout.stdout "_Z3barv" "_Z3bazv"
+check final_layout.stdout "_Z3bazv" "_Z3foov"
+check final_layout.stdout "global_varb" "global_vara"
+check final_layout.stdout "global_vara" "global_varc"

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-05-14  0:58     ` Sriraman Tallam
@ 2010-05-22  1:42       ` Sriraman Tallam
  2010-05-24 19:27         ` Taras Glek
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-05-22  1:42 UTC (permalink / raw)
  To: Ian Lance Taylor, Taras Glek; +Cc: binutils, Cary Coutant, Xinliang David Li

Hi Taras,

     Did you get a chance to test the timings with the new patch ?

Thanks,
-Sri.

On Thu, May 13, 2010 at 5:58 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Hi Ian and Taras,
>
>     Sorry for taking this long to get back. I have modified the patch
> to address Taras' timing issue. Basically, I split patterns specified
> into glob and non-glob by just looking for wild-card characters in the
> pattern. Non-glob look-ups are through a hash table and fast. Glob
> patterns are be searched linearly. So, if you specify non-glob symbol
> names in the section ordering file, the link should be faster than
> before.
>
>     Further, I have prioritized non-glob patters over glob patterns.
> Let me explain with an example :
>
> If object main.o has sections .text._foov, .text._barv, and
> .text._zapv and the seection ordering file is :
> ===============================
> *bar*
> .text.*
> .text._barv
> ===============================
>
> Then, .text._barv will be the last section in .text even though it
> matches the first glob pattern.
>
>
> The changes are :
>
>        * gold.h (is_wildcard_string): New function.
>        * layout.cc (Layout::layout): Pass this pointer to add_input_section.
>        (Layout::layout_eh_frame): Ditto.
>        (Layout::find_section_order_index): New method.
>        (Layout::read_layout_from_file): New method.
>        * layout.h (Layout::find_section_order_index): New method.
>        (Layout::read_layout_from_file): New method.
>        (Layout::input_section_position_): New private member.
>        (Layout::input_section_glob_): New private member.
>        * main.cc (main): Call read_layout_from_file here.
>        * options.h (--section-ordering-file): New option.
>        * output.cc (Output_section::input_section_order_specified_): New
>        member.
>        (Output_section::Output_section): Initialize new member.
>        (Output_section::add_input_section): Add new parameter.
>        Keep input sections when --section-ordering-file is used.
>        (Output_section::set_final_data_size): Sort input sections when
>        section ordering file is specified.
>        (Output_section::Input_section_sort_entry): Add new parameter.
>        Check sorting type.
>        (Output_section::Input_section_sort_entry::is_before_in_sequence):
>        New method.
>        (Output_section::Input_section_sort_compare::operator()): Change to
>        consider section_order_index.
>        (Output_section::Input_section_sort_init_fini_compare::operator()):
>        Change to consider section_order_index.
>        (Output_section::Input_section_sort_section_order_index_compare
>        ::operator()): New method.
>        (Output_section::sort_attached_input_sections): Change to sort
>        according to section order when specified.
>        (Output_section::add_input_section<32, true>): Add new parameter.
>        (Output_section::add_input_section<64, true>): Add new parameter.
>        (Output_section::add_input_section<32, false>): Add new parameter.
>        (Output_section::add_input_section<64, false>): Add new parameter.
>        * output.h (Output_section::add_input_section): Add new parameter.
>        (Output_section::input_section_order_specified): New
>        method.
>        (Output_section::set_input_section_order_specified): New method.
>        (Input_section::Input_section): Initialize section_order_index_.
>        (Input_section::section_order_index): New method.
>        (Input_section::set_section_order_index): New method.
>        (Input_section::section_order_index_): New member.
>        (Input_section::Input_section_sort_section_order_index_compare): New
>        struct.
>        (Output_section::input_section_order_specified_): New member.
>        * script-sections.cc (is_wildcard_string): Delete and move modified
>        method to gold.h.
>        (Output_section_element_input::Output_section_element_input): Modify
>        call to is_wildcard_string.
>        (Output_section_element_input::Input_section_pattern
>        ::Input_section_pattern): Ditto.
>        (Output_section_element_input::Output_section_element_input): Ditto.
>        * testsuite/Makefile.am (final_layout): New test case.
>        * testsuite/Makefile.in: Regenerate.
>        * testsuite/final_layout.cc: New file.
>        * testsuite/final_layout.sh: New file.
>
> Please let me know what you think.
>
> Thanks,
> -Sri.
>
>
>
> On Tue, Mar 2, 2010 at 7:43 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> Hi Ian,
>>
>>    I finally got around to making the changes you specified. Please
>> take a look when you get a chance.
>>
>>        * layout.cc (Layout::read_layout_from_file): New method.
>>        (Layout::layout): Pass this pointer to add_input_section.
>>        (Layout::layout_eh_frame): Ditto.
>>        * layout.h (Layout::read_layout_from_file): New method.
>>        (Layout::input_section_order): New method.
>>        (Layout::input_section_order_): New private member.
>>        * main.cc (main): Call read_layout_from_file here.
>>        * options.h (--section-ordering-file): New option.
>>        * output.cc (Output_section::input_section_order_specified_): New
>>        member.
>>        (Output_section::add_input_section): Add new parameter.
>>        Keep input sections when --section-ordering-file is used.
>>        (Output_section::set_final_data_size): Sort input sections when
>>        section ordering file is specified.
>>        (Output_section::Input_section_sort_entry::section_order_index_): New
>>        member.
>>        (Output_section::Input_section_sort_entry::section_order_index): New
>>        method.
>>        (Output_section::Input_section_sort_compare::operator()): Change to
>>        consider section_order_index.
>>        (Output_section::Input_section_sort_init_fini_compare::operator()):
>>        Change to consider section_order_index.
>>        (Output_section::Input_section_sort_section_order_index_compare
>>        ::operator()): New method.
>>        (Output_section::sort_attached_input_sections): Change to sort
>>        according to section order when specified.
>>        * output.h (Output_section::input_section_order_specified): New
>>        method.
>>        (Output_section::set_input_section_order_specified): New method.
>>        (Input_section::glob_pattern_number): New method.
>>        (Input_section::set_glob_pattern_number): New method.
>>        (Input_section::glob_pattern_number_): New member.
>>        (Input_section::Input_section_sort_section_order_index_compare): New
>>        struct.
>>        (Output_section::input_section_order_specified_): New member.
>>        * testsuite/Makefile.am (final_layout): New test case.
>>        * testsuite/Makefile.in: Regenerate.
>>        * testsuite/final_layout.cc: New file.
>>        * testsuite/final_layout.sh: New file
>>
>>
>> Thanks,
>> -Sriraman.
>>
>>
>> On Thu, Feb 11, 2010 at 9:29 PM, Ian Lance Taylor <iant@google.com> wrote:
>>> Sriraman Tallam <tmsriram@google.com> writes:
>>>
>>>>    I have attached a patch to reorder text and data sections in the
>>>> linker according to a user specified sequence. I have added a new
>>>> option --final-layout to do this. Currently, this could be done using
>>>> linker scripts but this patch makes it really easy to do this. Let me
>>>> explain using a simple example.
>>>>
>>>> test.cc
>>>>
>>>> void foo()
>>>> { }
>>>>
>>>> void bar()
>>>> { }
>>>>
>>>> int main()
>>>> {
>>>>   return 0;
>>>> }
>>>>
>>>> $ g++ -ffunction-sections test.cc
>>>> $ nm -n a.out
>>>> ...
>>>> 000000000040038c T _Z3foov
>>>> 0000000000400392 T _Z3barv
>>>> ....
>>>>
>>>> foo is laid to be before bar. Now, I can change this order as follows.
>>>>
>>>> $ (echo "_Z3barv" && echo "_Z3foov") > sequence.txt
>>>> $ g++ -ffunction-sections test.cc -Wl,--final-layout,sequence.txt
>>>> $ nm -n a.out
>>>> ...
>>>> 0000000000400658 T _Z3barv
>>>> 000000000040065e T _Z3foov
>>>> ...
>>>>
>>>> The order is changed.
>>>>
>>>> This can be done for text or data sections.
>>>
>>>
>>> As I understand it, in linker terms, you are sorting the sections by
>>> suffixes.  When two input sections are in the same output section, and
>>> both input sections have suffixes which appear in the file, then the
>>> input sections are sorted in the order in which the suffixes appear in
>>> the file.
>>>
>>> I think it would be more natural to sort the input sections by name
>>> rather than by suffix.  Since you don't want to fuss with writing
>>> ".text." all the time, suppose we say that we sort the input sections
>>> by name, and we match the input section names using glob patterns.  We
>>> already use glob patterns in linker scripts, so that is not a big
>>> stretch.
>>>
>>> Just a few comments on the rest of the patch.
>>>
>>>
>>>
>>>> +// Read the sequence of input sections from the file specified with
>>>> +// --final-layout.
>>>> +
>>>> +bool
>>>> +Layout::read_layout_from_file()
>>>> +{
>>>> +  const char* filename = parameters->options().final_layout();
>>>> +  char *buf = NULL;
>>>> +  size_t len = 0;
>>>> +  FILE* fp = fopen(filename, "r");
>>>> +
>>>> +  if (fp == NULL)
>>>> +    {
>>>> +      gold_error(_("Error opening layout file : %s\n"), filename);
>>>> +      gold_exit(false);
>>>> +    }
>>>> +
>>>> +  while (getline(&buf, &len, fp) != -1)
>>>> +    {
>>>> +      buf[strlen(buf) - 1] = 0;
>>>> +      this->input_section_order_.push_back(std::string(buf));
>>>> +    }
>>>> +
>>>> +  if (buf != NULL)
>>>> +    free(buf);
>>>> +
>>>> +  fclose(fp);
>>>> +  return true;
>>>> +}
>>>
>>> The getline function is insufficient portable for use in gold.  Search
>>> for std::getline in options.cc for an alternate approach you can use.
>>> Emulate the error message style you see there too--no capital letter,
>>> name the file, no space before colon, no \n.  And if you really want
>>> to exit on failure, call gold_fatal.
>>>
>>>
>>>> +// If --final-layout option is used, reorder the input sections in
>>>> +// .text, .data, .bss and .rodata according to the specified sequence.
>>>> +
>>>> +void
>>>> +Layout::section_reorder()
>>>> +{
>>>> +  this->read_layout_from_file();
>>>> +
>>>> +  for (Section_list::iterator p = this->section_list_.begin();
>>>> +       p != this->section_list_.end();
>>>> +       ++p)
>>>> +    {
>>>> +      if (strcmp(".text", (*p)->name()) == 0
>>>> +          || strcmp(".data", (*p)->name()) == 0
>>>> +          || strcmp(".bss", (*p)->name()) == 0
>>>> +          || strcmp(".rodata", (*p)->name()) == 0)
>>>> +        (*p)->reorder_layout(this);
>>>> +    }
>>>> +}
>>>
>>> Why restrict this to those output sections?  Why not sort input
>>> sections in any output section?
>>>
>>>
>>>> +  DEFINE_string(final_layout, options::TWO_DASHES, '\0', NULL,
>>>> +             N_("Layout functions and data in the order specified."),
>>>> +             N_("FILENAME"));
>>>
>>> I'm not sure I care for --final-layout as the option name.  Perhaps
>>> --section-ordering-file?  Perhaps somebody else has a better idea.
>>>
>>>
>>>> +  // the future, we keep track of the sections.  If the --final-layout
>>>> +  // option is used to specify the order of sections, we need to keep
>>>> +  // track of sections.
>>>>    if (have_sections_script
>>>>        || !this->input_sections_.empty()
>>>>        || this->may_sort_attached_input_sections()
>>>>        || this->must_sort_attached_input_sections()
>>>>        || parameters->options().user_set_Map()
>>>> -      || parameters->target().may_relax())
>>>> -    this->input_sections_.push_back(Input_section(object, shndx,
>>>> -                                               shdr.get_sh_size(),
>>>> -                                               addralign));
>>>> +      || parameters->target().may_relax()
>>>> +      || parameters->options().final_layout())
>>>> +    {
>>>> +      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
>>>> +      isecn.set_section_name(secname);
>>>> +      this->input_sections_.push_back(isecn);
>>>> +    }
>>>
>>> Don't save the string here, that's just going to bloat memory usage.
>>> Instead, when you read the file, give each line a number.  Then match
>>> the section name which you have here against the list of patterns.  If
>>> you find a match, store the number in the Input_section structure.
>>> Also, if you find a match, set a flag in the output section.  Then
>>> sort the sections, by number, in a function called from
>>> set_final_data_size.
>>>
>>> You will see that there is already some section ordering in that
>>> function, which is used to implement constructor/destructor priority
>>> ordering.  I guess the priority ordering should take precedence.
>>> Maybe.
>>>
>>> Ian
>>>
>>
>

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-05-22  1:42       ` Sriraman Tallam
@ 2010-05-24 19:27         ` Taras Glek
  2010-05-29  1:49           ` Sriraman Tallam
  0 siblings, 1 reply; 24+ messages in thread
From: Taras Glek @ 2010-05-24 19:27 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: binutils

On 05/21/2010 06:41 PM, Sriraman Tallam wrote:
> Hi Taras,
>
>       Did you get a chance to test the timings with the new patch ?
>    
I've only tested it briefly on Friday. I'm having some problems with the 
new patch. It ranges from working perfectly to occasionally taking 
longer than the previous one and/or crashing. I'll let you know more 
once I have more time to play with it

Taras
> Thanks,
> -Sri.
>
> On Thu, May 13, 2010 at 5:58 PM, Sriraman Tallam<tmsriram@google.com>  wrote:
>    
>> Hi Ian and Taras,
>>
>>      Sorry for taking this long to get back. I have modified the patch
>> to address Taras' timing issue. Basically, I split patterns specified
>> into glob and non-glob by just looking for wild-card characters in the
>> pattern. Non-glob look-ups are through a hash table and fast. Glob
>> patterns are be searched linearly. So, if you specify non-glob symbol
>> names in the section ordering file, the link should be faster than
>> before.
>>
>>      Further, I have prioritized non-glob patters over glob patterns.
>> Let me explain with an example :
>>
>> If object main.o has sections .text._foov, .text._barv, and
>> .text._zapv and the seection ordering file is :
>> ===============================
>> *bar*
>> .text.*
>> .text._barv
>> ===============================
>>
>> Then, .text._barv will be the last section in .text even though it
>> matches the first glob pattern.
>>
>>
>> The changes are :
>>
>>         * gold.h (is_wildcard_string): New function.
>>         * layout.cc (Layout::layout): Pass this pointer to add_input_section.
>>         (Layout::layout_eh_frame): Ditto.
>>         (Layout::find_section_order_index): New method.
>>         (Layout::read_layout_from_file): New method.
>>         * layout.h (Layout::find_section_order_index): New method.
>>         (Layout::read_layout_from_file): New method.
>>         (Layout::input_section_position_): New private member.
>>         (Layout::input_section_glob_): New private member.
>>         * main.cc (main): Call read_layout_from_file here.
>>         * options.h (--section-ordering-file): New option.
>>         * output.cc (Output_section::input_section_order_specified_): New
>>         member.
>>         (Output_section::Output_section): Initialize new member.
>>         (Output_section::add_input_section): Add new parameter.
>>         Keep input sections when --section-ordering-file is used.
>>         (Output_section::set_final_data_size): Sort input sections when
>>         section ordering file is specified.
>>         (Output_section::Input_section_sort_entry): Add new parameter.
>>         Check sorting type.
>>         (Output_section::Input_section_sort_entry::is_before_in_sequence):
>>         New method.
>>         (Output_section::Input_section_sort_compare::operator()): Change to
>>         consider section_order_index.
>>         (Output_section::Input_section_sort_init_fini_compare::operator()):
>>         Change to consider section_order_index.
>>         (Output_section::Input_section_sort_section_order_index_compare
>>         ::operator()): New method.
>>         (Output_section::sort_attached_input_sections): Change to sort
>>         according to section order when specified.
>>         (Output_section::add_input_section<32, true>): Add new parameter.
>>         (Output_section::add_input_section<64, true>): Add new parameter.
>>         (Output_section::add_input_section<32, false>): Add new parameter.
>>         (Output_section::add_input_section<64, false>): Add new parameter.
>>         * output.h (Output_section::add_input_section): Add new parameter.
>>         (Output_section::input_section_order_specified): New
>>         method.
>>         (Output_section::set_input_section_order_specified): New method.
>>         (Input_section::Input_section): Initialize section_order_index_.
>>         (Input_section::section_order_index): New method.
>>         (Input_section::set_section_order_index): New method.
>>         (Input_section::section_order_index_): New member.
>>         (Input_section::Input_section_sort_section_order_index_compare): New
>>         struct.
>>         (Output_section::input_section_order_specified_): New member.
>>         * script-sections.cc (is_wildcard_string): Delete and move modified
>>         method to gold.h.
>>         (Output_section_element_input::Output_section_element_input): Modify
>>         call to is_wildcard_string.
>>         (Output_section_element_input::Input_section_pattern
>>         ::Input_section_pattern): Ditto.
>>         (Output_section_element_input::Output_section_element_input): Ditto.
>>         * testsuite/Makefile.am (final_layout): New test case.
>>         * testsuite/Makefile.in: Regenerate.
>>         * testsuite/final_layout.cc: New file.
>>         * testsuite/final_layout.sh: New file.
>>
>> Please let me know what you think.
>>
>> Thanks,
>> -Sri.
>>
>>
>>
>> On Tue, Mar 2, 2010 at 7:43 PM, Sriraman Tallam<tmsriram@google.com>  wrote:
>>      
>>> Hi Ian,
>>>
>>>     I finally got around to making the changes you specified. Please
>>> take a look when you get a chance.
>>>
>>>         * layout.cc (Layout::read_layout_from_file): New method.
>>>         (Layout::layout): Pass this pointer to add_input_section.
>>>         (Layout::layout_eh_frame): Ditto.
>>>         * layout.h (Layout::read_layout_from_file): New method.
>>>         (Layout::input_section_order): New method.
>>>         (Layout::input_section_order_): New private member.
>>>         * main.cc (main): Call read_layout_from_file here.
>>>         * options.h (--section-ordering-file): New option.
>>>         * output.cc (Output_section::input_section_order_specified_): New
>>>         member.
>>>         (Output_section::add_input_section): Add new parameter.
>>>         Keep input sections when --section-ordering-file is used.
>>>         (Output_section::set_final_data_size): Sort input sections when
>>>         section ordering file is specified.
>>>         (Output_section::Input_section_sort_entry::section_order_index_): New
>>>         member.
>>>         (Output_section::Input_section_sort_entry::section_order_index): New
>>>         method.
>>>         (Output_section::Input_section_sort_compare::operator()): Change to
>>>         consider section_order_index.
>>>         (Output_section::Input_section_sort_init_fini_compare::operator()):
>>>         Change to consider section_order_index.
>>>         (Output_section::Input_section_sort_section_order_index_compare
>>>         ::operator()): New method.
>>>         (Output_section::sort_attached_input_sections): Change to sort
>>>         according to section order when specified.
>>>         * output.h (Output_section::input_section_order_specified): New
>>>         method.
>>>         (Output_section::set_input_section_order_specified): New method.
>>>         (Input_section::glob_pattern_number): New method.
>>>         (Input_section::set_glob_pattern_number): New method.
>>>         (Input_section::glob_pattern_number_): New member.
>>>         (Input_section::Input_section_sort_section_order_index_compare): New
>>>         struct.
>>>         (Output_section::input_section_order_specified_): New member.
>>>         * testsuite/Makefile.am (final_layout): New test case.
>>>         * testsuite/Makefile.in: Regenerate.
>>>         * testsuite/final_layout.cc: New file.
>>>         * testsuite/final_layout.sh: New file
>>>
>>>
>>> Thanks,
>>> -Sriraman.
>>>
>>>
>>> On Thu, Feb 11, 2010 at 9:29 PM, Ian Lance Taylor<iant@google.com>  wrote:
>>>        
>>>> Sriraman Tallam<tmsriram@google.com>  writes:
>>>>
>>>>          
>>>>>     I have attached a patch to reorder text and data sections in the
>>>>> linker according to a user specified sequence. I have added a new
>>>>> option --final-layout to do this. Currently, this could be done using
>>>>> linker scripts but this patch makes it really easy to do this. Let me
>>>>> explain using a simple example.
>>>>>
>>>>> test.cc
>>>>>
>>>>> void foo()
>>>>> { }
>>>>>
>>>>> void bar()
>>>>> { }
>>>>>
>>>>> int main()
>>>>> {
>>>>>    return 0;
>>>>> }
>>>>>
>>>>> $ g++ -ffunction-sections test.cc
>>>>> $ nm -n a.out
>>>>> ...
>>>>> 000000000040038c T _Z3foov
>>>>> 0000000000400392 T _Z3barv
>>>>> ....
>>>>>
>>>>> foo is laid to be before bar. Now, I can change this order as follows.
>>>>>
>>>>> $ (echo "_Z3barv"&&  echo "_Z3foov")>  sequence.txt
>>>>> $ g++ -ffunction-sections test.cc -Wl,--final-layout,sequence.txt
>>>>> $ nm -n a.out
>>>>> ...
>>>>> 0000000000400658 T _Z3barv
>>>>> 000000000040065e T _Z3foov
>>>>> ...
>>>>>
>>>>> The order is changed.
>>>>>
>>>>> This can be done for text or data sections.
>>>>>            
>>>>
>>>> As I understand it, in linker terms, you are sorting the sections by
>>>> suffixes.  When two input sections are in the same output section, and
>>>> both input sections have suffixes which appear in the file, then the
>>>> input sections are sorted in the order in which the suffixes appear in
>>>> the file.
>>>>
>>>> I think it would be more natural to sort the input sections by name
>>>> rather than by suffix.  Since you don't want to fuss with writing
>>>> ".text." all the time, suppose we say that we sort the input sections
>>>> by name, and we match the input section names using glob patterns.  We
>>>> already use glob patterns in linker scripts, so that is not a big
>>>> stretch.
>>>>
>>>> Just a few comments on the rest of the patch.
>>>>
>>>>
>>>>
>>>>          
>>>>> +// Read the sequence of input sections from the file specified with
>>>>> +// --final-layout.
>>>>> +
>>>>> +bool
>>>>> +Layout::read_layout_from_file()
>>>>> +{
>>>>> +  const char* filename = parameters->options().final_layout();
>>>>> +  char *buf = NULL;
>>>>> +  size_t len = 0;
>>>>> +  FILE* fp = fopen(filename, "r");
>>>>> +
>>>>> +  if (fp == NULL)
>>>>> +    {
>>>>> +      gold_error(_("Error opening layout file : %s\n"), filename);
>>>>> +      gold_exit(false);
>>>>> +    }
>>>>> +
>>>>> +  while (getline(&buf,&len, fp) != -1)
>>>>> +    {
>>>>> +      buf[strlen(buf) - 1] = 0;
>>>>> +      this->input_section_order_.push_back(std::string(buf));
>>>>> +    }
>>>>> +
>>>>> +  if (buf != NULL)
>>>>> +    free(buf);
>>>>> +
>>>>> +  fclose(fp);
>>>>> +  return true;
>>>>> +}
>>>>>            
>>>> The getline function is insufficient portable for use in gold.  Search
>>>> for std::getline in options.cc for an alternate approach you can use.
>>>> Emulate the error message style you see there too--no capital letter,
>>>> name the file, no space before colon, no \n.  And if you really want
>>>> to exit on failure, call gold_fatal.
>>>>
>>>>
>>>>          
>>>>> +// If --final-layout option is used, reorder the input sections in
>>>>> +// .text, .data, .bss and .rodata according to the specified sequence.
>>>>> +
>>>>> +void
>>>>> +Layout::section_reorder()
>>>>> +{
>>>>> +  this->read_layout_from_file();
>>>>> +
>>>>> +  for (Section_list::iterator p = this->section_list_.begin();
>>>>> +       p != this->section_list_.end();
>>>>> +       ++p)
>>>>> +    {
>>>>> +      if (strcmp(".text", (*p)->name()) == 0
>>>>> +          || strcmp(".data", (*p)->name()) == 0
>>>>> +          || strcmp(".bss", (*p)->name()) == 0
>>>>> +          || strcmp(".rodata", (*p)->name()) == 0)
>>>>> +        (*p)->reorder_layout(this);
>>>>> +    }
>>>>> +}
>>>>>            
>>>> Why restrict this to those output sections?  Why not sort input
>>>> sections in any output section?
>>>>
>>>>
>>>>          
>>>>> +  DEFINE_string(final_layout, options::TWO_DASHES, '\0', NULL,
>>>>> +             N_("Layout functions and data in the order specified."),
>>>>> +             N_("FILENAME"));
>>>>>            
>>>> I'm not sure I care for --final-layout as the option name.  Perhaps
>>>> --section-ordering-file?  Perhaps somebody else has a better idea.
>>>>
>>>>
>>>>          
>>>>> +  // the future, we keep track of the sections.  If the --final-layout
>>>>> +  // option is used to specify the order of sections, we need to keep
>>>>> +  // track of sections.
>>>>>     if (have_sections_script
>>>>>         || !this->input_sections_.empty()
>>>>>         || this->may_sort_attached_input_sections()
>>>>>         || this->must_sort_attached_input_sections()
>>>>>         || parameters->options().user_set_Map()
>>>>> -      || parameters->target().may_relax())
>>>>> -    this->input_sections_.push_back(Input_section(object, shndx,
>>>>> -                                               shdr.get_sh_size(),
>>>>> -                                               addralign));
>>>>> +      || parameters->target().may_relax()
>>>>> +      || parameters->options().final_layout())
>>>>> +    {
>>>>> +      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
>>>>> +      isecn.set_section_name(secname);
>>>>> +      this->input_sections_.push_back(isecn);
>>>>> +    }
>>>>>            
>>>> Don't save the string here, that's just going to bloat memory usage.
>>>> Instead, when you read the file, give each line a number.  Then match
>>>> the section name which you have here against the list of patterns.  If
>>>> you find a match, store the number in the Input_section structure.
>>>> Also, if you find a match, set a flag in the output section.  Then
>>>> sort the sections, by number, in a function called from
>>>> set_final_data_size.
>>>>
>>>> You will see that there is already some section ordering in that
>>>> function, which is used to implement constructor/destructor priority
>>>> ordering.  I guess the priority ordering should take precedence.
>>>> Maybe.
>>>>
>>>> Ian
>>>>
>>>>          
>>>        
>>      

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-05-24 19:27         ` Taras Glek
@ 2010-05-29  1:49           ` Sriraman Tallam
  2010-06-01 14:41             ` Ian Lance Taylor
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-05-29  1:49 UTC (permalink / raw)
  To: Taras Glek; +Cc: binutils

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

Hi,

    I fixed a bug Taras ran into. Here is the new patch.

	* gold.h (is_wildcard_string): New function.
	* layout.cc (Layout::layout): Pass this pointer to add_input_section.
	(Layout::layout_eh_frame): Ditto.
	(Layout::find_section_order_index): New method.
	(Layout::read_layout_from_file): New method.
	* layout.h (Layout::find_section_order_index): New method.
	(Layout::read_layout_from_file): New method.
	(Layout::input_section_position_): New private member.
	(Layout::input_section_glob_): New private member.
	* main.cc (main): Call read_layout_from_file here.
	* options.h (--section-ordering-file): New option.
	* output.cc (Output_section::input_section_order_specified_): New
	member.
	(Output_section::Output_section): Initialize new member.
	(Output_section::add_input_section): Add new parameter.
	Keep input sections when --section-ordering-file is used.
	(Output_section::set_final_data_size): Sort input sections when
	section ordering file is specified.
	(Output_section::Input_section_sort_entry): Add new parameter.
	Check sorting type.
	(Output_section::Input_section_sort_entry::is_before_in_sequence):
	New method.
	(Output_section::Input_section_sort_compare::operator()): Change to
	consider section_order_index.
	(Output_section::Input_section_sort_init_fini_compare::operator()):
	Change to consider section_order_index.
	(Output_section::Input_section_sort_section_order_index_compare
	::operator()): New method.
	(Output_section::sort_attached_input_sections): Change to sort
	according to section order when specified.
	(Output_section::add_input_section<32, true>): Add new parameter.
	(Output_section::add_input_section<64, true>): Add new parameter.
	(Output_section::add_input_section<32, false>): Add new parameter.
	(Output_section::add_input_section<64, false>): Add new parameter.
	* output.h (Output_section::add_input_section): Add new parameter.
	(Output_section::input_section_order_specified): New
	method.
	(Output_section::set_input_section_order_specified): New method.
	(Input_section::Input_section): Initialize section_order_index_.
	(Input_section::section_order_index): New method.
	(Input_section::set_section_order_index): New method.
	(Input_section::section_order_index_): New member.
	(Input_section::Input_section_sort_section_order_index_compare): New
	struct.
	(Output_section::input_section_order_specified_): New member.
	* script-sections.cc (is_wildcard_string): Delete and move modified
	method to gold.h.
	(Output_section_element_input::Output_section_element_input): Modify
	call to is_wildcard_string.
	(Output_section_element_input::Input_section_pattern
	::Input_section_pattern): Ditto.
	(Output_section_element_input::Output_section_element_input): Ditto.
	* testsuite/Makefile.am (final_layout): New test case.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/final_layout.cc: New file.
	* testsuite/final_layout.sh: New file.


Thanks,
-Sri.

On Mon, May 24, 2010 at 12:26 PM, Taras Glek <tglek@mozilla.com> wrote:
> On 05/21/2010 06:41 PM, Sriraman Tallam wrote:
>>
>> Hi Taras,
>>
>>      Did you get a chance to test the timings with the new patch ?
>>
>
> I've only tested it briefly on Friday. I'm having some problems with the new
> patch. It ranges from working perfectly to occasionally taking longer than
> the previous one and/or crashing. I'll let you know more once I have more
> time to play with it
>
> Taras
>>
>> Thanks,
>> -Sri.
>>
>> On Thu, May 13, 2010 at 5:58 PM, Sriraman Tallam<tmsriram@google.com>
>>  wrote:
>>
>>>
>>> Hi Ian and Taras,
>>>
>>>     Sorry for taking this long to get back. I have modified the patch
>>> to address Taras' timing issue. Basically, I split patterns specified
>>> into glob and non-glob by just looking for wild-card characters in the
>>> pattern. Non-glob look-ups are through a hash table and fast. Glob
>>> patterns are be searched linearly. So, if you specify non-glob symbol
>>> names in the section ordering file, the link should be faster than
>>> before.
>>>
>>>     Further, I have prioritized non-glob patters over glob patterns.
>>> Let me explain with an example :
>>>
>>> If object main.o has sections .text._foov, .text._barv, and
>>> .text._zapv and the seection ordering file is :
>>> ===============================
>>> *bar*
>>> .text.*
>>> .text._barv
>>> ===============================
>>>
>>> Then, .text._barv will be the last section in .text even though it
>>> matches the first glob pattern.
>>>
>>>
>>> The changes are :
>>>
>>>        * gold.h (is_wildcard_string): New function.
>>>        * layout.cc (Layout::layout): Pass this pointer to
>>> add_input_section.
>>>        (Layout::layout_eh_frame): Ditto.
>>>        (Layout::find_section_order_index): New method.
>>>        (Layout::read_layout_from_file): New method.
>>>        * layout.h (Layout::find_section_order_index): New method.
>>>        (Layout::read_layout_from_file): New method.
>>>        (Layout::input_section_position_): New private member.
>>>        (Layout::input_section_glob_): New private member.
>>>        * main.cc (main): Call read_layout_from_file here.
>>>        * options.h (--section-ordering-file): New option.
>>>        * output.cc (Output_section::input_section_order_specified_): New
>>>        member.
>>>        (Output_section::Output_section): Initialize new member.
>>>        (Output_section::add_input_section): Add new parameter.
>>>        Keep input sections when --section-ordering-file is used.
>>>        (Output_section::set_final_data_size): Sort input sections when
>>>        section ordering file is specified.
>>>        (Output_section::Input_section_sort_entry): Add new parameter.
>>>        Check sorting type.
>>>        (Output_section::Input_section_sort_entry::is_before_in_sequence):
>>>        New method.
>>>        (Output_section::Input_section_sort_compare::operator()): Change
>>> to
>>>        consider section_order_index.
>>>
>>>  (Output_section::Input_section_sort_init_fini_compare::operator()):
>>>        Change to consider section_order_index.
>>>        (Output_section::Input_section_sort_section_order_index_compare
>>>        ::operator()): New method.
>>>        (Output_section::sort_attached_input_sections): Change to sort
>>>        according to section order when specified.
>>>        (Output_section::add_input_section<32, true>): Add new parameter.
>>>        (Output_section::add_input_section<64, true>): Add new parameter.
>>>        (Output_section::add_input_section<32, false>): Add new parameter.
>>>        (Output_section::add_input_section<64, false>): Add new parameter.
>>>        * output.h (Output_section::add_input_section): Add new parameter.
>>>        (Output_section::input_section_order_specified): New
>>>        method.
>>>        (Output_section::set_input_section_order_specified): New method.
>>>        (Input_section::Input_section): Initialize section_order_index_.
>>>        (Input_section::section_order_index): New method.
>>>        (Input_section::set_section_order_index): New method.
>>>        (Input_section::section_order_index_): New member.
>>>        (Input_section::Input_section_sort_section_order_index_compare):
>>> New
>>>        struct.
>>>        (Output_section::input_section_order_specified_): New member.
>>>        * script-sections.cc (is_wildcard_string): Delete and move
>>> modified
>>>        method to gold.h.
>>>        (Output_section_element_input::Output_section_element_input):
>>> Modify
>>>        call to is_wildcard_string.
>>>        (Output_section_element_input::Input_section_pattern
>>>        ::Input_section_pattern): Ditto.
>>>        (Output_section_element_input::Output_section_element_input):
>>> Ditto.
>>>        * testsuite/Makefile.am (final_layout): New test case.
>>>        * testsuite/Makefile.in: Regenerate.
>>>        * testsuite/final_layout.cc: New file.
>>>        * testsuite/final_layout.sh: New file.
>>>
>>> Please let me know what you think.
>>>
>>> Thanks,
>>> -Sri.
>>>
>>>
>>>
>>> On Tue, Mar 2, 2010 at 7:43 PM, Sriraman Tallam<tmsriram@google.com>
>>>  wrote:
>>>
>>>>
>>>> Hi Ian,
>>>>
>>>>    I finally got around to making the changes you specified. Please
>>>> take a look when you get a chance.
>>>>
>>>>        * layout.cc (Layout::read_layout_from_file): New method.
>>>>        (Layout::layout): Pass this pointer to add_input_section.
>>>>        (Layout::layout_eh_frame): Ditto.
>>>>        * layout.h (Layout::read_layout_from_file): New method.
>>>>        (Layout::input_section_order): New method.
>>>>        (Layout::input_section_order_): New private member.
>>>>        * main.cc (main): Call read_layout_from_file here.
>>>>        * options.h (--section-ordering-file): New option.
>>>>        * output.cc (Output_section::input_section_order_specified_): New
>>>>        member.
>>>>        (Output_section::add_input_section): Add new parameter.
>>>>        Keep input sections when --section-ordering-file is used.
>>>>        (Output_section::set_final_data_size): Sort input sections when
>>>>        section ordering file is specified.
>>>>        (Output_section::Input_section_sort_entry::section_order_index_):
>>>> New
>>>>        member.
>>>>        (Output_section::Input_section_sort_entry::section_order_index):
>>>> New
>>>>        method.
>>>>        (Output_section::Input_section_sort_compare::operator()): Change
>>>> to
>>>>        consider section_order_index.
>>>>
>>>>  (Output_section::Input_section_sort_init_fini_compare::operator()):
>>>>        Change to consider section_order_index.
>>>>        (Output_section::Input_section_sort_section_order_index_compare
>>>>        ::operator()): New method.
>>>>        (Output_section::sort_attached_input_sections): Change to sort
>>>>        according to section order when specified.
>>>>        * output.h (Output_section::input_section_order_specified): New
>>>>        method.
>>>>        (Output_section::set_input_section_order_specified): New method.
>>>>        (Input_section::glob_pattern_number): New method.
>>>>        (Input_section::set_glob_pattern_number): New method.
>>>>        (Input_section::glob_pattern_number_): New member.
>>>>        (Input_section::Input_section_sort_section_order_index_compare):
>>>> New
>>>>        struct.
>>>>        (Output_section::input_section_order_specified_): New member.
>>>>        * testsuite/Makefile.am (final_layout): New test case.
>>>>        * testsuite/Makefile.in: Regenerate.
>>>>        * testsuite/final_layout.cc: New file.
>>>>        * testsuite/final_layout.sh: New file
>>>>
>>>>
>>>> Thanks,
>>>> -Sriraman.
>>>>
>>>>
>>>> On Thu, Feb 11, 2010 at 9:29 PM, Ian Lance Taylor<iant@google.com>
>>>>  wrote:
>>>>
>>>>>
>>>>> Sriraman Tallam<tmsriram@google.com>  writes:
>>>>>
>>>>>
>>>>>>
>>>>>>    I have attached a patch to reorder text and data sections in the
>>>>>> linker according to a user specified sequence. I have added a new
>>>>>> option --final-layout to do this. Currently, this could be done using
>>>>>> linker scripts but this patch makes it really easy to do this. Let me
>>>>>> explain using a simple example.
>>>>>>
>>>>>> test.cc
>>>>>>
>>>>>> void foo()
>>>>>> { }
>>>>>>
>>>>>> void bar()
>>>>>> { }
>>>>>>
>>>>>> int main()
>>>>>> {
>>>>>>   return 0;
>>>>>> }
>>>>>>
>>>>>> $ g++ -ffunction-sections test.cc
>>>>>> $ nm -n a.out
>>>>>> ...
>>>>>> 000000000040038c T _Z3foov
>>>>>> 0000000000400392 T _Z3barv
>>>>>> ....
>>>>>>
>>>>>> foo is laid to be before bar. Now, I can change this order as follows.
>>>>>>
>>>>>> $ (echo "_Z3barv"&&  echo "_Z3foov")>  sequence.txt
>>>>>> $ g++ -ffunction-sections test.cc -Wl,--final-layout,sequence.txt
>>>>>> $ nm -n a.out
>>>>>> ...
>>>>>> 0000000000400658 T _Z3barv
>>>>>> 000000000040065e T _Z3foov
>>>>>> ...
>>>>>>
>>>>>> The order is changed.
>>>>>>
>>>>>> This can be done for text or data sections.
>>>>>>
>>>>>
>>>>> As I understand it, in linker terms, you are sorting the sections by
>>>>> suffixes.  When two input sections are in the same output section, and
>>>>> both input sections have suffixes which appear in the file, then the
>>>>> input sections are sorted in the order in which the suffixes appear in
>>>>> the file.
>>>>>
>>>>> I think it would be more natural to sort the input sections by name
>>>>> rather than by suffix.  Since you don't want to fuss with writing
>>>>> ".text." all the time, suppose we say that we sort the input sections
>>>>> by name, and we match the input section names using glob patterns.  We
>>>>> already use glob patterns in linker scripts, so that is not a big
>>>>> stretch.
>>>>>
>>>>> Just a few comments on the rest of the patch.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> +// Read the sequence of input sections from the file specified with
>>>>>> +// --final-layout.
>>>>>> +
>>>>>> +bool
>>>>>> +Layout::read_layout_from_file()
>>>>>> +{
>>>>>> +  const char* filename = parameters->options().final_layout();
>>>>>> +  char *buf = NULL;
>>>>>> +  size_t len = 0;
>>>>>> +  FILE* fp = fopen(filename, "r");
>>>>>> +
>>>>>> +  if (fp == NULL)
>>>>>> +    {
>>>>>> +      gold_error(_("Error opening layout file : %s\n"), filename);
>>>>>> +      gold_exit(false);
>>>>>> +    }
>>>>>> +
>>>>>> +  while (getline(&buf,&len, fp) != -1)
>>>>>> +    {
>>>>>> +      buf[strlen(buf) - 1] = 0;
>>>>>> +      this->input_section_order_.push_back(std::string(buf));
>>>>>> +    }
>>>>>> +
>>>>>> +  if (buf != NULL)
>>>>>> +    free(buf);
>>>>>> +
>>>>>> +  fclose(fp);
>>>>>> +  return true;
>>>>>> +}
>>>>>>
>>>>>
>>>>> The getline function is insufficient portable for use in gold.  Search
>>>>> for std::getline in options.cc for an alternate approach you can use.
>>>>> Emulate the error message style you see there too--no capital letter,
>>>>> name the file, no space before colon, no \n.  And if you really want
>>>>> to exit on failure, call gold_fatal.
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> +// If --final-layout option is used, reorder the input sections in
>>>>>> +// .text, .data, .bss and .rodata according to the specified
>>>>>> sequence.
>>>>>> +
>>>>>> +void
>>>>>> +Layout::section_reorder()
>>>>>> +{
>>>>>> +  this->read_layout_from_file();
>>>>>> +
>>>>>> +  for (Section_list::iterator p = this->section_list_.begin();
>>>>>> +       p != this->section_list_.end();
>>>>>> +       ++p)
>>>>>> +    {
>>>>>> +      if (strcmp(".text", (*p)->name()) == 0
>>>>>> +          || strcmp(".data", (*p)->name()) == 0
>>>>>> +          || strcmp(".bss", (*p)->name()) == 0
>>>>>> +          || strcmp(".rodata", (*p)->name()) == 0)
>>>>>> +        (*p)->reorder_layout(this);
>>>>>> +    }
>>>>>> +}
>>>>>>
>>>>>
>>>>> Why restrict this to those output sections?  Why not sort input
>>>>> sections in any output section?
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> +  DEFINE_string(final_layout, options::TWO_DASHES, '\0', NULL,
>>>>>> +             N_("Layout functions and data in the order specified."),
>>>>>> +             N_("FILENAME"));
>>>>>>
>>>>>
>>>>> I'm not sure I care for --final-layout as the option name.  Perhaps
>>>>> --section-ordering-file?  Perhaps somebody else has a better idea.
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> +  // the future, we keep track of the sections.  If the
>>>>>> --final-layout
>>>>>> +  // option is used to specify the order of sections, we need to keep
>>>>>> +  // track of sections.
>>>>>>    if (have_sections_script
>>>>>>        || !this->input_sections_.empty()
>>>>>>        || this->may_sort_attached_input_sections()
>>>>>>        || this->must_sort_attached_input_sections()
>>>>>>        || parameters->options().user_set_Map()
>>>>>> -      || parameters->target().may_relax())
>>>>>> -    this->input_sections_.push_back(Input_section(object, shndx,
>>>>>> -                                               shdr.get_sh_size(),
>>>>>> -                                               addralign));
>>>>>> +      || parameters->target().may_relax()
>>>>>> +      || parameters->options().final_layout())
>>>>>> +    {
>>>>>> +      Input_section isecn(object, shndx, shdr.get_sh_size(),
>>>>>> addralign);
>>>>>> +      isecn.set_section_name(secname);
>>>>>> +      this->input_sections_.push_back(isecn);
>>>>>> +    }
>>>>>>
>>>>>
>>>>> Don't save the string here, that's just going to bloat memory usage.
>>>>> Instead, when you read the file, give each line a number.  Then match
>>>>> the section name which you have here against the list of patterns.  If
>>>>> you find a match, store the number in the Input_section structure.
>>>>> Also, if you find a match, set a flag in the output section.  Then
>>>>> sort the sections, by number, in a function called from
>>>>> set_final_data_size.
>>>>>
>>>>> You will see that there is already some section ordering in that
>>>>> function, which is used to implement constructor/destructor priority
>>>>> ordering.  I guess the priority ordering should take precedence.
>>>>> Maybe.
>>>>>
>>>>> Ian
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>
>

[-- Attachment #2: final_layout_patch.txt --]
[-- Type: text/plain, Size: 26969 bytes --]

Index: gold.h
===================================================================
RCS file: /cvs/src/src/gold/gold.h,v
retrieving revision 1.43
diff -u -u -p -r1.43 gold.h
--- gold.h	18 May 2010 19:18:31 -0000	1.43
+++ gold.h	29 May 2010 01:44:54 -0000
@@ -401,6 +401,15 @@ string_hash(const Char_type* s)
   return h;
 }
 
+// Return whether STRING contains a wildcard character.  This is used
+// to speed up matching.
+
+inline bool
+is_wildcard_string(const char* s)
+{
+  return strpbrk(s, "?*[") != NULL;
+}
+
 } // End namespace gold.
 
 #endif // !defined(GOLD_GOLD_H)
Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.169
diff -u -u -p -r1.169 layout.cc
--- layout.cc	24 Apr 2010 14:32:23 -0000	1.169
+++ layout.cc	29 May 2010 01:44:54 -0000
@@ -26,8 +26,10 @@
 #include <cstring>
 #include <algorithm>
 #include <iostream>
+#include <fstream>
 #include <utility>
 #include <fcntl.h>
+#include <fnmatch.h>
 #include <unistd.h>
 #include "libiberty.h"
 #include "md5.h"
@@ -669,7 +671,8 @@ Layout::layout(Sized_relobj<size, big_en
   // FIXME: Handle SHF_LINK_ORDER somewhere.
 
   *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
-			       this->script_options_->saw_sections_clause());
+			       this->script_options_->saw_sections_clause(),
+                               this);
   this->have_added_input_section_ = true;
 
   return os;
@@ -887,7 +890,7 @@ Layout::layout_eh_frame(Sized_relobj<siz
       // Add it as a normal section.
       bool saw_sections_clause = this->script_options_->saw_sections_clause();
       *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
-				   saw_sections_clause);
+				   saw_sections_clause, this);
       this->have_added_input_section_ = true;
     }
 
@@ -1642,6 +1645,65 @@ Layout::relaxation_loop_body(
   return off;
 }
 
+// Search the list of patterns and find the postion of the given section
+// name in the output section.  If the section name matches a glob
+// pattern and a non-glob name, then the non-glob position takes
+// precedence.  Return 0 if no match is found.
+
+unsigned int
+Layout::find_section_order_index(const std::string& section_name)
+{
+  std::map<std::string, unsigned int>::iterator map_it;
+  map_it = this->input_section_position_.find(section_name);
+  if (map_it != this->input_section_position_.end())
+    return map_it->second;
+  
+  // Absolute match failed.  Linear search the glob patterns.   
+  std::vector<std::string>::iterator it;
+  for (it = this->input_section_glob_.begin();
+       it != this->input_section_glob_.end();
+       ++it)
+    {
+       if (fnmatch((*it).c_str(), section_name.c_str(), FNM_NOESCAPE) == 0)
+         {
+           map_it = this->input_section_position_.find(*it);
+           gold_assert(map_it != this->input_section_position_.end());
+           return map_it->second;
+         }
+    }
+  return 0;
+}
+
+// Read the sequence of input sections from the file specified with
+// --section-ordering-file.
+
+void
+Layout::read_layout_from_file()
+{
+  const char* filename = parameters->options().section_ordering_file();
+  std::ifstream in;
+  std::string line;
+
+  in.open(filename);
+  std::getline(in, line);   // this chops off the trailing \n, if any
+  if (!in)
+    gold_fatal(_("unable to open --section-ordering-file file %s: %s"),
+               filename, strerror(errno));
+
+  unsigned int position = 1;
+  while (in)
+    {
+      if (!line.empty() && line[line.length() - 1] == '\r')   // Windows
+        line.resize(line.length() - 1);
+      this->input_section_position_[line] = position;
+      // Store all glob patterns in a vector.
+      if (is_wildcard_string(line.c_str()))
+        this->input_section_glob_.push_back(line);
+      position++;
+      std::getline(in, line);
+    }
+}
+
 // Finalize the layout.  When this is called, we have created all the
 // output sections and all the output segments which are based on
 // input sections.  We have several things to do, and we have to do
Index: layout.h
===================================================================
RCS file: /cvs/src/src/gold/layout.h,v
retrieving revision 1.80
diff -u -u -p -r1.80 layout.h
--- layout.h	9 Apr 2010 17:32:58 -0000	1.80
+++ layout.h	29 May 2010 01:44:54 -0000
@@ -308,6 +308,12 @@ class Layout
 	 const char* name, const elfcpp::Shdr<size, big_endian>& shdr,
 	 unsigned int reloc_shndx, unsigned int reloc_type, off_t* offset);
 
+  unsigned int
+  find_section_order_index(const std::string&);
+
+  void
+  read_layout_from_file();
+
   // Layout an input reloc section when doing a relocatable link.  The
   // section is RELOC_SHNDX in OBJECT, with data in SHDR.
   // DATA_SECTION is the reloc section to which it refers.  RR is the
@@ -1037,6 +1043,10 @@ class Layout
   Segment_states* segment_states_;
   // A relaxation debug checker.  We only create one when in debugging mode.
   Relaxation_debug_check* relaxation_debug_check_;
+  // Hash a pattern to its position in the section ordering file.
+  std::map<std::string, unsigned int> input_section_position_;
+  // Vector of glob only patterns in the section_ordering file.
+  std::vector<std::string> input_section_glob_;
 };
 
 // This task handles writing out data in output sections which is not
Index: main.cc
===================================================================
RCS file: /cvs/src/src/gold/main.cc,v
retrieving revision 1.38
diff -u -u -p -r1.38 main.cc
--- main.cc	22 Mar 2010 14:18:24 -0000	1.38
+++ main.cc	29 May 2010 01:44:54 -0000
@@ -234,6 +234,9 @@ main(int argc, char** argv)
       layout.incremental_inputs()->report_inputs(command_line.inputs());
     }
 
+  if (parameters->options().section_ordering_file())
+    layout.read_layout_from_file();
+
   // Get the search path from the -L options.
   Dirsearch search_path;
   search_path.initialize(&workqueue, &command_line.options().library_path());
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.146
diff -u -u -p -r1.146 options.h
--- options.h	18 May 2010 18:08:03 -0000	1.146
+++ options.h	29 May 2010 01:44:54 -0000
@@ -892,6 +892,10 @@ class General_options
                  N_("Add DIR to link time shared library search path"),
                  N_("DIR"));
 
+  DEFINE_string(section_ordering_file, options::TWO_DASHES, '\0', NULL,
+		N_("Layout functions and data in the order specified."),
+		N_("FILENAME"));
+
   DEFINE_special(section_start, options::TWO_DASHES, '\0',
 		 N_("Set address of section"), N_("SECTION=ADDRESS"));
 
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.126
diff -u -u -p -r1.126 output.cc
--- output.cc	23 May 2010 07:43:39 -0000	1.126
+++ output.cc	29 May 2010 01:44:54 -0000
@@ -1933,6 +1933,7 @@ Output_section::Output_section(const cha
     found_in_sections_clause_(false),
     has_load_address_(false),
     info_uses_section_index_(false),
+    input_section_order_specified_(false),
     may_sort_attached_input_sections_(false),
     must_sort_attached_input_sections_(false),
     attached_input_sections_are_sorted_(false),
@@ -1999,7 +2000,8 @@ Output_section::add_input_section(Sized_
 				  const char* secname,
 				  const elfcpp::Shdr<size, big_endian>& shdr,
 				  unsigned int reloc_shndx,
-				  bool have_sections_script)
+				  bool have_sections_script,
+				  Layout* layout)
 {
   elfcpp::Elf_Xword addralign = shdr.get_sh_addralign();
   if ((addralign & (addralign - 1)) != 0)
@@ -2090,16 +2092,30 @@ Output_section::add_input_section(Sized_
   // We need to keep track of this section if we are already keeping
   // track of sections, or if we are relaxing.  Also, if this is a
   // section which requires sorting, or which may require sorting in
-  // the future, we keep track of the sections.
+  // the future, we keep track of the sections.  If the
+  // --section-ordering-file option is used to specify the order of
+  // sections, we need to keep track of sections.
   if (have_sections_script
       || !this->input_sections_.empty()
       || this->may_sort_attached_input_sections()
       || this->must_sort_attached_input_sections()
       || parameters->options().user_set_Map()
-      || parameters->target().may_relax())
-    this->input_sections_.push_back(Input_section(object, shndx,
-						  shdr.get_sh_size(),
-						  addralign));
+      || parameters->target().may_relax()
+      || parameters->options().section_ordering_file())
+    {
+      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
+      if (parameters->options().section_ordering_file())
+        {
+          unsigned int section_order_index =
+            layout->find_section_order_index(std::string(secname));
+	  if (section_order_index)
+            {
+              isecn.set_section_order_index(section_order_index);
+              this->set_input_section_order_specified();
+            }
+        }
+      this->input_sections_.push_back(isecn);
+    }
 
   return aligned_offset_in_section;
 }
@@ -2623,7 +2639,8 @@ Output_section::set_final_data_size()
       return;
     }
 
-  if (this->must_sort_attached_input_sections())
+  if (this->must_sort_attached_input_sections()
+      || this->input_section_order_specified())
     this->sort_attached_input_sections();
 
   uint64_t address = this->address();
@@ -2700,12 +2717,14 @@ class Output_section::Input_section_sort
   { }
 
   Input_section_sort_entry(const Input_section& input_section,
-			   unsigned int index)
+			   unsigned int index,
+			   bool must_sort_attached_input_sections)
     : input_section_(input_section), index_(index),
       section_has_name_(input_section.is_input_section()
 			|| input_section.is_relaxed_input_section())
   {
-    if (this->section_has_name_)
+    if (this->section_has_name_
+        && must_sort_attached_input_sections)
       {
 	// This is only called single-threaded from Layout::finalize,
 	// so it is OK to lock.  Unfortunately we have no way to pass
@@ -2782,6 +2801,22 @@ class Output_section::Input_section_sort
     return memcmp(base_name + base_len - 2, ".o", 2) == 0;
   }
 
+  // Returns 0 if no order is specified. Returns 1 if "this" is the
+  // first section in order (is_before), returns 2 for s.
+  unsigned int
+  is_before_in_sequence(const Input_section_sort_entry& s) const
+  {
+    gold_assert(this->index_ != -1U);
+    if (this->input_section_.section_order_index() == 0
+        || s.input_section().section_order_index() == 0)
+      return 0;
+    if (this->input_section_.section_order_index()
+         < s.input_section().section_order_index())
+      return 1;
+    else
+      return 2;
+  }
+
  private:
   // The Input_section we are sorting.
   Input_section input_section_;
@@ -2843,6 +2878,12 @@ Output_section::Input_section_sort_compa
   if (!s1_has_priority && s2_has_priority)
     return true;
 
+  // Check if a section order exists for these sections through a section
+  // ordering file.  If sequence_num is 0, an order does not exist.
+  unsigned int sequence_num = s1.is_before_in_sequence(s2);
+  if (sequence_num)
+    return sequence_num == 1;
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2879,6 +2920,12 @@ Output_section::Input_section_sort_init_
   if (!s1_has_priority && s2_has_priority)
     return false;
 
+  // Check if a section order exists for these sections through a section
+  // ordering file.  If sequence_num is 0, an order does not exist.
+  unsigned int sequence_num = s1.is_before_in_sequence(s2);
+  if (sequence_num)
+    return sequence_num == 1;
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2888,6 +2935,22 @@ Output_section::Input_section_sort_init_
   return s1.index() < s2.index();
 }
 
+// Return true if S1 should come before S2.
+bool
+Output_section::Input_section_sort_section_order_index_compare::operator()(
+    const Output_section::Input_section_sort_entry& s1,
+    const Output_section::Input_section_sort_entry& s2) const
+{  
+  // Check if a section order exists for these sections through a section
+  // ordering file.  If sequence_num is 0, an order does not exist.
+  unsigned int sequence_num = s1.is_before_in_sequence(s2);
+  if (sequence_num)
+    return sequence_num == 1;
+
+  // Otherwise we keep the input order.
+  return s1.index() < s2.index();
+}
+
 // Sort the input sections attached to an output section.
 
 void
@@ -2913,17 +2976,27 @@ Output_section::sort_attached_input_sect
   for (Input_section_list::iterator p = this->input_sections_.begin();
        p != this->input_sections_.end();
        ++p, ++i)
-    sort_list.push_back(Input_section_sort_entry(*p, i));
+      sort_list.push_back(Input_section_sort_entry(*p, i,
+  			    this->must_sort_attached_input_sections()));
 
   // Sort the input sections.
-  if (this->type() == elfcpp::SHT_PREINIT_ARRAY
-      || this->type() == elfcpp::SHT_INIT_ARRAY
-      || this->type() == elfcpp::SHT_FINI_ARRAY)
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_init_fini_compare());
+  if (this->must_sort_attached_input_sections())
+    {
+      if (this->type() == elfcpp::SHT_PREINIT_ARRAY
+          || this->type() == elfcpp::SHT_INIT_ARRAY
+          || this->type() == elfcpp::SHT_FINI_ARRAY)
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_init_fini_compare());
+      else
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_compare());
+    }
   else
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_compare());
+    {
+      gold_assert(parameters->options().section_ordering_file());
+      std::sort(sort_list.begin(), sort_list.end(),
+	        Input_section_sort_section_order_index_compare());
+    }
 
   // Copy the sorted input sections back to our list.
   this->input_sections_.clear();
@@ -2931,6 +3004,7 @@ Output_section::sort_attached_input_sect
        p != sort_list.end();
        ++p)
     this->input_sections_.push_back(p->input_section());
+  sort_list.clear();
 
   // Remember that we sorted the input sections, since we might get
   // called again.
@@ -4471,7 +4545,8 @@ Output_section::add_input_section<32, fa
     const char* secname,
     const elfcpp::Shdr<32, false>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -4483,7 +4558,8 @@ Output_section::add_input_section<32, tr
     const char* secname,
     const elfcpp::Shdr<32, true>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -4495,7 +4571,8 @@ Output_section::add_input_section<64, fa
     const char* secname,
     const elfcpp::Shdr<64, false>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -4507,7 +4584,8 @@ Output_section::add_input_section<64, tr
     const char* secname,
     const elfcpp::Shdr<64, true>& shdr,
     unsigned int reloc_shndx,
-    bool have_sections_script);
+    bool have_sections_script,
+    Layout* layout);
 #endif
 
 #ifdef HAVE_TARGET_32_LITTLE
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.106
diff -u -u -p -r1.106 output.h
--- output.h	23 May 2010 07:43:39 -0000	1.106
+++ output.h	29 May 2010 01:44:54 -0000
@@ -2518,7 +2518,8 @@ class Output_section : public Output_dat
   add_input_section(Sized_relobj<size, big_endian>* object, unsigned int shndx,
 		    const char *name,
 		    const elfcpp::Shdr<size, big_endian>& shdr,
-		    unsigned int reloc_shndx, bool have_sections_script);
+		    unsigned int reloc_shndx, bool have_sections_script,
+		    Layout* layout);
 
   // Add generated data POSD to this output section.
   void
@@ -2736,6 +2737,18 @@ class Output_section : public Output_dat
   set_may_sort_attached_input_sections()
   { this->may_sort_attached_input_sections_ = true; }
 
+   // Returns true if input sections must be sorted according to the
+  // order in which their name appear in the --section-ordering-file.
+  bool
+  input_section_order_specified()
+  { return this->input_section_order_specified_; }
+
+  // Record that input sections must be sorted as some of their names
+  // match the patterns specified through --section-ordering-file.
+  void
+  set_input_section_order_specified()
+  { this->input_section_order_specified_ = true; }
+
   // Return whether the input sections attached to this output section
   // require sorting.  This is used to handle constructor priorities
   // compatibly with GNU ld.
@@ -2972,7 +2985,8 @@ class Output_section : public Output_dat
     Input_section(Relobj* object, unsigned int shndx, off_t data_size,
 		  uint64_t addralign)
       : shndx_(shndx),
-	p2align_(ffsll(static_cast<long long>(addralign)))
+	p2align_(ffsll(static_cast<long long>(addralign))),
+	section_order_index_(0)
     {
       gold_assert(shndx != OUTPUT_SECTION_CODE
 		  && shndx != MERGE_DATA_SECTION_CODE
@@ -2984,7 +2998,8 @@ class Output_section : public Output_dat
 
     // For a non-merge output section.
     Input_section(Output_section_data* posd)
-      : shndx_(OUTPUT_SECTION_CODE), p2align_(0)
+      : shndx_(OUTPUT_SECTION_CODE), p2align_(0),
+	section_order_index_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.posd = posd;
@@ -2995,7 +3010,8 @@ class Output_section : public Output_dat
       : shndx_(is_string
 	       ? MERGE_STRING_SECTION_CODE
 	       : MERGE_DATA_SECTION_CODE),
-	p2align_(0)
+	p2align_(0),
+	section_order_index_(0)
     {
       this->u1_.entsize = entsize;
       this->u2_.posd = posd;
@@ -3003,12 +3019,25 @@ class Output_section : public Output_dat
 
     // For a relaxed input section.
     Input_section(Output_relaxed_input_section *psection)
-      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0)
+      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0),
+	section_order_index_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.poris = psection;
     }
 
+    unsigned int
+    section_order_index() const
+    {
+      return this->section_order_index_;
+    }
+
+    void
+    set_section_order_index(unsigned int number)
+    {
+      this->section_order_index_ = number;  
+    }
+
     // The required alignment.
     uint64_t
     addralign() const
@@ -3234,6 +3263,9 @@ class Output_section : public Output_dat
       // For RELAXED_INPUT_SECTION_CODE, the data.
       Output_relaxed_input_section* poris;
     } u2_;
+    // The line number of the pattern it matches in the --section-ordering-file
+    // file.  It is 0 if does not match any pattern.
+    unsigned int section_order_index_;    
   };
 
   // Store the list of input sections for this Output_section into the
@@ -3540,6 +3572,15 @@ class Output_section : public Output_dat
 	       const Input_section_sort_entry&) const;
   };
 
+  // This is the sort comparison function when a section order is specified
+  // from an input file.
+  struct Input_section_sort_section_order_index_compare
+  {
+    bool
+    operator()(const Input_section_sort_entry&,
+	       const Input_section_sort_entry&) const;
+  };
+
   // Fill data.  This is used to fill in data between input sections.
   // It is also used for data statements (BYTE, WORD, etc.) in linker
   // scripts.  When we have to keep track of the input sections, we
@@ -3707,6 +3748,9 @@ class Output_section : public Output_dat
   // section, false if it means the symbol index of the corresponding
   // section symbol.
   bool info_uses_section_index_ : 1;
+  // True if input sections attached to this output section have to be
+  // sorted according to a specified order.
+  bool input_section_order_specified_ : 1;
   // True if the input sections attached to this output section may
   // need sorting.
   bool may_sort_attached_input_sections_ : 1;
Index: script-sections.cc
===================================================================
RCS file: /cvs/src/src/gold/script-sections.cc,v
retrieving revision 1.36
diff -u -u -p -r1.36 script-sections.cc
--- script-sections.cc	26 May 2010 15:15:05 -0000	1.36
+++ script-sections.cc	29 May 2010 01:44:54 -0000
@@ -983,15 +983,6 @@ class Output_section_element_fill : publ
   Expression* val_;
 };
 
-// Return whether STRING contains a wildcard character.  This is used
-// to speed up matching.
-
-static inline bool
-is_wildcard_string(const std::string& s)
-{
-  return strpbrk(s.c_str(), "?*[") != NULL;
-}
-
 // An input section specification in an output section
 
 class Output_section_element_input : public Output_section_element
@@ -1035,7 +1026,7 @@ class Output_section_element_input : pub
     Input_section_pattern(const char* patterna, size_t patternlena,
 			  Sort_wildcard sorta)
       : pattern(patterna, patternlena),
-	pattern_is_wildcard(is_wildcard_string(this->pattern)),
+	pattern_is_wildcard(is_wildcard_string(this->pattern.c_str())),
 	sort(sorta)
     { }
   };
@@ -1102,7 +1093,7 @@ Output_section_element_input::Output_sec
   if (spec->file.name.length != 1 || spec->file.name.value[0] != '*')
     this->filename_pattern_.assign(spec->file.name.value,
 				   spec->file.name.length);
-  this->filename_is_wildcard_ = is_wildcard_string(this->filename_pattern_);
+  this->filename_is_wildcard_ = is_wildcard_string(this->filename_pattern_.c_str());
 
   if (spec->input_sections.exclude != NULL)
     {
@@ -1111,7 +1102,7 @@ Output_section_element_input::Output_sec
 	   p != spec->input_sections.exclude->end();
 	   ++p)
 	{
-	  bool is_wildcard = is_wildcard_string(*p);
+	  bool is_wildcard = is_wildcard_string((*p).c_str());
 	  this->filename_exclusions_.push_back(std::make_pair(*p,
 							      is_wildcard));
 	}
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.135
diff -u -u -p -r1.135 Makefile.am
--- testsuite/Makefile.am	26 May 2010 15:47:39 -0000	1.135
+++ testsuite/Makefile.am	29 May 2010 01:44:54 -0000
@@ -193,6 +193,18 @@ icf_safe_so_test_1.stdout: icf_safe_so_t
 icf_safe_so_test_2.stdout: icf_safe_so_test
 	$(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
 
+check_SCRIPTS += final_layout.sh
+check_DATA += final_layout.stdout
+MOSTLYCLEANFILES += final_layout
+final_layout.o: final_layout.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections  -fdata-sections -g -o $@ $<
+final_layout_sequence.txt:
+	(echo "*_Z3barv*" && echo ".text._Z3bazv" && echo "*_Z3foov*" && echo "*global_varb*" && echo "*global_vara*" && echo "*global_varc*") > final_layout_sequence.txt
+final_layout: final_layout.o final_layout_sequence.txt gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,--section-ordering-file,final_layout_sequence.txt final_layout.o
+final_layout.stdout: final_layout
+	$(TEST_NM) final_layout > final_layout.stdout
+
 check_PROGRAMS += icf_virtual_function_folding_test
 MOSTLYCLEANFILES += icf_virtual_function_folding_test
 icf_virtual_function_folding_test.o: icf_virtual_function_folding_test.cc
Index: testsuite/final_layout.cc
===================================================================
RCS file: testsuite/final_layout.cc
diff -N testsuite/final_layout.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.cc	29 May 2010 01:44:55 -0000
@@ -0,0 +1,48 @@
+// final_layout.cc -- a test case for gold
+
+// Copyright 2010 Free Software Foundation, Inc.
+// Written by Sriraman Tallam <tmsriram@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify if --section-ordering-file orders
+// the .text and .data sections correctly as specified.
+
+int global_vara;
+int global_varb;
+int global_varc;
+
+int foo()
+{
+  return 1;
+}
+
+int bar()
+{
+  return 1;
+}
+
+int baz()
+{
+  return 1;
+}
+
+int main()
+{
+  return 1;
+}
Index: testsuite/final_layout.sh
===================================================================
RCS file: testsuite/final_layout.sh
diff -N testsuite/final_layout.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.sh	29 May 2010 01:44:55 -0000
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+# final_layout.sh -- test --final-layout
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Sriraman Tallam <tmsriram@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# The goal of this program is to verify if --section-ordering-file works as
+# intended.  File final_layout.cc is in this test.
+
+check()
+{
+    func_addr_1=$((16#`grep $2 $1 | awk '{print $1}'`))
+    func_addr_2=$((16#`grep $3 $1 | awk '{print $1}'`))
+    if [ $func_addr_1 -gt $func_addr_2 ]
+    then
+        echo "final layout of" $2 "and" $3 "is not right."
+	exit 1
+    fi
+}
+
+check final_layout.stdout "_Z3barv" "_Z3bazv"
+check final_layout.stdout "_Z3bazv" "_Z3foov"
+check final_layout.stdout "global_varb" "global_vara"
+check final_layout.stdout "global_vara" "global_varc"

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-05-29  1:49           ` Sriraman Tallam
@ 2010-06-01 14:41             ` Ian Lance Taylor
  2010-06-01 23:41               ` Sriraman Tallam
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Lance Taylor @ 2010-06-01 14:41 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: Taras Glek, binutils

Sriraman Tallam <tmsriram@google.com> writes:

> 	* gold.h (is_wildcard_string): New function.
> 	* layout.cc (Layout::layout): Pass this pointer to add_input_section.
> 	(Layout::layout_eh_frame): Ditto.
> 	(Layout::find_section_order_index): New method.
> 	(Layout::read_layout_from_file): New method.
> 	* layout.h (Layout::find_section_order_index): New method.
> 	(Layout::read_layout_from_file): New method.
> 	(Layout::input_section_position_): New private member.
> 	(Layout::input_section_glob_): New private member.
> 	* main.cc (main): Call read_layout_from_file here.
> 	* options.h (--section-ordering-file): New option.
> 	* output.cc (Output_section::input_section_order_specified_): New
> 	member.
> 	(Output_section::Output_section): Initialize new member.
> 	(Output_section::add_input_section): Add new parameter.
> 	Keep input sections when --section-ordering-file is used.
> 	(Output_section::set_final_data_size): Sort input sections when
> 	section ordering file is specified.
> 	(Output_section::Input_section_sort_entry): Add new parameter.
> 	Check sorting type.
> 	(Output_section::Input_section_sort_entry::is_before_in_sequence):
> 	New method.
> 	(Output_section::Input_section_sort_compare::operator()): Change to
> 	consider section_order_index.
> 	(Output_section::Input_section_sort_init_fini_compare::operator()):
> 	Change to consider section_order_index.
> 	(Output_section::Input_section_sort_section_order_index_compare
> 	::operator()): New method.
> 	(Output_section::sort_attached_input_sections): Change to sort
> 	according to section order when specified.
> 	(Output_section::add_input_section<32, true>): Add new parameter.
> 	(Output_section::add_input_section<64, true>): Add new parameter.
> 	(Output_section::add_input_section<32, false>): Add new parameter.
> 	(Output_section::add_input_section<64, false>): Add new parameter.
> 	* output.h (Output_section::add_input_section): Add new parameter.
> 	(Output_section::input_section_order_specified): New
> 	method.
> 	(Output_section::set_input_section_order_specified): New method.
> 	(Input_section::Input_section): Initialize section_order_index_.
> 	(Input_section::section_order_index): New method.
> 	(Input_section::set_section_order_index): New method.
> 	(Input_section::section_order_index_): New member.
> 	(Input_section::Input_section_sort_section_order_index_compare): New
> 	struct.
> 	(Output_section::input_section_order_specified_): New member.
> 	* script-sections.cc (is_wildcard_string): Delete and move modified
> 	method to gold.h.
> 	(Output_section_element_input::Output_section_element_input): Modify
> 	call to is_wildcard_string.
> 	(Output_section_element_input::Input_section_pattern
> 	::Input_section_pattern): Ditto.
> 	(Output_section_element_input::Output_section_element_input): Ditto.
> 	* testsuite/Makefile.am (final_layout): New test case.
> 	* testsuite/Makefile.in: Regenerate.
> 	* testsuite/final_layout.cc: New file.
> 	* testsuite/final_layout.sh: New file.


> +void
> +Layout::read_layout_from_file()
> +{
> +  const char* filename = parameters->options().section_ordering_file();
> +  std::ifstream in;
> +  std::string line;
> +
> +  in.open(filename);
> +  std::getline(in, line);   // this chops off the trailing \n, if any
> +  if (!in)
> +    gold_fatal(_("unable to open --section-ordering-file file %s: %s"),
> +               filename, strerror(errno));

You should check the result of in.open before calling std::getline.

> +
> +  unsigned int position = 1;
> +  while (in)
> +    {
> +      if (!line.empty() && line[line.length() - 1] == '\r')   // Windows
> +        line.resize(line.length() - 1);
> +      this->input_section_position_[line] = position;
> +      // Store all glob patterns in a vector.
> +      if (is_wildcard_string(line.c_str()))
> +        this->input_section_glob_.push_back(line);
> +      position++;
> +      std::getline(in, line);
> +    }
> +}

I think it would be useful to have some provision for comments in this
file.  I suggest that all lines which begin with '#' be discarded.  I
don't think we need to worry about section names which begin with '#'.


> +  // Hash a pattern to its position in the section ordering file.
> +  std::map<std::string, unsigned int> input_section_position_;

The comment says "Hash" but the code uses a map.  It does seem that a
hash would be appropriate here--e.g., use Unordered_map.


> +  DEFINE_string(section_ordering_file, options::TWO_DASHES, '\0', NULL,
> +		N_("Layout functions and data in the order specified."),
> +		N_("FILENAME"));

s/functions and data/sections/


> @@ -1999,7 +2000,8 @@ Output_section::add_input_section(Sized_
>  				  const char* secname,
>  				  const elfcpp::Shdr<size, big_endian>& shdr,
>  				  unsigned int reloc_shndx,
> -				  bool have_sections_script)
> +				  bool have_sections_script,
> +				  Layout* layout)

The convention in gold is that general capability objects are passed
first.  In this case layout should be the first parameter, not the
last.

> @@ -2090,16 +2092,30 @@ Output_section::add_input_section(Sized_
>    // We need to keep track of this section if we are already keeping
>    // track of sections, or if we are relaxing.  Also, if this is a
>    // section which requires sorting, or which may require sorting in
> -  // the future, we keep track of the sections.
> +  // the future, we keep track of the sections.  If the
> +  // --section-ordering-file option is used to specify the order of
> +  // sections, we need to keep track of sections.
>    if (have_sections_script
>        || !this->input_sections_.empty()
>        || this->may_sort_attached_input_sections()
>        || this->must_sort_attached_input_sections()
>        || parameters->options().user_set_Map()
> -      || parameters->target().may_relax())
> -    this->input_sections_.push_back(Input_section(object, shndx,
> -						  shdr.get_sh_size(),
> -						  addralign));
> +      || parameters->target().may_relax()
> +      || parameters->options().section_ordering_file())
> +    {
> +      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
> +      if (parameters->options().section_ordering_file())
> +        {
> +          unsigned int section_order_index =
> +            layout->find_section_order_index(std::string(secname));
> +	  if (section_order_index)
> +            {
> +              isecn.set_section_order_index(section_order_index);
> +              this->set_input_section_order_specified();
> +            }
> +        }
> +      this->input_sections_.push_back(isecn);
> +    }

Write if (section_order_index != 0); it's not a boolean value.

> @@ -2782,6 +2801,22 @@ class Output_section::Input_section_sort
>      return memcmp(base_name + base_len - 2, ".o", 2) == 0;
>    }
>  
> +  // Returns 0 if no order is specified. Returns 1 if "this" is the
> +  // first section in order (is_before), returns 2 for s.

s/"this"/THIS/.  s/for s/for S/.

> +  unsigned int
> +  is_before_in_sequence(const Input_section_sort_entry& s) const
> +  {
> +    gold_assert(this->index_ != -1U);
> +    if (this->input_section_.section_order_index() == 0
> +        || s.input_section().section_order_index() == 0)
> +      return 0;
> +    if (this->input_section_.section_order_index()
> +         < s.input_section().section_order_index())

Indentation looks wrong.

> +      return 1;
> +    else
> +      return 2;
> +  }

A more conventional comparator would return an int value, and return
-1 if THIS is first, 1 if S is first, 0 if they are incomparable.  I
think we should use that convention unless there is some reason it
won't work.

The name "is_before_in_sequence" implies that this function returns a
boolean value, which it does not.  Change the name to something like
"compare_section_ordering".

> @@ -2843,6 +2878,12 @@ Output_section::Input_section_sort_compa
>    if (!s1_has_priority && s2_has_priority)
>      return true;
>  
> +  // Check if a section order exists for these sections through a section
> +  // ordering file.  If sequence_num is 0, an order does not exist.
> +  unsigned int sequence_num = s1.is_before_in_sequence(s2);
> +  if (sequence_num)
> +    return sequence_num == 1;

Write "if (sequence_num != 0)".  When the function changes to return
an int, write "return sequence_num < 0".

> @@ -2879,6 +2920,12 @@ Output_section::Input_section_sort_init_
>    if (!s1_has_priority && s2_has_priority)
>      return false;
>  
> +  // Check if a section order exists for these sections through a section
> +  // ordering file.  If sequence_num is 0, an order does not exist.
> +  unsigned int sequence_num = s1.is_before_in_sequence(s2);
> +  if (sequence_num)
> +    return sequence_num == 1;

Same here.

> +// Return true if S1 should come before S2.
> +bool
> +Output_section::Input_section_sort_section_order_index_compare::operator()(
> +    const Output_section::Input_section_sort_entry& s1,
> +    const Output_section::Input_section_sort_entry& s2) const
> +{  
> +  // Check if a section order exists for these sections through a section
> +  // ordering file.  If sequence_num is 0, an order does not exist.
> +  unsigned int sequence_num = s1.is_before_in_sequence(s2);
> +  if (sequence_num)
> +    return sequence_num == 1;

Same here.

> @@ -2913,17 +2976,27 @@ Output_section::sort_attached_input_sect
>    for (Input_section_list::iterator p = this->input_sections_.begin();
>         p != this->input_sections_.end();
>         ++p, ++i)
> -    sort_list.push_back(Input_section_sort_entry(*p, i));
> +      sort_list.push_back(Input_section_sort_entry(*p, i,
> +  			    this->must_sort_attached_input_sections()));

Indentation change looks wrong.

>    // Sort the input sections.
> -  if (this->type() == elfcpp::SHT_PREINIT_ARRAY
> -      || this->type() == elfcpp::SHT_INIT_ARRAY
> -      || this->type() == elfcpp::SHT_FINI_ARRAY)
> -    std::sort(sort_list.begin(), sort_list.end(),
> -	      Input_section_sort_init_fini_compare());
> +  if (this->must_sort_attached_input_sections())
> +    {
> +      if (this->type() == elfcpp::SHT_PREINIT_ARRAY
> +          || this->type() == elfcpp::SHT_INIT_ARRAY
> +          || this->type() == elfcpp::SHT_FINI_ARRAY)
> +        std::sort(sort_list.begin(), sort_list.end(),
> +	          Input_section_sort_init_fini_compare());
> +      else
> +        std::sort(sort_list.begin(), sort_list.end(),
> +	          Input_section_sort_compare());
> +    }
>    else
> -    std::sort(sort_list.begin(), sort_list.end(),
> -	      Input_section_sort_compare());
> +    {
> +      gold_assert(parameters->options().section_ordering_file());
> +      std::sort(sort_list.begin(), sort_list.end(),
> +	        Input_section_sort_section_order_index_compare());
> +    }

This code is probably right but it means that in some cases people
will not get the expected result from --section-ordering-file.  I
wonder if there is some good way that we can warn if somebody tries to
use --section-ordering-file to reorder the input sections which appear
in a SORT clause in the section script.


This is OK with those changes.  If you're not sure how to handle the
warning, commit everything else and send a separate small patch to add
a warning.

Thanks.

Ian

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-01 14:41             ` Ian Lance Taylor
@ 2010-06-01 23:41               ` Sriraman Tallam
  2010-06-02 17:17                 ` H.J. Lu
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-06-01 23:41 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Taras Glek, binutils

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

Hi Ian,

     I committed the patch after making the changes you mentioned. I
did not add a warning when using the SORT clause and
--section-ordering file. Like you said, I will do this as a separate
patch after I figure out how to do this effectively.

2010-06-01  Sriraman Tallam  <tmsriram@google.com>

	* gold.h (is_wildcard_string): New function.
	* layout.cc (Layout::layout): Pass this pointer to add_input_section.
	(Layout::layout_eh_frame): Ditto.
	(Layout::find_section_order_index): New method.
	(Layout::read_layout_from_file): New method.
	* layout.h (Layout::find_section_order_index): New method.
	(Layout::read_layout_from_file): New method.
	(Layout::input_section_position_): New private member.
	(Layout::input_section_glob_): New private member.
	* main.cc (main): Call read_layout_from_file here.
	* options.h (--section-ordering-file): New option.
	* output.cc (Output_section::input_section_order_specified_): New
	member.
	(Output_section::Output_section): Initialize new member.
	(Output_section::add_input_section): Add new parameter.
	Keep input sections when --section-ordering-file is used.
	(Output_section::set_final_data_size): Sort input sections when
	section ordering file is specified.
	(Output_section::Input_section_sort_entry): Add new parameter.
	Check sorting type.
	(Output_section::Input_section_sort_entry::compare_section_ordering):
	New method.
	(Output_section::Input_section_sort_compare::operator()): Change to
	consider section_order_index.
	(Output_section::Input_section_sort_init_fini_compare::operator()):
	Change to consider section_order_index.
	(Output_section::Input_section_sort_section_order_index_compare
	::operator()): New method.
	(Output_section::sort_attached_input_sections): Change to sort
	according to section order when specified.
	(Output_section::add_input_section<32, true>): Add new parameter.
	(Output_section::add_input_section<64, true>): Add new parameter.
	(Output_section::add_input_section<32, false>): Add new parameter.
	(Output_section::add_input_section<64, false>): Add new parameter.
	* output.h (Output_section::add_input_section): Add new parameter.
	(Output_section::input_section_order_specified): New
	method.
	(Output_section::set_input_section_order_specified): New method.
	(Input_section::Input_section): Initialize section_order_index_.
	(Input_section::section_order_index): New method.
	(Input_section::set_section_order_index): New method.
	(Input_section::section_order_index_): New member.
	(Input_section::Input_section_sort_section_order_index_compare): New
	struct.
	(Output_section::input_section_order_specified_): New member.
	* script-sections.cc (is_wildcard_string): Delete and move modified
	method to gold.h.
	(Output_section_element_input::Output_section_element_input): Modify
	call to is_wildcard_string.
	(Output_section_element_input::Input_section_pattern
	::Input_section_pattern): Ditto.
	(Output_section_element_input::Output_section_element_input): Ditto.
	* testsuite/Makefile.am (final_layout): New test case.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/final_layout.cc: New file.
	* testsuite/final_layout.sh: New file.


Thanks,
-Sri.

On Tue, Jun 1, 2010 at 7:40 AM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>>       * gold.h (is_wildcard_string): New function.
>>       * layout.cc (Layout::layout): Pass this pointer to add_input_section.
>>       (Layout::layout_eh_frame): Ditto.
>>       (Layout::find_section_order_index): New method.
>>       (Layout::read_layout_from_file): New method.
>>       * layout.h (Layout::find_section_order_index): New method.
>>       (Layout::read_layout_from_file): New method.
>>       (Layout::input_section_position_): New private member.
>>       (Layout::input_section_glob_): New private member.
>>       * main.cc (main): Call read_layout_from_file here.
>>       * options.h (--section-ordering-file): New option.
>>       * output.cc (Output_section::input_section_order_specified_): New
>>       member.
>>       (Output_section::Output_section): Initialize new member.
>>       (Output_section::add_input_section): Add new parameter.
>>       Keep input sections when --section-ordering-file is used.
>>       (Output_section::set_final_data_size): Sort input sections when
>>       section ordering file is specified.
>>       (Output_section::Input_section_sort_entry): Add new parameter.
>>       Check sorting type.
>>       (Output_section::Input_section_sort_entry::is_before_in_sequence):
>>       New method.
>>       (Output_section::Input_section_sort_compare::operator()): Change to
>>       consider section_order_index.
>>       (Output_section::Input_section_sort_init_fini_compare::operator()):
>>       Change to consider section_order_index.
>>       (Output_section::Input_section_sort_section_order_index_compare
>>       ::operator()): New method.
>>       (Output_section::sort_attached_input_sections): Change to sort
>>       according to section order when specified.
>>       (Output_section::add_input_section<32, true>): Add new parameter.
>>       (Output_section::add_input_section<64, true>): Add new parameter.
>>       (Output_section::add_input_section<32, false>): Add new parameter.
>>       (Output_section::add_input_section<64, false>): Add new parameter.
>>       * output.h (Output_section::add_input_section): Add new parameter.
>>       (Output_section::input_section_order_specified): New
>>       method.
>>       (Output_section::set_input_section_order_specified): New method.
>>       (Input_section::Input_section): Initialize section_order_index_.
>>       (Input_section::section_order_index): New method.
>>       (Input_section::set_section_order_index): New method.
>>       (Input_section::section_order_index_): New member.
>>       (Input_section::Input_section_sort_section_order_index_compare): New
>>       struct.
>>       (Output_section::input_section_order_specified_): New member.
>>       * script-sections.cc (is_wildcard_string): Delete and move modified
>>       method to gold.h.
>>       (Output_section_element_input::Output_section_element_input): Modify
>>       call to is_wildcard_string.
>>       (Output_section_element_input::Input_section_pattern
>>       ::Input_section_pattern): Ditto.
>>       (Output_section_element_input::Output_section_element_input): Ditto.
>>       * testsuite/Makefile.am (final_layout): New test case.
>>       * testsuite/Makefile.in: Regenerate.
>>       * testsuite/final_layout.cc: New file.
>>       * testsuite/final_layout.sh: New file.
>
>
>> +void
>> +Layout::read_layout_from_file()
>> +{
>> +  const char* filename = parameters->options().section_ordering_file();
>> +  std::ifstream in;
>> +  std::string line;
>> +
>> +  in.open(filename);
>> +  std::getline(in, line);   // this chops off the trailing \n, if any
>> +  if (!in)
>> +    gold_fatal(_("unable to open --section-ordering-file file %s: %s"),
>> +               filename, strerror(errno));
>
> You should check the result of in.open before calling std::getline.
>
>> +
>> +  unsigned int position = 1;
>> +  while (in)
>> +    {
>> +      if (!line.empty() && line[line.length() - 1] == '\r')   // Windows
>> +        line.resize(line.length() - 1);
>> +      this->input_section_position_[line] = position;
>> +      // Store all glob patterns in a vector.
>> +      if (is_wildcard_string(line.c_str()))
>> +        this->input_section_glob_.push_back(line);
>> +      position++;
>> +      std::getline(in, line);
>> +    }
>> +}
>
> I think it would be useful to have some provision for comments in this
> file.  I suggest that all lines which begin with '#' be discarded.  I
> don't think we need to worry about section names which begin with '#'.
>
>
>> +  // Hash a pattern to its position in the section ordering file.
>> +  std::map<std::string, unsigned int> input_section_position_;
>
> The comment says "Hash" but the code uses a map.  It does seem that a
> hash would be appropriate here--e.g., use Unordered_map.
>
>
>> +  DEFINE_string(section_ordering_file, options::TWO_DASHES, '\0', NULL,
>> +             N_("Layout functions and data in the order specified."),
>> +             N_("FILENAME"));
>
> s/functions and data/sections/
>
>
>> @@ -1999,7 +2000,8 @@ Output_section::add_input_section(Sized_
>>                                 const char* secname,
>>                                 const elfcpp::Shdr<size, big_endian>& shdr,
>>                                 unsigned int reloc_shndx,
>> -                               bool have_sections_script)
>> +                               bool have_sections_script,
>> +                               Layout* layout)
>
> The convention in gold is that general capability objects are passed
> first.  In this case layout should be the first parameter, not the
> last.
>
>> @@ -2090,16 +2092,30 @@ Output_section::add_input_section(Sized_
>>    // We need to keep track of this section if we are already keeping
>>    // track of sections, or if we are relaxing.  Also, if this is a
>>    // section which requires sorting, or which may require sorting in
>> -  // the future, we keep track of the sections.
>> +  // the future, we keep track of the sections.  If the
>> +  // --section-ordering-file option is used to specify the order of
>> +  // sections, we need to keep track of sections.
>>    if (have_sections_script
>>        || !this->input_sections_.empty()
>>        || this->may_sort_attached_input_sections()
>>        || this->must_sort_attached_input_sections()
>>        || parameters->options().user_set_Map()
>> -      || parameters->target().may_relax())
>> -    this->input_sections_.push_back(Input_section(object, shndx,
>> -                                               shdr.get_sh_size(),
>> -                                               addralign));
>> +      || parameters->target().may_relax()
>> +      || parameters->options().section_ordering_file())
>> +    {
>> +      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
>> +      if (parameters->options().section_ordering_file())
>> +        {
>> +          unsigned int section_order_index =
>> +            layout->find_section_order_index(std::string(secname));
>> +       if (section_order_index)
>> +            {
>> +              isecn.set_section_order_index(section_order_index);
>> +              this->set_input_section_order_specified();
>> +            }
>> +        }
>> +      this->input_sections_.push_back(isecn);
>> +    }
>
> Write if (section_order_index != 0); it's not a boolean value.
>
>> @@ -2782,6 +2801,22 @@ class Output_section::Input_section_sort
>>      return memcmp(base_name + base_len - 2, ".o", 2) == 0;
>>    }
>>
>> +  // Returns 0 if no order is specified. Returns 1 if "this" is the
>> +  // first section in order (is_before), returns 2 for s.
>
> s/"this"/THIS/.  s/for s/for S/.
>
>> +  unsigned int
>> +  is_before_in_sequence(const Input_section_sort_entry& s) const
>> +  {
>> +    gold_assert(this->index_ != -1U);
>> +    if (this->input_section_.section_order_index() == 0
>> +        || s.input_section().section_order_index() == 0)
>> +      return 0;
>> +    if (this->input_section_.section_order_index()
>> +         < s.input_section().section_order_index())
>
> Indentation looks wrong.
>
>> +      return 1;
>> +    else
>> +      return 2;
>> +  }
>
> A more conventional comparator would return an int value, and return
> -1 if THIS is first, 1 if S is first, 0 if they are incomparable.  I
> think we should use that convention unless there is some reason it
> won't work.
>
> The name "is_before_in_sequence" implies that this function returns a
> boolean value, which it does not.  Change the name to something like
> "compare_section_ordering".
>
>> @@ -2843,6 +2878,12 @@ Output_section::Input_section_sort_compa
>>    if (!s1_has_priority && s2_has_priority)
>>      return true;
>>
>> +  // Check if a section order exists for these sections through a section
>> +  // ordering file.  If sequence_num is 0, an order does not exist.
>> +  unsigned int sequence_num = s1.is_before_in_sequence(s2);
>> +  if (sequence_num)
>> +    return sequence_num == 1;
>
> Write "if (sequence_num != 0)".  When the function changes to return
> an int, write "return sequence_num < 0".
>
>> @@ -2879,6 +2920,12 @@ Output_section::Input_section_sort_init_
>>    if (!s1_has_priority && s2_has_priority)
>>      return false;
>>
>> +  // Check if a section order exists for these sections through a section
>> +  // ordering file.  If sequence_num is 0, an order does not exist.
>> +  unsigned int sequence_num = s1.is_before_in_sequence(s2);
>> +  if (sequence_num)
>> +    return sequence_num == 1;
>
> Same here.
>
>> +// Return true if S1 should come before S2.
>> +bool
>> +Output_section::Input_section_sort_section_order_index_compare::operator()(
>> +    const Output_section::Input_section_sort_entry& s1,
>> +    const Output_section::Input_section_sort_entry& s2) const
>> +{
>> +  // Check if a section order exists for these sections through a section
>> +  // ordering file.  If sequence_num is 0, an order does not exist.
>> +  unsigned int sequence_num = s1.is_before_in_sequence(s2);
>> +  if (sequence_num)
>> +    return sequence_num == 1;
>
> Same here.
>
>> @@ -2913,17 +2976,27 @@ Output_section::sort_attached_input_sect
>>    for (Input_section_list::iterator p = this->input_sections_.begin();
>>         p != this->input_sections_.end();
>>         ++p, ++i)
>> -    sort_list.push_back(Input_section_sort_entry(*p, i));
>> +      sort_list.push_back(Input_section_sort_entry(*p, i,
>> +                         this->must_sort_attached_input_sections()));
>
> Indentation change looks wrong.
>
>>    // Sort the input sections.
>> -  if (this->type() == elfcpp::SHT_PREINIT_ARRAY
>> -      || this->type() == elfcpp::SHT_INIT_ARRAY
>> -      || this->type() == elfcpp::SHT_FINI_ARRAY)
>> -    std::sort(sort_list.begin(), sort_list.end(),
>> -           Input_section_sort_init_fini_compare());
>> +  if (this->must_sort_attached_input_sections())
>> +    {
>> +      if (this->type() == elfcpp::SHT_PREINIT_ARRAY
>> +          || this->type() == elfcpp::SHT_INIT_ARRAY
>> +          || this->type() == elfcpp::SHT_FINI_ARRAY)
>> +        std::sort(sort_list.begin(), sort_list.end(),
>> +               Input_section_sort_init_fini_compare());
>> +      else
>> +        std::sort(sort_list.begin(), sort_list.end(),
>> +               Input_section_sort_compare());
>> +    }
>>    else
>> -    std::sort(sort_list.begin(), sort_list.end(),
>> -           Input_section_sort_compare());
>> +    {
>> +      gold_assert(parameters->options().section_ordering_file());
>> +      std::sort(sort_list.begin(), sort_list.end(),
>> +             Input_section_sort_section_order_index_compare());
>> +    }
>
> This code is probably right but it means that in some cases people
> will not get the expected result from --section-ordering-file.  I
> wonder if there is some good way that we can warn if somebody tries to
> use --section-ordering-file to reorder the input sections which appear
> in a SORT clause in the section script.
>
>
> This is OK with those changes.  If you're not sure how to handle the
> warning, commit everything else and send a separate small patch to add
> a warning.
>
> Thanks.
>
> Ian
>

[-- Attachment #2: final_layout_patch.txt --]
[-- Type: text/plain, Size: 27053 bytes --]

Index: gold.h
===================================================================
RCS file: /cvs/src/src/gold/gold.h,v
retrieving revision 1.43
diff -u -u -p -r1.43 gold.h
--- gold.h	18 May 2010 19:18:31 -0000	1.43
+++ gold.h	1 Jun 2010 23:11:08 -0000
@@ -401,6 +401,15 @@ string_hash(const Char_type* s)
   return h;
 }
 
+// Return whether STRING contains a wildcard character.  This is used
+// to speed up matching.
+
+inline bool
+is_wildcard_string(const char* s)
+{
+  return strpbrk(s, "?*[") != NULL;
+}
+
 } // End namespace gold.
 
 #endif // !defined(GOLD_GOLD_H)
Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.169
diff -u -u -p -r1.169 layout.cc
--- layout.cc	24 Apr 2010 14:32:23 -0000	1.169
+++ layout.cc	1 Jun 2010 23:11:08 -0000
@@ -26,8 +26,10 @@
 #include <cstring>
 #include <algorithm>
 #include <iostream>
+#include <fstream>
 #include <utility>
 #include <fcntl.h>
+#include <fnmatch.h>
 #include <unistd.h>
 #include "libiberty.h"
 #include "md5.h"
@@ -668,7 +670,7 @@ Layout::layout(Sized_relobj<size, big_en
 
   // FIXME: Handle SHF_LINK_ORDER somewhere.
 
-  *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
+  *off = os->add_input_section(this, object, shndx, name, shdr, reloc_shndx,
 			       this->script_options_->saw_sections_clause());
   this->have_added_input_section_ = true;
 
@@ -886,7 +888,7 @@ Layout::layout_eh_frame(Sized_relobj<siz
       // We couldn't handle this .eh_frame section for some reason.
       // Add it as a normal section.
       bool saw_sections_clause = this->script_options_->saw_sections_clause();
-      *off = os->add_input_section(object, shndx, name, shdr, reloc_shndx,
+      *off = os->add_input_section(this, object, shndx, name, shdr, reloc_shndx,
 				   saw_sections_clause);
       this->have_added_input_section_ = true;
     }
@@ -1642,6 +1644,72 @@ Layout::relaxation_loop_body(
   return off;
 }
 
+// Search the list of patterns and find the postion of the given section
+// name in the output section.  If the section name matches a glob
+// pattern and a non-glob name, then the non-glob position takes
+// precedence.  Return 0 if no match is found.
+
+unsigned int
+Layout::find_section_order_index(const std::string& section_name)
+{
+  Unordered_map<std::string, unsigned int>::iterator map_it;
+  map_it = this->input_section_position_.find(section_name);
+  if (map_it != this->input_section_position_.end())
+    return map_it->second;
+
+  // Absolute match failed.  Linear search the glob patterns.
+  std::vector<std::string>::iterator it;
+  for (it = this->input_section_glob_.begin();
+       it != this->input_section_glob_.end();
+       ++it)
+    {
+       if (fnmatch((*it).c_str(), section_name.c_str(), FNM_NOESCAPE) == 0)
+         {
+           map_it = this->input_section_position_.find(*it);
+           gold_assert(map_it != this->input_section_position_.end());
+           return map_it->second;
+         }
+    }
+  return 0;
+}
+
+// Read the sequence of input sections from the file specified with
+// --section-ordering-file.
+
+void
+Layout::read_layout_from_file()
+{
+  const char* filename = parameters->options().section_ordering_file();
+  std::ifstream in;
+  std::string line;
+
+  in.open(filename);
+  if (!in)
+    gold_fatal(_("unable to open --section-ordering-file file %s: %s"),
+               filename, strerror(errno));
+
+  std::getline(in, line);   // this chops off the trailing \n, if any
+  unsigned int position = 1;
+
+  while (in)
+    {
+      if (!line.empty() && line[line.length() - 1] == '\r')   // Windows
+        line.resize(line.length() - 1);
+      // Ignore comments, beginning with '#'
+      if (line[0] == '#')
+        {
+          std::getline(in, line);
+          continue;
+        }
+      this->input_section_position_[line] = position;
+      // Store all glob patterns in a vector.
+      if (is_wildcard_string(line.c_str()))
+        this->input_section_glob_.push_back(line);
+      position++;
+      std::getline(in, line);
+    }
+}
+
 // Finalize the layout.  When this is called, we have created all the
 // output sections and all the output segments which are based on
 // input sections.  We have several things to do, and we have to do
Index: layout.h
===================================================================
RCS file: /cvs/src/src/gold/layout.h,v
retrieving revision 1.80
diff -u -u -p -r1.80 layout.h
--- layout.h	9 Apr 2010 17:32:58 -0000	1.80
+++ layout.h	1 Jun 2010 23:11:08 -0000
@@ -308,6 +308,12 @@ class Layout
 	 const char* name, const elfcpp::Shdr<size, big_endian>& shdr,
 	 unsigned int reloc_shndx, unsigned int reloc_type, off_t* offset);
 
+  unsigned int
+  find_section_order_index(const std::string&);
+
+  void
+  read_layout_from_file();
+
   // Layout an input reloc section when doing a relocatable link.  The
   // section is RELOC_SHNDX in OBJECT, with data in SHDR.
   // DATA_SECTION is the reloc section to which it refers.  RR is the
@@ -1037,6 +1043,10 @@ class Layout
   Segment_states* segment_states_;
   // A relaxation debug checker.  We only create one when in debugging mode.
   Relaxation_debug_check* relaxation_debug_check_;
+  // Hash a pattern to its position in the section ordering file.
+  Unordered_map<std::string, unsigned int> input_section_position_;
+  // Vector of glob only patterns in the section_ordering file.
+  std::vector<std::string> input_section_glob_;
 };
 
 // This task handles writing out data in output sections which is not
Index: main.cc
===================================================================
RCS file: /cvs/src/src/gold/main.cc,v
retrieving revision 1.38
diff -u -u -p -r1.38 main.cc
--- main.cc	22 Mar 2010 14:18:24 -0000	1.38
+++ main.cc	1 Jun 2010 23:11:08 -0000
@@ -234,6 +234,9 @@ main(int argc, char** argv)
       layout.incremental_inputs()->report_inputs(command_line.inputs());
     }
 
+  if (parameters->options().section_ordering_file())
+    layout.read_layout_from_file();
+
   // Get the search path from the -L options.
   Dirsearch search_path;
   search_path.initialize(&workqueue, &command_line.options().library_path());
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.146
diff -u -u -p -r1.146 options.h
--- options.h	18 May 2010 18:08:03 -0000	1.146
+++ options.h	1 Jun 2010 23:11:08 -0000
@@ -892,6 +892,10 @@ class General_options
                  N_("Add DIR to link time shared library search path"),
                  N_("DIR"));
 
+  DEFINE_string(section_ordering_file, options::TWO_DASHES, '\0', NULL,
+		N_("Layout sections in the order specified."),
+		N_("FILENAME"));
+
   DEFINE_special(section_start, options::TWO_DASHES, '\0',
 		 N_("Set address of section"), N_("SECTION=ADDRESS"));
 
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.126
diff -u -u -p -r1.126 output.cc
--- output.cc	23 May 2010 07:43:39 -0000	1.126
+++ output.cc	1 Jun 2010 23:11:08 -0000
@@ -1933,6 +1933,7 @@ Output_section::Output_section(const cha
     found_in_sections_clause_(false),
     has_load_address_(false),
     info_uses_section_index_(false),
+    input_section_order_specified_(false),
     may_sort_attached_input_sections_(false),
     must_sort_attached_input_sections_(false),
     attached_input_sections_are_sorted_(false),
@@ -1994,7 +1995,8 @@ Output_section::set_entsize(uint64_t v)
 
 template<int size, bool big_endian>
 off_t
-Output_section::add_input_section(Sized_relobj<size, big_endian>* object,
+Output_section::add_input_section(Layout* layout,
+				  Sized_relobj<size, big_endian>* object,
 				  unsigned int shndx,
 				  const char* secname,
 				  const elfcpp::Shdr<size, big_endian>& shdr,
@@ -2090,16 +2092,30 @@ Output_section::add_input_section(Sized_
   // We need to keep track of this section if we are already keeping
   // track of sections, or if we are relaxing.  Also, if this is a
   // section which requires sorting, or which may require sorting in
-  // the future, we keep track of the sections.
+  // the future, we keep track of the sections.  If the
+  // --section-ordering-file option is used to specify the order of
+  // sections, we need to keep track of sections.
   if (have_sections_script
       || !this->input_sections_.empty()
       || this->may_sort_attached_input_sections()
       || this->must_sort_attached_input_sections()
       || parameters->options().user_set_Map()
-      || parameters->target().may_relax())
-    this->input_sections_.push_back(Input_section(object, shndx,
-						  shdr.get_sh_size(),
-						  addralign));
+      || parameters->target().may_relax()
+      || parameters->options().section_ordering_file())
+    {
+      Input_section isecn(object, shndx, shdr.get_sh_size(), addralign);
+      if (parameters->options().section_ordering_file())
+        {
+          unsigned int section_order_index =
+            layout->find_section_order_index(std::string(secname));
+	  if (section_order_index != 0)
+            {
+              isecn.set_section_order_index(section_order_index);
+              this->set_input_section_order_specified();
+            }
+        }
+      this->input_sections_.push_back(isecn);
+    }
 
   return aligned_offset_in_section;
 }
@@ -2623,7 +2639,8 @@ Output_section::set_final_data_size()
       return;
     }
 
-  if (this->must_sort_attached_input_sections())
+  if (this->must_sort_attached_input_sections()
+      || this->input_section_order_specified())
     this->sort_attached_input_sections();
 
   uint64_t address = this->address();
@@ -2700,12 +2717,14 @@ class Output_section::Input_section_sort
   { }
 
   Input_section_sort_entry(const Input_section& input_section,
-			   unsigned int index)
+			   unsigned int index,
+			   bool must_sort_attached_input_sections)
     : input_section_(input_section), index_(index),
       section_has_name_(input_section.is_input_section()
 			|| input_section.is_relaxed_input_section())
   {
-    if (this->section_has_name_)
+    if (this->section_has_name_
+        && must_sort_attached_input_sections)
       {
 	// This is only called single-threaded from Layout::finalize,
 	// so it is OK to lock.  Unfortunately we have no way to pass
@@ -2782,6 +2801,22 @@ class Output_section::Input_section_sort
     return memcmp(base_name + base_len - 2, ".o", 2) == 0;
   }
 
+  // Returns 0 if sections are not comparable. Returns 1 if THIS is the
+  // first section in order, returns -1 for S.
+  int
+  compare_section_ordering(const Input_section_sort_entry& s) const
+  {
+    gold_assert(this->index_ != -1U);
+    if (this->input_section_.section_order_index() == 0
+        || s.input_section().section_order_index() == 0)
+      return 0;
+    if (this->input_section_.section_order_index()
+        < s.input_section().section_order_index())
+      return 1;
+    else
+      return -1;
+  }
+
  private:
   // The Input_section we are sorting.
   Input_section input_section_;
@@ -2843,6 +2878,12 @@ Output_section::Input_section_sort_compa
   if (!s1_has_priority && s2_has_priority)
     return true;
 
+  // Check if a section order exists for these sections through a section
+  // ordering file.  If sequence_num is 0, an order does not exist.
+  int sequence_num = s1.compare_section_ordering(s2);
+  if (sequence_num != 0)
+    return sequence_num == 1;
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2879,6 +2920,12 @@ Output_section::Input_section_sort_init_
   if (!s1_has_priority && s2_has_priority)
     return false;
 
+  // Check if a section order exists for these sections through a section
+  // ordering file.  If sequence_num is 0, an order does not exist.
+  int sequence_num = s1.compare_section_ordering(s2);
+  if (sequence_num != 0)
+    return sequence_num == 1;
+
   // Otherwise we sort by name.
   int compare = s1.section_name().compare(s2.section_name());
   if (compare != 0)
@@ -2888,6 +2935,22 @@ Output_section::Input_section_sort_init_
   return s1.index() < s2.index();
 }
 
+// Return true if S1 should come before S2.
+bool
+Output_section::Input_section_sort_section_order_index_compare::operator()(
+    const Output_section::Input_section_sort_entry& s1,
+    const Output_section::Input_section_sort_entry& s2) const
+{
+  // Check if a section order exists for these sections through a section
+  // ordering file.  If sequence_num is 0, an order does not exist.
+  int sequence_num = s1.compare_section_ordering(s2);
+  if (sequence_num != 0)
+    return sequence_num == 1;
+
+  // Otherwise we keep the input order.
+  return s1.index() < s2.index();
+}
+
 // Sort the input sections attached to an output section.
 
 void
@@ -2913,17 +2976,27 @@ Output_section::sort_attached_input_sect
   for (Input_section_list::iterator p = this->input_sections_.begin();
        p != this->input_sections_.end();
        ++p, ++i)
-    sort_list.push_back(Input_section_sort_entry(*p, i));
+      sort_list.push_back(Input_section_sort_entry(*p, i,
+                            this->must_sort_attached_input_sections()));
 
   // Sort the input sections.
-  if (this->type() == elfcpp::SHT_PREINIT_ARRAY
-      || this->type() == elfcpp::SHT_INIT_ARRAY
-      || this->type() == elfcpp::SHT_FINI_ARRAY)
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_init_fini_compare());
+  if (this->must_sort_attached_input_sections())
+    {
+      if (this->type() == elfcpp::SHT_PREINIT_ARRAY
+          || this->type() == elfcpp::SHT_INIT_ARRAY
+          || this->type() == elfcpp::SHT_FINI_ARRAY)
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_init_fini_compare());
+      else
+        std::sort(sort_list.begin(), sort_list.end(),
+	          Input_section_sort_compare());
+    }
   else
-    std::sort(sort_list.begin(), sort_list.end(),
-	      Input_section_sort_compare());
+    {
+      gold_assert(parameters->options().section_ordering_file());
+      std::sort(sort_list.begin(), sort_list.end(),
+	        Input_section_sort_section_order_index_compare());
+    }
 
   // Copy the sorted input sections back to our list.
   this->input_sections_.clear();
@@ -2931,6 +3004,7 @@ Output_section::sort_attached_input_sect
        p != sort_list.end();
        ++p)
     this->input_sections_.push_back(p->input_section());
+  sort_list.clear();
 
   // Remember that we sorted the input sections, since we might get
   // called again.
@@ -4466,6 +4540,7 @@ Output_file::close()
 template
 off_t
 Output_section::add_input_section<32, false>(
+    Layout* layout,
     Sized_relobj<32, false>* object,
     unsigned int shndx,
     const char* secname,
@@ -4478,6 +4553,7 @@ Output_section::add_input_section<32, fa
 template
 off_t
 Output_section::add_input_section<32, true>(
+    Layout* layout,
     Sized_relobj<32, true>* object,
     unsigned int shndx,
     const char* secname,
@@ -4490,6 +4566,7 @@ Output_section::add_input_section<32, tr
 template
 off_t
 Output_section::add_input_section<64, false>(
+    Layout* layout,
     Sized_relobj<64, false>* object,
     unsigned int shndx,
     const char* secname,
@@ -4502,6 +4579,7 @@ Output_section::add_input_section<64, fa
 template
 off_t
 Output_section::add_input_section<64, true>(
+    Layout* layout,
     Sized_relobj<64, true>* object,
     unsigned int shndx,
     const char* secname,
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.106
diff -u -u -p -r1.106 output.h
--- output.h	23 May 2010 07:43:39 -0000	1.106
+++ output.h	1 Jun 2010 23:11:08 -0000
@@ -2515,8 +2515,8 @@ class Output_section : public Output_dat
   // within the output section.
   template<int size, bool big_endian>
   off_t
-  add_input_section(Sized_relobj<size, big_endian>* object, unsigned int shndx,
-		    const char *name,
+  add_input_section(Layout* layout, Sized_relobj<size, big_endian>* object,
+                    unsigned int shndx, const char *name,
 		    const elfcpp::Shdr<size, big_endian>& shdr,
 		    unsigned int reloc_shndx, bool have_sections_script);
 
@@ -2736,6 +2736,18 @@ class Output_section : public Output_dat
   set_may_sort_attached_input_sections()
   { this->may_sort_attached_input_sections_ = true; }
 
+   // Returns true if input sections must be sorted according to the
+  // order in which their name appear in the --section-ordering-file.
+  bool
+  input_section_order_specified()
+  { return this->input_section_order_specified_; }
+
+  // Record that input sections must be sorted as some of their names
+  // match the patterns specified through --section-ordering-file.
+  void
+  set_input_section_order_specified()
+  { this->input_section_order_specified_ = true; }
+
   // Return whether the input sections attached to this output section
   // require sorting.  This is used to handle constructor priorities
   // compatibly with GNU ld.
@@ -2972,7 +2984,8 @@ class Output_section : public Output_dat
     Input_section(Relobj* object, unsigned int shndx, off_t data_size,
 		  uint64_t addralign)
       : shndx_(shndx),
-	p2align_(ffsll(static_cast<long long>(addralign)))
+	p2align_(ffsll(static_cast<long long>(addralign))),
+	section_order_index_(0)
     {
       gold_assert(shndx != OUTPUT_SECTION_CODE
 		  && shndx != MERGE_DATA_SECTION_CODE
@@ -2984,7 +2997,8 @@ class Output_section : public Output_dat
 
     // For a non-merge output section.
     Input_section(Output_section_data* posd)
-      : shndx_(OUTPUT_SECTION_CODE), p2align_(0)
+      : shndx_(OUTPUT_SECTION_CODE), p2align_(0),
+	section_order_index_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.posd = posd;
@@ -2995,7 +3009,8 @@ class Output_section : public Output_dat
       : shndx_(is_string
 	       ? MERGE_STRING_SECTION_CODE
 	       : MERGE_DATA_SECTION_CODE),
-	p2align_(0)
+	p2align_(0),
+	section_order_index_(0)
     {
       this->u1_.entsize = entsize;
       this->u2_.posd = posd;
@@ -3003,12 +3018,25 @@ class Output_section : public Output_dat
 
     // For a relaxed input section.
     Input_section(Output_relaxed_input_section *psection)
-      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0)
+      : shndx_(RELAXED_INPUT_SECTION_CODE), p2align_(0),
+	section_order_index_(0)
     {
       this->u1_.data_size = 0;
       this->u2_.poris = psection;
     }
 
+    unsigned int
+    section_order_index() const
+    {
+      return this->section_order_index_;
+    }
+
+    void
+    set_section_order_index(unsigned int number)
+    {
+      this->section_order_index_ = number;
+    }
+
     // The required alignment.
     uint64_t
     addralign() const
@@ -3234,6 +3262,9 @@ class Output_section : public Output_dat
       // For RELAXED_INPUT_SECTION_CODE, the data.
       Output_relaxed_input_section* poris;
     } u2_;
+    // The line number of the pattern it matches in the --section-ordering-file
+    // file.  It is 0 if does not match any pattern.
+    unsigned int section_order_index_;
   };
 
   // Store the list of input sections for this Output_section into the
@@ -3540,6 +3571,15 @@ class Output_section : public Output_dat
 	       const Input_section_sort_entry&) const;
   };
 
+  // This is the sort comparison function when a section order is specified
+  // from an input file.
+  struct Input_section_sort_section_order_index_compare
+  {
+    bool
+    operator()(const Input_section_sort_entry&,
+	       const Input_section_sort_entry&) const;
+  };
+
   // Fill data.  This is used to fill in data between input sections.
   // It is also used for data statements (BYTE, WORD, etc.) in linker
   // scripts.  When we have to keep track of the input sections, we
@@ -3707,6 +3747,9 @@ class Output_section : public Output_dat
   // section, false if it means the symbol index of the corresponding
   // section symbol.
   bool info_uses_section_index_ : 1;
+  // True if input sections attached to this output section have to be
+  // sorted according to a specified order.
+  bool input_section_order_specified_ : 1;
   // True if the input sections attached to this output section may
   // need sorting.
   bool may_sort_attached_input_sections_ : 1;
Index: script-sections.cc
===================================================================
RCS file: /cvs/src/src/gold/script-sections.cc,v
retrieving revision 1.36
diff -u -u -p -r1.36 script-sections.cc
--- script-sections.cc	26 May 2010 15:15:05 -0000	1.36
+++ script-sections.cc	1 Jun 2010 23:11:08 -0000
@@ -983,15 +983,6 @@ class Output_section_element_fill : publ
   Expression* val_;
 };
 
-// Return whether STRING contains a wildcard character.  This is used
-// to speed up matching.
-
-static inline bool
-is_wildcard_string(const std::string& s)
-{
-  return strpbrk(s.c_str(), "?*[") != NULL;
-}
-
 // An input section specification in an output section
 
 class Output_section_element_input : public Output_section_element
@@ -1035,7 +1026,7 @@ class Output_section_element_input : pub
     Input_section_pattern(const char* patterna, size_t patternlena,
 			  Sort_wildcard sorta)
       : pattern(patterna, patternlena),
-	pattern_is_wildcard(is_wildcard_string(this->pattern)),
+	pattern_is_wildcard(is_wildcard_string(this->pattern.c_str())),
 	sort(sorta)
     { }
   };
@@ -1102,7 +1093,7 @@ Output_section_element_input::Output_sec
   if (spec->file.name.length != 1 || spec->file.name.value[0] != '*')
     this->filename_pattern_.assign(spec->file.name.value,
 				   spec->file.name.length);
-  this->filename_is_wildcard_ = is_wildcard_string(this->filename_pattern_);
+  this->filename_is_wildcard_ = is_wildcard_string(this->filename_pattern_.c_str());
 
   if (spec->input_sections.exclude != NULL)
     {
@@ -1111,7 +1102,7 @@ Output_section_element_input::Output_sec
 	   p != spec->input_sections.exclude->end();
 	   ++p)
 	{
-	  bool is_wildcard = is_wildcard_string(*p);
+	  bool is_wildcard = is_wildcard_string((*p).c_str());
 	  this->filename_exclusions_.push_back(std::make_pair(*p,
 							      is_wildcard));
 	}
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.135
diff -u -u -p -r1.135 Makefile.am
--- testsuite/Makefile.am	26 May 2010 15:47:39 -0000	1.135
+++ testsuite/Makefile.am	1 Jun 2010 23:11:08 -0000
@@ -193,6 +193,18 @@ icf_safe_so_test_1.stdout: icf_safe_so_t
 icf_safe_so_test_2.stdout: icf_safe_so_test
 	$(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
 
+check_SCRIPTS += final_layout.sh
+check_DATA += final_layout.stdout
+MOSTLYCLEANFILES += final_layout
+final_layout.o: final_layout.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections  -fdata-sections -g -o $@ $<
+final_layout_sequence.txt:
+	(echo "*_Z3barv*" && echo ".text._Z3bazv" && echo "*_Z3foov*" && echo "*global_varb*" && echo "*global_vara*" && echo "*global_varc*") > final_layout_sequence.txt
+final_layout: final_layout.o final_layout_sequence.txt gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,--section-ordering-file,final_layout_sequence.txt final_layout.o
+final_layout.stdout: final_layout
+	$(TEST_NM) final_layout > final_layout.stdout
+
 check_PROGRAMS += icf_virtual_function_folding_test
 MOSTLYCLEANFILES += icf_virtual_function_folding_test
 icf_virtual_function_folding_test.o: icf_virtual_function_folding_test.cc
Index: testsuite/final_layout.cc
===================================================================
RCS file: testsuite/final_layout.cc
diff -N testsuite/final_layout.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.cc	1 Jun 2010 23:11:08 -0000
@@ -0,0 +1,48 @@
+// final_layout.cc -- a test case for gold
+
+// Copyright 2010 Free Software Foundation, Inc.
+// Written by Sriraman Tallam <tmsriram@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify if --section-ordering-file orders
+// the .text and .data sections correctly as specified.
+
+int global_vara;
+int global_varb;
+int global_varc;
+
+int foo()
+{
+  return 1;
+}
+
+int bar()
+{
+  return 1;
+}
+
+int baz()
+{
+  return 1;
+}
+
+int main()
+{
+  return 1;
+}
Index: testsuite/final_layout.sh
===================================================================
RCS file: testsuite/final_layout.sh
diff -N testsuite/final_layout.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/final_layout.sh	1 Jun 2010 23:11:08 -0000
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+# final_layout.sh -- test --final-layout
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Sriraman Tallam <tmsriram@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# The goal of this program is to verify if --section-ordering-file works as
+# intended.  File final_layout.cc is in this test.
+
+check()
+{
+    func_addr_1=$((16#`grep $2 $1 | awk '{print $1}'`))
+    func_addr_2=$((16#`grep $3 $1 | awk '{print $1}'`))
+    if [ $func_addr_1 -gt $func_addr_2 ]
+    then
+        echo "final layout of" $2 "and" $3 "is not right."
+	exit 1
+    fi
+}
+
+check final_layout.stdout "_Z3barv" "_Z3bazv"
+check final_layout.stdout "_Z3bazv" "_Z3foov"
+check final_layout.stdout "global_varb" "global_vara"
+check final_layout.stdout "global_vara" "global_varc"

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-01 23:41               ` Sriraman Tallam
@ 2010-06-02 17:17                 ` H.J. Lu
  2010-06-02 21:04                   ` Sriraman Tallam
  0 siblings, 1 reply; 24+ messages in thread
From: H.J. Lu @ 2010-06-02 17:17 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: Ian Lance Taylor, Taras Glek, binutils

On Tue, Jun 1, 2010 at 4:40 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Hi Ian,
>
>     I committed the patch after making the changes you mentioned. I
> did not add a warning when using the SORT clause and
> --section-ordering file. Like you said, I will do this as a separate
> patch after I figure out how to do this effectively.
>
> 2010-06-01  Sriraman Tallam  <tmsriram@google.com>
>
>        * gold.h (is_wildcard_string): New function.
>        * layout.cc (Layout::layout): Pass this pointer to add_input_section.
>        (Layout::layout_eh_frame): Ditto.
>        (Layout::find_section_order_index): New method.
>        (Layout::read_layout_from_file): New method.
>        * layout.h (Layout::find_section_order_index): New method.
>        (Layout::read_layout_from_file): New method.
>        (Layout::input_section_position_): New private member.
>        (Layout::input_section_glob_): New private member.
>        * main.cc (main): Call read_layout_from_file here.
>        * options.h (--section-ordering-file): New option.
>        * output.cc (Output_section::input_section_order_specified_): New
>        member.
>        (Output_section::Output_section): Initialize new member.
>        (Output_section::add_input_section): Add new parameter.
>        Keep input sections when --section-ordering-file is used.
>        (Output_section::set_final_data_size): Sort input sections when
>        section ordering file is specified.
>        (Output_section::Input_section_sort_entry): Add new parameter.
>        Check sorting type.
>        (Output_section::Input_section_sort_entry::compare_section_ordering):
>        New method.
>        (Output_section::Input_section_sort_compare::operator()): Change to
>        consider section_order_index.
>        (Output_section::Input_section_sort_init_fini_compare::operator()):
>        Change to consider section_order_index.
>        (Output_section::Input_section_sort_section_order_index_compare
>        ::operator()): New method.
>        (Output_section::sort_attached_input_sections): Change to sort
>        according to section order when specified.
>        (Output_section::add_input_section<32, true>): Add new parameter.
>        (Output_section::add_input_section<64, true>): Add new parameter.
>        (Output_section::add_input_section<32, false>): Add new parameter.
>        (Output_section::add_input_section<64, false>): Add new parameter.
>        * output.h (Output_section::add_input_section): Add new parameter.
>        (Output_section::input_section_order_specified): New
>        method.
>        (Output_section::set_input_section_order_specified): New method.
>        (Input_section::Input_section): Initialize section_order_index_.
>        (Input_section::section_order_index): New method.
>        (Input_section::set_section_order_index): New method.
>        (Input_section::section_order_index_): New member.
>        (Input_section::Input_section_sort_section_order_index_compare): New
>        struct.
>        (Output_section::input_section_order_specified_): New member.
>        * script-sections.cc (is_wildcard_string): Delete and move modified
>        method to gold.h.
>        (Output_section_element_input::Output_section_element_input): Modify
>        call to is_wildcard_string.
>        (Output_section_element_input::Input_section_pattern
>        ::Input_section_pattern): Ditto.
>        (Output_section_element_input::Output_section_element_input): Ditto.
>        * testsuite/Makefile.am (final_layout): New test case.
>        * testsuite/Makefile.in: Regenerate.
>        * testsuite/final_layout.cc: New file.
>        * testsuite/final_layout.sh: New file.
>
>

testsuite/final_layout.sh failed on Linux/x86-64:

http://www.sourceware.org/bugzilla/show_bug.cgi?id=11658

-- 
H.J.

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-02 17:17                 ` H.J. Lu
@ 2010-06-02 21:04                   ` Sriraman Tallam
  2010-06-03  6:51                     ` Sriraman Tallam
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-06-02 21:04 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Ian Lance Taylor, Taras Glek, binutils

The bug seems to be with the sort compare function. It tries to order
input sections based on the section ordering specified. If there are
some input sections with no ordering, then it tries to preserve the
original order. The way it is designed currently is causing conflicts.
I will prepare a patch to fix this.

Thanks,
-Sri.

On Wed, Jun 2, 2010 at 10:16 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jun 1, 2010 at 4:40 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> Hi Ian,
>>
>>     I committed the patch after making the changes you mentioned. I
>> did not add a warning when using the SORT clause and
>> --section-ordering file. Like you said, I will do this as a separate
>> patch after I figure out how to do this effectively.
>>
>> 2010-06-01  Sriraman Tallam  <tmsriram@google.com>
>>
>>        * gold.h (is_wildcard_string): New function.
>>        * layout.cc (Layout::layout): Pass this pointer to add_input_section.
>>        (Layout::layout_eh_frame): Ditto.
>>        (Layout::find_section_order_index): New method.
>>        (Layout::read_layout_from_file): New method.
>>        * layout.h (Layout::find_section_order_index): New method.
>>        (Layout::read_layout_from_file): New method.
>>        (Layout::input_section_position_): New private member.
>>        (Layout::input_section_glob_): New private member.
>>        * main.cc (main): Call read_layout_from_file here.
>>        * options.h (--section-ordering-file): New option.
>>        * output.cc (Output_section::input_section_order_specified_): New
>>        member.
>>        (Output_section::Output_section): Initialize new member.
>>        (Output_section::add_input_section): Add new parameter.
>>        Keep input sections when --section-ordering-file is used.
>>        (Output_section::set_final_data_size): Sort input sections when
>>        section ordering file is specified.
>>        (Output_section::Input_section_sort_entry): Add new parameter.
>>        Check sorting type.
>>        (Output_section::Input_section_sort_entry::compare_section_ordering):
>>        New method.
>>        (Output_section::Input_section_sort_compare::operator()): Change to
>>        consider section_order_index.
>>        (Output_section::Input_section_sort_init_fini_compare::operator()):
>>        Change to consider section_order_index.
>>        (Output_section::Input_section_sort_section_order_index_compare
>>        ::operator()): New method.
>>        (Output_section::sort_attached_input_sections): Change to sort
>>        according to section order when specified.
>>        (Output_section::add_input_section<32, true>): Add new parameter.
>>        (Output_section::add_input_section<64, true>): Add new parameter.
>>        (Output_section::add_input_section<32, false>): Add new parameter.
>>        (Output_section::add_input_section<64, false>): Add new parameter.
>>        * output.h (Output_section::add_input_section): Add new parameter.
>>        (Output_section::input_section_order_specified): New
>>        method.
>>        (Output_section::set_input_section_order_specified): New method.
>>        (Input_section::Input_section): Initialize section_order_index_.
>>        (Input_section::section_order_index): New method.
>>        (Input_section::set_section_order_index): New method.
>>        (Input_section::section_order_index_): New member.
>>        (Input_section::Input_section_sort_section_order_index_compare): New
>>        struct.
>>        (Output_section::input_section_order_specified_): New member.
>>        * script-sections.cc (is_wildcard_string): Delete and move modified
>>        method to gold.h.
>>        (Output_section_element_input::Output_section_element_input): Modify
>>        call to is_wildcard_string.
>>        (Output_section_element_input::Input_section_pattern
>>        ::Input_section_pattern): Ditto.
>>        (Output_section_element_input::Output_section_element_input): Ditto.
>>        * testsuite/Makefile.am (final_layout): New test case.
>>        * testsuite/Makefile.in: Regenerate.
>>        * testsuite/final_layout.cc: New file.
>>        * testsuite/final_layout.sh: New file.
>>
>>
>
> testsuite/final_layout.sh failed on Linux/x86-64:
>
> http://www.sourceware.org/bugzilla/show_bug.cgi?id=11658
>
> --
> H.J.
>

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-02 21:04                   ` Sriraman Tallam
@ 2010-06-03  6:51                     ` Sriraman Tallam
  2010-06-03 14:04                       ` H.J. Lu
  2010-06-03 15:56                       ` Ian Lance Taylor
  0 siblings, 2 replies; 24+ messages in thread
From: Sriraman Tallam @ 2010-06-03  6:51 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Ian Lance Taylor, Taras Glek, binutils

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

Hi,

    I have attached the patch to fix the bug in section ordering.
Sections which do not match any pattern in the file will be laid out
ahead of sections that match some pattern to remove the ambiguity that
was causing the problem. Please let me know if this is ok.

	* output.cc
	(Output_section::Input_section_sort_entry::compare_section_ordering):
	Change to return non-zero correctly.
	(Output_section::Input_section_sort_section_order_index_compare::operator()):
        Change to fix ambiguity in comparisons.

Thanks,
-Sri.

On Wed, Jun 2, 2010 at 2:04 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> The bug seems to be with the sort compare function. It tries to order
> input sections based on the section ordering specified. If there are
> some input sections with no ordering, then it tries to preserve the
> original order. The way it is designed currently is causing conflicts.
> I will prepare a patch to fix this.
>
> Thanks,
> -Sri.
>
> On Wed, Jun 2, 2010 at 10:16 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Jun 1, 2010 at 4:40 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> Hi Ian,
>>>
>>>     I committed the patch after making the changes you mentioned. I
>>> did not add a warning when using the SORT clause and
>>> --section-ordering file. Like you said, I will do this as a separate
>>> patch after I figure out how to do this effectively.
>>>
>>> 2010-06-01  Sriraman Tallam  <tmsriram@google.com>
>>>
>>>        * gold.h (is_wildcard_string): New function.
>>>        * layout.cc (Layout::layout): Pass this pointer to add_input_section.
>>>        (Layout::layout_eh_frame): Ditto.
>>>        (Layout::find_section_order_index): New method.
>>>        (Layout::read_layout_from_file): New method.
>>>        * layout.h (Layout::find_section_order_index): New method.
>>>        (Layout::read_layout_from_file): New method.
>>>        (Layout::input_section_position_): New private member.
>>>        (Layout::input_section_glob_): New private member.
>>>        * main.cc (main): Call read_layout_from_file here.
>>>        * options.h (--section-ordering-file): New option.
>>>        * output.cc (Output_section::input_section_order_specified_): New
>>>        member.
>>>        (Output_section::Output_section): Initialize new member.
>>>        (Output_section::add_input_section): Add new parameter.
>>>        Keep input sections when --section-ordering-file is used.
>>>        (Output_section::set_final_data_size): Sort input sections when
>>>        section ordering file is specified.
>>>        (Output_section::Input_section_sort_entry): Add new parameter.
>>>        Check sorting type.
>>>        (Output_section::Input_section_sort_entry::compare_section_ordering):
>>>        New method.
>>>        (Output_section::Input_section_sort_compare::operator()): Change to
>>>        consider section_order_index.
>>>        (Output_section::Input_section_sort_init_fini_compare::operator()):
>>>        Change to consider section_order_index.
>>>        (Output_section::Input_section_sort_section_order_index_compare
>>>        ::operator()): New method.
>>>        (Output_section::sort_attached_input_sections): Change to sort
>>>        according to section order when specified.
>>>        (Output_section::add_input_section<32, true>): Add new parameter.
>>>        (Output_section::add_input_section<64, true>): Add new parameter.
>>>        (Output_section::add_input_section<32, false>): Add new parameter.
>>>        (Output_section::add_input_section<64, false>): Add new parameter.
>>>        * output.h (Output_section::add_input_section): Add new parameter.
>>>        (Output_section::input_section_order_specified): New
>>>        method.
>>>        (Output_section::set_input_section_order_specified): New method.
>>>        (Input_section::Input_section): Initialize section_order_index_.
>>>        (Input_section::section_order_index): New method.
>>>        (Input_section::set_section_order_index): New method.
>>>        (Input_section::section_order_index_): New member.
>>>        (Input_section::Input_section_sort_section_order_index_compare): New
>>>        struct.
>>>        (Output_section::input_section_order_specified_): New member.
>>>        * script-sections.cc (is_wildcard_string): Delete and move modified
>>>        method to gold.h.
>>>        (Output_section_element_input::Output_section_element_input): Modify
>>>        call to is_wildcard_string.
>>>        (Output_section_element_input::Input_section_pattern
>>>        ::Input_section_pattern): Ditto.
>>>        (Output_section_element_input::Output_section_element_input): Ditto.
>>>        * testsuite/Makefile.am (final_layout): New test case.
>>>        * testsuite/Makefile.in: Regenerate.
>>>        * testsuite/final_layout.cc: New file.
>>>        * testsuite/final_layout.sh: New file.
>>>
>>>
>>
>> testsuite/final_layout.sh failed on Linux/x86-64:
>>
>> http://www.sourceware.org/bugzilla/show_bug.cgi?id=11658
>>
>> --
>> H.J.
>>
>

[-- Attachment #2: section_ordering_bug_patch.txt --]
[-- Type: text/plain, Size: 2660 bytes --]

Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.127
diff -u -u -p -r1.127 output.cc
--- output.cc	1 Jun 2010 23:37:57 -0000	1.127
+++ output.cc	3 Jun 2010 06:17:16 -0000
@@ -2801,20 +2801,21 @@ class Output_section::Input_section_sort
     return memcmp(base_name + base_len - 2, ".o", 2) == 0;
   }
 
-  // Returns 0 if sections are not comparable. Returns 1 if THIS is the
-  // first section in order, returns -1 for S.
+  // Returns 1 if THIS should appear before S in section order, -1 if S
+  // appears before THIS and 0 if they are not comparable.
   int
   compare_section_ordering(const Input_section_sort_entry& s) const
   {
-    gold_assert(this->index_ != -1U);
-    if (this->input_section_.section_order_index() == 0
-        || s.input_section().section_order_index() == 0)
-      return 0;
-    if (this->input_section_.section_order_index()
-        < s.input_section().section_order_index())
-      return 1;
-    else
-      return -1;
+    unsigned int this_secn_index = this->input_section_.section_order_index();
+    unsigned int s_secn_index = s.input_section().section_order_index();
+    if (this_secn_index > 0 && s_secn_index > 0)
+      {
+        if (this_secn_index < s_secn_index)
+          return 1;
+        else if (this_secn_index > s_secn_index)
+          return -1;
+      }
+    return 0;
   }
 
  private:
@@ -2935,20 +2936,23 @@ Output_section::Input_section_sort_init_
   return s1.index() < s2.index();
 }
 
-// Return true if S1 should come before S2.
+// Return true if S1 should come before S2.  Sections that do not match
+// any pattern in the section ordering file are placed ahead of the sections
+// that match some pattern.
+
 bool
 Output_section::Input_section_sort_section_order_index_compare::operator()(
     const Output_section::Input_section_sort_entry& s1,
     const Output_section::Input_section_sort_entry& s2) const
 {
-  // Check if a section order exists for these sections through a section
-  // ordering file.  If sequence_num is 0, an order does not exist.
-  int sequence_num = s1.compare_section_ordering(s2);
-  if (sequence_num != 0)
-    return sequence_num == 1;
+  unsigned int s1_secn_index = s1.input_section().section_order_index();
+  unsigned int s2_secn_index = s2.input_section().section_order_index();
 
-  // Otherwise we keep the input order.
-  return s1.index() < s2.index();
+  // Keep input order if section ordering cannot determine order.
+  if (s1_secn_index == s2_secn_index)
+    return s1.index() < s2.index();
+  
+  return s1_secn_index < s2_secn_index;
 }

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-03  6:51                     ` Sriraman Tallam
@ 2010-06-03 14:04                       ` H.J. Lu
  2010-06-03 15:56                       ` Ian Lance Taylor
  1 sibling, 0 replies; 24+ messages in thread
From: H.J. Lu @ 2010-06-03 14:04 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: Ian Lance Taylor, Taras Glek, binutils

On Wed, Jun 2, 2010 at 11:50 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Hi,
>
>    I have attached the patch to fix the bug in section ordering.
> Sections which do not match any pattern in the file will be laid out
> ahead of sections that match some pattern to remove the ambiguity that
> was causing the problem. Please let me know if this is ok.
>

Please add "PR gold/11658" in ChangeLog entry.

>        * output.cc
>        (Output_section::Input_section_sort_entry::compare_section_ordering):
>        Change to return non-zero correctly.
>        (Output_section::Input_section_sort_section_order_index_compare::operator()):
>        Change to fix ambiguity in comparisons.

Thanks.


H.J.
---
> Thanks,
> -Sri.
>
> On Wed, Jun 2, 2010 at 2:04 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> The bug seems to be with the sort compare function. It tries to order
>> input sections based on the section ordering specified. If there are
>> some input sections with no ordering, then it tries to preserve the
>> original order. The way it is designed currently is causing conflicts.
>> I will prepare a patch to fix this.
>>
>> Thanks,
>> -Sri.
>>
>> On Wed, Jun 2, 2010 at 10:16 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Jun 1, 2010 at 4:40 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>> Hi Ian,
>>>>
>>>>     I committed the patch after making the changes you mentioned. I
>>>> did not add a warning when using the SORT clause and
>>>> --section-ordering file. Like you said, I will do this as a separate
>>>> patch after I figure out how to do this effectively.
>>>>
>>>> 2010-06-01  Sriraman Tallam  <tmsriram@google.com>
>>>>
>>>>        * gold.h (is_wildcard_string): New function.
>>>>        * layout.cc (Layout::layout): Pass this pointer to add_input_section.
>>>>        (Layout::layout_eh_frame): Ditto.
>>>>        (Layout::find_section_order_index): New method.
>>>>        (Layout::read_layout_from_file): New method.
>>>>        * layout.h (Layout::find_section_order_index): New method.
>>>>        (Layout::read_layout_from_file): New method.
>>>>        (Layout::input_section_position_): New private member.
>>>>        (Layout::input_section_glob_): New private member.
>>>>        * main.cc (main): Call read_layout_from_file here.
>>>>        * options.h (--section-ordering-file): New option.
>>>>        * output.cc (Output_section::input_section_order_specified_): New
>>>>        member.
>>>>        (Output_section::Output_section): Initialize new member.
>>>>        (Output_section::add_input_section): Add new parameter.
>>>>        Keep input sections when --section-ordering-file is used.
>>>>        (Output_section::set_final_data_size): Sort input sections when
>>>>        section ordering file is specified.
>>>>        (Output_section::Input_section_sort_entry): Add new parameter.
>>>>        Check sorting type.
>>>>        (Output_section::Input_section_sort_entry::compare_section_ordering):
>>>>        New method.
>>>>        (Output_section::Input_section_sort_compare::operator()): Change to
>>>>        consider section_order_index.
>>>>        (Output_section::Input_section_sort_init_fini_compare::operator()):
>>>>        Change to consider section_order_index.
>>>>        (Output_section::Input_section_sort_section_order_index_compare
>>>>        ::operator()): New method.
>>>>        (Output_section::sort_attached_input_sections): Change to sort
>>>>        according to section order when specified.
>>>>        (Output_section::add_input_section<32, true>): Add new parameter.
>>>>        (Output_section::add_input_section<64, true>): Add new parameter.
>>>>        (Output_section::add_input_section<32, false>): Add new parameter.
>>>>        (Output_section::add_input_section<64, false>): Add new parameter.
>>>>        * output.h (Output_section::add_input_section): Add new parameter.
>>>>        (Output_section::input_section_order_specified): New
>>>>        method.
>>>>        (Output_section::set_input_section_order_specified): New method.
>>>>        (Input_section::Input_section): Initialize section_order_index_.
>>>>        (Input_section::section_order_index): New method.
>>>>        (Input_section::set_section_order_index): New method.
>>>>        (Input_section::section_order_index_): New member.
>>>>        (Input_section::Input_section_sort_section_order_index_compare): New
>>>>        struct.
>>>>        (Output_section::input_section_order_specified_): New member.
>>>>        * script-sections.cc (is_wildcard_string): Delete and move modified
>>>>        method to gold.h.
>>>>        (Output_section_element_input::Output_section_element_input): Modify
>>>>        call to is_wildcard_string.
>>>>        (Output_section_element_input::Input_section_pattern
>>>>        ::Input_section_pattern): Ditto.
>>>>        (Output_section_element_input::Output_section_element_input): Ditto.
>>>>        * testsuite/Makefile.am (final_layout): New test case.
>>>>        * testsuite/Makefile.in: Regenerate.
>>>>        * testsuite/final_layout.cc: New file.
>>>>        * testsuite/final_layout.sh: New file.
>>>>
>>>>
>>>
>>> testsuite/final_layout.sh failed on Linux/x86-64:
>>>
>>> http://www.sourceware.org/bugzilla/show_bug.cgi?id=11658
>>>
>>> --
>>> H.J.
>>>
>>
>



-- 
H.J.

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-03  6:51                     ` Sriraman Tallam
  2010-06-03 14:04                       ` H.J. Lu
@ 2010-06-03 15:56                       ` Ian Lance Taylor
  2010-06-03 18:02                         ` Sriraman Tallam
  1 sibling, 1 reply; 24+ messages in thread
From: Ian Lance Taylor @ 2010-06-03 15:56 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: H.J. Lu, Taras Glek, binutils

Sriraman Tallam <tmsriram@google.com> writes:

> 	* output.cc
> 	(Output_section::Input_section_sort_entry::compare_section_ordering):
> 	Change to return non-zero correctly.
> 	(Output_section::Input_section_sort_section_order_index_compare::operator()):
>         Change to fix ambiguity in comparisons.

This is OK.

Thanks.

Ian

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-03 15:56                       ` Ian Lance Taylor
@ 2010-06-03 18:02                         ` Sriraman Tallam
  2010-06-30 22:31                           ` Taras Glek
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-06-03 18:02 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: H.J. Lu, Taras Glek, binutils

Thanks. I committed the patch.

2010-06-03  Sriraman Tallam  <tmsriram@google.com>

	PR gold/11658
	* output.cc
	(Output_section::Input_section_sort_entry::compare_section_ordering):
	Change to return non-zero correctly.
	(Output_section::Input_section_sort_section_order_index_compare
	::operator()): Change to fix ambiguity in comparisons.

On Thu, Jun 3, 2010 at 8:55 AM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>>       * output.cc
>>       (Output_section::Input_section_sort_entry::compare_section_ordering):
>>       Change to return non-zero correctly.
>>       (Output_section::Input_section_sort_section_order_index_compare::operator()):
>>         Change to fix ambiguity in comparisons.
>
> This is OK.
>
> Thanks.
>
> Ian
>

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-03 18:02                         ` Sriraman Tallam
@ 2010-06-30 22:31                           ` Taras Glek
  2010-06-30 23:00                             ` Sriraman Tallam
  0 siblings, 1 reply; 24+ messages in thread
From: Taras Glek @ 2010-06-30 22:31 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: Ian Lance Taylor, H.J. Lu, binutils

Hi,
I noticed weird "** fill"ing behavior while looking at mapfiles. Ian 
says this is a bug, so I'm sending this to the list.

When compiling with -ffunction-sections, many sections usually have some 
fill after them. I'm not sure why that's needed..but a more interesting 
problem is that once you order those sections, the remaining fill gets 
lumped together like this:

.rel.dyn        0x0007c21c   0x18eaf0
  ** dynamic relocs
                 0x0007c21c   0x18eaf0

.rel.plt        0x0020ad0c     0xb0d0
  ** dynamic relocs
                 0x0020ad0c     0xb0d0

.text           0x00215de0   0xb35709
  ** fill        0x00215de0        0xa
  ** fill        0x00215dea        0x1
  ** fill        0x00215deb        0x1
  ** fill        0x00215dec        0x2
  ** fill        0x00215dee        0x1
  ** fill        0x00215def        0x1
  ** fill        0x00215df0        0x1
  ** fill        0x00215df1        0x1
  ** fill        0x00215df2        0x1
  ** fill        0x00215df3        0x1
  ** fill        0x00215df4        0x1
  ** fill        0x00215df5        0x1
  ** fill        0x00215df6        0x2
  ** fill        0x00215df8        0x1
  ** fill        0x00215df9        0x1
  ** fill        0x00215dfa        0x1
  ** fill        0x00215dfb        0x1
  ** fill        0x00215dfc        0x1
  ** fill        0x00215dfd        0x1
  ** fill        0x00215dfe        0x1
  ** fill        0x00215dff        0x1

This keeps going, in my binary I ended up with 50K of "** fill". Before 
ordering the fill found among sections like so:
  .text._Z11IsSameCCMapPtS_
                 0x002163f6       0x43 nsCompressedCharMap.o
                 0x002163f6                _Z11IsSameCCMapPtS_
  ** fill        0x00216439        0x1
  .text._ZN19nsCompressedCharMap9FillCCMapEPt
                 0x0021643a       0x23 nsCompressedCharMap.o
                 0x0021643a                
_ZN19nsCompressedCharMap9FillCCMapEPt
  ** fill        0x0021645d        0x1
  .text._ZN19nsCompressedCharMapD2Ev
                 0x0021645e       0x41 nsCompressedCharMap.o
                 0x0021645e                _ZN19nsCompressedCharMapD2Ev
                 0x0021645e                _ZN19nsCompressedCharMapD1Ev
  ** fill        0x0021649f        0x1
  .text._ZN19nsCompressedCharMapC2Ev
                 0x002164a0       0x7e nsCompressedCharMap.o
                 0x002164a0                _ZN19nsCompressedCharMapC2Ev
                 0x002164a0                _ZN19nsCompressedCharMapC1Ev
  .text._ZN19nsCompressedCharMap7SetCharEj
                 0x0021651e      0x137 nsCompressedCharMap.o
                 0x0021651e                
_ZN19nsCompressedCharMap7SetCharEj
  ** fill        0x00216655        0x1
  .text._ZN19nsCompressedCharMap8SetCharsEtPj
                 0x00216656      0x121 nsCompressedCharMap.o
                 0x00216656                
_ZN19nsCompressedCharMap8SetCharsEtPj
  ** fill        0x00216777        0x1
  .text._ZN19nsCompressedCharMap8SetCharsEPt
                 0x00216778      0x175 nsCompressedCharMap.o
                 0x00216778                
_ZN19nsCompressedCharMap8SetCharsEPt
  ** fill        0x002168ed        0x1
  .text._ZN19nsCompressedCharMap8SetCharsEPj
                 0x002168ee       0x2d nsCompressedCharMap.o
                 0x002168ee                
_ZN19nsCompressedCharMap8SetCharsEPj
  .text._Z10MapToCCMapPj
                 0x0021691b       0x84 nsCompressedCharMap.o
                 0x0021691b                _Z10MapToCCMapPj
  .text._Z13MapperToCCMapP20nsICharRepresentable
                 0x0021699f       0x45 nsCompressedCharMap.o
                 0x0021699f                
_Z13MapperToCCMapP20nsICharRepresentable
  .text._Z13MapToCCMapExtPjPS_j
                 0x002169e4      0x1f5 nsCompressedCharMap.o
                 0x002169e4                _Z13MapToCCMapExtPjPS_j
  ** fill        0x00216bd9        0x1
  .text._ZN19nsCompressedCharMap8NewCCMapEv
                 0x00216bda       0x75 nsCompressedCharMap.o
                 0x00216bda                
_ZN19nsCompressedCharMap8NewCCMapEv
  ** fill        0x00216c4f        0x1


On 06/03/2010 11:01 AM, Sriraman Tallam wrote:
> Thanks. I committed the patch.
>
> 2010-06-03  Sriraman Tallam<tmsriram@google.com>
>
> 	PR gold/11658
> 	* output.cc
> 	(Output_section::Input_section_sort_entry::compare_section_ordering):
> 	Change to return non-zero correctly.
> 	(Output_section::Input_section_sort_section_order_index_compare
> 	::operator()): Change to fix ambiguity in comparisons.
>
> On Thu, Jun 3, 2010 at 8:55 AM, Ian Lance Taylor<iant@google.com>  wrote:
>    
>> Sriraman Tallam<tmsriram@google.com>  writes:
>>
>>      
>>>        * output.cc
>>>        (Output_section::Input_section_sort_entry::compare_section_ordering):
>>>        Change to return non-zero correctly.
>>>        (Output_section::Input_section_sort_section_order_index_compare::operator()):
>>>          Change to fix ambiguity in comparisons.
>>>        
>> This is OK.
>>
>> Thanks.
>>
>> Ian
>>
>>      
>    

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-30 22:31                           ` Taras Glek
@ 2010-06-30 23:00                             ` Sriraman Tallam
  2010-06-30 23:09                               ` Taras Glek
  0 siblings, 1 reply; 24+ messages in thread
From: Sriraman Tallam @ 2010-06-30 23:00 UTC (permalink / raw)
  To: Taras Glek; +Cc: Ian Lance Taylor, H.J. Lu, binutils

On Wed, Jun 30, 2010 at 3:31 PM, Taras Glek <tglek@mozilla.com> wrote:
> Hi,
> I noticed weird "** fill"ing behavior while looking at mapfiles. Ian says
> this is a bug, so I'm sending this to the list.
>
> When compiling with -ffunction-sections, many sections usually have some
> fill after them. I'm not sure why that's needed..but a more interesting
> problem is that once you order those sections, the remaining fill gets
> lumped together like this:

Does this lumping happen when you re-order with linker scripts or with
--section-ordering-file, or both ? Could this fill be for alignment
purposes ?

>
> .rel.dyn        0x0007c21c   0x18eaf0
>  ** dynamic relocs
>                0x0007c21c   0x18eaf0
>
> .rel.plt        0x0020ad0c     0xb0d0
>  ** dynamic relocs
>                0x0020ad0c     0xb0d0
>
> .text           0x00215de0   0xb35709
>  ** fill        0x00215de0        0xa
>  ** fill        0x00215dea        0x1
>  ** fill        0x00215deb        0x1
>  ** fill        0x00215dec        0x2
>  ** fill        0x00215dee        0x1
>  ** fill        0x00215def        0x1
>  ** fill        0x00215df0        0x1
>  ** fill        0x00215df1        0x1
>  ** fill        0x00215df2        0x1
>  ** fill        0x00215df3        0x1
>  ** fill        0x00215df4        0x1
>  ** fill        0x00215df5        0x1
>  ** fill        0x00215df6        0x2
>  ** fill        0x00215df8        0x1
>  ** fill        0x00215df9        0x1
>  ** fill        0x00215dfa        0x1
>  ** fill        0x00215dfb        0x1
>  ** fill        0x00215dfc        0x1
>  ** fill        0x00215dfd        0x1
>  ** fill        0x00215dfe        0x1
>  ** fill        0x00215dff        0x1
>
> This keeps going, in my binary I ended up with 50K of "** fill". Before
> ordering the fill found among sections like so:
>  .text._Z11IsSameCCMapPtS_
>                0x002163f6       0x43 nsCompressedCharMap.o
>                0x002163f6                _Z11IsSameCCMapPtS_
>  ** fill        0x00216439        0x1
>  .text._ZN19nsCompressedCharMap9FillCCMapEPt
>                0x0021643a       0x23 nsCompressedCharMap.o
>                0x0021643a
>  _ZN19nsCompressedCharMap9FillCCMapEPt
>  ** fill        0x0021645d        0x1
>  .text._ZN19nsCompressedCharMapD2Ev
>                0x0021645e       0x41 nsCompressedCharMap.o
>                0x0021645e                _ZN19nsCompressedCharMapD2Ev
>                0x0021645e                _ZN19nsCompressedCharMapD1Ev
>  ** fill        0x0021649f        0x1
>  .text._ZN19nsCompressedCharMapC2Ev
>                0x002164a0       0x7e nsCompressedCharMap.o
>                0x002164a0                _ZN19nsCompressedCharMapC2Ev
>                0x002164a0                _ZN19nsCompressedCharMapC1Ev
>  .text._ZN19nsCompressedCharMap7SetCharEj
>                0x0021651e      0x137 nsCompressedCharMap.o
>                0x0021651e                _ZN19nsCompressedCharMap7SetCharEj
>  ** fill        0x00216655        0x1
>  .text._ZN19nsCompressedCharMap8SetCharsEtPj
>                0x00216656      0x121 nsCompressedCharMap.o
>                0x00216656
>  _ZN19nsCompressedCharMap8SetCharsEtPj
>  ** fill        0x00216777        0x1
>  .text._ZN19nsCompressedCharMap8SetCharsEPt
>                0x00216778      0x175 nsCompressedCharMap.o
>                0x00216778
>  _ZN19nsCompressedCharMap8SetCharsEPt
>  ** fill        0x002168ed        0x1
>  .text._ZN19nsCompressedCharMap8SetCharsEPj
>                0x002168ee       0x2d nsCompressedCharMap.o
>                0x002168ee
>  _ZN19nsCompressedCharMap8SetCharsEPj
>  .text._Z10MapToCCMapPj
>                0x0021691b       0x84 nsCompressedCharMap.o
>                0x0021691b                _Z10MapToCCMapPj
>  .text._Z13MapperToCCMapP20nsICharRepresentable
>                0x0021699f       0x45 nsCompressedCharMap.o
>                0x0021699f
>  _Z13MapperToCCMapP20nsICharRepresentable
>  .text._Z13MapToCCMapExtPjPS_j
>                0x002169e4      0x1f5 nsCompressedCharMap.o
>                0x002169e4                _Z13MapToCCMapExtPjPS_j
>  ** fill        0x00216bd9        0x1
>  .text._ZN19nsCompressedCharMap8NewCCMapEv
>                0x00216bda       0x75 nsCompressedCharMap.o
>                0x00216bda                _ZN19nsCompressedCharMap8NewCCMapEv
>  ** fill        0x00216c4f        0x1
>
>
> On 06/03/2010 11:01 AM, Sriraman Tallam wrote:
>>
>> Thanks. I committed the patch.
>>
>> 2010-06-03  Sriraman Tallam<tmsriram@google.com>
>>
>>        PR gold/11658
>>        * output.cc
>>
>>  (Output_section::Input_section_sort_entry::compare_section_ordering):
>>        Change to return non-zero correctly.
>>        (Output_section::Input_section_sort_section_order_index_compare
>>        ::operator()): Change to fix ambiguity in comparisons.
>>
>> On Thu, Jun 3, 2010 at 8:55 AM, Ian Lance Taylor<iant@google.com>  wrote:
>>
>>>
>>> Sriraman Tallam<tmsriram@google.com>  writes:
>>>
>>>
>>>>
>>>>       * output.cc
>>>>
>>>> (Output_section::Input_section_sort_entry::compare_section_ordering):
>>>>       Change to return non-zero correctly.
>>>>
>>>> (Output_section::Input_section_sort_section_order_index_compare::operator()):
>>>>         Change to fix ambiguity in comparisons.
>>>>
>>>
>>> This is OK.
>>>
>>> Thanks.
>>>
>>> Ian
>>>
>>>
>>
>>
>
>

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

* Re: Patch to do reorder text and data sections according to a user  specified sequence.
  2010-06-30 23:00                             ` Sriraman Tallam
@ 2010-06-30 23:09                               ` Taras Glek
  0 siblings, 0 replies; 24+ messages in thread
From: Taras Glek @ 2010-06-30 23:09 UTC (permalink / raw)
  To: Sriraman Tallam; +Cc: Ian Lance Taylor, H.J. Lu, binutils

On 06/30/2010 04:00 PM, Sriraman Tallam wrote:
> On Wed, Jun 30, 2010 at 3:31 PM, Taras Glek<tglek@mozilla.com>  wrote:
>    
>> Hi,
>> I noticed weird "** fill"ing behavior while looking at mapfiles. Ian says
>> this is a bug, so I'm sending this to the list.
>>
>> When compiling with -ffunction-sections, many sections usually have some
>> fill after them. I'm not sure why that's needed..but a more interesting
>> problem is that once you order those sections, the remaining fill gets
>> lumped together like this:
>>      
> Does this lumping happen when you re-order with linker scripts or with
> --section-ordering-file, or both ? Could this fill be for alignment
> purposes ?
>    
I've abandoned the linker script approach, so I've only tried this with 
--section-ordering-file.

I doubt there is a purpose for 50K of fill.

Taras

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

end of thread, other threads:[~2010-06-30 23:09 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-11  2:20 Patch to do reorder text and data sections according to a user specified sequence Sriraman Tallam
2010-02-12  5:29 ` Ian Lance Taylor
2010-03-03  2:43   ` Sriraman Tallam
2010-03-05 23:11     ` Taras Glek
2010-03-06  2:35       ` Taras Glek
2010-03-08  8:28       ` Sriraman Tallam
2010-03-08 19:48         ` Sriraman Tallam
2010-03-08 20:08           ` Taras Glek
2010-03-08 21:41             ` Ian Lance Taylor
2010-05-14  0:58     ` Sriraman Tallam
2010-05-22  1:42       ` Sriraman Tallam
2010-05-24 19:27         ` Taras Glek
2010-05-29  1:49           ` Sriraman Tallam
2010-06-01 14:41             ` Ian Lance Taylor
2010-06-01 23:41               ` Sriraman Tallam
2010-06-02 17:17                 ` H.J. Lu
2010-06-02 21:04                   ` Sriraman Tallam
2010-06-03  6:51                     ` Sriraman Tallam
2010-06-03 14:04                       ` H.J. Lu
2010-06-03 15:56                       ` Ian Lance Taylor
2010-06-03 18:02                         ` Sriraman Tallam
2010-06-30 22:31                           ` Taras Glek
2010-06-30 23:00                             ` Sriraman Tallam
2010-06-30 23:09                               ` Taras Glek

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).