On 31/08/16 07:42, Tom de Vries wrote:
> On 30/08/16 11:38, Bernd Edlinger wrote:
>> On 08/30/16 10:21, Tom de Vries wrote:
>>> On 29/08/16 18:43, Bernd Edlinger wrote:
>>>> Thanks!
>>>>
>>>> Actually my patch missed to fix one combination: -m32 with -fpic
>>>>
>>>> make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c --tool_opts
>>>> '-m32 -fpic'"
>>>>
>>>> FAIL: c-c++-common/ubsan/object-size-9.c   -O2  execution test
>>>> FAIL: c-c++-common/ubsan/object-size-9.c   -O2 -flto
>>>> -fno-use-linker-plugin -flto-partition=none  execution test
>>>>
>>>> The problem here is that the functions f2 and f3 access a stack-
>>>> based object out of bounds and that is inlined in main and
>>>> therefore smashes the return address of main in this case.
>>>>
>>>> A possible fix could look like follows:
>>>>
>>>> Index: object-size-9.c
>>>> ===================================================================
>>>> --- object-size-9.c    (revision 239794)
>>>> +++ object-size-9.c    (working copy)
>>>> @@ -93,5 +93,9 @@
>>>>   #endif
>>>>     f4 (12);
>>>>     f5 (12);
>>>> +#ifdef __cplusplus
>>>> +  /* Stack may be smashed by f2/f3 above.  */
>>>> +  __builtin_exit (0);
>>>> +#endif
>>>>     return 0;
>>>>   }
>>>>
>>>>
>>>> Do you think that this should be fixed too?
>>>
>>> I think it should be fixed. Ideally, we'd prevent the out-of-bounds
>>> writes to have harmful effects, but I'm not sure how to enforce that.
>>>
>>> This works for me:
>>> ...
>>> diff --git a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
>>> b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
>>> index 46f1fb9..fec920d 100644
>>> --- a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
>>> +++ b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
>>> @@ -31,6 +31,7 @@ static struct C
>>>   f2 (int i)
>>>   {
>>>     struct C x;
>>> +  struct C x2;
>>>     x.d[i] = 'z';
>>>     return x;
>>>   }
>>> @@ -45,6 +46,7 @@ static struct C
>>>   f3 (int i)
>>>   {
>>>     struct C x;
>>> +  struct C x2;
>>>     char *p = x.d;
>>>     p += i;
>>>     *p = 'z';
>>> ...
>>>
>>> But I have no idea how stable this solution is.
>>>
>>
>> At least x2 could not be opimized away, as it is no POD,
>> but there is no guarantee, that x2 comes after x on the stack.
>> Another possibility, which seems to work as well:
>>
>>
>> Index: gcc/testsuite/c-c++-common/ubsan/object-size-9.c
>> ===================================================================
>> --- gcc/testsuite/c-c++-common/ubsan/object-size-9.c    (revision 239794)
>> +++ gcc/testsuite/c-c++-common/ubsan/object-size-9.c    (working copy)
>> @@ -30,7 +30,7 @@ f1 (struct T x, int i)
>>   static struct C
>>   f2 (int i)
>>   {
>> -  struct C x;
>> +  struct C x __attribute__ ((aligned(16)));
>>     x.d[i] = 'z';
>>     return x;
>>   }
>> @@ -44,7 +44,7 @@ f2 (int i)
>>   static struct C
>>   f3 (int i)
>>   {
>> -  struct C x;
>> +  struct C x __attribute ((aligned(16)));
>>     char *p = x.d;
>>     p += i;
>>     *p = 'z';
>>
>
> Works for me.

OK for trunk, 5 & 6 branch?

Thanks,
- Tom