Re: [Patch, PR 4643] Allow symbols in MEMORY region specification

[Senthil Kumar Selvaraj <senthil_kumar dot selvaraj at atmel dot com>]

On Tue, Jan 20, 2015 at 02:13:52PM +1030, Alan Modra wrote:
> On Tue, Jan 20, 2015 at 06:29:38AM +0530, Senthil Kumar Selvaraj wrote:
> >   I ran a make check-ld for an AVR and native build, and both passed without
> >   regressions.
> 
> You need to run the testsuite on more than just a couple of targets
> when making changes of this nature.  Most of the following are
> testsuite issues, which you'll need to fix either by disabling the
> test (eg. AOUT doesn't allow you to set text start) or modifying your
> .d to match different output (eg. COFF symbol dump differs from ELF).
> 
> The spu-elf failures need investigating too.  At a guess, they are
> because you fail to initialise r->current in lang_do_memory_regions.

Yes, I'd missed that. I've fixed that, and this time, instead of
a separate test, I modified script.exp, which was already testing MEMORY
regions, to test for MEMORY regions with symbols. I figured this is 
better, because they're expected to work the same way anyway.

I ran the testsuite on the standard set of targets (picked up from
https://sourceware.org/ml/binutils/2015-01/msg00132.html). The test
fails for alpha-dec-vms (missing libraries) and powerpcle-cygwin-ld
(internal error), but both of them fail the existing MEMORY test on 
master too, on my machine.

Is this ok? If yes, could you commit it for me please? I don't have
commit access.

Regards
Senthil

ld/ChangeLog

2015-01-20  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	PR 4643
	* ldexp.c (fold_name): Fold LENGTH only after 
	lang_first_phase_enum.
	* ldgram.y (memory_spec): Don't evaluate ORIGIN and LENGTH 
	rightaway.
	* ldlang.h (struct memory_region_struct): Add origin_exp and 
	length_exp fields.
	* ldlang.c (lang_do_memory_regions): New.
	(lang_memory_region_lookup): Initialize origin_exp and 
	length_exp fields.
	(lang_process): Call lang_do_memory_regions.

ld/testsuite/ChangeLog
2015-01-20  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	* ld-scripts/memory.t: Define new symbol tred.
	* ld-scripts/memory_sym.t: New.
	* ld-scripts/script.exp: Perform MEMORY with symbols test, and
	conditionally check values of linker symbols.

diff --git a/ld/ldexp.c b/ld/ldexp.c
index f2c8620..ac66cc0 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -828,15 +828,18 @@ fold_name (etree_type *tree)
 
     case LENGTH:
       {
-        lang_memory_region_type *mem;
-
-        mem = lang_memory_region_lookup (tree->name.name, FALSE);
-        if (mem != NULL)
-          new_number (mem->length);
-        else
-          einfo (_("%F%S: undefined MEMORY region `%s'"
-		   " referenced in expression\n"),
-		 tree, tree->name.name);
+      if (expld.phase != lang_first_phase_enum)
+        {
+          lang_memory_region_type *mem;
+
+          mem = lang_memory_region_lookup (tree->name.name, FALSE);
+          if (mem != NULL)
+            new_number (mem->length);
+          else
+            einfo (_("%F%S: undefined MEMORY region `%s'"
+             " referenced in expression\n"),
+           tree, tree->name.name);
+        }
       }
       break;
 
diff --git a/ld/ldgram.y b/ld/ldgram.y
index 736f77d..f46aa9e 100644
--- a/ld/ldgram.y
+++ b/ld/ldgram.y
@@ -817,7 +817,7 @@ memory_spec: 	NAME
 origin_spec:
 	ORIGIN '=' mustbe_exp
 		{
-		  region->origin = exp_get_vma ($3, 0, "origin");
+		  region->origin_exp = $3;
 		  region->current = region->origin;
 		}
 	;
@@ -825,7 +825,7 @@ origin_spec:
 length_spec:
              LENGTH '=' mustbe_exp
 		{
-		  region->length = exp_get_vma ($3, -1, "length");
+		  region->length_exp = $3;
 		}
 	;
 
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 3ea22c2..5344e5e 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -85,6 +85,7 @@ static void lang_record_phdrs (void);
 static void lang_do_version_exports_section (void);
 static void lang_finalize_version_expr_head
   (struct bfd_elf_version_expr_head *);
+static void lang_do_memory_regions (void);
 
 /* Exported variables.  */
 const char *output_target;
@@ -1305,7 +1306,9 @@ lang_memory_region_lookup (const char *const name, bfd_boolean create)
   new_region->name_list.name = xstrdup (name);
   new_region->name_list.next = NULL;
   new_region->next = NULL;
+  new_region->origin_exp = NULL;
   new_region->origin = 0;
+  new_region->length_exp = NULL;
   new_region->length = ~(bfd_size_type) 0;
   new_region->current = 0;
   new_region->last_os = NULL;
@@ -6707,6 +6710,8 @@ lang_process (void)
   /* PR 13683: We must rerun the assignments prior to running garbage
      collection in order to make sure that all symbol aliases are resolved.  */
   lang_do_assignments (lang_mark_phase_enum);
+
+  lang_do_memory_regions();
   expld.phase = lang_first_phase_enum;
 
   /* Size up the common data.  */
@@ -7970,6 +7975,37 @@ lang_do_version_exports_section (void)
 			   lang_new_vers_node (greg, lreg), NULL);
 }
 
+/* Evaluate LENGTH and ORIGIN parts of MEMORY spec */
+
+static void
+lang_do_memory_regions (void)
+{
+  lang_memory_region_type *r = lang_memory_region_list;
+
+  for (; r != NULL; r = r->next)
+    {
+      if (r->origin_exp)
+      {
+        exp_fold_tree_no_dot (r->origin_exp);
+        if (expld.result.valid_p)
+          {
+            r->origin = expld.result.value;
+            r->current = r->origin;
+          }
+        else
+          einfo (_("%F%P: invalid origin for memory region %s\n"), r->name_list.name);
+      }
+      if (r->length_exp)
+      {
+        exp_fold_tree_no_dot (r->length_exp);
+        if (expld.result.valid_p)
+          r->length = expld.result.value;
+        else
+          einfo (_("%F%P: invalid length for memory region %s\n"), r->name_list.name);
+      }
+    }
+}
+
 void
 lang_add_unique (const char *name)
 {
diff --git a/ld/ldlang.h b/ld/ldlang.h
index 48d7e4e..69d21a7 100644
--- a/ld/ldlang.h
+++ b/ld/ldlang.h
@@ -55,8 +55,10 @@ typedef struct memory_region_struct
 {
   lang_memory_region_name name_list;
   struct memory_region_struct *next;
+  union etree_union *origin_exp;
   bfd_vma origin;
   bfd_size_type length;
+  union etree_union *length_exp;
   bfd_vma current;
   union lang_statement_union *last_os;
   flagword flags;
diff --git a/ld/testsuite/ld-scripts/memory.t b/ld/testsuite/ld-scripts/memory.t
index 129bd7c..937394f 100644
--- a/ld/testsuite/ld-scripts/memory.t
+++ b/ld/testsuite/ld-scripts/memory.t
@@ -29,5 +29,6 @@ SECTIONS
     data_end = .;
   } >DATAMEM
 
-  fred = ORIGIN(DATAMEM) + LENGTH(DATAMEM);  
+  fred = ORIGIN(DATAMEM) + LENGTH(DATAMEM);
+  tred = ORIGIN(TEXTMEM) + LENGTH(TEXTMEM);
 }
diff --git a/ld/testsuite/ld-scripts/memory_sym.t b/ld/testsuite/ld-scripts/memory_sym.t
new file mode 100644
index 0000000..4ccec55
--- /dev/null
+++ b/ld/testsuite/ld-scripts/memory_sym.t
@@ -0,0 +1,36 @@
+TXT_ORIGIN = 0x100;
+TXT_LENGTH = 32K;
+MEMORY
+{
+  R_TEXTMEM (ARX) : ORIGIN = TXT_ORIGIN, LENGTH = TXT_LENGTH
+  R_DATAMEM (AW)  : org = DATA_ORIGIN, l = DATA_LENGTH
+}
+
+REGION_ALIAS ("A_TEXTMEM", R_TEXTMEM);
+REGION_ALIAS ("A_DATAMEM", R_DATAMEM);
+
+REGION_ALIAS ("TEXTMEM", A_TEXTMEM);
+REGION_ALIAS ("DATAMEM", A_DATAMEM);
+
+SECTIONS
+{
+  . = 0;
+  .text :
+  {
+    text_start = ORIGIN (TEXTMEM);
+    *(.text)
+    *(.pr)
+    text_end = .;
+  } > TEXTMEM
+  
+  data_start = ORIGIN (DATAMEM);
+  .data :
+  {
+    *(.data)
+    *(.rw)
+    data_end = .;
+  } >DATAMEM
+
+  fred = ORIGIN(DATAMEM) + LENGTH(DATAMEM);  
+  tred = ORIGIN(TEXTMEM) + LENGTH(TEXTMEM);  
+}
diff --git a/ld/testsuite/ld-scripts/script.exp b/ld/testsuite/ld-scripts/script.exp
index cee706f..6ab04ec 100644
--- a/ld/testsuite/ld-scripts/script.exp
+++ b/ld/testsuite/ld-scripts/script.exp
@@ -66,6 +66,13 @@ proc check_script { } {
 	set passes 0
     } 
 
+    if {[info exists nm_output(tred)] \
+        && $nm_output(tred) != (0x100 + 0x8000)} {
+	send_log "tred == $nm_output(tred)\n"
+	verbose "tred == $nm_output(tred)"
+	set passes 0
+    }
+
     if {$nm_output(text_end) < $text_end \
 	    || $nm_output(text_end) > 0x110} {
 	send_log "text_end == $nm_output(text_end)\n"
@@ -79,6 +86,13 @@ proc check_script { } {
 	set passes 0
     } 
 
+    if {[info exists nm_output(fred)] \
+        && $nm_output(fred) != (0x1000 + 0x10000)} {
+	send_log "fred == $nm_output(fred)\n"
+	verbose "fred == $nm_output(fred)"
+	set passes 0
+    }
+
     if {$nm_output(data_end) < $data_end \
 	    || $nm_output(data_end) > 0x1010} {
 	send_log "data_end == $nm_output(data_end)\n"
@@ -126,6 +140,13 @@ if ![ld_simple_link $ld tmpdir/script "$flags -T $srcdir/$subdir/memory.t tmpdir
     check_script
 }
 
+set testname "MEMORY with symbols"
+if ![ld_simple_link $ld tmpdir/script "$flags -defsym DATA_ORIGIN=0x1000 -defsym DATA_LENGTH=0x10000 -T $srcdir/$subdir/memory_sym.t tmpdir/script.o"] {
+    fail $testname
+} else {
+    check_script
+}
+
 set test_script_list [lsort [glob $srcdir/$subdir/region-alias-*.t]]
 
 foreach test_script $test_script_list {
> 
> alpha-dec-vms  +FAIL: ld-scripts/pr4643
> alpha-linuxecoff  +FAIL: ld-scripts/pr4643
> arm-aout  +FAIL: ld-scripts/pr4643
> arm-coff  +FAIL: ld-scripts/pr4643
> arm-epoc-pe  +FAIL: ld-scripts/pr4643
> arm-pe  +FAIL: ld-scripts/pr4643
> arm-wince-pe  +FAIL: ld-scripts/pr4643
> i386-linuxaout  +FAIL: ld-scripts/pr4643
> i586-aout  +FAIL: ld-scripts/pr4643
> i586-coff  +FAIL: ld-scripts/pr4643
> i686-pe  +FAIL: ld-scripts/pr4643
> m68k-netbsd  +FAIL: ld-scripts/pr4643
> mcore-pe  +FAIL: ld-scripts/pr4643
> mips64-linux  +FAIL: ld-scripts/pr4643
> mipsel-linux-gnu  +FAIL: ld-scripts/pr4643
> mipsisa32el-linux  +FAIL: ld-scripts/pr4643
> mips-linux  +FAIL: ld-scripts/pr4643
> ns32k-netbsd  +FAIL: ld-scripts/pr4643
> pdp11-dec-aout  +FAIL: ld-scripts/pr4643
> powerpcle-cygwin  +FAIL: ld-scripts/pr4643
> rs6000-aix4.3.3  +FAIL: ld-scripts/pr4643
> rs6000-aix5.1  +FAIL: ld-scripts/pr4643
> sh-pe  +FAIL: ld-scripts/pr4643
> sparc-aout  +FAIL: ld-scripts/pr4643
> sparc-coff  +FAIL: ld-scripts/pr4643
> spu-elf  +FAIL: ld-scripts/empty-orphan
> spu-elf  +FAIL: ld-scripts/pr4643
> spu-elf  +FAIL: overlay size
> spu-elf  +FAIL: rgn-at1
> spu-elf  +FAIL: ld-scripts/rgn-at10
> spu-elf  +FAIL: ld-scripts/rgn-at11
> spu-elf  +FAIL: rgn-at2
> spu-elf  +FAIL: rgn-at3
> spu-elf  +FAIL: rgn-at4
> spu-elf  +FAIL: ld-scripts/rgn-at6
> spu-elf  +FAIL: ld-scripts/rgn-at7
> spu-elf  +FAIL: ld-scripts/rgn-at8
> spu-elf  +FAIL: ld-scripts/rgn-at9
> spu-elf  +FAIL: MEMORY
> spu-elf  +FAIL: section-flags-1
> spu-elf  +FAIL: section-flags-2
> tic30-unknown-aout  +FAIL: ld-scripts/pr4643
> tic30-unknown-coff  +FAIL: ld-scripts/pr4643
> tic4x-coff  +FAIL: ld-scripts/pr4643
> tic54x-coff  +FAIL: ld-scripts/pr4643
> tilegx-linux  +FAIL: ld-scripts/pr4643
> tilepro-linux  +FAIL: ld-scripts/pr4643
> x86_64-mingw32  +FAIL: ld-scripts/pr4643
> z80-coff  +FAIL: ld-scripts/pr4643
> z8k-coff  +FAIL: ld-scripts/pr4643
> 
> -- 
> Alan Modra
> Australia Development Lab, IBM