[PATCH 4/4] gas/arc: Make member of arc_flags const

[PATCH 4/4] gas/arc: Make member of arc_flags const

[Andrew Burgess]

By making the flgp field of struct arc_flags constant we can remove a
place where we cast away the const-ness of a variable.  Also, given that
the value assigned to this field almost always comes from compile-time
constant data, having the field non-constant is probably a bad thing.

gas/ChangeLog:

	* config/tc-arc.c (find_opcode_match): Remove casting away of
	const.
	* config/tc-arc.h (struct arc_flags): Make flgp field const.
---
 gas/ChangeLog       | 6 ++++++
 gas/config/tc-arc.c | 2 +-
 gas/config/tc-arc.h | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index b217e06..eca6817 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -1884,7 +1884,7 @@ find_opcode_match (const struct arc_opcode_hash_entry *entry,
 		      if (pflag->flgp != NULL)
 			goto match_failed;
 		      cl_matches++;
-		      pflag->flgp = (struct arc_flag_operand *) flg_operand;
+		      pflag->flgp = flg_operand;
 		      lnflg--;
 		      break; /* goto next flag class and parsed flag.  */
 		    }
diff --git a/gas/config/tc-arc.h b/gas/config/tc-arc.h
index 16f6a06..b61342b 100644
--- a/gas/config/tc-arc.h
+++ b/gas/config/tc-arc.h
@@ -225,7 +225,7 @@ struct arc_flags
   char name[MAX_FLAG_NAME_LENGTH + 1];
 
   /* Pointer to arc flags.  */
-  struct arc_flag_operand *flgp;
+  const struct arc_flag_operand *flgp;
 };
 
 extern const relax_typeS md_relax_table[];
-- 
2.6.4

[PATCH 1/4] gas/arc: Add guard against operand array overflow.

[Andrew Burgess]

Currently supplying an input file with too many operands to an
instruction will cause the assembler to overflow and array and trigger
undefined behaviour.

This change checks that we don't access outside the limits of the
operand array.

gas/ChangeLog:

	* config/tc-arc.c (tokenize_arguments): Add checks for array
	overflow.
	* testsuite/gas/arc/asm-errors.s: Addition test line added.
	* testsuite/gas/arc/asm-errors.err: Update expected results.
---
 gas/ChangeLog                        |  7 +++++++
 gas/config/tc-arc.c                  | 12 +++++++-----
 gas/testsuite/gas/arc/asm-errors.err |  2 ++
 gas/testsuite/gas/arc/asm-errors.s   |  1 +
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index b64174f..4c9b08a 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -1040,7 +1040,7 @@ tokenize_arguments (char *str,
 	case ']':
 	  ++input_line_pointer;
 	  --brk_lvl;
-	  if (!saw_arg)
+	  if (!saw_arg || num_args == ntok)
 	    goto err;
 	  tok->X_op = O_bracket;
 	  ++tok;
@@ -1050,7 +1050,7 @@ tokenize_arguments (char *str,
 	case '{':
 	case '[':
 	  input_line_pointer++;
-	  if (brk_lvl)
+	  if (brk_lvl || num_args == ntok)
 	    goto err;
 	  ++brk_lvl;
 	  tok->X_op = O_bracket;
@@ -1061,7 +1061,7 @@ tokenize_arguments (char *str,
 	case '@':
 	  /* We have labels, function names and relocations, all
 	     starting with @ symbol.  Sort them out.  */
-	  if (saw_arg && !saw_comma)
+	  if ((saw_arg && !saw_comma) || num_args == ntok)
 	    goto err;
 
 	  /* Parse @label.  */
@@ -1166,7 +1166,7 @@ tokenize_arguments (char *str,
 	  /* Fall through.  */
 	default:
 
-	  if (saw_arg && !saw_comma)
+	  if ((saw_arg && !saw_comma) || num_args == ntok)
 	    goto err;
 
 	  tok->X_op = O_absent;
@@ -1182,7 +1182,9 @@ tokenize_arguments (char *str,
 	normalsymbol:
 	  debug_exp (tok);
 
-	  if (tok->X_op == O_illegal || tok->X_op == O_absent)
+	  if (tok->X_op == O_illegal
+              || tok->X_op == O_absent
+              || num_args == ntok)
 	    goto err;
 
 	  saw_comma = FALSE;
diff --git a/gas/testsuite/gas/arc/asm-errors.err b/gas/testsuite/gas/arc/asm-errors.err
index 35390fc..e889eb8 100644
--- a/gas/testsuite/gas/arc/asm-errors.err
+++ b/gas/testsuite/gas/arc/asm-errors.err
@@ -2,3 +2,5 @@
 [^:]*:2: Error: inappropriate arguments for opcode 'adc'
 [^:]*:3: Error: inappropriate arguments for opcode 'adc'
 [^:]*:4: Error: inappropriate arguments for opcode 'adc'
+[^:]*:5: Error: extra comma
+[^:]*:5: Error: syntax error
diff --git a/gas/testsuite/gas/arc/asm-errors.s b/gas/testsuite/gas/arc/asm-errors.s
index 6e0fd6a..d3f16c0 100644
--- a/gas/testsuite/gas/arc/asm-errors.s
+++ b/gas/testsuite/gas/arc/asm-errors.s
@@ -2,3 +2,4 @@
         adc.al.ra       r0,r0,r2
         adc.eq.eq       r0,r0,r2
         adc.n.eq        r0,r0,r2
+        add             r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0
-- 
2.6.4

[PATCH 3/4] gas/arc: Use BFD_VMA_FMT for printf format specifier

[Andrew Burgess]

Some debug code has the wrong printf format specifier for some types
that are (ultimately) bfd_vma.  Fixed by using BFD_VMA_FMT string.  This
only becomes an issue when building the tc-arc.c file with -DDEBUG=1 to
build in the debug code.

gas/ChangeLog:

	* config/tc-arc.c (md_pcrel_from_section): Use BFD_VMA_FMT where
	appropriate.
	(md_convert_frag): Likewise.
---
 gas/ChangeLog       | 6 ++++++
 gas/config/tc-arc.c | 8 +++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 38d4e8f..b217e06 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -2508,7 +2508,8 @@ md_pcrel_from_section (fixS *fixP,
 	}
     }
 
-  pr_debug ("pcrel from %x + %lx = %x, symbol: %s (%x)\n",
+  pr_debug ("pcrel from %"BFD_VMA_FMT"x + %lx = %"BFD_VMA_FMT"x, "
+            "symbol: %s (%"BFD_VMA_FMT"x)\n",
 	    fixP->fx_frag->fr_address, fixP->fx_where, base,
 	    fixP->fx_addsy ? S_GET_NAME (fixP->fx_addsy) : "(null)",
 	    fixP->fx_addsy ? S_GET_VALUE (fixP->fx_addsy) : 0);
@@ -3024,8 +3025,9 @@ md_convert_frag (bfd *abfd ATTRIBUTE_UNUSED,
   dest = fragP->fr_literal + fix;
   table_entry = TC_GENERIC_RELAX_TABLE + fragP->fr_subtype;
 
-  pr_debug ("%s:%d: md_convert_frag, subtype: %d, fix: %d, var: %d\n",
-	    fragP->fr_file, fragP->fr_line,
+  pr_debug ("%s:%d: md_convert_frag, subtype: %d, fix: %d, "
+            "var: %"BFD_VMA_FMT"d\n",
+            fragP->fr_file, fragP->fr_line,
 	    fragP->fr_subtype, fix, fragP->fr_var);
 
   if (fragP->fr_subtype <= 0
-- 
2.6.4

[PATCH 0/4] [ARC] Set of miscellaneous fixes

[Andrew Burgess]

A set of fixes, they are all disjoint I've grouped them simply because
they are all ARC related.

--

Andrew Burgess (4):
  gas/arc: Add guard against operand array overflow.
  gas/arc: Fix array overrun when checking opcode array
  gas/arc: Use BFD_VMA_FMT for printf format specifier
  gas/arc: Make member of arc_flags const

 gas/ChangeLog                          | 27 +++++++++++++++++++++++++++
 gas/config/tc-arc.c                    | 28 ++++++++++++++++------------
 gas/config/tc-arc.h                    |  2 +-
 gas/testsuite/gas/arc/asm-errors-2.d   |  2 ++
 gas/testsuite/gas/arc/asm-errors-2.err |  2 ++
 gas/testsuite/gas/arc/asm-errors-2.s   |  2 ++
 gas/testsuite/gas/arc/asm-errors.err   |  2 ++
 gas/testsuite/gas/arc/asm-errors.s     |  1 +
 8 files changed, 53 insertions(+), 13 deletions(-)
 create mode 100644 gas/testsuite/gas/arc/asm-errors-2.d
 create mode 100644 gas/testsuite/gas/arc/asm-errors-2.err
 create mode 100644 gas/testsuite/gas/arc/asm-errors-2.s

-- 
2.6.4

[PATCH 2/4] gas/arc: Fix array overrun when checking opcode array

[Andrew Burgess]

The opcode array iterator mechanism can, in some situations, result in
reading memory outside of the opcode array.  When using the
iterator-next mechanism to find the next possible arc_opcode, if we find
an opcode where the name field is NULL, or the name does not match, then
the cached opcode pointer is not set to NULL.  The result is that
another call to iterator-next will again increment the opcode
pointer (which might now point outside the opcode array) and attempt to
access the name field of this undefined opcode.

Fixed in this commit by clearing the cached opcode pointer.

I've added a test case, which currently shows the bug, however, this
will only expose this bug while the opcode used (dsp_fp_cmp) is the last
opcode in the table.

gas/ChangeLog:

	* config/tc-arc.c (arc_opcode_hash_entry_iterator_next): Set
	cached opcode to NULL when we reach a non-matching opcode.
	* gas/testsuite/gas/arc/asm-errors-2.d: New file.
	* gas/testsuite/gas/arc/asm-errors-2.err: New file.
	* gas/testsuite/gas/arc/asm-errors-2.s: New file.
---
 gas/ChangeLog                          | 8 ++++++++
 gas/config/tc-arc.c                    | 6 +++---
 gas/testsuite/gas/arc/asm-errors-2.d   | 2 ++
 gas/testsuite/gas/arc/asm-errors-2.err | 2 ++
 gas/testsuite/gas/arc/asm-errors-2.s   | 2 ++
 5 files changed, 17 insertions(+), 3 deletions(-)
 create mode 100644 gas/testsuite/gas/arc/asm-errors-2.d
 create mode 100644 gas/testsuite/gas/arc/asm-errors-2.err
 create mode 100644 gas/testsuite/gas/arc/asm-errors-2.s

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 4c9b08a..38d4e8f 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -674,9 +674,9 @@ arc_opcode_hash_entry_iterator_next (const struct arc_opcode_hash_entry *entry,
       const char *old_name = iter->opcode->name;
 
       iter->opcode++;
-      if (iter->opcode->name
-	  && (strcmp (old_name, iter->opcode->name) != 0))
-	{
+      if (iter->opcode->name == NULL
+          || strcmp (old_name, iter->opcode->name) != 0)
+        {
 	  iter->index++;
 	  if (iter->index == entry->count)
 	    iter->opcode = NULL;
diff --git a/gas/testsuite/gas/arc/asm-errors-2.d b/gas/testsuite/gas/arc/asm-errors-2.d
new file mode 100644
index 0000000..fd3c09a
--- /dev/null
+++ b/gas/testsuite/gas/arc/asm-errors-2.d
@@ -0,0 +1,2 @@
+#as: -mcpu=arcem
+#error-output: asm-errors-2.err
diff --git a/gas/testsuite/gas/arc/asm-errors-2.err b/gas/testsuite/gas/arc/asm-errors-2.err
new file mode 100644
index 0000000..64fdc9a
--- /dev/null
+++ b/gas/testsuite/gas/arc/asm-errors-2.err
@@ -0,0 +1,2 @@
+[^:]*: Assembler messages:
+[^:]*:2: Error: inappropriate arguments for opcode 'dsp_fp_cmp'
diff --git a/gas/testsuite/gas/arc/asm-errors-2.s b/gas/testsuite/gas/arc/asm-errors-2.s
new file mode 100644
index 0000000..f5bf8da
--- /dev/null
+++ b/gas/testsuite/gas/arc/asm-errors-2.s
@@ -0,0 +1,2 @@
+        .text
+        dsp_fp_cmp      r0
-- 
2.6.4

Re: [PATCH 1/4] gas/arc: Add guard against operand array overflow.

[Claudiu Zissulescu]

On 11/05/16 21:21, Andrew Burgess wrote:
> Currently supplying an input file with too many operands to an
> instruction will cause the assembler to overflow and array and trigger
> undefined behaviour.
>
> This change checks that we don't access outside the limits of the
> operand array.
>
> gas/ChangeLog:
>
> 	* config/tc-arc.c (tokenize_arguments): Add checks for array
> 	overflow.
> 	* testsuite/gas/arc/asm-errors.s: Addition test line added.
> 	* testsuite/gas/arc/asm-errors.err: Update expected results.
> ---
>   gas/ChangeLog                        |  7 +++++++
>   gas/config/tc-arc.c                  | 12 +++++++-----
>   gas/testsuite/gas/arc/asm-errors.err |  2 ++
>   gas/testsuite/gas/arc/asm-errors.s   |  1 +
>   4 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
> index b64174f..4c9b08a 100644
> --- a/gas/config/tc-arc.c
> +++ b/gas/config/tc-arc.c
> @@ -1040,7 +1040,7 @@ tokenize_arguments (char *str,
>   	case ']':
>   	  ++input_line_pointer;
>   	  --brk_lvl;
> -	  if (!saw_arg)
> +	  if (!saw_arg || num_args == ntok)
>   	    goto err;
>   	  tok->X_op = O_bracket;
>   	  ++tok;
> @@ -1050,7 +1050,7 @@ tokenize_arguments (char *str,
>   	case '{':
>   	case '[':
>   	  input_line_pointer++;
> -	  if (brk_lvl)
> +	  if (brk_lvl || num_args == ntok)
>   	    goto err;
>   	  ++brk_lvl;
>   	  tok->X_op = O_bracket;
> @@ -1061,7 +1061,7 @@ tokenize_arguments (char *str,
>   	case '@':
>   	  /* We have labels, function names and relocations, all
>   	     starting with @ symbol.  Sort them out.  */
> -	  if (saw_arg && !saw_comma)
> +	  if ((saw_arg && !saw_comma) || num_args == ntok)
>   	    goto err;
>
>   	  /* Parse @label.  */
> @@ -1166,7 +1166,7 @@ tokenize_arguments (char *str,
>   	  /* Fall through.  */
>   	default:
>
> -	  if (saw_arg && !saw_comma)
> +	  if ((saw_arg && !saw_comma) || num_args == ntok)
>   	    goto err;
>
>   	  tok->X_op = O_absent;
> @@ -1182,7 +1182,9 @@ tokenize_arguments (char *str,
>   	normalsymbol:
>   	  debug_exp (tok);
>
> -	  if (tok->X_op == O_illegal || tok->X_op == O_absent)
> +	  if (tok->X_op == O_illegal
> +              || tok->X_op == O_absent
> +              || num_args == ntok)
>   	    goto err;
>
>   	  saw_comma = FALSE;
> diff --git a/gas/testsuite/gas/arc/asm-errors.err b/gas/testsuite/gas/arc/asm-errors.err
> index 35390fc..e889eb8 100644
> --- a/gas/testsuite/gas/arc/asm-errors.err
> +++ b/gas/testsuite/gas/arc/asm-errors.err
> @@ -2,3 +2,5 @@
>   [^:]*:2: Error: inappropriate arguments for opcode 'adc'
>   [^:]*:3: Error: inappropriate arguments for opcode 'adc'
>   [^:]*:4: Error: inappropriate arguments for opcode 'adc'
> +[^:]*:5: Error: extra comma
> +[^:]*:5: Error: syntax error
> diff --git a/gas/testsuite/gas/arc/asm-errors.s b/gas/testsuite/gas/arc/asm-errors.s
> index 6e0fd6a..d3f16c0 100644
> --- a/gas/testsuite/gas/arc/asm-errors.s
> +++ b/gas/testsuite/gas/arc/asm-errors.s
> @@ -2,3 +2,4 @@
>           adc.al.ra       r0,r0,r2
>           adc.eq.eq       r0,r0,r2
>           adc.n.eq        r0,r0,r2
> +        add             r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0,r0
>
This patch seems ok.

//Claudiu

Re: [PATCH 2/4] gas/arc: Fix array overrun when checking opcode array

[Claudiu Zissulescu]

On 11/05/16 21:21, Andrew Burgess wrote:
> gas/ChangeLog:
>
> 	* config/tc-arc.c (arc_opcode_hash_entry_iterator_next): Set
> 	cached opcode to NULL when we reach a non-matching opcode.
> 	* gas/testsuite/gas/arc/asm-errors-2.d: New file.
> 	* gas/testsuite/gas/arc/asm-errors-2.err: New file.
> 	* gas/testsuite/gas/arc/asm-errors-2.s: New file.

This seems also OK,

Claudiu

Re: [PATCH 3/4] gas/arc: Use BFD_VMA_FMT for printf format specifier

[Claudiu Zissulescu]

> gas/ChangeLog:
>
> 	* config/tc-arc.c (md_pcrel_from_section): Use BFD_VMA_FMT where
> 	appropriate.
> 	(md_convert_frag): Likewise.

This is OK, probably this patch may be consider as obvious.

Claudiu

Re: [PATCH 4/4] gas/arc: Make member of arc_flags const

[Claudiu Zissulescu]

> gas/ChangeLog:
>
> 	* config/tc-arc.c (find_opcode_match): Remove casting away of
> 	const.
> 	* config/tc-arc.h (struct arc_flags): Make flgp field const.
> ---

This is OK, probably this patch may be consider also obvious.

Claudiu

Re: [PATCH 3/4] gas/arc: Use BFD_VMA_FMT for printf format specifier

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (md_pcrel_from_section): Use BFD_VMA_FMT where
> 	appropriate.
> 	(md_convert_frag): Likewise.

Approved - please apply.

Cheers
  Nick

Re: [PATCH 4/4] gas/arc: Make member of arc_flags const

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (find_opcode_match): Remove casting away of
> 	const.
> 	* config/tc-arc.h (struct arc_flags): Make flgp field const.

Approved - please apply.

Cheers
  Nick

Re: [PATCH 2/4] gas/arc: Fix array overrun when checking opcode array

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (arc_opcode_hash_entry_iterator_next): Set
> 	cached opcode to NULL when we reach a non-matching opcode.
> 	* gas/testsuite/gas/arc/asm-errors-2.d: New file.
> 	* gas/testsuite/gas/arc/asm-errors-2.err: New file.
> 	* gas/testsuite/gas/arc/asm-errors-2.s: New file.
 
Approved - please apply.

Cheers
  Nick

Re: [PATCH 1/4] gas/arc: Add guard against operand array overflow.

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 	* config/tc-arc.c (tokenize_arguments): Add checks for array
> 	overflow.
> 	* testsuite/gas/arc/asm-errors.s: Addition test line added.
> 	* testsuite/gas/arc/asm-errors.err: Update expected results.
 
Approved - please apply.

Cheers
  Nick