This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.

> Other than that, the patch series looks fine to me.
>

I am checking it in together with the rest of series.

Thanks.

-- 
H.J.
From 24c80251adceb8b7bdd3cdc1688e395383b495ea 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 | 61 +++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 6bb9e472a2..a48ffd0041 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.  */
@@ -6858,7 +6866,8 @@ find_relro_section_callback (lang_wild_statement_type *ptr ATTRIBUTE_UNUSED,
 /* Iterate over sections for relro sections.  */
 
 static void
-lang_find_relro_sections_1 (lang_statement_union_type *s,
+lang_find_relro_sections_1 (seg_align_type *seg,
+			    lang_statement_union_type *s,
 			    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)
@@ -6877,15 +6886,18 @@ lang_find_relro_sections_1 (lang_statement_union_type *s,
 		     has_relro_section);
 	  break;
 	case lang_constructors_statement_enum:
-	  lang_find_relro_sections_1 (constructor_list.head,
+	  lang_find_relro_sections_1 (seg,
+				      constructor_list.head,
 				      has_relro_section);
 	  break;
 	case lang_output_section_statement_enum:
-	  lang_find_relro_sections_1 (s->output_section_statement.children.head,
+	  lang_find_relro_sections_1 (seg,
+				      s->output_section_statement.children.head,
 				      has_relro_section);
 	  break;
 	case lang_group_statement_enum:
-	  lang_find_relro_sections_1 (s->group_statement.children.head,
+	  lang_find_relro_sections_1 (seg,
+				      s->group_statement.children.head,
 				      has_relro_section);
 	  break;
 	default:
@@ -6901,7 +6913,8 @@ lang_find_relro_sections (void)
 
   /* Check all sections in the link script.  */
 
-  lang_find_relro_sections_1 (expld.dataseg.relro_start_stat,
+  lang_find_relro_sections_1 (&expld.dataseg,
+			      expld.dataseg.relro_start_stat,
 			      &has_relro_section);
 
   if (!has_relro_section)
-- 
2.13.6


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]