Enable _FORTIFY_SOURCE=3 for gcc 12 and above
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
gcc 12 now has support for the __builtin_dynamic_object_size builtin.
Adapt the macro checks to enable _FORTIFY_SOURCE=3 on gcc 12 and above.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
include/features.h | 4 +++-
misc/sys/cdefs.h | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
Comments
On 17/12/2021 01:07, Siddhesh Poyarekar via Libc-alpha wrote:
> gcc 12 now has support for the __builtin_dynamic_object_size builtin.
> Adapt the macro checks to enable _FORTIFY_SOURCE=3 on gcc 12 and above.
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
With gcc version 12.0.0 20211217 (experimental) [master r12-6038-g411ac94611f]
I am seeing two new failures:
FAIL: debug/tst-chk7
FAIL: debug/tst-chk8
And both explicit set _FORTIFY_SOURCE to 3. I am not if is really related to
_FORTIFY_SOURCE=3:
__GI_getenv (name=0x7ffff7f6ec25 "CPATH", name@entry=0x7ffff7f6ec23 "LOCPATH") at get
env.c:84
84 if (name_start == ep_start && !strncmp (*ep + 2, name, len)
(gdb) bt
#0 __GI_getenv (name=0x7ffff7f6ec25 "CPATH", name@entry=0x7ffff7f6ec23 "LOCPATH")
at getenv.c:84
#1 0x00007ffff7dd87bc in __GI_setlocale (category=category@entry=6,
locale=locale@entry=0x555555564195 "de_DE.UTF-8") at setlocale.c:252
#2 0x000055555555e956 in do_test ()
at /home/azanella/Projects/glibc/glibc-git/debug/tst-chk1.c:1424
#3 0x0000555555562cd2 in support_test_main (argc=1, argv=0x7fffffffd810,
config=config@entry=0x7fffffffd670) at support_test_main.c:415
#4 0x0000555555559bad in main (argc=<optimized out>, argv=<optimized out>)
at ../support/test-driver.c:170
> ---
> include/features.h | 4 +++-
> misc/sys/cdefs.h | 3 ++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/features.h b/include/features.h
> index d974eabfaf..933499bcff 100644
> --- a/include/features.h
> +++ b/include/features.h
> @@ -412,7 +412,9 @@
> # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> # elif !__GNUC_PREREQ (4, 1)
> # warning _FORTIFY_SOURCE requires GCC 4.1 or later
> -# elif _FORTIFY_SOURCE > 2 && __glibc_clang_prereq (9, 0)
> +# elif _FORTIFY_SOURCE > 2 && (__glibc_clang_prereq (9, 0) \
> + || __GNUC_PREREQ (12, 0))
> +
> # if _FORTIFY_SOURCE > 3
> # warning _FORTIFY_SOURCE > 3 is treated like 3 on this platform
> # endif
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index a05b538579..1875e5c6ed 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -143,7 +143,8 @@
> #define __bos0(ptr) __builtin_object_size (ptr, 0)
>
> /* Use __builtin_dynamic_object_size at _FORTIFY_SOURCE=3 when available. */
> -#if __USE_FORTIFY_LEVEL == 3 && __glibc_clang_prereq (9, 0)
> +#if __USE_FORTIFY_LEVEL == 3 && (__glibc_clang_prereq (9, 0) \
> + || __GNUC_PREREQ (12, 0))
> # define __glibc_objsize0(__o) __builtin_dynamic_object_size (__o, 0)
> # define __glibc_objsize(__o) __builtin_dynamic_object_size (__o, 1)
> #else
On 12/17/21 18:47, Adhemerval Zanella wrote:
> On 17/12/2021 01:07, Siddhesh Poyarekar via Libc-alpha wrote:
>> gcc 12 now has support for the __builtin_dynamic_object_size builtin.
>> Adapt the macro checks to enable _FORTIFY_SOURCE=3 on gcc 12 and above.
>>
>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
> With gcc version 12.0.0 20211217 (experimental) [master r12-6038-g411ac94611f]
> I am seeing two new failures:
>
> FAIL: debug/tst-chk7
> FAIL: debug/tst-chk8
>
> And both explicit set _FORTIFY_SOURCE to 3. I am not if is really related to
> _FORTIFY_SOURCE=3:
>
> __GI_getenv (name=0x7ffff7f6ec25 "CPATH", name@entry=0x7ffff7f6ec23 "LOCPATH") at get
> env.c:84
> 84 if (name_start == ep_start && !strncmp (*ep + 2, name, len)
> (gdb) bt
> #0 __GI_getenv (name=0x7ffff7f6ec25 "CPATH", name@entry=0x7ffff7f6ec23 "LOCPATH")
> at getenv.c:84
> #1 0x00007ffff7dd87bc in __GI_setlocale (category=category@entry=6,
> locale=locale@entry=0x555555564195 "de_DE.UTF-8") at setlocale.c:252
> #2 0x000055555555e956 in do_test ()
> at /home/azanella/Projects/glibc/glibc-git/debug/tst-chk1.c:1424
> #3 0x0000555555562cd2 in support_test_main (argc=1, argv=0x7fffffffd810,
> config=config@entry=0x7fffffffd670) at support_test_main.c:415
> #4 0x0000555555559bad in main (argc=<optimized out>, argv=<optimized out>)
> at ../support/test-driver.c:170
That looks unrelated but I'll take a look. I've been testing with the
full __bdos patchset and that did not fail.
Thanks,
Siddhesh
On 12/17/21 19:24, Siddhesh Poyarekar wrote:
> On 12/17/21 18:47, Adhemerval Zanella wrote:
>> On 17/12/2021 01:07, Siddhesh Poyarekar via Libc-alpha wrote:
>>> gcc 12 now has support for the __builtin_dynamic_object_size builtin.
>>> Adapt the macro checks to enable _FORTIFY_SOURCE=3 on gcc 12 and above.
>>>
>>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>>
>> With gcc version 12.0.0 20211217 (experimental) [master
>> r12-6038-g411ac94611f]
>> I am seeing two new failures:
>>
>> FAIL: debug/tst-chk7
>> FAIL: debug/tst-chk8
>>
>> And both explicit set _FORTIFY_SOURCE to 3. I am not if is really
>> related to
>> _FORTIFY_SOURCE=3:
>>
>> __GI_getenv (name=0x7ffff7f6ec25 "CPATH", name@entry=0x7ffff7f6ec23
>> "LOCPATH") at get
>> env.c:84
>> 84 if (name_start == ep_start && !strncmp (*ep + 2,
>> name, len)
>> (gdb) bt
>> #0 __GI_getenv (name=0x7ffff7f6ec25 "CPATH",
>> name@entry=0x7ffff7f6ec23 "LOCPATH")
>> at getenv.c:84
>> #1 0x00007ffff7dd87bc in __GI_setlocale (category=category@entry=6,
>> locale=locale@entry=0x555555564195 "de_DE.UTF-8") at setlocale.c:252
>> #2 0x000055555555e956 in do_test ()
>> at /home/azanella/Projects/glibc/glibc-git/debug/tst-chk1.c:1424
>> #3 0x0000555555562cd2 in support_test_main (argc=1, argv=0x7fffffffd810,
>> config=config@entry=0x7fffffffd670) at support_test_main.c:415
>> #4 0x0000555555559bad in main (argc=<optimized out>, argv=<optimized
>> out>)
>> at ../support/test-driver.c:170
>
> That looks unrelated but I'll take a look. I've been testing with the
> full __bdos patchset and that did not fail.
Sorry that was tardy of me; that's just the test failing to catch buffer
overflows, which ends up corrupting state for later tests and
eventually, getenv. I should have anticipated that.
I'm basically trying to get the support in early in case the remaining
gcc patches take longer and glibc 2.35 freezes over. Should I xfail
these tests for gcc for now and then revert that bit once the rest of
the support goes in? If it's not acceptable to push this change before
all of the compiler support is in, then I'll settle for the possibility
of having to wait until 2.36 development opens and then backport the
change to 2.35.
What would you suggest?
Thanks,
Siddhesh
On 17/12/2021 12:24, Siddhesh Poyarekar wrote:
> On 12/17/21 19:24, Siddhesh Poyarekar wrote:
>> On 12/17/21 18:47, Adhemerval Zanella wrote:
>>> On 17/12/2021 01:07, Siddhesh Poyarekar via Libc-alpha wrote:
>>>> gcc 12 now has support for the __builtin_dynamic_object_size builtin.
>>>> Adapt the macro checks to enable _FORTIFY_SOURCE=3 on gcc 12 and above.
>>>>
>>>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>>>
>>> With gcc version 12.0.0 20211217 (experimental) [master r12-6038-g411ac94611f]
>>> I am seeing two new failures:
>>>
>>> FAIL: debug/tst-chk7
>>> FAIL: debug/tst-chk8
>>>
>>> And both explicit set _FORTIFY_SOURCE to 3. I am not if is really related to
>>> _FORTIFY_SOURCE=3:
>>>
>>> __GI_getenv (name=0x7ffff7f6ec25 "CPATH", name@entry=0x7ffff7f6ec23 "LOCPATH") at get
>>> env.c:84
>>> 84 if (name_start == ep_start && !strncmp (*ep + 2, name, len)
>>> (gdb) bt
>>> #0 __GI_getenv (name=0x7ffff7f6ec25 "CPATH", name@entry=0x7ffff7f6ec23 "LOCPATH")
>>> at getenv.c:84
>>> #1 0x00007ffff7dd87bc in __GI_setlocale (category=category@entry=6,
>>> locale=locale@entry=0x555555564195 "de_DE.UTF-8") at setlocale.c:252
>>> #2 0x000055555555e956 in do_test ()
>>> at /home/azanella/Projects/glibc/glibc-git/debug/tst-chk1.c:1424
>>> #3 0x0000555555562cd2 in support_test_main (argc=1, argv=0x7fffffffd810,
>>> config=config@entry=0x7fffffffd670) at support_test_main.c:415
>>> #4 0x0000555555559bad in main (argc=<optimized out>, argv=<optimized out>)
>>> at ../support/test-driver.c:170
>>
>> That looks unrelated but I'll take a look. I've been testing with the full __bdos patchset and that did not fail.
>
> Sorry that was tardy of me; that's just the test failing to catch buffer overflows, which ends up corrupting state for later tests and eventually, getenv. I should have anticipated that.
>
> I'm basically trying to get the support in early in case the remaining gcc patches take longer and glibc 2.35 freezes over. Should I xfail these tests for gcc for now and then revert that bit once the rest of the support goes in? If it's not acceptable to push this change before all of the compiler support is in, then I'll settle for the possibility of having to wait until 2.36 development opens and then backport the change to 2.35.
>
> What would you suggest?
I am not sure I am following what is happening here, is the test wrong for
_FORTIFY_SOURCE=3, our _FORTIFY_SOURCE=3 lacking some hardening, or missing
upstream gcc support?
On 12/18/21 00:34, Adhemerval Zanella wrote:
> I am not sure I am following what is happening here, is the test wrong for
> _FORTIFY_SOURCE=3, our _FORTIFY_SOURCE=3 lacking some hardening, or missing
> upstream gcc support?
It's missing some support upstream. Basically the series is this:
https://patchwork.sourceware.org/project/gcc/list/?series=5387&state=*
where I've committed 1/6 and 2/6, thus adding
__builtin_dynamic_object_size identification to gcc. However, the
actual dynamic expression generation is in patches 3-6, which is why the
tests are failing.
I may not be able to finish updating and pushing all the patches before
glibc freeze.
Siddhesh
On 17/12/2021 16:30, Siddhesh Poyarekar wrote:
> On 12/18/21 00:34, Adhemerval Zanella wrote:
>> I am not sure I am following what is happening here, is the test wrong for
>> _FORTIFY_SOURCE=3, our _FORTIFY_SOURCE=3 lacking some hardening, or missing
>> upstream gcc support?
>
> It's missing some support upstream. Basically the series is this:
>
> https://patchwork.sourceware.org/project/gcc/list/?series=5387&state=*
>
> where I've committed 1/6 and 2/6, thus adding __builtin_dynamic_object_size identification to gcc. However, the actual dynamic expression generation is in patches 3-6, which is why the tests are failing.
>
> I may not be able to finish updating and pushing all the patches before glibc freeze.
Right, it this case I am afraid we will need to hold enabling _FORTIFY_SOURCE=3
until we get proper and complete support from compiler.
On 12/18/21 01:18, Adhemerval Zanella wrote:
>
>
> On 17/12/2021 16:30, Siddhesh Poyarekar wrote:
>> On 12/18/21 00:34, Adhemerval Zanella wrote:
>>> I am not sure I am following what is happening here, is the test wrong for
>>> _FORTIFY_SOURCE=3, our _FORTIFY_SOURCE=3 lacking some hardening, or missing
>>> upstream gcc support?
>>
>> It's missing some support upstream. Basically the series is this:
>>
>> https://patchwork.sourceware.org/project/gcc/list/?series=5387&state=*
>>
>> where I've committed 1/6 and 2/6, thus adding __builtin_dynamic_object_size identification to gcc. However, the actual dynamic expression generation is in patches 3-6, which is why the tests are failing.
>>
>> I may not be able to finish updating and pushing all the patches before glibc freeze.
>
> Right, it this case I am afraid we will need to hold enabling _FORTIFY_SOURCE=3
> until we get proper and complete support from compiler.
I agree that's fair.
Thanks,
Siddhesh
On 17/12/2021 18:47, Adhemerval Zanella via Libc-alpha wrote:
>
>
> On 17/12/2021 01:07, Siddhesh Poyarekar via Libc-alpha wrote:
>> gcc 12 now has support for the __builtin_dynamic_object_size builtin.
>> Adapt the macro checks to enable _FORTIFY_SOURCE=3 on gcc 12 and above.
>>
>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
> With gcc version 12.0.0 20211217 (experimental) [master r12-6038-g411ac94611f]
> I am seeing two new failures:
>
> FAIL: debug/tst-chk7
> FAIL: debug/tst-chk8
This should now work on gcc trunk as of r12-6482, tested with:
gcc version 12.0.0 20220112 (experimental) (GCC)
OK to commit?
Thanks,
Siddhesh
On 12/01/2022 01:14, Siddhesh Poyarekar wrote:
> On 17/12/2021 18:47, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 17/12/2021 01:07, Siddhesh Poyarekar via Libc-alpha wrote:
>>> gcc 12 now has support for the __builtin_dynamic_object_size builtin.
>>> Adapt the macro checks to enable _FORTIFY_SOURCE=3 on gcc 12 and above.
>>>
>>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>>
>> With gcc version 12.0.0 20211217 (experimental) [master r12-6038-g411ac94611f]
>> I am seeing two new failures:
>>
>> FAIL: debug/tst-chk7
>> FAIL: debug/tst-chk8
>
> This should now work on gcc trunk as of r12-6482, tested with:
>
> gcc version 12.0.0 20220112 (experimental) (GCC)
>
> OK to commit?
Ok, I have verified that it now work with gcc 12 current.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.or>
@@ -412,7 +412,9 @@
# warning _FORTIFY_SOURCE requires compiling with optimization (-O)
# elif !__GNUC_PREREQ (4, 1)
# warning _FORTIFY_SOURCE requires GCC 4.1 or later
-# elif _FORTIFY_SOURCE > 2 && __glibc_clang_prereq (9, 0)
+# elif _FORTIFY_SOURCE > 2 && (__glibc_clang_prereq (9, 0) \
+ || __GNUC_PREREQ (12, 0))
+
# if _FORTIFY_SOURCE > 3
# warning _FORTIFY_SOURCE > 3 is treated like 3 on this platform
# endif
@@ -143,7 +143,8 @@
#define __bos0(ptr) __builtin_object_size (ptr, 0)
/* Use __builtin_dynamic_object_size at _FORTIFY_SOURCE=3 when available. */
-#if __USE_FORTIFY_LEVEL == 3 && __glibc_clang_prereq (9, 0)
+#if __USE_FORTIFY_LEVEL == 3 && (__glibc_clang_prereq (9, 0) \
+ || __GNUC_PREREQ (12, 0))
# define __glibc_objsize0(__o) __builtin_dynamic_object_size (__o, 0)
# define __glibc_objsize(__o) __builtin_dynamic_object_size (__o, 1)
#else