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

[Uros Bizjak]

On Mon, Mar 10, 2014 at 11:27 AM, Uros Bizjak <ubiz...@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  <ubiz...@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.

Index: crtstuff.c
===================================================================
--- crtstuff.c  (revision 208448)
+++ crtstuff.c  (working copy)
@@ -460,12 +460,14 @@ frame_dummy (void)
 #endif /* USE_EH_FRAME_REGISTRY */
 
 #ifdef JCR_SECTION_NAME
-  if (__JCR_LIST__[0])
+  void **jcr_list;
+  __asm ("" : "=g" (jcr_list) : "0" (__JCR_LIST__));
+  if (__builtin_expect (*jcr_list != NULL, 0))
     {
       void (*register_classes) (void *) = _Jv_RegisterClasses;
       __asm ("" : "+r" (register_classes));
       if (register_classes)
-       register_classes (__JCR_LIST__);
+       register_classes (jcr_list);
     }
 #endif /* JCR_SECTION_NAME */
 
@@ -565,12 +567,14 @@ __do_global_ctors_1(void)
 #endif
 
 #ifdef JCR_SECTION_NAME
-  if (__JCR_LIST__[0])
+  void **jcr_list
+  __asm ("" : "=g" (jcr_list) : "0" (__JCR_LIST__));
+  if (__builtin_expect (*jcr_list != NULL, 0))
     {
       void (*register_classes) (void *) = _Jv_RegisterClasses;
       __asm ("" : "+r" (register_classes));
       if (register_classes)
-       register_classes (__JCR_LIST__);
+       register_classes (jcr_list);
     }
 #endif