[PATCH] gas: require an operand to .startof.()/.sizeof.()

[PATCH] gas: require an operand to .startof.()/.sizeof.()

[Jan Beulich]

gas/
2017-02-21  Jan Beulich  <jbeulich@suse.com>

	* expr.c (operand): Handle missing operand to .startof.() and
	.sizeof.().
	* testsuite/gas/all/err-sizeof.s: New.

--- 2017-02-21/gas/expr.c
+++ 2017-02-21/gas/expr.c
@@ -1154,6 +1154,10 @@ operand (expressionS *expressionP, enum
 		   || input_line_pointer[1] == 'T');
 	  input_line_pointer += start ? 8 : 7;
 	  SKIP_WHITESPACE ();
+
+	  /* Cover for the as_bad () invocations below.  */
+	  expressionP->X_op = O_absent;
+
 	  if (*input_line_pointer != '(')
 	    as_bad (_("syntax error in .startof. or .sizeof."));
 	  else
@@ -1163,6 +1167,16 @@ operand (expressionS *expressionP, enum
 	      ++input_line_pointer;
 	      SKIP_WHITESPACE ();
 	      c = get_symbol_name (& name);
+	      if (! *name)
+		{
+		  as_bad (_("expected symbol name"));
+		  (void) restore_line_pointer (c);
+		  if (c != ')')
+		    ignore_rest_of_line ();
+		  else
+		    ++input_line_pointer;
+		  break;
+		}
 
 	      buf = concat (start ? ".startof." : ".sizeof.", name,
 			    (char *) NULL);
@@ -1306,6 +1320,14 @@ operand (expressionS *expressionP, enum
 	      SKIP_WHITESPACE_AFTER_NAME ();
 
 	      c = get_symbol_name (& name);
+	      if (! *name)
+		{
+		  as_bad (_("expected symbol name"));
+		  expressionP->X_op = O_absent;
+		  (void) restore_line_pointer (c);
+		  ignore_rest_of_line ();
+		  break;
+		}
 
 	      buf = concat (start ? ".startof." : ".sizeof.", name,
 			    (char *) NULL);
--- 2017-02-21/gas/testsuite/gas/all/err-sizeof.s
+++ 2017-02-21/gas/testsuite/gas/all/err-sizeof.s
@@ -0,0 +1,21 @@
+;# .sizeof. and .startof. operator diagnostics
+;# { dg-do assemble }
+	.long	.sizeof.(a b)		;# { dg-error "Error: syntax error" }
+	.long	.startof.(x y)		;# { dg-error "Error: syntax error" }
+	.long	.sizeof.(a+b)		;# { dg-error "Error: syntax error" }
+	.long	.startof.(x-y)		;# { dg-error "Error: syntax error" }
+	.long	.sizeof.("a+b")
+	.long	.startof.("x-y")
+	.long	.sizeof.()		;# { dg-error "Error: expected symbol name" }
+	.long	.startof.()		;# { dg-error "Error: expected symbol name" }
+;# We don't really care about these, but I didn't find a way to discard
+;# them, and I also don't want to use dg-excess-errors here.
+;# { dg-error "junk at end" "" { target *-*-* } 3 }
+;# { dg-error "junk at end" "junk" { target *-*-* } 4 }
+;# { dg-error "junk at end" "junk" { target *-*-* } 5 }
+;# { dg-error "UND" "undefined" { target *-*-* } 5 }
+;# { dg-error "junk at end" "junk" { target *-*-* } 6 }
+;# { dg-error "UND" "undefined" { target *-*-* } 6 }
+;# { dg-error "too complex" "too complex" { target powerpc*-*-* } 6 }
+;# { dg-warning "zero assumed" "missing" { target *-*-* } 9 }
+;# { dg-warning "zero assumed" "missing" { target *-*-* } 10 }

Re: [PATCH] gas: require an operand to .startof.()/.sizeof.()

[Alan Modra]

On Tue, Feb 21, 2017 at 03:15:10AM -0700, Jan Beulich wrote:
> 	* expr.c (operand): Handle missing operand to .startof.() and
> 	.sizeof.().
> 	* testsuite/gas/all/err-sizeof.s: New.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] gas: require an operand to .startof.()/.sizeof.()

[Hans-Peter Nilsson]

> Date: Tue, 21 Feb 2017 03:15:10 -0700
> From: "Jan Beulich" <JBeulich@suse.com>

> gas/
> 2017-02-21  Jan Beulich  <jbeulich@suse.com>
> 
> 	* expr.c (operand): Handle missing operand to .startof.() and
> 	.sizeof.().
> 	* testsuite/gas/all/err-sizeof.s: New.

Target is cris-axis-linux-gnu
Target is cris-axis-elf
Host is x86_64-pc-linux-gnu but compiling with -m32 (unknown if
that matters, but please consider your undoubtedly large list of
targets tested for your patches).

Running /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/all/gas.exp ...
FAIL: gas/all/err-sizeof.s (test for excess errors)

gas.log:
FAIL: gas/all/err-sizeof.s (test for excess errors)
Excess errors:
/tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/all//err-sizeof.s:6: Error: expression too complex


Committed:
gas/
	* testsuite/gas/all/err-sizeof.s: Include cris*-*-* in the list of
	targets	yielding an error message matching "too complex".

diff --git a/gas/testsuite/gas/all/err-sizeof.s b/gas/testsuite/gas/all/err-sizeof.s
index 4ddb5bd..457856b 100644
--- a/gas/testsuite/gas/all/err-sizeof.s
+++ b/gas/testsuite/gas/all/err-sizeof.s
@@ -16,6 +16,6 @@
 ;# { dg-error "UND" "undefined" { target *-*-* } 5 }
 ;# { dg-error "junk at end" "junk" { target *-*-* } 6 }
 ;# { dg-error "UND" "undefined" { target *-*-* } 6 }
-;# { dg-error "too complex" "too complex" { target powerpc*-*-* } 6 }
+;# { dg-error "too complex" "too complex" { target powerpc*-*-* cris*-*-* } 6 }
 ;# { dg-warning "zero assumed" "missing" { target *-*-* } 9 }
 ;# { dg-warning "zero assumed" "missing" { target *-*-* } 10 }


brgds, H-P

Re: [PATCH] gas: require an operand to .startof.()/.sizeof.()

[Jan Beulich]

>>> On 22.02.17 at 17:31, <nickc@redhat.com> wrote:
>> gas/
>> 2017-02-21  Jan Beulich  <jbeulich@suse.com>
>> 
>> 	* expr.c (operand): Handle missing operand to .startof.() and
>> 	.sizeof.().
>> 	* testsuite/gas/all/err-sizeof.s: New.
> 
> The cris architecture is not the only one affected by this new test.
> The following are also affected:
> 
>  alpha-dec-vms 
>  cr16-elf 
>  crx-elf 
>  d10v-elf 
>  d30v-elf 
>  epiphany-elf 
>  fr30-elf 
>  frv-elf 
>  h8300-elf 
>  h8300-rtems 
>  i386-darwin 
>  ip2k-elf 
>  iq2000-elf 
>  lm32-rtems4.10 
>  m32c-elf 
>  m32r-elf 
>  m68hc11-elf 
>  mep-elf 
>  microblaze-elf 
>  mn10200-elf 
>  mn10300-elf 
>  ms1-elf 
>  msp430-elf 
>  mt-elf 
>  or1k-elf 
>  pdp11-dec-aout 
>  riscv32-elf 
>  riscv64-elf 
>  rl78-elf 
>  rs6000-aix4.3.3 
>  rx-elf 
>  s390-linux 
>  s390x-ibm-tpf 
>  spu-elf 
>  tilegx-linux-gnu 
>  tilegxbe-linux-gnu 
>  tilepro-elf 
>  v850-elf 
>  vax-netbsdelf 
>  xc16x-elf 
>  xgate-elf 
>  xstormy16-elf 
> 
> Given the large number of targets affected, maybe the test itself needs
> to be reconsidered ?  Or maybe split into two tests, one general one,
> and one for targets which do support the difference of two symbols in an
> expression ?

I'll commit the patch below as obvious - using a number as second
operand is good enough for the purpose of the test and should
eliminate all dependencies on target specific behavior.

Jan

gas: slightly relax .startof.()/.sizeof.() testcase

gas/
2017-02-23  Jan Beulich  <jbeulich@suse.com>

	* testsuite/gas/all/err-sizeof.s: Don't use sums or differences
	of symbols as expression.

--- 2017-02-21/gas/testsuite/gas/all/err-sizeof.s
+++ 2017-02-21/gas/testsuite/gas/all/err-sizeof.s
@@ -2,8 +2,8 @@
 ;# { dg-do assemble }
 	.long	.sizeof.(a b)		;# { dg-error "Error: syntax error" }
 	.long	.startof.(x y)		;# { dg-error "Error: syntax error" }
-	.long	.sizeof.(a+b)		;# { dg-error "Error: syntax error" }
-	.long	.startof.(x-y)		;# { dg-error "Error: syntax error" }
+	.long	.sizeof.(a+1)		;# { dg-error "Error: syntax error" }
+	.long	.startof.(x-1)		;# { dg-error "Error: syntax error" }
 	.long	.sizeof.("a+b")
 	.long	.startof.("x-y")
 	.long	.sizeof.()		;# { dg-error "Error: expected symbol name" }
@@ -13,9 +13,6 @@
 ;# { dg-error "junk at end" "" { target *-*-* } 3 }
 ;# { dg-error "junk at end" "junk" { target *-*-* } 4 }
 ;# { dg-error "junk at end" "junk" { target *-*-* } 5 }
-;# { dg-error "UND" "undefined" { target *-*-* } 5 }
 ;# { dg-error "junk at end" "junk" { target *-*-* } 6 }
-;# { dg-error "UND" "undefined" { target *-*-* } 6 }
-;# { dg-error "too complex" "too complex" { target powerpc*-*-* cris*-*-* } 6 }
 ;# { dg-warning "zero assumed" "missing" { target *-*-* } 9 }
 ;# { dg-warning "zero assumed" "missing" { target *-*-* } 10 }

Re: [PATCH] gas: require an operand to .startof.()/.sizeof.()

[Jan Beulich]

>>> On 22.02.17 at 17:31, <nickc@redhat.com> wrote:
>> gas/
>> 2017-02-21  Jan Beulich  <jbeulich@suse.com>
>> 
>> 	* expr.c (operand): Handle missing operand to .startof.() and
>> 	.sizeof.().
>> 	* testsuite/gas/all/err-sizeof.s: New.
> 
> The cris architecture is not the only one affected by this new test.
> The following are also affected:

Hmm, I had thought powerpc would be a very strange exception,
rather than the rule.

> Given the large number of targets affected, maybe the test itself needs
> to be reconsidered ?  Or maybe split into two tests, one general one,
> and one for targets which do support the difference of two symbols in an
> expression ?

Thing is - the test doesn't use the difference of symbols. There
ought to be a syntax error long before that expression would
need evaluating. I'll need to go look again, but perhaps best is
going to be to remove the problematic line altogether?

Jan

Re: [PATCH] gas: require an operand to .startof.()/.sizeof.()

[Nick Clifton]

Hi Jan,

> gas/
> 2017-02-21  Jan Beulich  <jbeulich@suse.com>
> 
> 	* expr.c (operand): Handle missing operand to .startof.() and
> 	.sizeof.().
> 	* testsuite/gas/all/err-sizeof.s: New.

The cris architecture is not the only one affected by this new test.
The following are also affected:

 alpha-dec-vms 
 cr16-elf 
 crx-elf 
 d10v-elf 
 d30v-elf 
 epiphany-elf 
 fr30-elf 
 frv-elf 
 h8300-elf 
 h8300-rtems 
 i386-darwin 
 ip2k-elf 
 iq2000-elf 
 lm32-rtems4.10 
 m32c-elf 
 m32r-elf 
 m68hc11-elf 
 mep-elf 
 microblaze-elf 
 mn10200-elf 
 mn10300-elf 
 ms1-elf 
 msp430-elf 
 mt-elf 
 or1k-elf 
 pdp11-dec-aout 
 riscv32-elf 
 riscv64-elf 
 rl78-elf 
 rs6000-aix4.3.3 
 rx-elf 
 s390-linux 
 s390x-ibm-tpf 
 spu-elf 
 tilegx-linux-gnu 
 tilegxbe-linux-gnu 
 tilepro-elf 
 v850-elf 
 vax-netbsdelf 
 xc16x-elf 
 xgate-elf 
 xstormy16-elf 

Given the large number of targets affected, maybe the test itself needs
to be reconsidered ?  Or maybe split into two tests, one general one,
and one for targets which do support the difference of two symbols in an
expression ?

Cheers
  Nick