Message ID | 20240215225806.2836388-1-sam@gentoo.org |
---|---|
State | New |
Headers | show |
Series | testsuite: Fix vfprintf-chk-1.c with -fhardened | expand |
Sam James <sam@gentoo.org> writes: > With _FORTIFY_SOURCE >= 2 (enabled by -fhardened), vfprintf-chk-1.c's > __vfprintf_chk ends up calling __vprintf_chk rather than vprintf. > > ``` > --- a/fortify.s > +++ b/no-fortify.s > @@ -8,27 +8,28 @@ > [...] > __vfprintf_chk: > [...] > movl $1, should_optimize(%rip) > - jmp __vfprintf_chk > + jmp vfprintf@PLT > ``` Ping. > > 2024-02-15 Sam James <sam@gentoo.org> > > gcc/testsuite/ChangeLog: > * gcc.c-torture/execute/vfprintf-chk-1.c (__vfprintf_chk): Undefine _FORTIFY_SOURCE > to call the real vfprintf. > --- > The test, AIUI, is trying to test GCC's own basic _chk bits rather than > any of e.g. glibc's _FORTIFY_SOURCE handling. > > I'm curious as to why only vfprintf triggers this right now. If this patch is right, > perhaps we should do printf-chk-1.c, fprintf-chk-1.c, and vprintf-chk-1. > > Please push if OK as I don't have access. > > gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c > index 401eaf4304a4..a8e5689e3fe6 100644 > --- a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c > +++ b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c > @@ -1,6 +1,7 @@ > /* { dg-skip-if "requires io" { freestanding } } */ > > #ifndef test > +#undef _FORTIFY_SOURCE > #include <stdio.h> > #include <stdlib.h> > #include <stdarg.h>
On Thu, Feb 15, 2024 at 10:53:08PM +0000, Sam James wrote: > With _FORTIFY_SOURCE >= 2 (enabled by -fhardened), vfprintf-chk-1.c's > __vfprintf_chk ends up calling __vprintf_chk rather than vprintf. s/__vprintf_chk/__vfprintf_chk/ above > > ``` > --- a/fortify.s > +++ b/no-fortify.s > @@ -8,27 +8,28 @@ > [...] > __vfprintf_chk: > [...] > movl $1, should_optimize(%rip) > - jmp __vfprintf_chk > + jmp vfprintf@PLT > ``` > > 2024-02-15 Sam James <sam@gentoo.org> > > gcc/testsuite/ChangeLog: > * gcc.c-torture/execute/vfprintf-chk-1.c (__vfprintf_chk): Undefine _FORTIFY_SOURCE > to call the real vfprintf. I think we should change vprintf-chk-1.c as well. > --- > The test, AIUI, is trying to test GCC's own basic _chk bits rather than > any of e.g. glibc's _FORTIFY_SOURCE handling. > > I'm curious as to why only vfprintf triggers this right now. If this patch is right, > perhaps we should do printf-chk-1.c, fprintf-chk-1.c, and vprintf-chk-1. That is easy. In the printf-chk-1.c case it calls vprintf which results in __vfprintf_chk or __vprintf_chk calls but redefines just __printf_chk. Similarly fprintf-chk-1.c case calls vfprintf which results in __vfprintf_chk call which isn't redefined either. In the __vfprintf_chk case it calls vfprintf, which the inline results in __vfprintf_chk call and so it self-recurses instead of calling glibc for the actual implementation. In the vprintf-chk-1.c case, it depends. __fortify_function int vprintf (const char *__restrict __fmt, __gnuc_va_list __ap) { #ifdef __USE_EXTERN_INLINES return __vfprintf_chk (stdout, __USE_FORTIFY_LEVEL - 1, __fmt, __ap); #else return __vprintf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, __ap); #endif } So, if __USE_EXTERN_INLINES is defined, it will work fine, if not, it will also self-recurse like vfprintf-chk-1.c. Jakub
On Tue, 2024-03-12 at 17:19 +0100, Jakub Jelinek wrote: > On Thu, Feb 15, 2024 at 10:53:08PM +0000, Sam James wrote: > > With _FORTIFY_SOURCE >= 2 (enabled by -fhardened), vfprintf-chk-1.c's > > __vfprintf_chk ends up calling __vprintf_chk rather than vprintf. Do we really want to support adding random CFLAGS running the test suite? AFAIK adding random CFLAGS will just cause test failures here or there. We are adjusting the test suite for -fPIE -pie and -fstack- protector-strong but it's because they can be implicitly enabled with -- enable-default-* options, and we don't have --enable-default-hardened as at now. If we need to bootstrap a hardened GCC and test it, pass -fhardened as how "info gccinstall" suggests: make BOOT_CFLAGS="-O2 -g -fhardened" instead of env C{,XX}FLAGS="-O2 -g -fhardened" /path/to/gcc/configure ... which will taint the test suite with -fhardened.
On Wed, Mar 13, 2024 at 06:05:29PM +0800, Xi Ruoyao wrote: > On Tue, 2024-03-12 at 17:19 +0100, Jakub Jelinek wrote: > > On Thu, Feb 15, 2024 at 10:53:08PM +0000, Sam James wrote: > > > With _FORTIFY_SOURCE >= 2 (enabled by -fhardened), vfprintf-chk-1.c's > > > __vfprintf_chk ends up calling __vprintf_chk rather than vprintf. > > Do we really want to support adding random CFLAGS running the test > suite? Random flags certainly not, but some flags should be supported and are very useful. We already support the various ABI changing options (-m32 -m64 -mx32 and the like) and ISA options in there (-march=whatever, -msse2, etc.), and testing with -fstack-protector-strong is what some distros do for years, testing with -fhardened is desirable if pretty much everything in the distros is built with that flag. Another thing is using --param whatever=whatever in the target_board flags, or -fno-tree-dce etc. that may or might not work and user needs to be prepared there will be extra fails. Jakub
--- a/fortify.s +++ b/no-fortify.s @@ -8,27 +8,28 @@ [...] __vfprintf_chk: [...] movl $1, should_optimize(%rip) - jmp __vfprintf_chk + jmp vfprintf@PLT ``` 2024-02-15 Sam James <sam@gentoo.org> gcc/testsuite/ChangeLog: * gcc.c-torture/execute/vfprintf-chk-1.c (__vfprintf_chk): Undefine _FORTIFY_SOURCE to call the real vfprintf. --- The test, AIUI, is trying to test GCC's own basic _chk bits rather than any of e.g. glibc's _FORTIFY_SOURCE handling. I'm curious as to why only vfprintf triggers this right now. If this patch is right, perhaps we should do printf-chk-1.c, fprintf-chk-1.c, and vprintf-chk-1. Please push if OK as I don't have access. gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c index 401eaf4304a4..a8e5689e3fe6 100644 --- a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c +++ b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c @@ -1,6 +1,7 @@ /* { dg-skip-if "requires io" { freestanding } } */