Bug 14887

Summary: Error: ARM register expected -- `str r1,[ r0 ]'
Product: binutils Reporter: Khem Raj <raj.khem>
Component: gasAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: john, mikpelinux, nickc, pbrobinson, roland, will.newton
Priority: P2    
Version: 2.23   
Target Milestone: ---   
Host: Target: arm-none-eabi
Build: Last reconfirmed:

Description Khem Raj 2012-11-28 05:27:40 UTC
We are now regressing on arm and getting the above error after this commit
This happens on both master as well as 2.23 branch

commit 65faec7cb829c58b20a5f26ee2908ac35165fc58
Author: Roland McGrath <roland@gnu.org>
Date:   Tue Nov 20 17:58:28 2012 +0000

    gas/
        * config/tc-arm.c (arm_symbol_chars): New variable.
        * config/tc-arm.h (tc_symbol_chars): New macro, defined to that.

    gas/testsuite/
        * gas/arm/macro-pld.s: New file.
        * gas/arm/macro-pld.d: New file.


Test case is below

a.s
====


.text
str r1,[ r0 ]


$ ./gas/as-new a.s
/home/kraj/a.s: Assembler messages:
/home/kraj/a.s:2: Error: ARM register expected -- `str r1,[ r0 ]'


If I remove spaces before and after r0 the errors goes away. Testcase works well on 2.22 branch and without the above commit.
Comment 1 Mikael Pettersson 2013-01-05 15:53:13 UTC
The problem of whitespace before the closing bracket is PR14987, fixed on head but not on 2.23 branch where it is a regression from the 2.23.1 release.

The problem of whitespace after the opening bracket is not fixed on head, but a patch similar to PR14987's fixes it on both head and 2.23 branch:

diff -u -p -r1.550 tc-arm.c
--- binutils-2.24-2013-01-05/gas/config/tc-arm.c        2 Jan 2013 13:38:55 -0000       1.550
+++ binutils-2.24-2013-01-05/gas/config/tc-arm.c        5 Jan 2013 15:23:42 -0000
@@ -5168,6 +5168,9 @@ parse_address_main (char **str, int i, i
       return PARSE_OPERAND_SUCCESS;
     }
 
+  /* PR gas/14887: Allow for whitespace after the opening bracket.  */
+  skip_whitespace (p);
+
   if ((reg = arm_reg_parse (&p, REG_TYPE_RN)) == FAIL)
     {
       inst.error = _(reg_expected_msgs[REG_TYPE_RN]);
Comment 2 Sourceware Commits 2013-01-07 12:49:17 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2013-01-07 12:49:12

Modified files:
	gas            : ChangeLog 
	gas/config     : tc-arm.c 
	gas/testsuite  : ChangeLog 
	gas/testsuite/gas/arm: neon-ldst-es.s 

Log message:
	PR gas/14887
	* config/tc-arm.c (skip_past_char): Skip whitespace before the
	anticipated character.
	* config/tc-arm.c (parse_address_main): Delete skip of whitespace
	here as it is no longer needed.
	
	PR gas/14887
	* gas/arm/neon-ldst-es.s: Add more whitespace.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/ChangeLog.diff?cvsroot=src&r1=1.4898&r2=1.4899
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/config/tc-arm.c.diff?cvsroot=src&r1=1.550&r2=1.551
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.2165&r2=1.2166
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/neon-ldst-es.s.diff?cvsroot=src&r1=1.4&r2=1.5
Comment 3 Nick Clifton 2013-01-07 12:52:54 UTC
Hi Guys,

  Mikael's patch works, but there are other cases where extra whitespace can cause bogux syntax errors.  So I have extended his patch by adding a call to skip_whitespace() inside skip_past_char().  This seems to take care of all of the cases that I could find.  If any more examples of bogus whitespace syntax errors do show up, feel free to reopen this issue.

Cheers
  Nick
Comment 4 John Tytgat 2013-04-01 18:55:07 UTC
This very same problem is now a regression in the 2.23.2 release, was not an issue for 2.23.1.
Comment 5 Sourceware Commits 2013-06-24 23:36:18 UTC
CVSROOT:	/cvs/src
Module name:	src
Branch: 	binutils-2_23-branch
Changes by:	roland@sourceware.org	2013-06-24 23:36:17

Modified files:
	gas            : ChangeLog 
	gas/config     : tc-arm.c 
	gas/testsuite  : ChangeLog 
	gas/testsuite/gas/arm: neon-ldst-es.s 

Log message:
	gas/
	PR gas/14887
	* config/tc-arm.c (skip_past_char): Skip whitespace before the
	anticipated character.
	* config/tc-arm.c (parse_address_main): Delete skip of whitespace
	here as it is no longer needed.
	
	gas/testsuite/
	PR gas/14887
	* gas/arm/neon-ldst-es.s: Add more whitespace.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/ChangeLog.diff?cvsroot=src&only_with_tag=binutils-2_23-branch&r1=1.4769.2.32&r2=1.4769.2.33
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/config/tc-arm.c.diff?cvsroot=src&only_with_tag=binutils-2_23-branch&r1=1.523.2.8&r2=1.523.2.9
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&only_with_tag=binutils-2_23-branch&r1=1.2057.2.33&r2=1.2057.2.34
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/neon-ldst-es.s.diff?cvsroot=src&only_with_tag=binutils-2_23-branch&r1=1.3.8.1&r2=1.3.8.2
Comment 6 Roland McGrath 2013-06-24 23:38:32 UTC
I've put that fix on the 2.23 branch too now.
Comment 7 Will Newton 2014-07-08 15:04:56 UTC
This issue appears to be resolved with 2.24.