Re: [PATCH 4/4] ld: Add ldlang_check_relro_region/update lang_find_relro_sections_1

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sun, Nov 12, 2017 at 5:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Nov 12, 2017 at 4:12 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Sun, Nov 12, 2017 at 09:31:51AM -0800, H.J. Lu wrote:
>>>       (ldlang_check_relro_region): New function.
>>>       (lang_find_relro_sections_1): Add an argument for pointer to
>>
>> This is a nitpick, but most other functions that have a
>> lang_statement_union_type* argument have it as the first parameter.
>> It obviously doesn't matter, but I think it would be nicer for
>> consistency if you kept the same parameter order here.
>
> Good idea.  Here is the updated patch.

This is the actual patch I checked in.

>> Other than that, the patch series looks fine to me.
>>
>
> I am checking it in together with the rest of series.
>
> Thanks.
>
> --
> H.J.



-- 
H.J.

From ed1794ee7a2bd8adc22e5bb8e7343b72758d5692 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 11 Nov 2017 07:02:30 -0800
Subject: [PATCH] ld: Add ldlang_check_relro_region/update
 lang_find_relro_sections_1

Extract GNU_RELRO region check into a new funtion and pass a pointer to
seg_align_type to lang_find_relro_sections_1 so that they can also be
used for text-only LOAD segment.

	* ldlang.c (lang_size_sections_1): Extract GNU_RELRO region check
	into ...
	(ldlang_check_relro_region): New function.
	(lang_find_relro_sections_1): Add an argument for pointer to
	seg_align_type and replace expld.dataseg with the pointer.
	(lang_find_relro_sections): Pass address of expld.dataseg to
	lang_find_relro_sections_1.
---
 ld/ldlang.c | 55 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 6bb9e472a2..674004ee38 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -4975,6 +4975,30 @@ os_region_check (lang_output_section_statement_type *os,
     }
 }
 
+static void
+ldlang_check_relro_region (lang_statement_union_type *s,
+			   seg_align_type *seg)
+{
+  if (seg->relro == exp_seg_relro_start)
+    {
+      if (!seg->relro_start_stat)
+	seg->relro_start_stat = s;
+      else
+	{
+	  ASSERT (seg->relro_start_stat == s);
+	}
+    }
+  else if (seg->relro == exp_seg_relro_end)
+    {
+      if (!seg->relro_end_stat)
+	seg->relro_end_stat = s;
+      else
+	{
+	  ASSERT (seg->relro_end_stat == s);
+	}
+    }
+}
+
 /* Set the sizes for all the output sections.  */
 
 static bfd_vma
@@ -5432,24 +5456,8 @@ lang_size_sections_1
 			   output_section_statement->bfd_section,
 			   &newdot);
 
-	    if (expld.dataseg.relro == exp_seg_relro_start)
-	      {
-		if (!expld.dataseg.relro_start_stat)
-		  expld.dataseg.relro_start_stat = s;
-		else
-		  {
-		    ASSERT (expld.dataseg.relro_start_stat == s);
-		  }
-	      }
-	    else if (expld.dataseg.relro == exp_seg_relro_end)
-	      {
-		if (!expld.dataseg.relro_end_stat)
-		  expld.dataseg.relro_end_stat = s;
-		else
-		  {
-		    ASSERT (expld.dataseg.relro_end_stat == s);
-		  }
-	      }
+	    ldlang_check_relro_region (s, &expld.dataseg);
+
 	    expld.dataseg.relro = exp_seg_relro_none;
 
 	    /* This symbol may be relative to this section.  */
@@ -6859,6 +6867,7 @@ find_relro_section_callback (lang_wild_statement_type *ptr ATTRIBUTE_UNUSED,
 
 static void
 lang_find_relro_sections_1 (lang_statement_union_type *s,
+			    seg_align_type *seg,
 			    bfd_boolean *has_relro_section)
 {
   if (*has_relro_section)
@@ -6866,7 +6875,7 @@ lang_find_relro_sections_1 (lang_statement_union_type *s,
 
   for (; s != NULL; s = s->header.next)
     {
-      if (s == expld.dataseg.relro_end_stat)
+      if (s == seg->relro_end_stat)
 	break;
 
       switch (s->header.type)
@@ -6878,15 +6887,15 @@ lang_find_relro_sections_1 (lang_statement_union_type *s,
 	  break;
 	case lang_constructors_statement_enum:
 	  lang_find_relro_sections_1 (constructor_list.head,
-				      has_relro_section);
+				      seg, has_relro_section);
 	  break;
 	case lang_output_section_statement_enum:
 	  lang_find_relro_sections_1 (s->output_section_statement.children.head,
-				      has_relro_section);
+				      seg, has_relro_section);
 	  break;
 	case lang_group_statement_enum:
 	  lang_find_relro_sections_1 (s->group_statement.children.head,
-				      has_relro_section);
+				      seg, has_relro_section);
 	  break;
 	default:
 	  break;
@@ -6902,7 +6911,7 @@ lang_find_relro_sections (void)
   /* Check all sections in the link script.  */
 
   lang_find_relro_sections_1 (expld.dataseg.relro_start_stat,
-			      &has_relro_section);
+			      &expld.dataseg, &has_relro_section);
 
   if (!has_relro_section)
     link_info.relro = FALSE;
-- 
2.13.6