| Message ID | AM4PR0701MB2162F4D5813449A38CA01422E4E00@AM4PR0701MB2162.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
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. Thanks, - Tom
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';