diff mbox series

gas: pru: Fix trailing whitespace handling

Message ID 20240814202521.1066938-1-dimitar@dinux.eu
State New
Headers
Series gas: pru: Fix trailing whitespace handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed

Commit Message

Dimitar Dimitrov Aug. 14, 2024, 8:25 p.m. UTC 
  With commit 6ae8a30d44f016cafb46a75843b5109316eb1996, arguments followed
by a C-style comment ended up with a trailing space.  That extra space
character confused the PRU register name matching, leading to spurious
errors about unrecognized registers.  This affected existing code like
newlib's setjmp.s for pru.

Fix by stripping the trailing whitespace for any argument.

Even with 6ae8a30d44f016cafb46a75843b5109316eb1996 reverted, this patch
is safe to be applied.

Successfully regression-tested with GCC and newlib testsuites for pru-unknown-elf.

Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
---
 gas/config/tc-pru.c             |  9 ++++++++-
 gas/testsuite/gas/pru/pr32073.d | 11 +++++++++++
 gas/testsuite/gas/pru/pr32073.s |  6 ++++++
 3 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 gas/testsuite/gas/pru/pr32073.d
 create mode 100644 gas/testsuite/gas/pru/pr32073.s

Comments

Jan Beulich Aug. 15, 2024, 8:15 a.m. UTC | #1 
On 14.08.2024 22:25, Dimitar Dimitrov wrote:
> With commit 6ae8a30d44f016cafb46a75843b5109316eb1996, arguments followed
> by a C-style comment ended up with a trailing space.  That extra space
> character confused the PRU register name matching, leading to spurious
> errors about unrecognized registers.  This affected existing code like
> newlib's setjmp.s for pru.
> 
> Fix by stripping the trailing whitespace for any argument.
> 
> Even with 6ae8a30d44f016cafb46a75843b5109316eb1996 reverted, this patch
> is safe to be applied.
> 
> Successfully regression-tested with GCC and newlib testsuites for pru-unknown-elf.
> 
> Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>

Okay, albeit preferably with ...

> --- a/gas/config/tc-pru.c
> +++ b/gas/config/tc-pru.c
> @@ -1400,7 +1400,7 @@ pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
>  {
>    char *p;
>    char *end = NULL;
> -  int i;
> +  int i, len;
>    p = argstr;
>    i = 0;
>    bool terminate = false;
> @@ -1437,6 +1437,13 @@ pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
>  	    as_bad (_("too many arguments"));
>  	}
>  
> +      /* Strip trailing whitespace.  */
> +      len = strlen (parsed_args[i]);

... len's type changed to size_t for the use here to be appropriate.

Jan
Dimitar Dimitrov Aug. 15, 2024, 3:55 p.m. UTC | #2 
On Thu, Aug 15, 2024 at 10:15:36AM +0200, Jan Beulich wrote:
> On 14.08.2024 22:25, Dimitar Dimitrov wrote:
> > With commit 6ae8a30d44f016cafb46a75843b5109316eb1996, arguments followed
> > by a C-style comment ended up with a trailing space.  That extra space
> > character confused the PRU register name matching, leading to spurious
> > errors about unrecognized registers.  This affected existing code like
> > newlib's setjmp.s for pru.
> > 
> > Fix by stripping the trailing whitespace for any argument.
> > 
> > Even with 6ae8a30d44f016cafb46a75843b5109316eb1996 reverted, this patch
> > is safe to be applied.
> > 
> > Successfully regression-tested with GCC and newlib testsuites for pru-unknown-elf.
> > 
> > Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
> 
> Okay, albeit preferably with ...
> 
> > --- a/gas/config/tc-pru.c
> > +++ b/gas/config/tc-pru.c
> > @@ -1400,7 +1400,7 @@ pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
> >  {
> >    char *p;
> >    char *end = NULL;
> > -  int i;
> > +  int i, len;
> >    p = argstr;
> >    i = 0;
> >    bool terminate = false;
> > @@ -1437,6 +1437,13 @@ pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
> >  	    as_bad (_("too many arguments"));
> >  	}
> >  
> > +      /* Strip trailing whitespace.  */
> > +      len = strlen (parsed_args[i]);
> 
> ... len's type changed to size_t for the use here to be appropriate.

Thanks. Pushed with the suggested change.

Regards,
Dimitar
> 
> Jan
diff mbox series

Patch

diff --git a/gas/config/tc-pru.c b/gas/config/tc-pru.c
index 99a3c1ef88c..f0bbc5b17ef 100644
--- a/gas/config/tc-pru.c
+++ b/gas/config/tc-pru.c
@@ -1400,7 +1400,7 @@  pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
 {
   char *p;
   char *end = NULL;
-  int i;
+  int i, len;
   p = argstr;
   i = 0;
   bool terminate = false;
@@ -1437,6 +1437,13 @@  pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
 	    as_bad (_("too many arguments"));
 	}
 
+      /* Strip trailing whitespace.  */
+      len = strlen (parsed_args[i]);
+      for (char *temp = parsed_args[i] + len - 1;
+	   len && ISSPACE (*temp);
+	   temp--, len--)
+	*temp = '\0';
+
       if (*parsestr == '\0' || (p != NULL && *p == '\0'))
 	terminate = true;
       ++i;
diff --git a/gas/testsuite/gas/pru/pr32073.d b/gas/testsuite/gas/pru/pr32073.d
new file mode 100644
index 00000000000..ac353bd8209
--- /dev/null
+++ b/gas/testsuite/gas/pru/pr32073.d
@@ -0,0 +1,11 @@ 
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: whitespace parsing
+
+# Test the whitespace parsing
+
+.*: +file format elf32-pru
+
+Disassembly of section .text:
+0+0000 <[^>]*> 1300e2e1 	mov	r1, sp
+0+0004 <[^>]*> 1300e2e1 	mov	r1, sp
+0+0008 <[^>]*> 1300e2e1 	mov	r1, sp
diff --git a/gas/testsuite/gas/pru/pr32073.s b/gas/testsuite/gas/pru/pr32073.s
new file mode 100644
index 00000000000..ebfab2367a6
--- /dev/null
+++ b/gas/testsuite/gas/pru/pr32073.s
@@ -0,0 +1,6 @@ 
+# Source file used to test the whitespace parsing.
+
+foo:
+	mov	r1 /* comment */, r2	/* ... */
+	mov	r1	/* comment */, r2	/* ... */
+	mov	/* x */ r1	/* comment */ , r2	/* ... */