PATCH: PR target/38781: PR38151: valgrind finds problem

Richard Guenther richard.guenther@gmail.com
Tue Jan 13 21:35:00 GMT 2009


On Tue, Jan 13, 2009 at 10:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jan 13, 2009 at 1:17 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Tue, Jan 13, 2009 at 6:30 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Jan 9, 2009 at 9:36 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> Zero sized arrays or structures are special cases for
>>>> x86-64. We shouldn't check classes on them. OK for
>>>> trunk?
>>>>
>>>>
>>>> --
>>>> H.J.
>>>> ---
>>>> 2008-01-09  H.J. Lu  <hongjiu.lu@intel.com>
>>>>
>>>>        PR target/38781
>>>>        * config/i386/i386.c (classify_argument): Check zero sized
>>>>        arrays or structures.
>>>>
>>>
>>> Ping.
>>
>> I don't see a testcase, so what is exactly the problem?
>>
>
> Valgrind finds that for gcc.c-torture/execute/pr38151.c:
>
> --
> void abort (void);
>
> struct S2848
> {
>  unsigned int a;
>  _Complex int b;
>  struct
>  {
>  } __attribute__ ((aligned)) c;
> };
>
> struct S2848 s2848;
>
> int fails;
>
> void  __attribute__((noinline))
> check2848va (int z, ...)
> {
>  struct S2848 arg;
>  __builtin_va_list ap;
>
>  __builtin_va_start (ap, z);
>
>  arg = __builtin_va_arg (ap, struct S2848);
>
>  if (s2848.a != arg.a)
>    ++fails;
>  if (s2848.b != arg.b)
>    ++fails;
>
>  __builtin_va_end (ap);
> }
>
> int main (void)
> {
>  s2848.a = 4027477739U;
>  s2848.b = (723419448 + -218144346 * __extension__ 1i);
>
>  check2848va (1, s2848);
>
>  if (fails)
>    abort ();
>
>  return 0;
> }
> --
>
> We are reading beyond array of classes in
>
>                      num = classify_argument (TYPE_MODE (TREE_TYPE (field)),
>                                               TREE_TYPE (field), subclasses,
>                                               (int_bit_position (field)
>                                                + bit_offset) % 256);
>                      if (!num)
>                        return 0;
>                      for (i = 0; i < num; i++)
>                        {
>                          int pos =
>                            (int_bit_position (field) + (bit_offset %
> 64)) / 8 / 8;
>                          classes[i + pos] =
>                            merge_classes (subclasses[i], classes[i + pos]);
>                        }
>                    }
>
> The problem is classify_argument may return:
>
>      /* Zero sized arrays or structures are NO_CLASS.  We return 0 to
>         signalize memory class, so handle it as special case.  */
>      if (!words)
>        {
>          classes[0] = X86_64_NO_CLASS;
>          return 1;
>        }
>
> When num == 1, we should check classes[0] before accessing
> classes[1].
>
> Even if we were reading garbage, we didn't crash.

I see.  Shouldn't the other places that recurse also check this?

Richard.

> --
> H.J.
>



More information about the Gcc-patches mailing list