Re: [PATCH, libgcc]: Avoid warning: array subscript is above array bounds when compiling crtstuff.c

[Uros Bizjak <ubizjak at gmail dot com>]

On Mon, Mar 10, 2014 at 11:27 AM, Uros Bizjak <ubizjak@gmail.com> wrote:

>>> > Well, better is non-obvious, while it is smaller (which is good for
>>> > initialization and thus rarely executed code), the common case is that
>>> > *jcr_list is 0 (gcj is used rarely these days) and for the common case it is
>>> > one instruction longer.
>>> > Perhaps at least use if (__builtin_expect (*jcr_list != NULL, 0))?
>>> > Otherwise looks good to me.
>>>
>>> Following source:
>>>
>>> void frame_dummy (void)
>>> {
>>>   void **jcr_list = __JCR_LIST__;
>>>   if (__builtin_expect (*jcr_list != 0, 0))
>>>     register_classes (jcr_list);
>>> }
>>>
>>> generates exactly the same code while avoiding the warning. So,
>>> following your concern, I am testing following patch:
>>
>> But then the asm is gone and it can start to break any time soon.
>> For GCC __JCR_LIST__ is simply a zero sized local array and thus
>> dereferencing it's first element is invalid.  It doesn't know that we use
>> linker magic to populate the array.
>
> OK, then we have to live with additional instruction (which isn't bad
> either, since the r/i compare can be fused with the cjump, where m/i
> compare can't be).
>
> I will bootstrap/regtest with additional __built_expect.

Attached patch is what I plan to commit to mainline soon. Ian, does it
look OK to you?

2014-03-10  Uros Bizjak  <ubizjak@gmail.com>

    * crtstuff.c (frame_dummy): Use void **jcr_list temporary
    variable to avoid array subscript is above array bounds warnings.
    Use __builtin_expect when checking *jcr_list for NULL.

Re-bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}
and alpha-linux-gnu.

Jakub, is it OK for mainline from RMs POV? Shall we backport this
patch to other release branches?

Uros.

Attachment: m.diff.txt
Description: Text document