Re: PING: [PATCH] i386: Add TARGET_INDIRECT_BRANCH_REGISTER

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Feb 22, 2018 at 6:29 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Sun, Jan 28, 2018 at 11:56 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> > On Sat, Jan 27, 2018 at 2:12 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> >> For
>> >>
>> >> ---
>> >> struct C {
>> >>   virtual ~C();
>> >>   virtual void f();
>> >> };
>> >>
>> >> void
>> >> f (C *p)
>> >> {
>> >>   p->f();
>> >>   p->f();
>> >> }
>> >> ---
>> >>
>> >> -mindirect-branch=thunk-extern -O2 on x86-64 GNU/Linux generates:
>> >>
>> >> _Z1fP1C:
>> >> .LFB0:
>> >>         .cfi_startproc
>> >>         pushq   %rbx
>> >>         .cfi_def_cfa_offset 16
>> >>         .cfi_offset 3, -16
>> >>         movq    (%rdi), %rax
>> >>         movq    %rdi, %rbx
>> >>         jmp     .LIND1
>> >> .LIND0:
>> >>         pushq   16(%rax)
>> >>         jmp     __x86_indirect_thunk
>> >> .LIND1:
>> >>         call    .LIND0
>> >>         movq    (%rbx), %rax
>> >>         movq    %rbx, %rdi
>> >>         popq    %rbx
>> >>         .cfi_def_cfa_offset 8
>> >>         movq    16(%rax), %rax
>> >>         jmp     __x86_indirect_thunk_rax
>> >>         .cfi_endproc
>> >>
>> >> x86-64 is supposed to have asynchronous unwind tables by default, but
>> >> there is nothing that reflects the change in the (relative) frame
>> >> address after .LIND0.  That region really has to be moved outside of
>> >> the .cfi_startproc/.cfi_endproc bracket.
>> >>
>> >> This patch adds TARGET_INDIRECT_BRANCH_REGISTER to force indirect
>> >> branch via register when -mindirect-branch=thunk-extern is used.  Now,
>> >> -mindirect-branch=thunk-extern -O2 on x86-64 GNU/Linux generates:
>> >>
>> >> _Z1fP1C:
>> >> .LFB0:
>> >>         .cfi_startproc
>> >>         pushq   %rbx
>> >>         .cfi_def_cfa_offset 16
>> >>         .cfi_offset 3, -16
>> >>         movq    (%rdi), %rax
>> >>         movq    %rdi, %rbx
>> >>         movq    16(%rax), %rax
>> >>         call    __x86_indirect_thunk_rax
>> >>         movq    (%rbx), %rax
>> >>         movq    %rbx, %rdi
>> >>         popq    %rbx
>> >>         .cfi_def_cfa_offset 8
>> >>         movq    16(%rax), %rax
>> >>         jmp     __x86_indirect_thunk_rax
>> >>         .cfi_endproc
>> >>
>> >> Now "-mindirect-branch=thunk-extern" is equivalent to
>> >> "-mindirect-branch=thunk-extern -mindirect-branch-register", which is
>> >> used by Linux kernel.
>> >>
>> >> Tested on i686 and x86-64.  OK for trunk?
>> >>
>> >> Thanks.
>> >>
>> >> H.J.
>> >> ----
>> >> gcc/
>> >>
>> >>         PR target/84039
>> >>         * config/i386/constraints.md (Bs): Replace
>> >>         ix86_indirect_branch_register with
>> >>         TARGET_INDIRECT_BRANCH_REGISTER.
>> >>         (Bw): Likewise.
>> >>         * config/i386/i386.md (indirect_jump): Likewise.
>> >>         (tablejump): Likewise.
>> >>         (*sibcall_memory): Likewise.
>> >>         (*sibcall_value_memory): Likewise.
>> >>         Peepholes of indirect call and jump via memory: Likewise.
>> >>         * config/i386/i386.opt: Likewise.
>> >>         * config/i386/predicates.md (indirect_branch_operand): Likewise.
>> >>         (GOT_memory_operand): Likewise.
>> >>         (call_insn_operand): Likewise.
>> >>         (sibcall_insn_operand): Likewise.
>> >>         (GOT32_symbol_operand): Likewise.
>> >>         * config/i386/i386.h (TARGET_INDIRECT_BRANCH_REGISTER): New.
>> >>
>> >
>> > Here is the updated patch  to disallow *sibcall_GOT_32 and *sibcall_value_GOT_32
>> > for TARGET_INDIRECT_BRANCH_REGISTER.
>> >
>> > Tested on i686 and x86-64.  OK for trunk?
>> >
>>
>> Hi Jan,
>>
>> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg02233.html
>>
>> Is OK for trunk?
>
> I see that using register makes the problem go away and pushing address to stack
> seemed bit odd anyway. However how does this work on other types of thunk?

Kernel only uses  -mindirect-branch=thunk-extern.  I am working on a proposal
to use -mindirect-branch=thunk-extern in user space to support CET in a single
binary.  So at the end of the day, only
-mindirect-branch=thunk-extern will be used.


-- 
H.J.