[02/15] debug: Increase tst-fortify checks for compiler without __va_arg_pack support
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
fail
|
Patch failed to apply
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
fail
|
Patch failed to apply
|
Commit Message
The fortify wrappers for varargs functions already add fallbacks to
builtins calls if __va_arg_pack is not supported.
Checked on aarch64, armhf, x86_64, and i686.
---
debug/tst-fortify.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
Comments
On 2023-12-21 13:59, Adhemerval Zanella wrote:
> The fortify wrappers for varargs functions already add fallbacks to
> builtins calls if __va_arg_pack is not supported.
... and in fact helps test the #else part with a different compiler,
like clang. BTW, I'm not sure if you've seen this, but Serge Guelton
used to maintain a _FORTIFY_SOURCE testsuite to do comparative testing
between clang and gcc:
https://github.com/serge-sans-paille/fortify-test-suite
It would be nice to subsume all of that, if there's additional coverage
there.
LGTM.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
> Checked on aarch64, armhf, x86_64, and i686.
> ---
> debug/tst-fortify.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/debug/tst-fortify.c b/debug/tst-fortify.c
> index 20e926751a..5cd9d22feb 100644
> --- a/debug/tst-fortify.c
> +++ b/debug/tst-fortify.c
> @@ -130,7 +130,7 @@ static int num2 = 987654;
> chk_fail_ok = 0; \
> FAIL (); \
> }
> -#if __USE_FORTIFY_LEVEL >= 2 && (!defined __cplusplus || defined __va_arg_pack)
> +#if __USE_FORTIFY_LEVEL >= 2
> # define CHK_FAIL2_START CHK_FAIL_START
> # define CHK_FAIL2_END CHK_FAIL_END
> #else
> @@ -419,7 +419,6 @@ do_test (void)
> stpncpy (buf + 6, "cd", l0 + 5);
> CHK_FAIL_END
>
> -# if !defined __cplusplus || defined __va_arg_pack
> CHK_FAIL_START
> sprintf (buf + 8, "%d", num1);
> CHK_FAIL_END
> @@ -439,7 +438,6 @@ do_test (void)
> CHK_FAIL_START
> swprintf (wbuf + 8, l0 + 3, L"%d", num1);
> CHK_FAIL_END
> -# endif
>
> memcpy (buf, str1 + 2, 9);
> CHK_FAIL_START
> @@ -550,7 +548,6 @@ do_test (void)
> FAIL ();
> }
>
> -# if !defined __cplusplus || defined __va_arg_pack
> CHK_FAIL_START
> sprintf (a.buf1 + (O + 7), "%d", num1);
> CHK_FAIL_END
> @@ -562,7 +559,6 @@ do_test (void)
> CHK_FAIL_START
> snprintf (a.buf1 + (O + 7), l0 + 3, "%d", num2);
> CHK_FAIL_END
> -# endif
>
> memcpy (a.buf1, str1 + (3 - O), 8 + O);
> CHK_FAIL_START
On Thu, 21 Dec 2023, Siddhesh Poyarekar wrote:
> On 2023-12-21 13:59, Adhemerval Zanella wrote:
> > The fortify wrappers for varargs functions already add fallbacks to
> > builtins calls if __va_arg_pack is not supported.
>
> ... and in fact helps test the #else part with a different compiler, like
> clang. BTW, I'm not sure if you've seen this, but Serge Guelton used to
> maintain a _FORTIFY_SOURCE testsuite to do comparative testing between clang
> and gcc:
>
> https://github.com/serge-sans-paille/fortify-test-suite
>
> It would be nice to subsume all of that, if there's additional coverage there.
A related (hard) issue is how to run the glibc testsuite with a different
compiler (whether in the glibc build tree, or, introducing a further
issue, against an installed copy of glibc) - as one built copy of glibc
ought to work with multiple compilers, including ones that aren't
supported for building glibc itself. (In principle such testing could
detect ABI incompatibilities as well as header issues; consider e.g. the
known issues where clang is incompatible with the x86-64 ABI's handling of
narrower-than-32-bit function arguments and return values, though it's
quite likely tests would fail to exercise such incompatibilities.)
On 21/12/23 17:02, Siddhesh Poyarekar wrote:
> On 2023-12-21 13:59, Adhemerval Zanella wrote:
>> The fortify wrappers for varargs functions already add fallbacks to
>> builtins calls if __va_arg_pack is not supported.
>
> ... and in fact helps test the #else part with a different compiler, like clang. BTW, I'm not sure if you've seen this, but Serge Guelton used to maintain a _FORTIFY_SOURCE testsuite to do comparative testing between clang and gcc:
>
> https://github.com/serge-sans-paille/fortify-test-suite
>
> It would be nice to subsume all of that, if there's additional coverage there.
Interesting, I will take a look. Our tests are still lacking a way to
proper check wrong usages that might trigger compiler warnings/error
(either through the builtins and/or wrapper).
But as I put on cover-letter, I actually tested it by using using on top
of my WIP branch that aims to enable clang build glibc.
>
> LGTM.
>
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
>>
>> Checked on aarch64, armhf, x86_64, and i686.
>> ---
>> debug/tst-fortify.c | 6 +-----
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/debug/tst-fortify.c b/debug/tst-fortify.c
>> index 20e926751a..5cd9d22feb 100644
>> --- a/debug/tst-fortify.c
>> +++ b/debug/tst-fortify.c
>> @@ -130,7 +130,7 @@ static int num2 = 987654;
>> chk_fail_ok = 0; \
>> FAIL (); \
>> }
>> -#if __USE_FORTIFY_LEVEL >= 2 && (!defined __cplusplus || defined __va_arg_pack)
>> +#if __USE_FORTIFY_LEVEL >= 2
>> # define CHK_FAIL2_START CHK_FAIL_START
>> # define CHK_FAIL2_END CHK_FAIL_END
>> #else
>> @@ -419,7 +419,6 @@ do_test (void)
>> stpncpy (buf + 6, "cd", l0 + 5);
>> CHK_FAIL_END
>> -# if !defined __cplusplus || defined __va_arg_pack
>> CHK_FAIL_START
>> sprintf (buf + 8, "%d", num1);
>> CHK_FAIL_END
>> @@ -439,7 +438,6 @@ do_test (void)
>> CHK_FAIL_START
>> swprintf (wbuf + 8, l0 + 3, L"%d", num1);
>> CHK_FAIL_END
>> -# endif
>> memcpy (buf, str1 + 2, 9);
>> CHK_FAIL_START
>> @@ -550,7 +548,6 @@ do_test (void)
>> FAIL ();
>> }
>> -# if !defined __cplusplus || defined __va_arg_pack
>> CHK_FAIL_START
>> sprintf (a.buf1 + (O + 7), "%d", num1);
>> CHK_FAIL_END
>> @@ -562,7 +559,6 @@ do_test (void)
>> CHK_FAIL_START
>> snprintf (a.buf1 + (O + 7), l0 + 3, "%d", num2);
>> CHK_FAIL_END
>> -# endif
>> memcpy (a.buf1, str1 + (3 - O), 8 + O);
>> CHK_FAIL_START
@@ -130,7 +130,7 @@ static int num2 = 987654;
chk_fail_ok = 0; \
FAIL (); \
}
-#if __USE_FORTIFY_LEVEL >= 2 && (!defined __cplusplus || defined __va_arg_pack)
+#if __USE_FORTIFY_LEVEL >= 2
# define CHK_FAIL2_START CHK_FAIL_START
# define CHK_FAIL2_END CHK_FAIL_END
#else
@@ -419,7 +419,6 @@ do_test (void)
stpncpy (buf + 6, "cd", l0 + 5);
CHK_FAIL_END
-# if !defined __cplusplus || defined __va_arg_pack
CHK_FAIL_START
sprintf (buf + 8, "%d", num1);
CHK_FAIL_END
@@ -439,7 +438,6 @@ do_test (void)
CHK_FAIL_START
swprintf (wbuf + 8, l0 + 3, L"%d", num1);
CHK_FAIL_END
-# endif
memcpy (buf, str1 + 2, 9);
CHK_FAIL_START
@@ -550,7 +548,6 @@ do_test (void)
FAIL ();
}
-# if !defined __cplusplus || defined __va_arg_pack
CHK_FAIL_START
sprintf (a.buf1 + (O + 7), "%d", num1);
CHK_FAIL_END
@@ -562,7 +559,6 @@ do_test (void)
CHK_FAIL_START
snprintf (a.buf1 + (O + 7), l0 + 3, "%d", num2);
CHK_FAIL_END
-# endif
memcpy (a.buf1, str1 + (3 - O), 8 + O);
CHK_FAIL_START