On 09/19/16 22:19, Jeff Law wrote: > On 09/15/2016 04:29 AM, Tom de Vries wrote: >> 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 >> >> >> 0001-Fix-object-size-9.c-with-fpic.patch >> >> >> Fix object-size-9.c with -fpic >> >> 2016-09-15 Bernd Edlinger <bernd.edlin...@hotmail.de> >> Tom de Vries <t...@codesourcery.com> >> >> PR testsuite/77411 >> * c-c++-common/ubsan/object-size-9.c (f2, f3): Declare struct C >> variable >> with __attribute__((aligned(16))). > Just so I'm sure on this stuff. > > The tests exist to verify that ubsan detects the out-of-bounds writes. > ubsan isn't terminating the process, so we end up with a smashed stack? > > I fail to see how using aligned like this should consistently work. It > feels like a hack that just happens to work now. > > Would it work to break this up into distinct tests, exit()-ing from each > function rather than returning back to main? >
Yes. I think how this test is designed, each function must be inlined, or it will fail anyway. It was for instance impossible to pass the ubsan test, if -fno-inline was used as RUNTESTFLAGS. Therefore it works as well, if main avoids to return and calls exit(0) instead, with a specific comment of course. See https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01985.html Bernd.