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

[Christophe Lyon]

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