[PATCH] ia64 diagnostics

[PATCH] ia64 diagnostics

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2796 bytes --]

Error messages resulting from wrong operands being used with insns were
partially meaningless.

Built and tested on ia64-unknown-linux-gnu.

Jan

gas/
2005-01-24  Jan Beulich  <jbeulich@novell.com>

	* config/tc-ia64.c (parse_operands): Parse all specified
operands,
	immediately discarding (but counting) those exceeding the
maximum
	possible amount. Track whether output and input operand counts
ever
	matched, and use this to better indicate which of the operands/
	operand types was wrong; specifically don't default to pointing
to
	the first operand.

---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-18
10:43:33.000000000 +0100
+++ 2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-24
11:16:17.997766627 +0100
@@ -5792,11 +5792,22 @@ parse_operands (idesc)
       ++num_outputs;
     }
 
-  for (; i < NELEMS (CURR_SLOT.opnd); ++i)
+  for (; ; ++i)
     {
-      sep = parse_operand (CURR_SLOT.opnd + i);
-      if (CURR_SLOT.opnd[i].X_op == O_absent)
-	break;
+      if (i < NELEMS (CURR_SLOT.opnd)) 
+	{
+	  sep = parse_operand (CURR_SLOT.opnd + i);
+	  if (CURR_SLOT.opnd[i].X_op == O_absent)
+	    break;
+	}
+      else
+	{
+	  expressionS dummy;
+
+	  sep = parse_operand (&dummy);
+	  if (dummy.X_op == O_absent)
+	    break;
+	}
 
       ++num_operands;
 
@@ -5847,14 +5858,21 @@ parse_operands (idesc)
 	}
     }
 
-  highest_unmatched_operand = 0;
+  highest_unmatched_operand = -4;
   curr_out_of_range_pos = -1;
   error_pos = 0;
-  expected_operand = idesc->operands[0];
   for (; idesc; idesc = get_next_opcode (idesc))
     {
       if (num_outputs != idesc->num_outputs)
 	continue;		/* mismatch in # of outputs */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 1;
+      if ((num_operands < NELEMS (idesc->operands)
+	   && idesc->operands[num_operands])
+	  || (num_operands > 0 && !idesc->operands[num_operands - 1]))
+	continue;		/* mismatch in number of arguments */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 2;
 
       CURR_SLOT.num_fixups = 0;
 
@@ -5907,10 +5925,6 @@ parse_operands (idesc)
 	  continue;
 	}
 
-      if (num_operands < NELEMS (idesc->operands)
-	  && idesc->operands[num_operands])
-	continue;		/* mismatch in number of arguments */
-
       break;
     }
   if (!idesc)
@@ -5919,6 +5933,10 @@ parse_operands (idesc)
 	as_bad ("Operand %u of `%s' should be %s",
 		error_pos + 1, mnemonic,
 		elf64_ia64_operands[expected_operand].desc);
+      else if (highest_unmatched_operand < 0 &&
!(highest_unmatched_operand & 1))
+	as_bad ("Wrong number of output operands");
+      else if (highest_unmatched_operand < 0 &&
!(highest_unmatched_operand & 2))
+	as_bad ("Wrong number of input operands");
       else
 	as_bad ("Operand mismatch");
       return 0;


[-- Attachment #2: binutils-mainline-ia64-diagnostics.patch --]
[-- Type: text/plain, Size: 2795 bytes --]

Error messages resulting from wrong operands being used with insns were
partially meaningless.

Built and tested on ia64-unknown-linux-gnu.

Jan

gas/
2005-01-24  Jan Beulich  <jbeulich@novell.com>

	* config/tc-ia64.c (parse_operands): Parse all specified operands,
	immediately discarding (but counting) those exceeding the maximum
	possible amount. Track whether output and input operand counts ever
	matched, and use this to better indicate which of the operands/
	operand types was wrong; specifically don't default to pointing to
	the first operand.

--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-18 10:43:33.000000000 +0100
+++ 2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-24 11:16:17.997766627 +0100
@@ -5792,11 +5792,22 @@ parse_operands (idesc)
       ++num_outputs;
     }
 
-  for (; i < NELEMS (CURR_SLOT.opnd); ++i)
+  for (; ; ++i)
     {
-      sep = parse_operand (CURR_SLOT.opnd + i);
-      if (CURR_SLOT.opnd[i].X_op == O_absent)
-	break;
+      if (i < NELEMS (CURR_SLOT.opnd)) 
+	{
+	  sep = parse_operand (CURR_SLOT.opnd + i);
+	  if (CURR_SLOT.opnd[i].X_op == O_absent)
+	    break;
+	}
+      else
+	{
+	  expressionS dummy;
+
+	  sep = parse_operand (&dummy);
+	  if (dummy.X_op == O_absent)
+	    break;
+	}
 
       ++num_operands;
 
@@ -5847,14 +5858,21 @@ parse_operands (idesc)
 	}
     }
 
-  highest_unmatched_operand = 0;
+  highest_unmatched_operand = -4;
   curr_out_of_range_pos = -1;
   error_pos = 0;
-  expected_operand = idesc->operands[0];
   for (; idesc; idesc = get_next_opcode (idesc))
     {
       if (num_outputs != idesc->num_outputs)
 	continue;		/* mismatch in # of outputs */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 1;
+      if ((num_operands < NELEMS (idesc->operands)
+	   && idesc->operands[num_operands])
+	  || (num_operands > 0 && !idesc->operands[num_operands - 1]))
+	continue;		/* mismatch in number of arguments */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 2;
 
       CURR_SLOT.num_fixups = 0;
 
@@ -5907,10 +5925,6 @@ parse_operands (idesc)
 	  continue;
 	}
 
-      if (num_operands < NELEMS (idesc->operands)
-	  && idesc->operands[num_operands])
-	continue;		/* mismatch in number of arguments */
-
       break;
     }
   if (!idesc)
@@ -5919,6 +5933,10 @@ parse_operands (idesc)
 	as_bad ("Operand %u of `%s' should be %s",
 		error_pos + 1, mnemonic,
 		elf64_ia64_operands[expected_operand].desc);
+      else if (highest_unmatched_operand < 0 && !(highest_unmatched_operand & 1))
+	as_bad ("Wrong number of output operands");
+      else if (highest_unmatched_operand < 0 && !(highest_unmatched_operand & 2))
+	as_bad ("Wrong number of input operands");
       else
 	as_bad ("Operand mismatch");
       return 0;

Re: [PATCH] ia64 diagnostics

[James E Wilson]

On Mon, 2005-01-24 at 02:24, Jan Beulich wrote:
> 	* config/tc-ia64.c (parse_operands): Parse all specified
> operands,
> 	immediately discarding (but counting) those exceeding the
> maximum

This looks mostly OK.  You have one comment that doesn't use a sentence,
but you copied it from elsewhere, so I will let that go.  I'd rather see
comments in the source file than in the ChangeLog entry.

A testcase would be useful.

> +	  || (num_operands > 0 && !idesc->operands[num_operands - 1]))

This part looks like an out-of-bounds array reference.  Since we are
counting all arguments, it is possible for num_operands to be larger
than the idesc->operands array size.  There are also other later places
that will result in out-of-bounds array references.  I think you need an
extra test before this line like
	|| (num_operands > NELEMS (CURR_SLOT.opnd))
Since this is a loop invariant, this test could be performed before
entering the loop, and just exit with an appropriate error if we have
too many operands.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: [PATCH] ia64 diagnostics

[James E Wilson]

On Fri, 2005-01-28 at 02:00, Jan Beulich wrote:
> 	* config/tc-ia64.c (parse_operands): Parse all specified
> operands,
> 	immediately discarding (but counting) those exceeding the
> maximum...

OK.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: [PATCH] ia64 diagnostics

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 4592 bytes --]

>You didn't cc binutils on this message.  The finished patch needs to
be
>sent to the binutils list as usual.  I'd prefer that all messages get
>cc'ed to the list so that they are archived.

Sorry, an oversight.

>The GCC testsuite driver does "setenv LANG C" to avoid problems with
>translated messages.  The gas testsuite does not do this, but it
would
>be easy to add.  Anyways, we already have tests that depend on
wording. 
>Some of your other patches adds tests that contain the string error
>which gets translated for instance.  Set LANG=fr and it comes out
>ERREUR.

Test added.

Built and tested on ia64-unknown-linux-gnu.

Jan

gas/
2005-01-28  Jan Beulich  <jbeulich@novell.com>

	* config/tc-ia64.c (parse_operands): Parse all specified
operands,
	immediately discarding (but counting) those exceeding the
maximum
	possible amount. Track whether output and input operand counts
ever
	matched, and use this to better indicate which of the operands/
	operand types was wrong; specifically don't default to pointing
to
	the first operand.

gas/testsuite/
2005-01-28  Jan Beulich  <jbeulich@novell.com>

	* gas/ia64/operands.[ls]: New.
	* gas/ia64/ia64.exp: Run new test.

---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-18
10:43:33.000000000 +0100
+++ 2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-28
10:46:43.134457440 +0100
@@ -5792,11 +5792,22 @@ parse_operands (idesc)
       ++num_outputs;
     }
 
-  for (; i < NELEMS (CURR_SLOT.opnd); ++i)
+  for (; ; ++i)
     {
-      sep = parse_operand (CURR_SLOT.opnd + i);
-      if (CURR_SLOT.opnd[i].X_op == O_absent)
-	break;
+      if (i < NELEMS (CURR_SLOT.opnd)) 
+	{
+	  sep = parse_operand (CURR_SLOT.opnd + i);
+	  if (CURR_SLOT.opnd[i].X_op == O_absent)
+	    break;
+	}
+      else
+	{
+	  expressionS dummy;
+
+	  sep = parse_operand (&dummy);
+	  if (dummy.X_op == O_absent)
+	    break;
+	}
 
       ++num_operands;
 
@@ -5847,14 +5858,22 @@ parse_operands (idesc)
 	}
     }
 
-  highest_unmatched_operand = 0;
+  highest_unmatched_operand = -4;
   curr_out_of_range_pos = -1;
   error_pos = 0;
-  expected_operand = idesc->operands[0];
   for (; idesc; idesc = get_next_opcode (idesc))
     {
       if (num_outputs != idesc->num_outputs)
 	continue;		/* mismatch in # of outputs */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 1;
+      if (num_operands > NELEMS (idesc->operands)
+	  || (num_operands < NELEMS (idesc->operands)
+	   && idesc->operands[num_operands])
+	  || (num_operands > 0 && !idesc->operands[num_operands - 1]))
+	continue;		/* mismatch in number of arguments */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 2;
 
       CURR_SLOT.num_fixups = 0;
 
@@ -5907,10 +5926,6 @@ parse_operands (idesc)
 	  continue;
 	}
 
-      if (num_operands < NELEMS (idesc->operands)
-	  && idesc->operands[num_operands])
-	continue;		/* mismatch in number of arguments */
-
       break;
     }
   if (!idesc)
@@ -5919,6 +5934,10 @@ parse_operands (idesc)
 	as_bad ("Operand %u of `%s' should be %s",
 		error_pos + 1, mnemonic,
 		elf64_ia64_operands[expected_operand].desc);
+      else if (highest_unmatched_operand < 0 &&
!(highest_unmatched_operand & 1))
+	as_bad ("Wrong number of output operands");
+      else if (highest_unmatched_operand < 0 &&
!(highest_unmatched_operand & 2))
+	as_bad ("Wrong number of input operands");
       else
 	as_bad ("Operand mismatch");
       return 0;
---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2004-07-02
08:26:34.000000000 +0200
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2005-01-28
10:50:55.133477790 +0100
@@ -24,6 +24,7 @@ if [istarget "ia64-*"] then {
     run_dump_test "opc-i"
     run_dump_test "opc-m"
     run_dump_test "opc-x"
+    run_list_test "operands" ""
 
     run_list_test "dv-raw-err" ""
     run_list_test "dv-waw-err" ""
---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/operands.l	1970-01-01
01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/operands.l	2005-01-28
10:56:36.064137676 +0100
@@ -0,0 +1,5 @@
+.*: Assembler messages:
+.*:3: Error: .* output .*
+.*:4: Error: .* input .*
+.*:5: Error: .* 1 .*
+.*:6: Error: .* 2 .*
---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/operands.s	1970-01-01
01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/operands.s	2005-01-28
10:54:08.385428548 +0100
@@ -0,0 +1,6 @@
+	.text
+_start:
+	zxt1	r1, r2 = r3
+	zxt2	r4 = r5, r6
+	zxt4	p1 = r8
+	sxt1	r7 = 0


[-- Attachment #2: binutils-mainline-ia64-diagnostics.patch --]
[-- Type: text/plain, Size: 4099 bytes --]

Error messages resulting from wrong operands being used with insns were
partially meaningless.

Built and tested on ia64-unknown-linux-gnu.

Jan

gas/
2005-01-28  Jan Beulich  <jbeulich@novell.com>

	* config/tc-ia64.c (parse_operands): Parse all specified operands,
	immediately discarding (but counting) those exceeding the maximum
	possible amount. Track whether output and input operand counts ever
	matched, and use this to better indicate which of the operands/
	operand types was wrong; specifically don't default to pointing to
	the first operand.

gas/testsuite/
2005-01-28  Jan Beulich  <jbeulich@novell.com>

	* gas/ia64/operands.[ls]: New.
	* gas/ia64/ia64.exp: Run new test.

--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-18 10:43:33.000000000 +0100
+++ 2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-28 10:46:43.134457440 +0100
@@ -5792,11 +5792,22 @@ parse_operands (idesc)
       ++num_outputs;
     }
 
-  for (; i < NELEMS (CURR_SLOT.opnd); ++i)
+  for (; ; ++i)
     {
-      sep = parse_operand (CURR_SLOT.opnd + i);
-      if (CURR_SLOT.opnd[i].X_op == O_absent)
-	break;
+      if (i < NELEMS (CURR_SLOT.opnd)) 
+	{
+	  sep = parse_operand (CURR_SLOT.opnd + i);
+	  if (CURR_SLOT.opnd[i].X_op == O_absent)
+	    break;
+	}
+      else
+	{
+	  expressionS dummy;
+
+	  sep = parse_operand (&dummy);
+	  if (dummy.X_op == O_absent)
+	    break;
+	}
 
       ++num_operands;
 
@@ -5847,14 +5858,22 @@ parse_operands (idesc)
 	}
     }
 
-  highest_unmatched_operand = 0;
+  highest_unmatched_operand = -4;
   curr_out_of_range_pos = -1;
   error_pos = 0;
-  expected_operand = idesc->operands[0];
   for (; idesc; idesc = get_next_opcode (idesc))
     {
       if (num_outputs != idesc->num_outputs)
 	continue;		/* mismatch in # of outputs */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 1;
+      if (num_operands > NELEMS (idesc->operands)
+	  || (num_operands < NELEMS (idesc->operands)
+	   && idesc->operands[num_operands])
+	  || (num_operands > 0 && !idesc->operands[num_operands - 1]))
+	continue;		/* mismatch in number of arguments */
+      if (highest_unmatched_operand < 0)
+	highest_unmatched_operand |= 2;
 
       CURR_SLOT.num_fixups = 0;
 
@@ -5907,10 +5926,6 @@ parse_operands (idesc)
 	  continue;
 	}
 
-      if (num_operands < NELEMS (idesc->operands)
-	  && idesc->operands[num_operands])
-	continue;		/* mismatch in number of arguments */
-
       break;
     }
   if (!idesc)
@@ -5919,6 +5934,10 @@ parse_operands (idesc)
 	as_bad ("Operand %u of `%s' should be %s",
 		error_pos + 1, mnemonic,
 		elf64_ia64_operands[expected_operand].desc);
+      else if (highest_unmatched_operand < 0 && !(highest_unmatched_operand & 1))
+	as_bad ("Wrong number of output operands");
+      else if (highest_unmatched_operand < 0 && !(highest_unmatched_operand & 2))
+	as_bad ("Wrong number of input operands");
       else
 	as_bad ("Operand mismatch");
       return 0;
--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2004-07-02 08:26:34.000000000 +0200
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2005-01-28 10:50:55.133477790 +0100
@@ -24,6 +24,7 @@ if [istarget "ia64-*"] then {
     run_dump_test "opc-i"
     run_dump_test "opc-m"
     run_dump_test "opc-x"
+    run_list_test "operands" ""
 
     run_list_test "dv-raw-err" ""
     run_list_test "dv-waw-err" ""
--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/operands.l	1970-01-01 01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/operands.l	2005-01-28 10:56:36.064137676 +0100
@@ -0,0 +1,5 @@
+.*: Assembler messages:
+.*:3: Error: .* output .*
+.*:4: Error: .* input .*
+.*:5: Error: .* 1 .*
+.*:6: Error: .* 2 .*
--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/operands.s	1970-01-01 01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/operands.s	2005-01-28 10:54:08.385428548 +0100
@@ -0,0 +1,6 @@
+	.text
+_start:
+	zxt1	r1, r2 = r3
+	zxt2	r4 = r5, r6
+	zxt4	p1 = r8
+	sxt1	r7 = 0