Message ID | 56581C2F.9070300@redhat.com |
---|---|
State | New |
Headers | show |
> Both of these are fixed in the i386 backend. As a further safety > measure, I've added some extra code to regrename to ignore stack regs > after regstack_complete - they can't be dealt with anymore. +#ifdef STACK_REGS + if (regstack_completed + && REG_P (recog_data.operand[i]) + && IN_RANGE (REGNO (recog_data.operand[i]), + FIRST_STACK_REG, LAST_STACK_REG)) + untracked_operands |= 1 << i; +#endif Why not use "op" instead of recog_data.operand[i] here? Don't this need to be placed before the conditional call to create_new_chain?
On 11/27/2015 10:26 AM, Eric Botcazou wrote: > +#ifdef STACK_REGS > + if (regstack_completed > + && REG_P (recog_data.operand[i]) > + && IN_RANGE (REGNO (recog_data.operand[i]), > + FIRST_STACK_REG, LAST_STACK_REG)) > + untracked_operands |= 1 << i; > +#endif > > Why not use "op" instead of recog_data.operand[i] here? Don't this need to be > placed before the conditional call to create_new_chain? Both good points. Ok with those changes (will retest)? Bernd
> Both good points. Ok with those changes (will retest)?
Yes, thanks.
On 11/27/2015 10:02 AM, Bernd Schmidt wrote: > This is a patch for PRs 68471 and 68472, which show problems with the > ROP mitigation: > * reg-stack doesn't call df_insn_update when it makes changes, and > if df checking is enabled, any subsequent df_analyze call will > abort > * Using -mcmodel=medium fails because of a pattern that has lea type > and needs its modrm_class overridden. > > Both of these are fixed in the i386 backend. As a further safety > measure, I've added some extra code to regrename to ignore stack regs > after regstack_complete - they can't be dealt with anymore. > > Bootstrapped and tested on x86_64-linux, with -mmitigate-rop forced on. Ok? > PR target/68471 > PR target/68472 > * config/i386/i386.c (ix86_mitigate_rop): Don't call > compute_bb_for_insn again. Call df_insn_rescan_all. > * config/i386/i386.md (set_got_rex64): Override modrm_class. > > * regrename.c (build_def_use): Ignore stack regs if regstack_completed. > > testsuite/ > * gcc.target/i386/rop1.c: New test. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 2ac6c25..14c99eb 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -45243,8 +45243,9 @@ ix86_mitigate_rop (void) > COPY_HARD_REG_SET (inout_risky, input_risky); > IOR_HARD_REG_SET (inout_risky, output_risky); > > - compute_bb_for_insn (); > df_note_add_problem (); > + /* Fix up what stack-regs did. */ > + df_insn_rescan_all (); > df_analyze (); > > regrename_init (true); > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index a57d165..671580d 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -12418,6 +12418,7 @@ > "lea{q}\t{_GLOBAL_OFFSET_TABLE_(%%rip), %0|%0, _GLOBAL_OFFSET_TABLE_[rip]}" > [(set_attr "type" "lea") > (set_attr "length_address" "4") > + (set_attr "modrm_class" "unknown") > (set_attr "mode" "DI")]) > > (define_insn "set_rip_rex64" > --- /dev/null 2015-11-23 12:05:22.553607702 +0100 > +++ gcc/testsuite/gcc.target/i386/rop1.c 2015-11-24 15:40:04.381086953 +0100 > @@ -0,0 +1,7 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target lp64 } */ > +/* { dg-options "-mcmodel=medium -mmitigate-rop" } */ > +void > +foo (void) > +{ > +} Ccing Uros for the i386 bits. Bernd
On Mon, Nov 30, 2015 at 10:38 PM, Bernd Schmidt <bschmidt@redhat.com> wrote: > On 11/27/2015 10:02 AM, Bernd Schmidt wrote: >> >> This is a patch for PRs 68471 and 68472, which show problems with the >> ROP mitigation: >> * reg-stack doesn't call df_insn_update when it makes changes, and >> if df checking is enabled, any subsequent df_analyze call will >> abort >> * Using -mcmodel=medium fails because of a pattern that has lea type >> and needs its modrm_class overridden. >> >> Both of these are fixed in the i386 backend. As a further safety >> measure, I've added some extra code to regrename to ignore stack regs >> after regstack_complete - they can't be dealt with anymore. >> >> Bootstrapped and tested on x86_64-linux, with -mmitigate-rop forced on. >> Ok? > > >> PR target/68471 >> PR target/68472 >> * config/i386/i386.c (ix86_mitigate_rop): Don't call >> compute_bb_for_insn again. Call df_insn_rescan_all. >> * config/i386/i386.md (set_got_rex64): Override modrm_class. >> >> * regrename.c (build_def_use): Ignore stack regs if >> regstack_completed. >> >> testsuite/ >> * gcc.target/i386/rop1.c: New test. >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 2ac6c25..14c99eb 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -45243,8 +45243,9 @@ ix86_mitigate_rop (void) >> COPY_HARD_REG_SET (inout_risky, input_risky); >> IOR_HARD_REG_SET (inout_risky, output_risky); >> >> - compute_bb_for_insn (); >> df_note_add_problem (); >> + /* Fix up what stack-regs did. */ >> + df_insn_rescan_all (); >> df_analyze (); >> >> regrename_init (true); >> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md >> index a57d165..671580d 100644 >> --- a/gcc/config/i386/i386.md >> +++ b/gcc/config/i386/i386.md >> @@ -12418,6 +12418,7 @@ >> "lea{q}\t{_GLOBAL_OFFSET_TABLE_(%%rip), %0|%0, >> _GLOBAL_OFFSET_TABLE_[rip]}" >> [(set_attr "type" "lea") >> (set_attr "length_address" "4") >> + (set_attr "modrm_class" "unknown") >> (set_attr "mode" "DI")]) >> >> (define_insn "set_rip_rex64" >> --- /dev/null 2015-11-23 12:05:22.553607702 +0100 >> +++ gcc/testsuite/gcc.target/i386/rop1.c 2015-11-24 >> 15:40:04.381086953 +0100 >> @@ -0,0 +1,7 @@ >> +/* { dg-do compile } */ >> +/* { dg-require-effective-target lp64 } */ >> +/* { dg-options "-mcmodel=medium -mmitigate-rop" } */ >> +void >> +foo (void) >> +{ >> +} > > > Ccing Uros for the i386 bits. These are OK. Thanks, Uros.
PR target/68471 PR target/68472 * config/i386/i386.c (ix86_mitigate_rop): Don't call compute_bb_for_insn again. Call df_insn_rescan_all. * config/i386/i386.md (set_got_rex64): Override modrm_class. * regrename.c (build_def_use): Ignore stack regs if regstack_completed. testsuite/ * gcc.target/i386/rop1.c: New test. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 2ac6c25..14c99eb 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -45243,8 +45243,9 @@ ix86_mitigate_rop (void) COPY_HARD_REG_SET (inout_risky, input_risky); IOR_HARD_REG_SET (inout_risky, output_risky); - compute_bb_for_insn (); df_note_add_problem (); + /* Fix up what stack-regs did. */ + df_insn_rescan_all (); df_analyze (); regrename_init (true); diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index a57d165..671580d 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -12418,6 +12418,7 @@ "lea{q}\t{_GLOBAL_OFFSET_TABLE_(%%rip), %0|%0, _GLOBAL_OFFSET_TABLE_[rip]}" [(set_attr "type" "lea") (set_attr "length_address" "4") + (set_attr "modrm_class" "unknown") (set_attr "mode" "DI")]) (define_insn "set_rip_rex64" diff --git a/gcc/regrename.c b/gcc/regrename.c index e2a1e83..47c8dfa 100644 --- a/gcc/regrename.c +++ b/gcc/regrename.c @@ -1685,6 +1685,13 @@ build_def_use (basic_block bb) && !verify_reg_tracked (op)) create_new_chain (REGNO (op), REG_NREGS (op), NULL, NULL, NO_REGS); +#ifdef STACK_REGS + if (regstack_completed + && REG_P (recog_data.operand[i]) + && IN_RANGE (REGNO (recog_data.operand[i]), + FIRST_STACK_REG, LAST_STACK_REG)) + untracked_operands |= 1 << i; +#endif } if (fail_current_block) --- /dev/null 2015-11-23 12:05:22.553607702 +0100 +++ gcc/testsuite/gcc.target/i386/rop1.c 2015-11-24 15:40:04.381086953 +0100 @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target lp64 } */ +/* { dg-options "-mcmodel=medium -mmitigate-rop" } */ +void +foo (void) +{ +}