* [PATCH] gas: Fix checking for backwards .org with negative offset
@ 2020-05-29 15:04 Alex Coplan
2020-05-31 22:52 ` Alan Modra
0 siblings, 1 reply; 5+ messages in thread
From: Alex Coplan @ 2020-05-29 15:04 UTC (permalink / raw)
To: binutils; +Cc: nd, rearnsha, marcus.shawcroft, nickc, ramana.radhakrishnan
[-- Attachment #1: Type: text/plain, Size: 1934 bytes --]
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.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2281 bytes --]
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;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gas: Fix checking for backwards .org with negative offset
2020-05-29 15:04 [PATCH] gas: Fix checking for backwards .org with negative offset Alex Coplan
@ 2020-05-31 22:52 ` Alan Modra
2020-06-01 8:06 ` Alex Coplan
0 siblings, 1 reply; 5+ messages in thread
From: Alan Modra @ 2020-05-31 22:52 UTC (permalink / raw)
To: Alex Coplan
Cc: binutils, ramana.radhakrishnan, nd, rearnsha, marcus.shawcroft
On Fri, May 29, 2020 at 04:04:50PM +0100, Alex Coplan wrote:
> * 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.
This is OK everywhere with the formatting fixed.
> - if (address + fragP->fr_fix > target)
> + if ((offsetT)(address + fragP->fr_fix) > target)
if ((offsetT) (address + fragP->fr_fix) > target)
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] gas: Fix checking for backwards .org with negative offset
2020-05-31 22:52 ` Alan Modra
@ 2020-06-01 8:06 ` Alex Coplan
2020-06-09 14:06 ` Alex Coplan
0 siblings, 1 reply; 5+ messages in thread
From: Alex Coplan @ 2020-06-01 8:06 UTC (permalink / raw)
To: Alan Modra
Cc: binutils, Ramana Radhakrishnan, nd, Richard Earnshaw, Marcus Shawcroft
> -----Original Message-----
> From: Alan Modra <amodra@gmail.com>
> Sent: 31 May 2020 23:52
> To: Alex Coplan <Alex.Coplan@arm.com>
> Cc: binutils@sourceware.org; Ramana Radhakrishnan
> <Ramana.Radhakrishnan@arm.com>; nd <nd@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft <Marcus.Shawcroft@arm.com>
> Subject: Re: [PATCH] gas: Fix checking for backwards .org with negative
> offset
>
> On Fri, May 29, 2020 at 04:04:50PM +0100, Alex Coplan wrote:
> > * 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.
>
> This is OK everywhere with the formatting fixed.
>
> > - if (address + fragP->fr_fix > target)
> > + if ((offsetT)(address + fragP->fr_fix) > target)
>
> if ((offsetT) (address + fragP->fr_fix) > target)
>
Hi Alan,
Thanks. I should have mentioned in my original email that I don't have
commit access so I'll need a maintainer to commit on my behalf.
Alex
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] gas: Fix checking for backwards .org with negative offset
2020-06-01 8:06 ` Alex Coplan
@ 2020-06-09 14:06 ` Alex Coplan
2020-06-10 5:59 ` Alan Modra
0 siblings, 1 reply; 5+ messages in thread
From: Alex Coplan @ 2020-06-09 14:06 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
> -----Original Message-----
> From: Alex Coplan
> Sent: 01 June 2020 09:07
> To: Alan Modra <amodra@gmail.com>
> Cc: binutils@sourceware.org; Ramana Radhakrishnan
> <Ramana.Radhakrishnan@arm.com>; nd <nd@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft <Marcus.Shawcroft@arm.com>
> Subject: RE: [PATCH] gas: Fix checking for backwards .org with negative
> offset
>
> > -----Original Message-----
> > From: Alan Modra <amodra@gmail.com>
> > Sent: 31 May 2020 23:52
> > To: Alex Coplan <Alex.Coplan@arm.com>
> > Cc: binutils@sourceware.org; Ramana Radhakrishnan
> > <Ramana.Radhakrishnan@arm.com>; nd <nd@arm.com>; Richard Earnshaw
> > <Richard.Earnshaw@arm.com>; Marcus Shawcroft <Marcus.Shawcroft@arm.com>
> > Subject: Re: [PATCH] gas: Fix checking for backwards .org with negative
> > offset
> >
> > On Fri, May 29, 2020 at 04:04:50PM +0100, Alex Coplan wrote:
> > > * 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.
> >
> > This is OK everywhere with the formatting fixed.
> >
> > > - if (address + fragP->fr_fix > target)
> > > + if ((offsetT)(address + fragP->fr_fix) > target)
> >
> > if ((offsetT) (address + fragP->fr_fix) > target)
> >
>
> Hi Alan,
>
> Thanks. I should have mentioned in my original email that I don't have
> commit access so I'll need a maintainer to commit on my behalf.
>
> Alex
Does this also need backporting to binutils-2.34? If so, I'll need a
maintainer to do it on my behalf.
Thanks,
Alex
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gas: Fix checking for backwards .org with negative offset
2020-06-09 14:06 ` Alex Coplan
@ 2020-06-10 5:59 ` Alan Modra
0 siblings, 0 replies; 5+ messages in thread
From: Alan Modra @ 2020-06-10 5:59 UTC (permalink / raw)
To: Alex Coplan; +Cc: binutils
On Tue, Jun 09, 2020 at 02:06:41PM +0000, Alex Coplan wrote:
> Does this also need backporting to binutils-2.34? If so, I'll need a
> maintainer to do it on my behalf.
Yes, I think patch could be safely backported. I'll commit it after
running regression tests on the 2.34 branch.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-10 5:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29 15:04 [PATCH] gas: Fix checking for backwards .org with negative offset Alex Coplan
2020-05-31 22:52 ` Alan Modra
2020-06-01 8:06 ` Alex Coplan
2020-06-09 14:06 ` Alex Coplan
2020-06-10 5:59 ` Alan Modra
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).