| Message ID | 20240213164052.300183-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] x86: Support x32 and IBT in heap trampoline | expand |
On Tue, Feb 13, 2024 at 08:40:52AM -0800, H.J. Lu wrote: > Add x32 and IBT support to x86 heap trampoline implementation with a > testcase. > > 2024-02-13 Jakub Jelinek <jakub@redhat.com> > H.J. Lu <hjl.tools@gmail.com> > > libgcc/ > > PR target/113855 > * config/i386/heap-trampoline.c (trampoline_insns): Add IBT > support and pad to the multiple of 4 bytes. Use movabsq > instead of movabs in comments. Add -mx32 variant. > > gcc/testsuite/ > > PR target/113855 > * gcc.dg/heap-trampoline-1.c: New test. > * lib/target-supports.exp (check_effective_target_heap_trampoline): > New. LGTM, but please give Iain a day or two to chime in. Jakub
On Tue, Feb 13, 2024 at 8:46 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Feb 13, 2024 at 08:40:52AM -0800, H.J. Lu wrote: > > Add x32 and IBT support to x86 heap trampoline implementation with a > > testcase. > > > > 2024-02-13 Jakub Jelinek <jakub@redhat.com> > > H.J. Lu <hjl.tools@gmail.com> > > > > libgcc/ > > > > PR target/113855 > > * config/i386/heap-trampoline.c (trampoline_insns): Add IBT > > support and pad to the multiple of 4 bytes. Use movabsq > > instead of movabs in comments. Add -mx32 variant. > > > > gcc/testsuite/ > > > > PR target/113855 > > * gcc.dg/heap-trampoline-1.c: New test. > > * lib/target-supports.exp (check_effective_target_heap_trampoline): > > New. > > LGTM, but please give Iain a day or two to chime in. > > Jakub > I am checking it in today.
> On 14 Feb 2024, at 18:12, H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Feb 13, 2024 at 8:46 AM Jakub Jelinek <jakub@redhat.com> wrote: >> >> On Tue, Feb 13, 2024 at 08:40:52AM -0800, H.J. Lu wrote: >>> Add x32 and IBT support to x86 heap trampoline implementation with a >>> testcase. >>> >>> 2024-02-13 Jakub Jelinek <jakub@redhat.com> >>> H.J. Lu <hjl.tools@gmail.com> >>> >>> libgcc/ >>> >>> PR target/113855 >>> * config/i386/heap-trampoline.c (trampoline_insns): Add IBT >>> support and pad to the multiple of 4 bytes. Use movabsq >>> instead of movabs in comments. Add -mx32 variant. >>> >>> gcc/testsuite/ >>> >>> PR target/113855 >>> * gcc.dg/heap-trampoline-1.c: New test. >>> * lib/target-supports.exp (check_effective_target_heap_trampoline): >>> New. >> >> LGTM, but please give Iain a day or two to chime in. >> >> Jakub >> > > I am checking it in today. I have just one question; from your patch the use of endbr* seems to be unconditionally based on the flags used to build libgcc. However, I was expecting that the use of extended trampolines like this would depend on command line flags used to compile the end-user’s code. As per the discussion in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113855#c4 I was expecting that we would need to extend this implementation to cover more cases (i.e. the GCC-14 implementation is “base”). any comments? Iain > > -- > H.J.
On Wed, Feb 14, 2024 at 07:59:26PM +0000, Iain Sandoe wrote: > I have just one question; > > from your patch the use of endbr* seems to be unconditionally based on the > flags used to build libgcc. > > However, I was expecting that the use of extended trampolines like this would > depend on command line flags used to compile the end-user’s code. I think for CET the rule is you need everything to be compiled with the CET options, including libgcc, trying to mix and match objects built one and another way unless one is lucky and there are no indirect calls to something that isn't marked is not going to work when enforcing it. And, the endbr* insn acts as a nop on older CPUs (ok, except for VIA or something similar or pre-i686?) or when not enforcing. So, if CET is enabled while building libgcc, the insns in there don't hurt, and if the gcc libraries aren't build with CET, one really can't use it. Jakub
On Wed, Feb 14, 2024 at 11:59 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > > > > > On 14 Feb 2024, at 18:12, H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Feb 13, 2024 at 8:46 AM Jakub Jelinek <jakub@redhat.com> wrote: > >> > >> On Tue, Feb 13, 2024 at 08:40:52AM -0800, H.J. Lu wrote: > >>> Add x32 and IBT support to x86 heap trampoline implementation with a > >>> testcase. > >>> > >>> 2024-02-13 Jakub Jelinek <jakub@redhat.com> > >>> H.J. Lu <hjl.tools@gmail.com> > >>> > >>> libgcc/ > >>> > >>> PR target/113855 > >>> * config/i386/heap-trampoline.c (trampoline_insns): Add IBT > >>> support and pad to the multiple of 4 bytes. Use movabsq > >>> instead of movabs in comments. Add -mx32 variant. > >>> > >>> gcc/testsuite/ > >>> > >>> PR target/113855 > >>> * gcc.dg/heap-trampoline-1.c: New test. > >>> * lib/target-supports.exp (check_effective_target_heap_trampoline): > >>> New. > >> > >> LGTM, but please give Iain a day or two to chime in. > >> > >> Jakub > >> > > > > I am checking it in today. > > I have just one question; > > from your patch the use of endbr* seems to be unconditionally based on the > flags used to build libgcc. > > However, I was expecting that the use of extended trampolines like this would > depend on command line flags used to compile the end-user’s code. We only ship ONE libgcc binary. You get the same libgcc binary regardless what options one uses to compile an application. Since ENBD64 is a NOP if IBT isn't enabled, so it isn't an issue. > As per the discussion in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113855#c4 > I was expecting that we would need to extend this implementation to cover more > cases (i.e. the GCC-14 implementation is “base”). > > any comments? > Iain > > > > > > -- > > H.J. >
diff --git a/gcc/testsuite/gcc.dg/heap-trampoline-1.c b/gcc/testsuite/gcc.dg/heap-trampoline-1.c new file mode 100644 index 00000000000..1aebe00d731 --- /dev/null +++ b/gcc/testsuite/gcc.dg/heap-trampoline-1.c @@ -0,0 +1,23 @@ +/* { dg-do run { target heap_trampoline } } */ +/* { dg-options "-ftrampoline-impl=heap" } */ + +__attribute__((noipa)) int +bar (int (*fn) (int)) +{ + return fn (42) + 1; +} + +int +main () +{ + int a = 0; + int foo (int x) { if (x != 42) __builtin_abort (); return ++a; } + if (bar (foo) != 2 || a != 1) + __builtin_abort (); + if (bar (foo) != 3 || a != 2) + __builtin_abort (); + a = 42; + if (bar (foo) != 44 || a != 43) + __builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 6ce8557c9a9..81715999f87 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -13477,3 +13477,15 @@ proc dg-require-python-h { args } { eval lappend extra-tool-flags $python_flags verbose "After appending, extra-tool-flags: ${extra-tool-flags}" 3 } + +# Return 1 if the target supports heap-trampoline, 0 otherwise. +proc check_effective_target_heap_trampoline {} { + if { [istarget aarch64*-*-linux*] + || [istarget i?86-*-darwin*] + || [istarget x86_64-*-darwin*] + || [istarget i?86-*-linux*] + || [istarget x86_64-*-linux*] } { + return 1 + } + return 0 +} diff --git a/libgcc/config/i386/heap-trampoline.c b/libgcc/config/i386/heap-trampoline.c index 1df0aa06108..a8637dc92d3 100644 --- a/libgcc/config/i386/heap-trampoline.c +++ b/libgcc/config/i386/heap-trampoline.c @@ -30,28 +30,64 @@ void __gcc_nested_func_ptr_created (void *chain, void *func, void *dst); void __gcc_nested_func_ptr_deleted (void); #if __x86_64__ + +#ifdef __LP64__ static const uint8_t trampoline_insns[] = { - /* movabs $<func>,%r11 */ +#if defined __CET__ && (__CET__ & 1) != 0 + /* endbr64. */ + 0xf3, 0x0f, 0x1e, 0xfa, +#endif + + /* movabsq $<func>,%r11 */ 0x49, 0xbb, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - /* movabs $<chain>,%r10 */ + /* movabsq $<chain>,%r10 */ 0x49, 0xba, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* rex.WB jmpq *%r11 */ - 0x41, 0xff, 0xe3 + 0x41, 0xff, 0xe3, + + /* Pad to the multiple of 4 bytes. */ + 0x90 }; +#else +static const uint8_t trampoline_insns[] = { +#if defined __CET__ && (__CET__ & 1) != 0 + /* endbr64. */ + 0xf3, 0x0f, 0x1e, 0xfa, +#endif + + /* movl $<func>,%r11d */ + 0x41, 0xbb, + 0x00, 0x00, 0x00, 0x00, + + /* movl $<chain>,%r10d */ + 0x41, 0xba, + 0x00, 0x00, 0x00, 0x00, + + /* rex.WB jmpq *%r11 */ + 0x41, 0xff, 0xe3, + + /* Pad to the multiple of 4 bytes. */ + 0x90 +}; +#endif union ix86_trampoline { uint8_t insns[sizeof(trampoline_insns)]; struct __attribute__((packed)) fields { +#if defined __CET__ && (__CET__ & 1) != 0 + uint8_t endbr64[4]; +#endif uint8_t insn_0[2]; void *func_ptr; uint8_t insn_1[2]; void *chain_ptr; uint8_t insn_2[3]; + uint8_t pad; } fields; };