gas/expr.c: 0b vs 0b0 vs 00b

gas/expr.c: 0b vs 0b0 vs 00b

[DJ Delorie]

For targets with both NUMBERS_WITH_SUFFIX and LOCAL_LABELS_FB, there
is a parse ambiquity between various binary constants:

0b	backward reference
1b	binary value, suffix
0b1	binary value, prefix

The third will be parsed as 0b followed by "junk".  This patch checks
for 0b followed by further binary digits and bypasses the b-as-suffix
rule.

I don't think a generic test case would work, since it would have to
test expressions that are only valid on some targets.  Would one
target's test suffice, or should the test be duplicated across many
targets?

diff --git a/gas/expr.c b/gas/expr.c
index 106f06d..2237c02 100644
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -854,7 +854,8 @@ operand (expressionS *expressionP, enum expr_mode mode)
 	  /* Fall through.  */
 	case 'B':
 	  input_line_pointer++;
-	  if (flag_m68k_mri || NUMBERS_WITH_SUFFIX)
+	  if ((flag_m68k_mri || NUMBERS_WITH_SUFFIX)
+	      && (input_line_pointer[0] != '0' && input_line_pointer[0] != '1'))
 	    goto default_case;
 	  integer_constant (2, expressionP);
 	  break;

Re: gas/expr.c: 0b vs 0b0 vs 00b

[Alan Modra]

On Fri, Aug 07, 2015 at 01:39:34AM -0400, DJ Delorie wrote:
> 
> For targets with both NUMBERS_WITH_SUFFIX and LOCAL_LABELS_FB, there
> is a parse ambiquity between various binary constants:
> 
> 0b	backward reference
> 1b	binary value, suffix
> 0b1	binary value, prefix

Actually 1b is a backward reference too.  I think both 0b and 1b
without following binary digits ought to be treated as backward refs
on these targets, since if you want to write a binary 0 or 1 with an
explicit suffix that can be done as 0B or 1B.

> The third will be parsed as 0b followed by "junk".  This patch checks
> for 0b followed by further binary digits and bypasses the b-as-suffix
> rule.
> 
> I don't think a generic test case would work, since it would have to
> test expressions that are only valid on some targets.  Would one
> target's test suffice, or should the test be duplicated across many
> targets?

I think a single target test is sufficient.  I've been playing with
variants of the following.

 .text
_start:
 .byte 0f - _start
 .byte 1f - _start
0:
 .byte 0b - _start
1:
 .byte 1b - _start
 .byte 0b0, 0b1, 0B0, 0B1
 .byte 0B, 1B

Here's a patch that extends yours a little.  What do you think?

    	* expr.c (integer_constant): Return O_absent expression if eol.
    	(operand): For targets with both LOCAL_LABELS_FB and
    	NUMBERS_WITH_SUFFIX set, treat "0b" not followed by binary
    	digits as a local label reference.  Correct handling of 0b prefix.
	If a suffix is not allowed, error on 0B.

diff --git a/gas/expr.c b/gas/expr.c
index 106f06d..2dae6ba 100644
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -285,6 +285,12 @@ integer_constant (int radix, expressionS *expressionP)
 #define valuesize 32
 #endif
 
+  if (is_end_of_line[(unsigned char) *input_line_pointer])
+    {
+      expressionP->X_op = O_absent;
+      return;
+    }
+
   if ((NUMBERS_WITH_SUFFIX || flag_m68k_mri) && radix == 0)
     {
       int flt = 0;
@@ -832,32 +838,28 @@ operand (expressionS *expressionP, enum expr_mode mode)
 	  break;
 
 	case 'b':
-	  if (LOCAL_LABELS_FB && ! (flag_m68k_mri || NUMBERS_WITH_SUFFIX))
+	  if (LOCAL_LABELS_FB && !flag_m68k_mri
+	      && input_line_pointer[1] != '0'
+	      && input_line_pointer[1] != '1')
 	    {
-	      /* This code used to check for '+' and '-' here, and, in
-		 some conditions, fall through to call
-		 integer_constant.  However, that didn't make sense,
-		 as integer_constant only accepts digits.  */
-	      /* Some of our code elsewhere does permit digits greater
-		 than the expected base; for consistency, do the same
-		 here.  */
-	      if (input_line_pointer[1] < '0'
-		  || input_line_pointer[1] > '9')
-		{
-		  /* Parse this as a back reference to label 0.  */
-		  input_line_pointer--;
-		  integer_constant (10, expressionP);
-		  break;
-		}
-	      /* Otherwise, parse this as a binary number.  */
+	      /* Parse this as a back reference to label 0.  */
+	      input_line_pointer--;
+	      integer_constant (10, expressionP);
+	      break;
 	    }
+	  /* Otherwise, parse this as a binary number.  */
 	  /* Fall through.  */
 	case 'B':
-	  input_line_pointer++;
+	  if (input_line_pointer[1] == '0'
+	      || input_line_pointer[1] == '1')
+	    {
+	      input_line_pointer++;
+	      integer_constant (2, expressionP);
+	      break;
+	    }
 	  if (flag_m68k_mri || NUMBERS_WITH_SUFFIX)
-	    goto default_case;
-	  integer_constant (2, expressionP);
-	  break;
+	    input_line_pointer++;
+	  goto default_case;
 
 	case '0':
 	case '1':

-- 
Alan Modra
Australia Development Lab, IBM

Re: gas/expr.c: 0b vs 0b0 vs 00b

[DJ Delorie]

> Actually 1b is a backward reference too.  I think both 0b and 1b
> without following binary digits ought to be treated as backward refs
> on these targets, since if you want to write a binary 0 or 1 with an
> explicit suffix that can be done as 0B or 1B.

Note that the gas docs are vague about this: one spot says local refs
are '1' through '9', another refers to the "first ten single-digit..."

But yeah, I was only trying to special-case the 0b[01]+ case.

> Here's a patch that extends yours a little.  What do you think?

Seems OK to me.

Re: gas/expr.c: 0b vs 0b0 vs 00b

[Alan Modra]

On Wed, Aug 12, 2015 at 06:05:50PM -0400, DJ Delorie wrote:
> 
> > Actually 1b is a backward reference too.  I think both 0b and 1b
> > without following binary digits ought to be treated as backward refs
> > on these targets, since if you want to write a binary 0 or 1 with an
> > explicit suffix that can be done as 0B or 1B.
> 
> Note that the gas docs are vague about this: one spot says local refs
> are '1' through '9', another refers to the "first ten single-digit..."
> 
> But yeah, I was only trying to special-case the 0b[01]+ case.
> 
> > Here's a patch that extends yours a little.  What do you think?
> 
> Seems OK to me.

OK, committing along with this doc fix.

	* doc/as.texinfo (Local Labels): Allowed range of N in local
	labels is non-negative integers, not positive integers.

diff --git a/gas/doc/as.texinfo b/gas/doc/as.texinfo
index dae7424..c2f2f86 100644
--- a/gas/doc/as.texinfo
+++ b/gas/doc/as.texinfo
@@ -3707,11 +3707,11 @@ Local labels are different from local symbols.  Local labels help compilers and
 programmers use names temporarily.  They create symbols which are guaranteed to
 be unique over the entire scope of the input source code and which can be
 referred to by a simple notation.  To define a local label, write a label of
-the form @samp{@b{N}:} (where @b{N} represents any positive integer).  To refer
-to the most recent previous definition of that label write @samp{@b{N}b}, using
-the same number as when you defined the label.  To refer to the next definition
-of a local label, write @samp{@b{N}f}---the @samp{b} stands for ``backwards''
-and the @samp{f} stands for ``forwards''.
+the form @samp{@b{N}:} (where @b{N} represents any non-negative integer).
+To refer to the most recent previous definition of that label write
+@samp{@b{N}b}, using the same number as when you defined the label.  To refer
+to the next definition of a local label, write @samp{@b{N}f}.  The @samp{b}
+stands for ``backwards'' and the @samp{f} stands for ``forwards''.
 
 There is no restriction on how you can use these labels, and you can reuse them
 too.  So that it is possible to repeatedly define the same local label (using

-- 
Alan Modra
Australia Development Lab, IBM