Re: [PATCH] testsuite: Fix vfprintf-chk-1.c 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 +, 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
Re: [PATCH] testsuite: Fix vfprintf-chk-1.c with -fhardened
On Tue, 2024-03-12 at 17:19 +0100, Jakub Jelinek wrote:
> On Thu, Feb 15, 2024 at 10:53:08PM +, 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.
--
Xi Ruoyao
School of Aerospace Science and Technology, Xidian University
Re: [PATCH] testsuite: Fix vfprintf-chk-1.c with -fhardened
On Thu, Feb 15, 2024 at 10:53:08PM +, 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-15Sam James
>
> 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
Re: [PATCH] testsuite: Fix vfprintf-chk-1.c with -fhardened
Sam James 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-15Sam James
>
> 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
> #include
> #include
[PATCH] testsuite: Fix vfprintf-chk-1.c with -fhardened
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
```
2024-02-15 Sam James
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
#include
#include
--
2.43.1
