This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH: PR target/38781: PR38151: valgrind finds problem
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.
>