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 1:33 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> 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?
>
I checked other places. They check classes[0] == X86_64_NO_CLASS
before accessing classes beyond index 0.
--
H.J.