public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][Arm][gas]: Fix forward thumb references [PR gas/25235]
@ 2021-05-25 12:03 Tamar Christina
  2021-05-25 12:24 ` Nick Clifton
  0 siblings, 1 reply; 2+ messages in thread
From: Tamar Christina @ 2021-05-25 12:03 UTC (permalink / raw)
  To: binutils; +Cc: nd, Richard.Earnshaw, nickc, ramana.radhakrishnan

[-- Attachment #1: Type: text/plain, Size: 4060 bytes --]

Hi All,

When assembling a forward reference the symbol will be unknown and so during
do_t_adr we cannot set the thumb bit.  The bit it set so early to prevent
relaxations that are invalid. i.e. relaxing a Thumb2 to Thumb1 insn when the
symbol is Thumb.

But because it's done so early we miss the case for forward references.
This patch changes it so that we additionally check the thumb bit during the
internal relocation processing.

In principle we should be able to only set the bit during reloc processing but
that would require changes to the other relocations that the instruction could
be relaxed to.

This approach still allows early relaxations (which means that we have less
iteration of internal reloc processing) while still fixing the forward reference
case.

build on native hardware and regtested on
  arm-none-elf, arm-none-elf (32 bit host),
  arm-none-linux-gnueabihf, arm-none-linux-gnueabihf (32 bit host)

Cross-compiled and regtested on
  arm-none-linux-gnueabihf, armeb-none-elf, arm-wince-pe

and no issues.

Ok for master? and for backport to binutils-2.36?

Thanks,
Tamar

gas/ChangeLog:

2021-05-25  Tamar Christina  <tamar.christina@arm.com>

	PR gas/25235
	* config/tc-arm.c (md_convert_frag): Set LSB when Thumb symbol.
	(relax_adr): Thumb symbols 4 bytes.
	* testsuite/gas/arm/pr25235.d: New test.
	* testsuite/gas/arm/pr25235.s: New test.

--- inline copy of patch -- 
diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 9229dd081589228997a135117053697db049a370..1e2ac65d422cb7829cde5c9f03bcc57ef297e2c5 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -26827,6 +26827,14 @@ md_convert_frag (bfd *abfd, segT asec ATTRIBUTE_UNUSED, fragS *fragp)
       pc_rel = (opcode == T_MNEM_ldr_pc2);
       break;
     case T_MNEM_adr:
+      /* Thumb bits should be set in the frag handling so we process them
+	 after all symbols have been seen.  PR gas/25235.  */
+      if (exp.X_op == O_symbol
+	  && exp.X_add_symbol != NULL
+	  && S_IS_DEFINED (exp.X_add_symbol)
+	  && THUMB_IS_FUNC (exp.X_add_symbol))
+	exp.X_add_number |= 1;
+
       if (fragp->fr_var == 4)
 	{
 	  insn = THUMB_OP32 (opcode);
@@ -27024,7 +27032,8 @@ relax_adr (fragS *fragp, asection *sec, long stretch)
   if (fragp->fr_symbol == NULL
       || !S_IS_DEFINED (fragp->fr_symbol)
       || sec != S_GET_SEGMENT (fragp->fr_symbol)
-      || S_IS_WEAK (fragp->fr_symbol))
+      || S_IS_WEAK (fragp->fr_symbol)
+      || THUMB_IS_FUNC (fragp->fr_symbol))
     return 4;
 
   val = relaxed_symbol_addr (fragp, stretch);
diff --git a/gas/testsuite/gas/arm/pr25235.d b/gas/testsuite/gas/arm/pr25235.d
new file mode 100644
index 0000000000000000000000000000000000000000..126950387014335afaf4a60b6292ab64cdd303f6
--- /dev/null
+++ b/gas/testsuite/gas/arm/pr25235.d
@@ -0,0 +1,24 @@
+#skip: *-*-pe *-*-wince *-*-vxworks
+#objdump: -dr
+#name: PR25235: Thumb forward references error
+
+.*: +file format .*arm.*
+
+Disassembly of section .text:
+
+00000000 <f1>:
+   0:	46c0      	nop			; \(mov r8, r8\)
+   2:	46c0      	nop			; \(mov r8, r8\)
+
+00000004 <f2>:
+   4:	f2af 0107 	subw	r1, pc, #7
+   8:	f20f 0305 	addw	r3, pc, #5
+   c:	a401      	add	r4, pc, #4	; \(adr r4, 14 <f4>\)
+   e:	46c0      	nop			; \(mov r8, r8\)
+
+00000010 <f3>:
+  10:	46c0      	nop			; \(mov r8, r8\)
+  12:	46c0      	nop			; \(mov r8, r8\)
+
+00000014 <f4>:
+  14:	e1a00000 	nop			; \(mov r0, r0\)
diff --git a/gas/testsuite/gas/arm/pr25235.s b/gas/testsuite/gas/arm/pr25235.s
new file mode 100644
index 0000000000000000000000000000000000000000..77637392f1c1b95e85ff2aeaf4a68507d4e466a9
--- /dev/null
+++ b/gas/testsuite/gas/arm/pr25235.s
@@ -0,0 +1,30 @@
+    .syntax unified
+    .thumb
+
+    .align 2
+    .type f1, %function
+    .thumb_func
+    f1:
+        nop
+
+    .align 2
+    .type f2, %function
+    .thumb_func
+    f2:
+        adr r1, f1
+        adr r3, f3
+        adr r4, f4
+
+
+    .align 2
+    .type f3, %function
+    .thumb_func
+    f3:
+        nop
+
+    .align 2
+    .type f3, %function
+    .arm
+    f4:
+        nop
+


-- 

[-- Attachment #2: rb14501.patch --]
[-- Type: text/x-diff, Size: 2652 bytes --]

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 9229dd081589228997a135117053697db049a370..1e2ac65d422cb7829cde5c9f03bcc57ef297e2c5 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -26827,6 +26827,14 @@ md_convert_frag (bfd *abfd, segT asec ATTRIBUTE_UNUSED, fragS *fragp)
       pc_rel = (opcode == T_MNEM_ldr_pc2);
       break;
     case T_MNEM_adr:
+      /* Thumb bits should be set in the frag handling so we process them
+	 after all symbols have been seen.  PR gas/25235.  */
+      if (exp.X_op == O_symbol
+	  && exp.X_add_symbol != NULL
+	  && S_IS_DEFINED (exp.X_add_symbol)
+	  && THUMB_IS_FUNC (exp.X_add_symbol))
+	exp.X_add_number |= 1;
+
       if (fragp->fr_var == 4)
 	{
 	  insn = THUMB_OP32 (opcode);
@@ -27024,7 +27032,8 @@ relax_adr (fragS *fragp, asection *sec, long stretch)
   if (fragp->fr_symbol == NULL
       || !S_IS_DEFINED (fragp->fr_symbol)
       || sec != S_GET_SEGMENT (fragp->fr_symbol)
-      || S_IS_WEAK (fragp->fr_symbol))
+      || S_IS_WEAK (fragp->fr_symbol)
+      || THUMB_IS_FUNC (fragp->fr_symbol))
     return 4;
 
   val = relaxed_symbol_addr (fragp, stretch);
diff --git a/gas/testsuite/gas/arm/pr25235.d b/gas/testsuite/gas/arm/pr25235.d
new file mode 100644
index 0000000000000000000000000000000000000000..126950387014335afaf4a60b6292ab64cdd303f6
--- /dev/null
+++ b/gas/testsuite/gas/arm/pr25235.d
@@ -0,0 +1,24 @@
+#skip: *-*-pe *-*-wince *-*-vxworks
+#objdump: -dr
+#name: PR25235: Thumb forward references error
+
+.*: +file format .*arm.*
+
+Disassembly of section .text:
+
+00000000 <f1>:
+   0:	46c0      	nop			; \(mov r8, r8\)
+   2:	46c0      	nop			; \(mov r8, r8\)
+
+00000004 <f2>:
+   4:	f2af 0107 	subw	r1, pc, #7
+   8:	f20f 0305 	addw	r3, pc, #5
+   c:	a401      	add	r4, pc, #4	; \(adr r4, 14 <f4>\)
+   e:	46c0      	nop			; \(mov r8, r8\)
+
+00000010 <f3>:
+  10:	46c0      	nop			; \(mov r8, r8\)
+  12:	46c0      	nop			; \(mov r8, r8\)
+
+00000014 <f4>:
+  14:	e1a00000 	nop			; \(mov r0, r0\)
diff --git a/gas/testsuite/gas/arm/pr25235.s b/gas/testsuite/gas/arm/pr25235.s
new file mode 100644
index 0000000000000000000000000000000000000000..77637392f1c1b95e85ff2aeaf4a68507d4e466a9
--- /dev/null
+++ b/gas/testsuite/gas/arm/pr25235.s
@@ -0,0 +1,30 @@
+    .syntax unified
+    .thumb
+
+    .align 2
+    .type f1, %function
+    .thumb_func
+    f1:
+        nop
+
+    .align 2
+    .type f2, %function
+    .thumb_func
+    f2:
+        adr r1, f1
+        adr r3, f3
+        adr r4, f4
+
+
+    .align 2
+    .type f3, %function
+    .thumb_func
+    f3:
+        nop
+
+    .align 2
+    .type f3, %function
+    .arm
+    f4:
+        nop
+


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH][Arm][gas]: Fix forward thumb references [PR gas/25235]
  2021-05-25 12:03 [PATCH][Arm][gas]: Fix forward thumb references [PR gas/25235] Tamar Christina
@ 2021-05-25 12:24 ` Nick Clifton
  0 siblings, 0 replies; 2+ messages in thread
From: Nick Clifton @ 2021-05-25 12:24 UTC (permalink / raw)
  To: Tamar Christina, binutils; +Cc: nd, Richard.Earnshaw, ramana.radhakrishnan

Hi Tamar,


> Ok for master? and for backport to binutils-2.36?

Approved for both.  Please apply.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-05-25 12:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 12:03 [PATCH][Arm][gas]: Fix forward thumb references [PR gas/25235] Tamar Christina
2021-05-25 12:24 ` Nick Clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).