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?
jeff
