Re: [PATCH] gold: -Trodata-segment and fix -Ttext-segment for isolate_execinstr targets

[Roland McGrath <mcgrathr at google dot com>]

Here's a new version of the patch.


Thanks,
Roland


gold/
2013-07-19  Roland McGrath  <mcgrathr@google.com>

	* options.h (General_options): Add -Trodata-segment option.
	* parameters.cc (Parameters::check_rodata_segment): New function.
	(Parameters::set_target_once): Call it.
	* parameters.h (Parameters): Declare it (private member function).
	* layout.cc (load_seg_unusable_for_headers): New function, broken
	out of Layout::relaxation_loop_body.  If TARGET->isolate_execinstr()
	then validate rodata segment rather than text segment.
	(relaxation_loop_body): Call that.
	(is_text_segment): New function.  Don't admit a non-executable
	segment if TARGET->isolate_execinstr().
	(set_segment_offsets): Call it.  Honor -Trodata-segment option.

--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -2369,6 +2369,22 @@ Layout::prepare_for_relaxation()
   this->record_output_section_data_from_script_ = true;
 }

+// If the user set the address of the text segment, that may not be
+// compatible with putting the segment headers and file headers into
+// that segment.  For isolate_execinstr() targets, it's the rodata
+// segment rather than text where we might put the headers.
+static inline bool
+load_seg_unusable_for_headers(const Target* target)
+{
+  const General_options& options = parameters->options();
+  if (target->isolate_execinstr())
+    return (options.user_set_Trodata_segment()
+	    && options.Trodata_segment() % target->abi_pagesize() != 0);
+  else
+    return (options.user_set_Ttext()
+	    && options.Ttext() % target->abi_pagesize() != 0);
+}
+
 // Relaxation loop body:  If target has no relaxation, this runs only once
 // Otherwise, the target relaxation hook is called at the end of
 // each iteration.  If the hook returns true, it means re-layout of
@@ -2421,11 +2437,7 @@ Layout::relaxation_loop_body(
       != General_options::OBJECT_FORMAT_ELF)
     load_seg = NULL;

-  // If the user set the address of the text segment, that may not be
-  // compatible with putting the segment headers and file headers into
-  // that segment.
-  if (parameters->options().user_set_Ttext()
-      && parameters->options().Ttext() % target->abi_pagesize() != 0)
+  if (load_seg_unusable_for_headers(target))
     {
       load_seg = NULL;
       phdr_seg = NULL;
@@ -3319,6 +3331,20 @@ align_file_offset(off_t off, uint64_t addr,
uint64_t abi_pagesize)
   return aligned_off;
 }

+// On targets where the text segment contains only executable code,
+// a non-executable segment is never the text segment.
+
+static inline bool
+is_text_segment(const Target* target, const Output_segment* seg)
+{
+  elfcpp::Elf_Xword flags = seg->flags();
+  if ((flags & elfcpp::PF_W) != 0)
+    return false;
+  if ((flags & elfcpp::PF_X) == 0)
+    return !target->isolate_execinstr();
+  return true;
+}
+
 // Set the file offsets of all the segments, and all the sections they
 // contain.  They have all been created.  LOAD_SEG must be be laid out
 // first.  Return the offset of the data to follow.
@@ -3411,8 +3437,14 @@ Layout::set_segment_offsets(const Target*
target, Output_segment* load_seg,
 	    }
 	  else if (parameters->options().user_set_Ttext()
 		   && (parameters->options().omagic()
-		       || ((*p)->flags() & elfcpp::PF_W) == 0))
+		       || is_text_segment(target, *p)))
+	    {
+	      are_addresses_set = true;
+	    }
+	  else if (parameters->options().user_set_Trodata_segment()
+		   && ((*p)->flags() & (elfcpp::PF_W | elfcpp::PF_X)) == 0)
 	    {
+	      addr = parameters->options().Trodata_segment();
 	      are_addresses_set = true;
 	    }
 	  else if (parameters->options().user_set_Tdata()
--- a/gold/options.h
+++ b/gold/options.h
@@ -1162,6 +1162,8 @@ class General_options
   DEFINE_uint64_alias(Ttext_segment, Ttext, options::ONE_DASH, '\0',
 		      N_("Set the address of the text segment"),
 		      N_("ADDRESS"));
+  DEFINE_uint64(Trodata_segment, options::ONE_DASH, '\0', -1U,
+		N_("Set the address of the rodata segment"), N_("ADDRESS"));

   DEFINE_bool(toc_optimize, options::TWO_DASHES, '\0', true,
 	      N_("(PowerPC64 only) Optimize TOC code sequences"),
--- a/gold/parameters.cc
+++ b/gold/parameters.cc
@@ -1,6 +1,6 @@
 // parameters.cc -- general parameters for a link using gold

-// Copyright 2006, 2007, 2008, 2009, 2010, 2011, 2012
+// Copyright 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013
 // Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.

@@ -127,7 +127,10 @@ Parameters::set_target_once(Target* target)
   gold_assert(this->target_ == NULL);
   this->target_ = target;
   if (this->options_valid())
-    this->check_target_endianness();
+    {
+      this->check_target_endianness();
+      this->check_rodata_segment();
+    }
 }

 // Clear the target, for testing.
@@ -219,6 +222,15 @@ Parameters::check_target_endianness()
     }
 }

+void
+Parameters::check_rodata_segment()
+{
+  if (this->options().user_set_Trodata_segment()
+      && !this->options().rosegment()
+      && !this->target().isolate_execinstr())
+    gold_error(_("-Trodata-segment is meaningless without --rosegment"));
+}
+
 // Return the name of the entry symbol.

 const char*
--- a/gold/parameters.h
+++ b/gold/parameters.h
@@ -1,6 +1,6 @@
 // parameters.h -- general parameters for a link using gold  -*- C++ -*-

-// Copyright 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2009, 2010, 2013 Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.

 // This file is part of gold.
@@ -183,6 +183,9 @@ class Parameters
   void
   check_target_endianness();

+  void
+  check_rodata_segment();
+
   friend class Set_parameters_target_once;

   Errors* errors_;