[v2] x86: Support x32 and IBT in heap trampoline
Checks
Commit Message
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.
---
gcc/testsuite/gcc.dg/heap-trampoline-1.c | 23 +++++++++++++
gcc/testsuite/lib/target-supports.exp | 12 +++++++
libgcc/config/i386/heap-trampoline.c | 42 ++++++++++++++++++++++--
3 files changed, 74 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/heap-trampoline-1.c
Comments
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.
>
new file mode 100644
@@ -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;
+}
@@ -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
+}
@@ -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;
};