[PATCH] gas: Fix checking for backwards .org with negative offset

Alex Coplan alex.coplan@arm.com
Fri May 29 15:04:50 GMT 2020


Hello,

This patch fixes internal errors in (at least) arm and aarch64 GAS when
assembling code that attempts a negative .org. For example, assembling:

.=-1
nop

would give:

test.s: Assembler messages:
test.s:1: Error: attempt to .org/.space/.nops backwards? (-1)
test.s: Internal error in check_mapping_symbols at ../../../binutils-gdb/gas/config/tc-aarch64.c:8557.
Please report this bug.

on aarch64, and:

test.s: Assembler messages:
test.s:1: Error: attempt to .org/.space/.nops backwards? (-1)
test.s: Internal error in make_mapping_symbol at ../../../../src/binutils-gdb/gas/config/tc-arm.c:3017.
Please report this bug.

on arm.

The bug appears to be a regression introduced in binutils-2.29 by this
commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=9875b36538d35f2292ddc3bb5e7c60e1582aa087

The problem is in the rs_org case of the main switch statement in
gas/write.c:relax_segment(). This code assigns the signed variable
`offset' to the unsigned variable `target' and then proceeds to do an
unsigned comparison:

  if (address + fragP->fr_fix > target)

which fails in this case because `offset' was -1 and so `target' wrapped
to become UINT64_MAX.

This patch uses a signed type for `target' and casts the LHS to ensure a
signed comparison is used here.

Testing:
 * New tests for arm and aarch64.
 * Regression tested on a wide variety of targets.
 * Bootstrapped on aarch64.

Ok for master and backport to binutils-2.34?

Thanks,
Alex

---

gas/ChangeLog:

2020-05-29  Alex Coplan  <alex.coplan@arm.com>

	* testsuite/gas/aarch64/org-neg.d: New test.
	* testsuite/gas/aarch64/org-neg.l: Error output for test.
	* testsuite/gas/aarch64/org-neg.s: Input for test.
	* testsuite/gas/arm/org-neg.d: New test.
	* testsuite/gas/arm/org-neg.l: Error output for test.
	* testsuite/gas/arm/org-neg.s: Input for test.
	* write.c (relax_segment): Fix handling of negative offset when
	  relaxing an rs_org frag.
-------------- next part --------------
diff --git a/gas/testsuite/gas/aarch64/org-neg.d b/gas/testsuite/gas/aarch64/org-neg.d
new file mode 100644
index 0000000000..83e6af6afb
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/org-neg.d
@@ -0,0 +1,3 @@
+#name: negative org should not cause internal error
+#source: org-neg.s
+#error_output: org-neg.l
diff --git a/gas/testsuite/gas/aarch64/org-neg.l b/gas/testsuite/gas/aarch64/org-neg.l
new file mode 100644
index 0000000000..f8414adc19
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/org-neg.l
@@ -0,0 +1,2 @@
+[^:]*: Assembler messages:
+.*: Error: attempt to move .org backwards
diff --git a/gas/testsuite/gas/aarch64/org-neg.s b/gas/testsuite/gas/aarch64/org-neg.s
new file mode 100644
index 0000000000..403e70d5e0
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/org-neg.s
@@ -0,0 +1,2 @@
+.=-1
+ret
diff --git a/gas/testsuite/gas/arm/org-neg.d b/gas/testsuite/gas/arm/org-neg.d
new file mode 100644
index 0000000000..83e6af6afb
--- /dev/null
+++ b/gas/testsuite/gas/arm/org-neg.d
@@ -0,0 +1,3 @@
+#name: negative org should not cause internal error
+#source: org-neg.s
+#error_output: org-neg.l
diff --git a/gas/testsuite/gas/arm/org-neg.l b/gas/testsuite/gas/arm/org-neg.l
new file mode 100644
index 0000000000..f8414adc19
--- /dev/null
+++ b/gas/testsuite/gas/arm/org-neg.l
@@ -0,0 +1,2 @@
+[^:]*: Assembler messages:
+.*: Error: attempt to move .org backwards
diff --git a/gas/testsuite/gas/arm/org-neg.s b/gas/testsuite/gas/arm/org-neg.s
new file mode 100644
index 0000000000..f60486e2c8
--- /dev/null
+++ b/gas/testsuite/gas/arm/org-neg.s
@@ -0,0 +1,2 @@
+.=-1
+nop
diff --git a/gas/write.c b/gas/write.c
index 5825117548..398c50bec0 100644
--- a/gas/write.c
+++ b/gas/write.c
@@ -2940,7 +2940,7 @@ relax_segment (struct frag *segment_frag_root, segT segment, int pass)
 
 	      case rs_org:
 		{
-		  addressT target = offset;
+		  offsetT target = offset;
 		  addressT after;
 
 		  if (symbolP)
@@ -2960,7 +2960,7 @@ relax_segment (struct frag *segment_frag_root, segT segment, int pass)
 		  /* Growth may be negative, but variable part of frag
 		     cannot have fewer than 0 chars.  That is, we can't
 		     .org backwards.  */
-		  if (address + fragP->fr_fix > target)
+		  if ((offsetT)(address + fragP->fr_fix) > target)
 		    {
 		      growth = 0;
 


More information about the Binutils mailing list