gas/config/tc-arm.c:7819: error: array subscript is above array bounds

Message ID alpine.BSF.2.20.16.1907102142130.51962@arjuna.pair.com
State New
Headers show
Series
  • gas/config/tc-arm.c:7819: error: array subscript is above array bounds
Related show

Commit Message

Hans-Peter Nilsson July 11, 2019, 1:45 a.m.
Compiling gas at 43dd762689eb8 for e.g. arm-eabi using an old gcc like
gcc-4.3 yields a warning that causes more than one concern:

/x/gas/config/tc-arm.c: In function 'parse_operands':
/x/gas/config/tc-arm.c:7819: error: array subscript is above array bounds

That line seems to have been introduced with commit 1b883319, says git
blame.  The author is CC:ed.

7819:	  po_reg_or_fail (REG_TYPE_ZR);

That po_reg_or_fail call is a macro, with the definition containing:

#define po_reg_or_fail(regtype)					\
  do								\
    {								\
      val = arm_typed_reg_parse (& str, regtype, & rtype,	\
				 & inst.operands[i].vectype);	\
      if (val == FAIL)						\
	{							\
	  first_error (_(reg_expected_msgs[regtype]));		\
	  goto failure;						\
	}							\
...

It's the reg_expected_msgs[REG_TYPE_ZR] that causes the warning.
Checking reg_expected_msgs[] shows:

const char * const reg_expected_msgs[] =
{
  [REG_TYPE_RN]	    = N_("ARM register expected"),
...
  [REG_TYPE_MQ]	    = N_("MVE vector register expected"),
  [REG_TYPE_RNB]    = N_("")
 };

That is, no entry for REG_TYPE_ZR, which would make the argument to
first_error NULL if reg_expected_msgs[REG_TYPE_ZR] was within bounds.
But:

enum arm_reg_type
{
  REG_TYPE_RN,
...
  REG_TYPE_RNB,
  REG_TYPE_ZR
};

To wit, the warning seems to be correct.  Now, that warning *can* be
trivially "fixed" with:


...but I'm worrying also about:

- I don't know if that error message would be valid and informative; I
don't know what a "ZR" register is; it's just a copy-paste to me.

- There's apparently no (error) test-case that would have exposed the
missing message.

- This is an *old* gcc, 4.3.  Building with a more recent gcc, say
gcc-6.3, does not yield this warning.  That seems like a regression,
assuming that the warning is as valid as it looks, at a glance.
Perhaps a warning that had too many false positives and was silenced?
Off-topic for this list though.

brgds, H-P

Comments

Christophe Lyon July 11, 2019, 8:36 a.m. | #1
On Thu, 11 Jul 2019 at 03:46, Hans-Peter Nilsson <hp@bitrange.com> wrote:
>

> Compiling gas at 43dd762689eb8 for e.g. arm-eabi using an old gcc like

> gcc-4.3 yields a warning that causes more than one concern:

>

> /x/gas/config/tc-arm.c: In function 'parse_operands':

> /x/gas/config/tc-arm.c:7819: error: array subscript is above array bounds

>

> That line seems to have been introduced with commit 1b883319, says git

> blame.  The author is CC:ed.

>

> 7819:     po_reg_or_fail (REG_TYPE_ZR);

>

> That po_reg_or_fail call is a macro, with the definition containing:

>

> #define po_reg_or_fail(regtype)                                 \

>   do                                                            \

>     {                                                           \

>       val = arm_typed_reg_parse (& str, regtype, & rtype,       \

>                                  & inst.operands[i].vectype);   \

>       if (val == FAIL)                                          \

>         {                                                       \

>           first_error (_(reg_expected_msgs[regtype]));          \

>           goto failure;                                         \

>         }                                                       \

> ...

>

> It's the reg_expected_msgs[REG_TYPE_ZR] that causes the warning.

> Checking reg_expected_msgs[] shows:

>

> const char * const reg_expected_msgs[] =

> {

>   [REG_TYPE_RN]     = N_("ARM register expected"),

> ...

>   [REG_TYPE_MQ]     = N_("MVE vector register expected"),

>   [REG_TYPE_RNB]    = N_("")

>  };

>

> That is, no entry for REG_TYPE_ZR, which would make the argument to

> first_error NULL if reg_expected_msgs[REG_TYPE_ZR] was within bounds.

> But:

>

> enum arm_reg_type

> {

>   REG_TYPE_RN,

> ...

>   REG_TYPE_RNB,

>   REG_TYPE_ZR

> };

>

> To wit, the warning seems to be correct.  Now, that warning *can* be

> trivially "fixed" with:

>

> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c

> index b70028f..1dab180 100644

> --- a/gas/config/tc-arm.c

> +++ b/gas/config/tc-arm.c

> @@ -690,7 +690,8 @@ const char * const reg_expected_msgs[] =

>    [REG_TYPE_MMXWCG] = N_("iWMMXt scalar register expected"),

>    [REG_TYPE_XSCALE] = N_("XScale accumulator register expected"),

>    [REG_TYPE_MQ]            = N_("MVE vector register expected"),

> -  [REG_TYPE_RNB]    = N_("")

> +  [REG_TYPE_RNB]    = N_(""),

> +  [REG_TYPE_ZR]            = N_("ZR register expected"),

>  };

>

>  /* Some well known registers that we refer to directly elsewhere.  */

>

> ...but I'm worrying also about:

>

> - I don't know if that error message would be valid and informative; I

> don't know what a "ZR" register is; it's just a copy-paste to me.

>

> - There's apparently no (error) test-case that would have exposed the

> missing message.

>

> - This is an *old* gcc, 4.3.  Building with a more recent gcc, say

> gcc-6.3, does not yield this warning.  That seems like a regression,

> assuming that the warning is as valid as it looks, at a glance.

> Perhaps a warning that had too many false positives and was silenced?

> Off-topic for this list though.

>


It also makes me wonder whether binutils is monitored by Coverity
(like gdb and gcc at least) ?

Christophe

> brgds, H-P

Patch

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index b70028f..1dab180 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -690,7 +690,8 @@  const char * const reg_expected_msgs[] =
   [REG_TYPE_MMXWCG] = N_("iWMMXt scalar register expected"),
   [REG_TYPE_XSCALE] = N_("XScale accumulator register expected"),
   [REG_TYPE_MQ]	    = N_("MVE vector register expected"),
-  [REG_TYPE_RNB]    = N_("")
+  [REG_TYPE_RNB]    = N_(""),
+  [REG_TYPE_ZR]	    = N_("ZR register expected"),
 };

 /* Some well known registers that we refer to directly elsewhere.  */