[PATCH v2 2/2] LD: Always make a SEGMENT_START expression section-relative

[Maciej W. Rozycki]

From: Maciej W. Rozycki <macro@mips.com>

Fix an issue with the SEGMENT_START builtin function where its result is 
absolute when taken from the default supplied, and section-relative when 
taken from a `-T' command-line override.  This is against documentation, 
inconsistent and unexpected, and with PIE executables gives an incorrect 
result with the `__executable_start' symbol.

Make the result of SEGMENT_START always section-relative then.

	ld/
	* ldexp.c (fold_binary): Always make the result of SEGMENT_START
	section-relative.
	* testsuite/ld-scripts/segment-start.d: New test.
	* testsuite/ld-scripts/segment-start.ld: New test linker script.
	* testsuite/ld-scripts/segment-start.s: New test source.
	* testsuite/ld-scripts/script.exp: Run the new test.
---
Hi Alan,

> > 	* ldexp.c (fold_binary): Always make the result of SEGMENT_START
> > 	section-relative.
> 
> The above is OK, but since you're changing this code it would be good
> to avoid the divide by zero exposed by your override testcase on
> non-ELF targets.  Please add a "config.maxpagesize != 0" test before
> we try to calculate "seg->value % config.maxpagesize".

 Thanks for catching this, fix now sent as 1/2 in this series.

> > 	* testsuite/ld-scripts/segment-start.d: New test.
> > 	* testsuite/ld-scripts/segment-start.ld: New test linker script.
> > 	* testsuite/ld-scripts/segment-start.s: New test source.
> > 	* testsuite/ld-scripts/script.exp: Run the new test.
> 
> I'm inclined to think these tests should only be run on ELF targets.
> Some formats don't support changing the text segment address.

 With the division by zero fixed these tests work however across all but 
just a bunch of targets.  So why not have all the good targets covered?

 I propose to have the few problematic targets XFAILed then, as in this 
update.  There are 6 such targets only really, the remaining ones are 
aliases.

  Maciej

Changes from v1:

- XFAIL targets that are not expected to handle SEGMENT_START correctly.
---
 ld/ldexp.c                               |    4 +++-
 ld/testsuite/ld-scripts/script.exp       |    4 ++++
 ld/testsuite/ld-scripts/segment-start.d  |   18 ++++++++++++++++++
 ld/testsuite/ld-scripts/segment-start.ld |   12 ++++++++++++
 ld/testsuite/ld-scripts/segment-start.s  |    2 ++
 5 files changed, 39 insertions(+), 1 deletion(-)

binutils-ld-segment-start-default-rel.diff
Index: src/ld/ldexp.c
===================================================================
--- src.orig/ld/ldexp.c
+++ src/ld/ldexp.c
@@ -534,6 +534,7 @@ fold_binary (etree_type *tree)
      operand, binary.rhs is first operand.  */
   if (expld.result.valid_p && tree->type.node_code == SEGMENT_START)
     {
+      bfd_vma value = expld.result.value;
       const char *segment_name;
       segment_type *seg;
 
@@ -551,9 +552,10 @@ fold_binary (etree_type *tree)
 		       "isn't multiple of maximum page size\n"),
 		     segment_name);
 	    seg->used = TRUE;
-	    new_rel_from_abs (seg->value);
+	    value = seg->value;
 	    break;
 	  }
+      new_rel_from_abs (value);
       return;
     }
 
Index: src/ld/testsuite/ld-scripts/script.exp
===================================================================
--- src.orig/ld/testsuite/ld-scripts/script.exp
+++ src/ld/testsuite/ld-scripts/script.exp
@@ -231,3 +231,7 @@ foreach test_script $test_script_list {
 
 run_dump_test "align-with-input"
 run_dump_test "pr20302"
+
+run_dump_test "segment-start" {{name (default)}}
+run_dump_test "segment-start" {{name (overridden)} \
+			       {ld -Ttext-segment=0x10000000}}
Index: src/ld/testsuite/ld-scripts/segment-start.d
===================================================================
--- /dev/null
+++ src/ld/testsuite/ld-scripts/segment-start.d
@@ -0,0 +1,19 @@
+#PROG: nm
+#name: SEGMENT_START expression not absolute
+#source: segment-start.s
+#ld: -e 0 -u __executable_start -T segment-start.ld
+#xfail: mmix-*-* pdp11-*-* powerpc-*-aix* powerpc-*-beos* rs6000-*-* sh-*-pe
+#xfail: c30-*-*aout* tic30-*-*aout* c54x*-*-*coff* tic54x-*-*coff*
+# XFAIL targets that are not expected to handle SEGMENT_START correctly.
+
+# Make sure `__executable_start' is regular:
+#
+# 10000000 T __executable_start
+#
+# not absolute:
+#
+# 10000000 A __executable_start
+
+#...
+0*10000000 T __executable_start
+#pass
Index: src/ld/testsuite/ld-scripts/segment-start.ld
===================================================================
--- /dev/null
+++ src/ld/testsuite/ld-scripts/segment-start.ld
@@ -0,0 +1,12 @@
+SECTIONS
+{
+  PROVIDE (__executable_start = SEGMENT_START ("text-segment", 0x10000000));
+  .text : { *(.text) }
+  .data : { *(.data) }
+  .bss : { *(.bss) }
+  .loader : { *(.loader) }
+  .symtab : { *(.symtab) }
+  .strtab : { *(.strtab) }
+  .shstrtab : { *(.shstrtab) }
+  /DISCARD/ : { *(*) }
+}
Index: src/ld/testsuite/ld-scripts/segment-start.s
===================================================================
--- /dev/null
+++ src/ld/testsuite/ld-scripts/segment-start.s
@@ -0,0 +1,2 @@
+	.text
+	.space	16