Re: [patch][gold] Don't put rodata and text in the same segment.

[Rafael Espindola <espindola at google dot com>]

> Note that I already approved the earlier patch you sent which always
> moves to a new page when it sees a new segment. ÂCan you commit that
> patch and then send this one as a new diff? ÂThanks.

I liked you suggestion to only move forward when there is a change in
flags, that is why I update the patch. Attached is one with only that
change.

Let me know which one you like best. This one or the one that always
moves to a new page and I will commit.

> Ian
>

Thanks,
-- 
Rafael Ãvila de EspÃndola

diff --git a/gold/layout.cc b/gold/layout.cc
index bfe6a5e..5d20dad 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -2567,7 +2567,7 @@ Layout::set_segment_offsets(const Target* target, Output_segment* load_seg,
   const bool check_sections = parameters->options().check_sections();
   Output_segment* last_load_segment = NULL;
 
-  bool was_readonly = false;
+  elfcpp::Elf_Word old_flags = -1;
   for (Segment_list::iterator p = this->segment_list_.begin();
        p != this->segment_list_.end();
        ++p)
@@ -2614,21 +2614,26 @@ Layout::set_segment_offsets(const Target* target, Output_segment* load_seg,
 
 	  if (!are_addresses_set)
 	    {
-	      // If the last segment was readonly, and this one is
-	      // not, then skip the address forward one page,
-	      // maintaining the same position within the page.  This
-	      // lets us store both segments overlapping on a single
-	      // page in the file, but the loader will put them on
-	      // different pages in memory.
+	      // If the R, W or X flags changed, skip the address forward one
+	      // page, maintaining the same position within the page.
+	      // Advancing to the next page lets the loader set the flags
+	      // correctly for each segment.
+	      // Maintaining the same position within the page lets us store
+	      // both segments overlapping on a single page in the file, but
+	      // the loader will put them on different pages in memory.
+	      // We will revisit this decision once we know the size of the
+	      // segment.
 
 	      addr = align_address(addr, (*p)->maximum_alignment());
 	      aligned_addr = addr;
 
-	      if (was_readonly && ((*p)->flags() & elfcpp::PF_W) != 0)
-		{
-		  if ((addr & (abi_pagesize - 1)) != 0)
-		    addr = addr + abi_pagesize;
-		}
+              const elfcpp::Elf_Word cur_flags =
+                ((*p)->flags() & (elfcpp::PF_W | elfcpp::PF_R | elfcpp::PF_X));
+
+	      if (old_flags != static_cast<elfcpp::Elf_Word>(-1)
+		  && cur_flags != old_flags
+		  && (addr & (abi_pagesize - 1)) != 0)
+		addr = addr + abi_pagesize;
 
 	      off = orig_off + ((addr - orig_addr) & (abi_pagesize - 1));
 	    }
@@ -2686,8 +2691,8 @@ Layout::set_segment_offsets(const Target* target, Output_segment* load_seg,
 
 	  addr = new_addr;
 
-	  if (((*p)->flags() & elfcpp::PF_W) == 0)
-	    was_readonly = true;
+          old_flags =
+            ((*p)->flags() & (elfcpp::PF_W | elfcpp::PF_R | elfcpp::PF_X));
 
 	  // Implement --check-sections.  We know that the segments
 	  // are sorted by LMA.