[MSP430,LD] Leave placement of .lower and .upper sections to generic linker code

Message ID 20190416214429.1fb8a73f@jozef-kubuntu
State New
Headers show
Series
  • [MSP430,LD] Leave placement of .lower and .upper sections to generic linker code
Related show

Commit Message

Jozef Lawrynowicz April 16, 2019, 8:44 p.m.
The MSP430 linker has the functionality to "shuffle" function and data
sections between upper and lower memory, if the lower memory region overflows
and --{code,data}-region=either is passed to ld.

The --{code,data}-region= options also accept values of "upper" and "lower",
and for these options, it will add ".upper" or ".lower" prefixes to
input section names.

Currently, the code also tries to move these ".upper" and ".lower"
prefixed sections to an assumed output section, based on the input
section name (e.g. .upper.data.foo -> .upper.data). This means the actual input
section rules in the linker script are ignored.
The patch fixes this by renaming the section in these situations using
bfd_rename_section, then leaving the generic parts of the linker to place the
section in the correct output section.

Furthermore, some erroneous free()s of the variables containing the newly
prefixed section names cause the section names in the map file to appear as
random garbage.

The attached patch fixes these issues and adds tests. The ld testsuite now
runs clean for msp430-elf.

If the patch is acceptable, I would appreciate if someone would commit it for
me, as I do not have write access.

Thanks,
Jozef

Comments

Nick Clifton April 17, 2019, 2:07 p.m. | #1
Hi Jozef,

> The attached patch fixes these issues and adds tests. The ld testsuite now

> runs clean for msp430-elf.

> 

> If the patch is acceptable, I would appreciate if someone would commit it for

> me, as I do not have write access.


Approved and applied.

Cheers
  Nick

Patch

From 38fd83452c9d532bdf4f9e7c0375433b4841e66b Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Tue, 16 Apr 2019 18:20:15 +0100
Subject: [PATCH] MSP430: Leave placement of .lower and .upper sections to
 generic linker code

ld/ChangeLog:

2019-04-16  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* emultempl/msp430.em (warn_no_output_section): New.
	(gld${EMULATION_NAME}_place_orphan): Handle orphaned .lower and .upper
	prefixed sections.
	(move_prefixed_section): Remove.
	(add_region_prefix): For REGION_UPPER and REGION_LOWER sections, use
	bfd_rename_section to add the .upper and .lower prefix. Don't call
	move_prefixed_section.
	Remove free()s of section name strings.
	* testsuite/ld-msp430-elf/msp430-elf.exp: Update guidance on using
	run_ld_link_tests.
	Add msp430warntests.
	Add valid-map test.
	* testsuite/ld-msp430-elf/valid-map-foo.s: New test source.
	* testsuite/ld-msp430-elf/valid-map.s: Likewise.
	* testsuite/ld-msp430-elf/warn-no-lower.s: Likewise.
	* testsuite/ld-msp430-elf/valid-map.d: New test.
	* testsuite/ld-msp430-elf/valid-map.map: New expected map file output.
	* testsuite/ld-msp430-elf/warn-no-lower-code.r: New expected warning
	output.
	* testsuite/ld-msp430-elf/warn-no-lower-data.r: Likewise.
	* testsuite/ld-msp430-elf/warn-no-lower.r: Likewise.
---
 ld/emultempl/msp430.em                        | 110 ++++++++----------
 ld/testsuite/ld-msp430-elf/msp430-elf.exp     |  52 +++++++--
 ld/testsuite/ld-msp430-elf/valid-map-foo.s    |  26 +++++
 ld/testsuite/ld-msp430-elf/valid-map.d        |   6 +
 ld/testsuite/ld-msp430-elf/valid-map.map      |   6 +
 ld/testsuite/ld-msp430-elf/valid-map.s        |  60 ++++++++++
 .../ld-msp430-elf/warn-no-lower-code.r        |   1 +
 .../ld-msp430-elf/warn-no-lower-data.r        |   3 +
 ld/testsuite/ld-msp430-elf/warn-no-lower.r    |   4 +
 ld/testsuite/ld-msp430-elf/warn-no-lower.s    |  44 +++++++
 10 files changed, 240 insertions(+), 72 deletions(-)
 create mode 100644 ld/testsuite/ld-msp430-elf/valid-map-foo.s
 create mode 100644 ld/testsuite/ld-msp430-elf/valid-map.d
 create mode 100644 ld/testsuite/ld-msp430-elf/valid-map.map
 create mode 100644 ld/testsuite/ld-msp430-elf/valid-map.s
 create mode 100644 ld/testsuite/ld-msp430-elf/warn-no-lower-code.r
 create mode 100644 ld/testsuite/ld-msp430-elf/warn-no-lower-data.r
 create mode 100644 ld/testsuite/ld-msp430-elf/warn-no-lower.r
 create mode 100644 ld/testsuite/ld-msp430-elf/warn-no-lower.s

diff --git a/ld/emultempl/msp430.em b/ld/emultempl/msp430.em
index 8d1b45bb7e..765a9ea288 100644
--- a/ld/emultempl/msp430.em
+++ b/ld/emultempl/msp430.em
@@ -215,6 +215,40 @@  scan_children (lang_statement_union_type * l)
   return amount;
 }
 
+#define WARN_UPPER 0
+#define WARN_LOWER 1
+#define WARN_TEXT 0
+#define WARN_DATA 1
+#define WARN_BSS 2
+#define WARN_RODATA 3
+
+/* Warn only once per output section.
+ * NAME starts with ".upper." or ".lower.".  */
+static void
+warn_no_output_section (const char *name)
+{
+  static bfd_boolean warned[2][4] = {{FALSE, FALSE, FALSE, FALSE},
+				     {FALSE, FALSE, FALSE, FALSE}};
+  int i = WARN_LOWER;
+
+  if (strncmp (name, ".upper.", 7) == 0)
+    i = WARN_UPPER;
+
+  if (!warned[i][WARN_TEXT] && strcmp (name + 6, ".text") == 0)
+    warned[i][WARN_TEXT] = TRUE;
+  else if (!warned[i][WARN_DATA] && strcmp (name + 6, ".data") == 0)
+    warned[i][WARN_DATA] = TRUE;
+  else if (!warned[i][WARN_BSS] && strcmp (name + 6, ".bss") == 0)
+    warned[i][WARN_BSS] = TRUE;
+  else if (!warned[i][WARN_RODATA] && strcmp (name + 6, ".rodata") == 0)
+    warned[i][WARN_RODATA] = TRUE;
+  else
+    return;
+  einfo ("%P: warning: no input section rule matches %s in linker script\n",
+	 name);
+}
+
+
 /* Place an orphan section.  We use this to put .either sections
    into either their lower or their upper equivalents.  */
 
@@ -240,6 +274,13 @@  gld${EMULATION_NAME}_place_orphan (asection * s,
   if (constraint != 0)
     return NULL;
 
+  if (strncmp (secname, ".upper.", 7) == 0
+      || strncmp (secname, ".lower.", 7) == 0)
+    {
+      warn_no_output_section (secname);
+      return NULL;
+    }
+
   /* We only need special handling for .either sections.  */
   if (strncmp (secname, ".either.", 8) != 0)
     return NULL;
@@ -340,52 +381,21 @@  change_output_section (lang_statement_union_type ** head,
   return FALSE;
 }
 
-static void
-move_prefixed_section (asection *s, char *new_name,
-		       lang_output_section_statement_type * new_output_sec)
-{
-  s->name = new_name;
-  if (s->output_section == NULL)
-    lang_add_section (& (new_output_sec->children), s, NULL, new_output_sec);
-  else
-    {
-      lang_output_section_statement_type * curr_output_sec
-	= lang_output_section_find (s->output_section->name);
-      change_output_section (&(curr_output_sec->children.head), s,
-			     new_output_sec);
-    }
-}
-
 static void
 add_region_prefix (bfd *abfd, asection *s,
 		   ATTRIBUTE_UNUSED void *unused)
 {
   const char *curr_name = bfd_get_section_name (abfd, s);
-  char * base_name;
-  char * new_input_sec_name = NULL;
-  char * new_output_sec_name = NULL;
   int region = REGION_NONE;
 
   if (strncmp (curr_name, ".text", 5) == 0)
-    {
-      region = code_region;
-      base_name = concat (".text", NULL);
-    }
+    region = code_region;
   else if (strncmp (curr_name, ".data", 5) == 0)
-    {
-      region = data_region;
-      base_name = concat (".data", NULL);
-    }
+    region = data_region;
   else if (strncmp (curr_name, ".bss", 4) == 0)
-    {
-      region = data_region;
-      base_name = concat (".bss", NULL);
-    }
+    region = data_region;
   else if (strncmp (curr_name, ".rodata", 7) == 0)
-    {
-      region = data_region;
-      base_name = concat (".rodata", NULL);
-    }
+    region = data_region;
   else
     return;
 
@@ -394,30 +404,10 @@  add_region_prefix (bfd *abfd, asection *s,
     case REGION_NONE:
       break;
     case REGION_UPPER:
-      new_input_sec_name = concat (".upper", curr_name, NULL);
-      new_output_sec_name = concat (".upper", base_name, NULL);
-      lang_output_section_statement_type * upper
-	= lang_output_section_find (new_output_sec_name);
-      if (upper != NULL)
-	{
-	  move_prefixed_section (s, new_input_sec_name, upper);
-	}
-      else
-	einfo (_("%P: error: no section named %s in linker script\n"),
-	       new_output_sec_name);
+      bfd_rename_section (abfd, s, concat (".upper", curr_name, NULL));
       break;
     case REGION_LOWER:
-      new_input_sec_name = concat (".lower", curr_name, NULL);
-      new_output_sec_name = concat (".lower", base_name, NULL);
-      lang_output_section_statement_type * lower
-	= lang_output_section_find (new_output_sec_name);
-      if (lower != NULL)
-	{
-	  move_prefixed_section (s, new_input_sec_name, lower);
-	}
-      else
-	einfo (_("%P: error: no section named %s in linker script\n"),
-	       new_output_sec_name);
+      bfd_rename_section (abfd, s, concat (".lower", curr_name, NULL));
       break;
     case REGION_EITHER:
       s->name = concat (".either", curr_name, NULL);
@@ -427,12 +417,6 @@  add_region_prefix (bfd *abfd, asection *s,
       FAIL ();
       break;
     }
-  free (base_name);
-  if (new_input_sec_name)
-    {
-      free (new_input_sec_name);
-      free (new_output_sec_name);
-    }
 }
 
 static void
diff --git a/ld/testsuite/ld-msp430-elf/msp430-elf.exp b/ld/testsuite/ld-msp430-elf/msp430-elf.exp
index 8996d3fcbf..b6f3151c80 100644
--- a/ld/testsuite/ld-msp430-elf/msp430-elf.exp
+++ b/ld/testsuite/ld-msp430-elf/msp430-elf.exp
@@ -23,15 +23,29 @@  if { ![istarget "msp430*elf*"] } {
     return
 }
 
-# List contains test-items with 3 items followed by 2 lists and one more item:
-# 0:name 1:ld early options 2:ld late options 3:assembler options
-# 4:filenames of assembler files 5: action and options. 6: name of output file
-
-# Actions:
-# objdump: Apply objdump options on result.  Compare with regex (last arg).
-# nm: Apply nm options on result.  Compare with regex (last arg).
-# readelf: Apply readelf options on result.  Compare with regex (last arg).
-
+# List contains test-items with 3 items followed by 2 lists, one item and
+# one optional item:
+#  0:name
+#  1:ld/ar leading options, placed before object files
+#  2:ld/ar trailing options, placed after object files
+#  3:assembler options
+#  4:filenames of assembler files
+#  5:list of actions, options and expected outputs.
+#  6:name of output file
+#  7:compiler flags (optional)
+#
+# Actions: { command command-line-options file-containg-expected-output-regexps }
+# Commands:
+#   objdump: Apply objdump options on result.
+#   nm: Apply nm options on result.
+#   readelf: Apply readelf options on result.
+#   ld: Don't apply anything on result.  Compare output during linking with
+#     the file containing regexps (which is the second arg, not the third).
+#     Note that this *must* be the first action if it is to be used at all;
+#     in all other cases, any output from the linker during linking is
+#     treated as a sign of an error and FAILs the test.
+#
+#
 set msp430regionprefixtests {
   {"Move main() to .upper.text" "-T msp430.ld --code-region=upper"
     "" "" {main-with-text-rodata.s} {{objdump -d main-text-upper.d}} "main-upper"}
@@ -136,6 +150,26 @@  set msp430eithershuffletests {
     {{objdump -D main-const-upper.d}} "either-to-upper-const-unique-sec"}
 }
 
+set msp430warntests {
+    {"Warn when section cannot be transformed because output section does not exist in linker script (text,data,bss,rodata)"
+        "-T msp430-no-lower.ld --code-region=lower --data-region=lower" "" "" {warn-no-lower.s}
+        {{ld warn-no-lower.r}} "warn-no-lower"}
+    {"Warn when section cannot be transformed because output section does not exist in linker script (text only)"
+        "-T msp430-no-lower.ld --code-region=lower" "" "" {warn-no-lower.s}
+        {{ld warn-no-lower-code.r}} "warn-no-lower-code"}
+    {"Warn when section cannot be transformed because output section does not exist in linker script (data,bss,rodata)"
+        "-T msp430-no-lower.ld --data-region=lower" "" "" {warn-no-lower.s}
+        {{ld warn-no-lower-data.r}} "warn-no-lower-data"}
+}
+
+# Don't run section shuffle tests when msp430 ISA is selected
+if {[string match "*-mcpu=msp430 *" [board_info [target_info name] multilib_flags]]
+  || [string match "*-mcpu=msp430" [board_info [target_info name] multilib_flags]]} {
+    return
+}
 run_ld_link_tests $msp430regionprefixtests
 run_ld_link_tests $msp430regionprefixuniquesectiontests
 run_ld_link_tests $msp430eithershuffletests
+run_ld_link_tests $msp430warntests
+
+run_dump_test valid-map
diff --git a/ld/testsuite/ld-msp430-elf/valid-map-foo.s b/ld/testsuite/ld-msp430-elf/valid-map-foo.s
new file mode 100644
index 0000000000..329613965c
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/valid-map-foo.s
@@ -0,0 +1,26 @@ 
+	.file	"foo.c"
+.text
+	.section	.text.foo1,"ax",@progbits
+	.balign 2
+	.global	foo1
+	.type	foo1, @function
+foo1:
+; start of function
+; framesize_regs:     0
+; framesize_locals:   0
+; framesize_outgoing: 0
+; framesize:          0
+; elim ap -> fp       2
+; elim fp -> sp       0
+; saved regs:(none)
+	; start of prologue
+	; end of prologue
+	NOP
+.L2:
+	MOV.W	&a, R12
+	CMP.W	#0, R12 { JNE	.L2
+	MOV.B	#0, R12
+	; start of epilogue
+	RET
+	.size	foo1, .-foo1
+	.ident	"GCC: (jozef) 7.3.2"
diff --git a/ld/testsuite/ld-msp430-elf/valid-map.d b/ld/testsuite/ld-msp430-elf/valid-map.d
new file mode 100644
index 0000000000..cb82406a88
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/valid-map.d
@@ -0,0 +1,6 @@ 
+# source: valid-map-foo.s
+# source: valid-map.s
+# ld: -Map=valid-map.map --code-region=lower --script=msp430.ld
+# map: valid-map.map
+
+#pass
diff --git a/ld/testsuite/ld-msp430-elf/valid-map.map b/ld/testsuite/ld-msp430-elf/valid-map.map
new file mode 100644
index 0000000000..39e39798ef
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/valid-map.map
@@ -0,0 +1,6 @@ 
+# Test that the transformed section name foo1 from foo.s appears correctly
+# in the map file
+
+#...
+ .lower.text.foo1
+#pass
diff --git a/ld/testsuite/ld-msp430-elf/valid-map.s b/ld/testsuite/ld-msp430-elf/valid-map.s
new file mode 100644
index 0000000000..feeb50362f
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/valid-map.s
@@ -0,0 +1,60 @@ 
+	.file	"tester.c"
+.text
+	.global	a
+.data
+	.balign 2
+	.type	a, @object
+	.size	a, 2
+a:
+	.short	5
+.text
+	.balign 2
+	.global	foo
+	.type	foo, @function
+foo:
+; start of function
+; framesize_regs:     0
+; framesize_locals:   2
+; framesize_outgoing: 0
+; framesize:          2
+; elim ap -> fp       2
+; elim fp -> sp       2
+; saved regs:(none)
+	; start of prologue
+	SUB.W	#2, R1
+	; end of prologue
+	MOV.W	R12, @R1
+	MOV.W	@R1, R12
+	ADD.W	#-2, R12
+	MOV.W	@R12, R12
+	CMP.W	#0, R12 { JEQ	.L2
+	MOV.B	#0, R12
+	BR	#.L3
+.L2:
+	MOV.B	#1, R12
+.L3:
+	; start of epilogue
+	ADD.W	#2, R1
+	RET
+	.size	foo, .-foo
+	.balign 2
+	.global	main
+	.type	main, @function
+main:
+; start of function
+; framesize_regs:     0
+; framesize_locals:   0
+; framesize_outgoing: 0
+; framesize:          0
+; elim ap -> fp       2
+; elim fp -> sp       0
+; saved regs:(none)
+	; start of prologue
+	; end of prologue
+	MOV.W	#a, R12
+	CALL	#foo
+	; start of epilogue
+	.refsym	__crt0_call_exit
+	RET
+	.size	main, .-main
+	.ident	"GCC: (jozef) 7.3.2"
diff --git a/ld/testsuite/ld-msp430-elf/warn-no-lower-code.r b/ld/testsuite/ld-msp430-elf/warn-no-lower-code.r
new file mode 100644
index 0000000000..de05e77691
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/warn-no-lower-code.r
@@ -0,0 +1 @@ 
+.*warning: no input section rule matches .lower.text in linker script
diff --git a/ld/testsuite/ld-msp430-elf/warn-no-lower-data.r b/ld/testsuite/ld-msp430-elf/warn-no-lower-data.r
new file mode 100644
index 0000000000..1c816121f0
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/warn-no-lower-data.r
@@ -0,0 +1,3 @@ 
+.*warning: no input section rule matches .lower.data in linker script
+.*warning: no input section rule matches .lower.bss in linker script
+.*warning: no input section rule matches .lower.rodata in linker script
diff --git a/ld/testsuite/ld-msp430-elf/warn-no-lower.r b/ld/testsuite/ld-msp430-elf/warn-no-lower.r
new file mode 100644
index 0000000000..53c82db9b6
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/warn-no-lower.r
@@ -0,0 +1,4 @@ 
+.*warning: no input section rule matches .lower.text in linker script
+.*warning: no input section rule matches .lower.data in linker script
+.*warning: no input section rule matches .lower.bss in linker script
+.*warning: no input section rule matches .lower.rodata in linker script
diff --git a/ld/testsuite/ld-msp430-elf/warn-no-lower.s b/ld/testsuite/ld-msp430-elf/warn-no-lower.s
new file mode 100644
index 0000000000..4c8e20d79b
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/warn-no-lower.s
@@ -0,0 +1,44 @@ 
+	.file	"tester.c"
+.text
+	.global	foo
+	.section	.rodata
+.LC0:
+	.string	"bar"
+	.section	.data,"aw",@progbits
+	.balign 2
+	.type	foo, @object
+	.size	foo, 2
+foo:
+	.short	.LC0
+	.section	.text,"ax",@progbits
+	.balign 2
+	.global	main
+	.type	main, @function
+main:
+; start of function
+; framesize_regs:     0
+; framesize_locals:   2
+; framesize_outgoing: 0
+; framesize:          2
+; elim ap -> fp       2
+; elim fp -> sp       2
+; saved regs:(none)
+	; start of prologue
+	SUB.W	#2, R1
+	; end of prologue
+	MOV.W	#1, @R1
+	BR	#.L2
+.L3:
+	MOV.W	&foo, R12
+	ADD.W	#-1, R12
+	MOV.W	R12, &foo
+.L2:
+	MOV.W	@R1, R12
+	CMP.W	#0, R12 { JNE	.L3
+	MOV.B	#0, R12
+	; start of epilogue
+	.refsym	__crt0_call_exit
+	ADD.W	#2, R1
+	RET
+	.size	main, .-main
+	.ident	"GCC: (jozef) 7.3.2"
-- 
2.17.1