Re: [PATCH] [gold] Implement -z stack-size option

[Roland McGrath <mcgrathr at google dot com>]

On Tue, Aug 23, 2016 at 4:25 PM, Cary Coutant <ccoutant@gmail.com> wrote:
> I'd rename this function to create_stack_segment, since it's no longer
> just about executable stack info.

Done.

> And it seems to me that it would be cleaner to set the size and
> alignment directly when creating the segment. I'd just add a new
> public method Output_segment::set_size_and_alignment(), and call it
> immediately after calling make_output_segment.

There was already one for alignment, so I just added a set_size method.
To go with this change, I elided the call to set_offset for PT_GNU_STACK
rather than making set_offset treat it specially.  Is that what you meant?

> To preserve existing behavior when -z stack-size isn't given,
> shouldn't we set min_p_align_ only if stack_size() > 0?

That would preserve Gold's existing behavior, which is inconsistent with
BFD ld's existing behavior.  With my change, Gold's behavior matches BFD
ld's in all cases (I think).


Thanks,
Roland


gold/
2016-08-23  Roland McGrath  <roland@hack.frob.com>

	* options.h (General_options): Grok -z stack-size.
	* output.h (Output_segment::set_size): New method.
	* layout.cc (Layout::create_executable_stack_info): Renamed to ...
	(Layout::create_stack_segment): ... this.  Always create the
	segment if -z stack-size was used.
	(Layout::set_segment_offsets): Don't call ->set_offset on the
	PT_GNU_STACK segment.

diff --git a/gold/layout.cc b/gold/layout.cc
index 376051d..d14f27b 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -2135,7 +2135,7 @@ void
 Layout::create_notes()
 {
   this->create_gold_note();
-  this->create_executable_stack_info();
+  this->create_stack_segment();
   this->create_build_id();
 }

@@ -2985,13 +2985,15 @@ Layout::create_gold_note()
 // executable.  Otherwise, if at least one input file a
 // .note.GNU-stack section, and some input file has no .note.GNU-stack
 // section, we use the target default for whether the stack should be
-// executable.  Otherwise, we don't generate a stack note.  When
-// generating a object file, we create a .note.GNU-stack section with
-// the appropriate marking.  When generating an executable or shared
-// library, we create a PT_GNU_STACK segment.
+// executable.  If -z stack-size was used to set a p_memsz value for
+// PT_GNU_STACK, we generate the segment regardless.  Otherwise, we
+// don't generate a stack note.  When generating a object file, we
+// create a .note.GNU-stack section with the appropriate marking.
+// When generating an executable or shared library, we create a
+// PT_GNU_STACK segment.

 void
-Layout::create_executable_stack_info()
+Layout::create_stack_segment()
 {
   bool is_stack_executable;
   if (parameters->options().is_execstack_set())
@@ -3003,7 +3005,9 @@ Layout::create_executable_stack_info()
 	gold_warning(_("one or more inputs require executable stack, "
 		       "but -z noexecstack was given"));
     }
-  else if (!this->input_with_gnu_stack_note_)
+  else if (!this->input_with_gnu_stack_note_
+	   && (!parameters->options().user_set_stack_size()
+	       || parameters->options().relocatable()))
     return;
   else
     {
@@ -3032,7 +3036,12 @@ Layout::create_executable_stack_info()
       int flags = elfcpp::PF_R | elfcpp::PF_W;
       if (is_stack_executable)
 	flags |= elfcpp::PF_X;
+      Output_segment* seg =
 	this->make_output_segment(elfcpp::PT_GNU_STACK, flags);
+      seg->set_size(parameters->options().stack_size());
+      // BFD lets targets override this default alignment, but the only
+      // targets that do so are ones that Gold does not support so far.
+      seg->set_minimum_p_align(16);
     }
 }

@@ -3718,7 +3727,9 @@ Layout::set_segment_offsets(const Target*
target, Output_segment* load_seg,
        p != this->segment_list_.end();
        ++p)
     {
-      if ((*p)->type() != elfcpp::PT_LOAD)
+      // PT_GNU_STACK was set up correctly when it was created.
+      if ((*p)->type() != elfcpp::PT_LOAD
+	  && (*p)->type() != elfcpp::PT_GNU_STACK)
 	(*p)->set_offset((*p)->type() == elfcpp::PT_GNU_RELRO
 			 ? increase_relro
 			 : 0);
diff --git a/gold/layout.h b/gold/layout.h
index c369fef..b2d699f 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -1037,9 +1037,9 @@ class Layout
   void
   create_gold_note();

-  // Record whether the stack must be executable.
+  // Record whether the stack must be executable, and a user-supplied size.
   void
-  create_executable_stack_info();
+  create_stack_segment();

   // Create a build ID note if needed.
   void
diff --git a/gold/options.h b/gold/options.h
index 8680635..a195179 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1342,6 +1342,8 @@ class General_options
   DEFINE_bool(relro, options::DASH_Z, '\0', DEFAULT_LD_Z_RELRO,
 	      N_("Where possible mark variables read-only after relocation"),
 	      N_("Don't mark variables read-only after relocation"));
+  DEFINE_uint64(stack_size, options::DASH_Z, '\0', 0,
+		N_("Set PT_GNU_STACK segment p_memsz to SIZE"), N_("SIZE"));
   DEFINE_bool(text, options::DASH_Z, '\0', false,
 	      N_("Do not permit relocations in read-only segments"),
 	      N_("Permit relocations in read-only segments (default)"));
diff --git a/gold/output.h b/gold/output.h
index d8a8aaa..6b9186b 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -4796,6 +4796,13 @@ class Output_segment
       this->min_p_align_ = align;
   }

+  // Set the memory size of this segment.
+  void
+  set_size(uint64_t size)
+  {
+    this->memsz_ = size;
+  }
+
   // Set the offset of this segment based on the section.  This should
   // only be called for a non-PT_LOAD segment.
   void