| Message ID | 10A4B8FF-4F9C-488F-9011-E1B0EBCA02BC@googlemail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [X86] Fix PR82920 (code part). | expand |
On Fri, May 10, 2019 at 11:03 PM Iain Sandoe <idsandoe@googlemail.com> wrote: > > Hi! > > PR82920 is about CET fails on Darwin. > > Initially, this was expected to be just a testsuite issue, however it turns out that the indirection thunks code has inconsistent handling of the output of labels. Thus some of the output is missing the leading “_” on Darwin, which breaks ABI and won’t link. > > Since most of the tests are scan-asms that check for what’s expected, they currently pass on Darwin but will begin failing when the codegen is fixed. Thus there is larger, but mechanical, testsuite change needed to deal with this. I will post that if anyone’s interested, but otherwise will just apply it once the codgen fix is agreed. > > The patch factors out some common code that writes out the jumps and uses the regular output scheme that accounts for __USER_LABEL_PREFIX__. > > I will note in passing that there’s very little PIC test coverage for the indirection thunks code, although Darwin is PIC-only for m64 - Linux has only a few tests. > > OK for trunk? OK for trunk if the patch doesn't regress x86_64-linux. > Backports? Also OK for backports after a couple of days of soaking in the trunk without problems (so autotesters will test the patched compiler from the trunk). Thanks, Uros. > Iain > > gcc/ > > * config/i386/i386.c (ix86_output_jmp_thunk_or_indirect): New. > (ix86_output_indirect_branch_via_reg): Use output mechanism accounting for > __USR_LABEL_PREFIX. > (ix86_output_indirect_branch_via_push): Likewise. > (ix86_output_function_return): Likewise. > (ix86_output_indirect_function_return): Likewise. > > From 4da5837cd7bbe61b6d2687e552e3afb5bfdb2765 Mon Sep 17 00:00:00 2001 > From: Iain Sandoe <iain@sandoe.co.uk> > Date: Tue, 7 May 2019 07:27:19 -0400 > Subject: [PATCH] [Darwin] Fix PR82920 - code changes. > > Emit labels using machinery that includes the __USER_LABEL_PREFIX__ > --- > gcc/config/i386/i386.c | 48 ++++++++++++++++++++++++------------------ > 1 file changed, 27 insertions(+), 21 deletions(-) > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index c51d775b89..08aa9d9475 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -15130,6 +15130,20 @@ ix86_nopic_noplt_attribute_p (rtx call_op) > return false; > } > > +/* Helper to output the jmp/call. */ > +static void > +ix86_output_jmp_thunk_or_indirect (const char *thunk_name, const int regno) > +{ > + if (thunk_name != NULL) > + { > + fprintf (asm_out_file, "\tjmp\t"); > + assemble_name (asm_out_file, thunk_name); > + putc ('\n', asm_out_file); > + } > + else > + output_indirect_thunk (regno); > +} > + > /* Output indirect branch via a call and return thunk. CALL_OP is a > register which contains the branch target. XASM is the assembly > template for CALL_OP. Branch is a tail call if SIBCALL_P is true. > @@ -15168,17 +15182,14 @@ ix86_output_indirect_branch_via_reg (rtx call_op, bool sibcall_p) > thunk_name = NULL; > > if (sibcall_p) > - { > - if (thunk_name != NULL) > - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); > - else > - output_indirect_thunk (regno); > - } > + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); > else > { > if (thunk_name != NULL) > { > - fprintf (asm_out_file, "\tcall\t%s\n", thunk_name); > + fprintf (asm_out_file, "\tcall\t"); > + assemble_name (asm_out_file, thunk_name); > + putc ('\n', asm_out_file); > return; > } > > @@ -15199,10 +15210,7 @@ ix86_output_indirect_branch_via_reg (rtx call_op, bool sibcall_p) > > ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, indirectlabel1); > > - if (thunk_name != NULL) > - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); > - else > - output_indirect_thunk (regno); > + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); > > ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, indirectlabel2); > > @@ -15259,10 +15267,7 @@ ix86_output_indirect_branch_via_push (rtx call_op, const char *xasm, > if (sibcall_p) > { > output_asm_insn (push_buf, &call_op); > - if (thunk_name != NULL) > - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); > - else > - output_indirect_thunk (regno); > + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); > } > else > { > @@ -15318,10 +15323,7 @@ ix86_output_indirect_branch_via_push (rtx call_op, const char *xasm, > > output_asm_insn (push_buf, &call_op); > > - if (thunk_name != NULL) > - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); > - else > - output_indirect_thunk (regno); > + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); > > ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, indirectlabel2); > > @@ -15420,7 +15422,9 @@ ix86_output_function_return (bool long_p) > indirect_thunk_name (thunk_name, INVALID_REGNUM, need_prefix, > true); > indirect_return_needed |= need_thunk; > - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); > + fprintf (asm_out_file, "\tjmp\t"); > + assemble_name (asm_out_file, thunk_name); > + putc ('\n', asm_out_file); > } > else > output_indirect_thunk (INVALID_REGNUM); > @@ -15460,7 +15464,9 @@ ix86_output_indirect_function_return (rtx ret_op) > indirect_return_via_cx = true; > indirect_thunks_used |= 1 << CX_REG; > } > - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); > + fprintf (asm_out_file, "\tjmp\t"); > + assemble_name (asm_out_file, thunk_name); > + putc ('\n', asm_out_file); > } > else > output_indirect_thunk (regno); > -- > 2.17.1 > >
> On 10 May 2019, at 22:22, Uros Bizjak <ubizjak@gmail.com> wrote: > > On Fri, May 10, 2019 at 11:03 PM Iain Sandoe <idsandoe@googlemail.com> wrote: >> >> PR82920 is about CET fails on Darwin. >> >> Initially, this was expected to be just a testsuite issue, however it turns out that the indirection thunks code has inconsistent handling of the output of labels. Thus some of the output is missing the leading “_” on Darwin, which breaks ABI and won’t link. >> >> Since most of the tests are scan-asms that check for what’s expected, they currently pass on Darwin but will begin failing when the codegen is fixed. Thus there is larger, but mechanical, testsuite change needed to deal with this. I will post that if anyone’s interested, but otherwise will just apply it once the codgen fix is agreed. >> >> The patch factors out some common code that writes out the jumps and uses the regular output scheme that accounts for __USER_LABEL_PREFIX__. >> >> I will note in passing that there’s very little PIC test coverage for the indirection thunks code, although Darwin is PIC-only for m64 - Linux has only a few tests. >> >> OK for trunk? > > OK for trunk if the patch doesn't regress x86_64-linux. tested on x86_64-linux-gnu and x86_64-darwin (m32 and m64 in both cases). Applied to trunk. Many thanks to Dominique for working though some of the many testsuite changes needed. There’s a second set of test changes needed to fix the PR completely, but those are disjoint to the ones below which result from the codegen changes. thanks Iain gcc/ 2019-05-12 Iain Sandoe <iain@sandoe.co.uk> PR target/82920 * config/i386/i386.c (ix86_output_jmp_thunk_or_indirect): New. (ix86_output_indirect_branch_via_reg): Use output mechanism accounting for __USER_LABEL_PREFIX__. (ix86_output_indirect_branch_via_push): Likewise. (ix86_output_function_return): Likewise. (ix86_output_indirect_function_return): Likewise. gcc/testsuite/ 2019-05-12 Iain Sandoe <iain@sandoe.co.uk> Dominique d'Humieres <dominiq@gcc.gnu.org> PR target/82920 * gcc.target/i386/indirect-thunk-1.c: Adjust scan-asms for Darwin, do not use -fno-pic on Darwin. * gcc.target/i386/indirect-thunk-2.c: Likewise. * gcc.target/i386/indirect-thunk-3.c: Likewise. * gcc.target/i386/indirect-thunk-4.c: Likewise. * gcc.target/i386/indirect-thunk-7.c: Likewise. * gcc.target/i386/indirect-thunk-attr-1.c: Likewise. * gcc.target/i386/indirect-thunk-attr-2.c: Likewise. * gcc.target/i386/indirect-thunk-attr-3.c: Likewise. * gcc.target/i386/indirect-thunk-attr-4.c: Likewise. * gcc.target/i386/indirect-thunk-attr-5.c: Likewise. * gcc.target/i386/indirect-thunk-attr-6.c: Likewise. * gcc.target/i386/indirect-thunk-attr-7.c: Likewise. * gcc.target/i386/indirect-thunk-attr-8.c: Likewise. * gcc.target/i386/indirect-thunk-extern-1.c: Likewise. * gcc.target/i386/indirect-thunk-extern-2.c: Likewise. * gcc.target/i386/indirect-thunk-extern-3.c: Likewise. * gcc.target/i386/indirect-thunk-extern-4.c: Likewise. * gcc.target/i386/indirect-thunk-extern-7.c: Likewise. * gcc.target/i386/indirect-thunk-inline-1.c: Likewise. * gcc.target/i386/indirect-thunk-inline-2.c: Likewise. * gcc.target/i386/indirect-thunk-inline-3.c: Likewise. * gcc.target/i386/indirect-thunk-inline-4.c: Likewise. * gcc.target/i386/indirect-thunk-inline-7.c: Likewise. * gcc.target/i386/indirect-thunk-register-1.c: Likewise. * gcc.target/i386/indirect-thunk-register-2.c: Likewise. * gcc.target/i386/indirect-thunk-register-3.c: Likewise. * gcc.target/i386/indirect-thunk-register-4.c: Likewise. * gcc.target/i386/ret-thunk-1.c: Likewise. * gcc.target/i386/ret-thunk-10.c: Likewise. * gcc.target/i386/ret-thunk-11.c: Likewise. * gcc.target/i386/ret-thunk-12.c: Likewise. * gcc.target/i386/ret-thunk-13.c: Likewise. * gcc.target/i386/ret-thunk-14.c: Likewise. * gcc.target/i386/ret-thunk-15.c: Likewise. * gcc.target/i386/ret-thunk-16.c: Likewise. * gcc.target/i386/ret-thunk-2.c: Likewise. * gcc.target/i386/ret-thunk-22.c: Likewise. * gcc.target/i386/ret-thunk-23.c: Likewise. * gcc.target/i386/ret-thunk-24.c: Likewise. * gcc.target/i386/ret-thunk-3.c: Likewise. * gcc.target/i386/ret-thunk-4.c: Likewise. * gcc.target/i386/ret-thunk-5.c: Likewise. * gcc.target/i386/ret-thunk-6.c: Likewise. * gcc.target/i386/ret-thunk-7.c: Likewise. * gcc.target/i386/ret-thunk-8.c: Likewise. * gcc.target/i386/ret-thunk-9.c: Likewise.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index c51d775b89..08aa9d9475 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -15130,6 +15130,20 @@ ix86_nopic_noplt_attribute_p (rtx call_op) return false; } +/* Helper to output the jmp/call. */ +static void +ix86_output_jmp_thunk_or_indirect (const char *thunk_name, const int regno) +{ + if (thunk_name != NULL) + { + fprintf (asm_out_file, "\tjmp\t"); + assemble_name (asm_out_file, thunk_name); + putc ('\n', asm_out_file); + } + else + output_indirect_thunk (regno); +} + /* Output indirect branch via a call and return thunk. CALL_OP is a register which contains the branch target. XASM is the assembly template for CALL_OP. Branch is a tail call if SIBCALL_P is true. @@ -15168,17 +15182,14 @@ ix86_output_indirect_branch_via_reg (rtx call_op, bool sibcall_p) thunk_name = NULL; if (sibcall_p) - { - if (thunk_name != NULL) - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); - else - output_indirect_thunk (regno); - } + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); else { if (thunk_name != NULL) { - fprintf (asm_out_file, "\tcall\t%s\n", thunk_name); + fprintf (asm_out_file, "\tcall\t"); + assemble_name (asm_out_file, thunk_name); + putc ('\n', asm_out_file); return; } @@ -15199,10 +15210,7 @@ ix86_output_indirect_branch_via_reg (rtx call_op, bool sibcall_p) ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, indirectlabel1); - if (thunk_name != NULL) - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); - else - output_indirect_thunk (regno); + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, indirectlabel2); @@ -15259,10 +15267,7 @@ ix86_output_indirect_branch_via_push (rtx call_op, const char *xasm, if (sibcall_p) { output_asm_insn (push_buf, &call_op); - if (thunk_name != NULL) - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); - else - output_indirect_thunk (regno); + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); } else { @@ -15318,10 +15323,7 @@ ix86_output_indirect_branch_via_push (rtx call_op, const char *xasm, output_asm_insn (push_buf, &call_op); - if (thunk_name != NULL) - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); - else - output_indirect_thunk (regno); + ix86_output_jmp_thunk_or_indirect (thunk_name, regno); ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, indirectlabel2); @@ -15420,7 +15422,9 @@ ix86_output_function_return (bool long_p) indirect_thunk_name (thunk_name, INVALID_REGNUM, need_prefix, true); indirect_return_needed |= need_thunk; - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); + fprintf (asm_out_file, "\tjmp\t"); + assemble_name (asm_out_file, thunk_name); + putc ('\n', asm_out_file); } else output_indirect_thunk (INVALID_REGNUM); @@ -15460,7 +15464,9 @@ ix86_output_indirect_function_return (rtx ret_op) indirect_return_via_cx = true; indirect_thunks_used |= 1 << CX_REG; } - fprintf (asm_out_file, "\tjmp\t%s\n", thunk_name); + fprintf (asm_out_file, "\tjmp\t"); + assemble_name (asm_out_file, thunk_name); + putc ('\n', asm_out_file); } else output_indirect_thunk (regno);
Hi! PR82920 is about CET fails on Darwin. Initially, this was expected to be just a testsuite issue, however it turns out that the indirection thunks code has inconsistent handling of the output of labels. Thus some of the output is missing the leading “_” on Darwin, which breaks ABI and won’t link. Since most of the tests are scan-asms that check for what’s expected, they currently pass on Darwin but will begin failing when the codegen is fixed. Thus there is larger, but mechanical, testsuite change needed to deal with this. I will post that if anyone’s interested, but otherwise will just apply it once the codgen fix is agreed. The patch factors out some common code that writes out the jumps and uses the regular output scheme that accounts for __USER_LABEL_PREFIX__. I will note in passing that there’s very little PIC test coverage for the indirection thunks code, although Darwin is PIC-only for m64 - Linux has only a few tests. OK for trunk? Backports? Iain gcc/ * config/i386/i386.c (ix86_output_jmp_thunk_or_indirect): New. (ix86_output_indirect_branch_via_reg): Use output mechanism accounting for __USR_LABEL_PREFIX. (ix86_output_indirect_branch_via_push): Likewise. (ix86_output_function_return): Likewise. (ix86_output_indirect_function_return): Likewise. From 4da5837cd7bbe61b6d2687e552e3afb5bfdb2765 Mon Sep 17 00:00:00 2001 From: Iain Sandoe <iain@sandoe.co.uk> Date: Tue, 7 May 2019 07:27:19 -0400 Subject: [PATCH] [Darwin] Fix PR82920 - code changes. Emit labels using machinery that includes the __USER_LABEL_PREFIX__ --- gcc/config/i386/i386.c | 48 ++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 21 deletions(-)