From patchwork Fri Aug 10 11:42:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Uros Bizjak X-Patchwork-Id: 956221 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-483499-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="mk8kByx9"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="c5OIoAjV"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41n3CM3Bgfz9s7Q for ; Fri, 10 Aug 2018 21:42:54 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:cc:content-type; q=dns; s=default; b=HTYs+HGGghR4KXu84tHhnixhk5Fjl7bqAtYPW/9ZSXt CGjX82HVDGYg/SvK9b9SVIIEGd9o+5nocN6e7BX4xM9UTPn3xoOpJPYpwZ3KKsie UlvewPE2IkCN7MBo+O5Tjwf7d+n9Wk8G9S0tT4RACwb+QmXoTmg/WNzTzUSaBqSM = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:cc:content-type; s=default; bh=ZKO28zKL/0x0JKnzpwNG28VeVqI=; b=mk8kByx95vLJwU2kx gOEsiS4oyIr3SmGepph87lK9o8Jzk0XzSbBu6m3H3agB8H14/qiXhf7z248bOuq/ F9jA0R6wKNULpZ3NWWIhwUa6gmccOd7cTFsnE645vLtidUB80VMTnYu+Cdz84lpf Nw0lG7heg6qReyNblL/7ZYF7fo= Received: (qmail 53826 invoked by alias); 10 Aug 2018 11:42:46 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 53201 invoked by uid 89); 10 Aug 2018 11:42:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=rop, cet, canary, alt X-HELO: mail-io0-f177.google.com Received: from mail-io0-f177.google.com (HELO mail-io0-f177.google.com) (209.85.223.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Aug 2018 11:42:42 +0000 Received: by mail-io0-f177.google.com with SMTP id q4-v6so7372772iob.8 for ; Fri, 10 Aug 2018 04:42:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to:cc; bh=reJmwoGfOMhM50EoA7rUd84hGPKZIetaWYBK6hi9FEM=; b=c5OIoAjVUtPuJlzckY9jLj+NpWoz4vTxjh7rWeVQyhat+F0pv7BgFVQ8ttj66U82/U AzeP83U6PiH13SdP86FUEXvgekkovaxMN5FuKiNaHLQS7s+vms3nawUEo61MoBNWbeHp 96Pk7x1firKnmFiZ/m9ruRiaxu4CXByA9Ij+ig7CA2PFkNbfxJskk9YJMnnPK7Tc6ROH o0NJCaTCt2pC2mFe4nAY88RX5a6PENBDMv7b77HoMu4MSwK5pZrVS8mFSFoKcXAD62LL nprCsM3kbOvmyjHuCwAvKkXfecH5+koI2705uy2yOrw6wAyr/pdOwRc7eKcEiV6gE5Aw lSeg== MIME-Version: 1.0 Received: by 2002:a02:9a06:0:0:0:0:0 with HTTP; Fri, 10 Aug 2018 04:42:40 -0700 (PDT) From: Uros Bizjak Date: Fri, 10 Aug 2018 13:42:40 +0200 Message-ID: Subject: [RFC PATCH, i386]: Deprecate -mmitigate-rop To: "gcc-patches@gcc.gnu.org" Cc: Jeff Law , Jakub Jelinek This option is fairly ineffective, and in the light of CET, nobody seems interested to improve it. Deprecate the option, so it won't lure developers to the land of false security. 2018-08-10 Uros Bizjak * config/i386/i386.opt (mmitigate-rop): Mark as deprecated. * doc/invoke.texi (mmitigate-rop): Remove. * config/i386/i386.c: Do not include "regrename.h". (ix86_rop_should_change_byte_p, reg_encoded_number) (ix86_get_modrm_for_rop, set_rop_modrm_reg_bits, ix86_mitigate_rop): Remove. (ix86_reorg): Remove call to ix86_mitigate_rop. * config/i386/i386.md (attr "modrm_class"): Remove. (cmp_ccno_1, mov_xor, movstrict_xor, x86_movcc_0_m1. x86_movcc_0_m1_se) (x86_movcc_0_m1_neg): Remove modrm_class attribute override. testsuite/Changelog: 2018-08-10 Uros Bizjak * gcc.target/i386/rop1.c: Remove. * gcc.target/i386/pr83554 (dg-options): Remove -mmitigate-rop. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Any opinion against the deprecation? Uros. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 7554fd1f6599..d70e92124ddc 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -75,7 +75,6 @@ along with GCC; see the file COPYING3. If not see #include "tree-iterator.h" #include "dbgcnt.h" #include "case-cfn-macros.h" -#include "regrename.h" #include "dojump.h" #include "fold-const-call.h" #include "tree-vrp.h" @@ -3135,15 +3134,6 @@ ix86_debug_options (void) return; } -/* Return true if T is one of the bytes we should avoid with - -mmitigate-rop. */ - -static bool -ix86_rop_should_change_byte_p (int t) -{ - return t == 0xc2 || t == 0xc3 || t == 0xca || t == 0xcb; -} - static const char *stringop_alg_names[] = { #define DEF_ENUM #define DEF_ALG(alg, name) #name, @@ -29110,98 +29100,6 @@ ix86_instantiate_decls (void) instantiate_decl_rtl (s->rtl); } -/* Return the number used for encoding REG, in the range 0..7. */ - -static int -reg_encoded_number (rtx reg) -{ - unsigned regno = REGNO (reg); - switch (regno) - { - case AX_REG: - return 0; - case CX_REG: - return 1; - case DX_REG: - return 2; - case BX_REG: - return 3; - case SP_REG: - return 4; - case BP_REG: - return 5; - case SI_REG: - return 6; - case DI_REG: - return 7; - default: - break; - } - if (IN_RANGE (regno, FIRST_STACK_REG, LAST_STACK_REG)) - return regno - FIRST_STACK_REG; - if (IN_RANGE (regno, FIRST_SSE_REG, LAST_SSE_REG)) - return regno - FIRST_SSE_REG; - if (IN_RANGE (regno, FIRST_MMX_REG, LAST_MMX_REG)) - return regno - FIRST_MMX_REG; - if (IN_RANGE (regno, FIRST_REX_SSE_REG, LAST_REX_SSE_REG)) - return regno - FIRST_REX_SSE_REG; - if (IN_RANGE (regno, FIRST_REX_INT_REG, LAST_REX_INT_REG)) - return regno - FIRST_REX_INT_REG; - if (IN_RANGE (regno, FIRST_MASK_REG, LAST_MASK_REG)) - return regno - FIRST_MASK_REG; - return -1; -} - -/* Given an insn INSN with NOPERANDS OPERANDS, return the modr/m byte used - in its encoding if it could be relevant for ROP mitigation, otherwise - return -1. If POPNO0 and POPNO1 are nonnull, store the operand numbers - used for calculating it into them. */ - -static int -ix86_get_modrm_for_rop (rtx_insn *insn, rtx *operands, int noperands, - int *popno0 = 0, int *popno1 = 0) -{ - if (asm_noperands (PATTERN (insn)) >= 0) - return -1; - int has_modrm = get_attr_modrm (insn); - if (!has_modrm) - return -1; - enum attr_modrm_class cls = get_attr_modrm_class (insn); - rtx op0, op1; - switch (cls) - { - case MODRM_CLASS_OP02: - gcc_assert (noperands >= 3); - if (popno0) - { - *popno0 = 0; - *popno1 = 2; - } - op0 = operands[0]; - op1 = operands[2]; - break; - case MODRM_CLASS_OP01: - gcc_assert (noperands >= 2); - if (popno0) - { - *popno0 = 0; - *popno1 = 1; - } - op0 = operands[0]; - op1 = operands[1]; - break; - default: - return -1; - } - if (REG_P (op0) && REG_P (op1)) - { - int enc0 = reg_encoded_number (op0); - int enc1 = reg_encoded_number (op1); - return 0xc0 + (enc1 << 3) + enc0; - } - return -1; -} - /* Check whether x86 address PARTS is a pc-relative address. */ bool @@ -42215,215 +42113,6 @@ ix86_seh_fixup_eh_fallthru (void) } } -/* Given a register number BASE, the lowest of a group of registers, update - regsets IN and OUT with the registers that should be avoided in input - and output operands respectively when trying to avoid generating a modr/m - byte for -mmitigate-rop. */ - -static void -set_rop_modrm_reg_bits (int base, HARD_REG_SET &in, HARD_REG_SET &out) -{ - SET_HARD_REG_BIT (out, base); - SET_HARD_REG_BIT (out, base + 1); - SET_HARD_REG_BIT (in, base + 2); - SET_HARD_REG_BIT (in, base + 3); -} - -/* Called if -mmitigate-rop is in effect. Try to rewrite instructions so - that certain encodings of modr/m bytes do not occur. */ -static void -ix86_mitigate_rop (void) -{ - HARD_REG_SET input_risky; - HARD_REG_SET output_risky; - HARD_REG_SET inout_risky; - - CLEAR_HARD_REG_SET (output_risky); - CLEAR_HARD_REG_SET (input_risky); - SET_HARD_REG_BIT (output_risky, AX_REG); - SET_HARD_REG_BIT (output_risky, CX_REG); - SET_HARD_REG_BIT (input_risky, BX_REG); - SET_HARD_REG_BIT (input_risky, DX_REG); - set_rop_modrm_reg_bits (FIRST_SSE_REG, input_risky, output_risky); - set_rop_modrm_reg_bits (FIRST_REX_INT_REG, input_risky, output_risky); - set_rop_modrm_reg_bits (FIRST_REX_SSE_REG, input_risky, output_risky); - set_rop_modrm_reg_bits (FIRST_EXT_REX_SSE_REG, input_risky, output_risky); - set_rop_modrm_reg_bits (FIRST_MASK_REG, input_risky, output_risky); - COPY_HARD_REG_SET (inout_risky, input_risky); - IOR_HARD_REG_SET (inout_risky, output_risky); - - df_note_add_problem (); - /* Fix up what stack-regs did. */ - df_insn_rescan_all (); - df_analyze (); - - regrename_init (true); - regrename_analyze (NULL); - - auto_vec cands; - - for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn)) - { - if (!NONDEBUG_INSN_P (insn)) - continue; - - if (GET_CODE (PATTERN (insn)) == USE - || GET_CODE (PATTERN (insn)) == CLOBBER) - continue; - - extract_insn (insn); - - int opno0, opno1; - int modrm = ix86_get_modrm_for_rop (insn, recog_data.operand, - recog_data.n_operands, &opno0, - &opno1); - - if (!ix86_rop_should_change_byte_p (modrm)) - continue; - - insn_rr_info *info = &insn_rr[INSN_UID (insn)]; - - /* This happens when regrename has to fail a block. */ - if (!info->op_info) - continue; - - if (info->op_info[opno0].n_chains != 0) - { - gcc_assert (info->op_info[opno0].n_chains == 1); - du_head_p op0c; - op0c = regrename_chain_from_id (info->op_info[opno0].heads[0]->id); - if (op0c->target_data_1 + op0c->target_data_2 == 0 - && !op0c->cannot_rename) - cands.safe_push (op0c); - - op0c->target_data_1++; - } - if (info->op_info[opno1].n_chains != 0) - { - gcc_assert (info->op_info[opno1].n_chains == 1); - du_head_p op1c; - op1c = regrename_chain_from_id (info->op_info[opno1].heads[0]->id); - if (op1c->target_data_1 + op1c->target_data_2 == 0 - && !op1c->cannot_rename) - cands.safe_push (op1c); - - op1c->target_data_2++; - } - } - - int i; - du_head_p head; - FOR_EACH_VEC_ELT (cands, i, head) - { - int old_reg, best_reg; - HARD_REG_SET unavailable; - - CLEAR_HARD_REG_SET (unavailable); - if (head->target_data_1) - IOR_HARD_REG_SET (unavailable, output_risky); - if (head->target_data_2) - IOR_HARD_REG_SET (unavailable, input_risky); - - int n_uses; - reg_class superclass = regrename_find_superclass (head, &n_uses, - &unavailable); - old_reg = head->regno; - best_reg = find_rename_reg (head, superclass, &unavailable, - old_reg, false); - bool ok = regrename_do_replace (head, best_reg); - gcc_assert (ok); - if (dump_file) - fprintf (dump_file, "Chain %d renamed as %s in %s\n", head->id, - reg_names[best_reg], reg_class_names[superclass]); - - } - - regrename_finish (); - - df_analyze (); - - basic_block bb; - regset_head live; - - INIT_REG_SET (&live); - - FOR_EACH_BB_FN (bb, cfun) - { - rtx_insn *insn; - - COPY_REG_SET (&live, DF_LR_OUT (bb)); - df_simulate_initialize_backwards (bb, &live); - - FOR_BB_INSNS_REVERSE (bb, insn) - { - if (!NONDEBUG_INSN_P (insn)) - continue; - - df_simulate_one_insn_backwards (bb, insn, &live); - - if (GET_CODE (PATTERN (insn)) == USE - || GET_CODE (PATTERN (insn)) == CLOBBER) - continue; - - extract_insn (insn); - constrain_operands_cached (insn, reload_completed); - int opno0, opno1; - int modrm = ix86_get_modrm_for_rop (insn, recog_data.operand, - recog_data.n_operands, &opno0, - &opno1); - if (modrm < 0 - || !ix86_rop_should_change_byte_p (modrm) - || opno0 == opno1) - continue; - - rtx oldreg = recog_data.operand[opno1]; - preprocess_constraints (insn); - const operand_alternative *alt = which_op_alt (); - - int i; - for (i = 0; i < recog_data.n_operands; i++) - if (i != opno1 - && alt[i].earlyclobber - && reg_overlap_mentioned_p (recog_data.operand[i], - oldreg)) - break; - - if (i < recog_data.n_operands) - continue; - - if (dump_file) - fprintf (dump_file, - "attempting to fix modrm byte in insn %d:" - " reg %d class %s", INSN_UID (insn), REGNO (oldreg), - reg_class_names[alt[opno1].cl]); - - HARD_REG_SET unavailable; - REG_SET_TO_HARD_REG_SET (unavailable, &live); - SET_HARD_REG_BIT (unavailable, REGNO (oldreg)); - IOR_COMPL_HARD_REG_SET (unavailable, call_used_reg_set); - IOR_HARD_REG_SET (unavailable, fixed_reg_set); - IOR_HARD_REG_SET (unavailable, output_risky); - IOR_COMPL_HARD_REG_SET (unavailable, - reg_class_contents[alt[opno1].cl]); - - for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) - if (!TEST_HARD_REG_BIT (unavailable, i)) - break; - if (i == FIRST_PSEUDO_REGISTER) - { - if (dump_file) - fprintf (dump_file, ", none available\n"); - continue; - } - if (dump_file) - fprintf (dump_file, " -> %d\n", i); - rtx newreg = gen_rtx_REG (recog_data.operand_mode[opno1], i); - validate_change (insn, recog_data.operand_loc[opno1], newreg, false); - insn = emit_insn_before (gen_move_insn (newreg, oldreg), insn); - } - } -} - /* Implement machine specific optimizations. We implement padding of returns for K8 CPUs and pass to avoid 4 jumps in the single 16 byte window. */ static void @@ -42433,9 +42122,6 @@ ix86_reorg (void) with old MDEP_REORGS that are not CFG based. Recompute it now. */ compute_bb_for_insn (); - if (flag_mitigate_rop) - ix86_mitigate_rop (); - if (TARGET_SEH && current_function_has_exception_handlers ()) ix86_seh_fixup_eh_fallthru (); diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 73948c12618d..b65a7fbde996 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -663,19 +663,6 @@ ] (const_int 1))) -(define_attr "modrm_class" "none,incdec,op0,op01,op02,pushpop,unknown" - (cond [(eq_attr "modrm" "0") - (const_string "none") - (eq_attr "type" "alu,imul,ishift") - (const_string "op02") - (eq_attr "type" "imov,imovx,lea,alu1,icmp") - (const_string "op01") - (eq_attr "type" "incdec") - (const_string "incdec") - (eq_attr "type" "push,pop") - (const_string "pushpop")] - (const_string "unknown"))) - ;; The (bounding maximum) length of an instruction in bytes. ;; ??? fistp and frndint are in fact fldcw/{fistp,frndint}/fldcw sequences. ;; Later we may want to split them and compute proper length as for @@ -1299,7 +1286,6 @@ ktest\t%0, %0" [(set_attr "type" "test,icmp,msklog") (set_attr "length_immediate" "0,1,*") - (set_attr "modrm_class" "op0,unknown,*") (set_attr "prefix" "*,*,vex") (set_attr "mode" "")]) @@ -1313,7 +1299,6 @@ cmp{}\t{%1, %0|%0, %1}" [(set_attr "type" "test,icmp") (set_attr "length_immediate" "0,1") - (set_attr "modrm_class" "op0,unknown") (set_attr "mode" "")]) (define_insn "*cmp_1" @@ -2028,7 +2013,6 @@ "reload_completed" "xor{l}\t%k0, %k0" [(set_attr "type" "alu1") - (set_attr "modrm_class" "op0") (set_attr "mode" "SI") (set_attr "length_immediate" "0")]) @@ -2913,7 +2897,6 @@ "reload_completed" "xor{}\t%0, %0" [(set_attr "type" "alu1") - (set_attr "modrm_class" "op0") (set_attr "mode" "") (set_attr "length_immediate" "0")]) @@ -13585,7 +13568,6 @@ "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" @@ -18214,7 +18196,6 @@ "" "sbb{}\t%0, %0" [(set_attr "type" "alu1") - (set_attr "modrm_class" "op0") (set_attr "use_carry" "1") (set_attr "pent_pair" "pu") (set_attr "mode" "") @@ -18230,7 +18211,6 @@ "" "sbb{}\t%0, %0" [(set_attr "type" "alu1") - (set_attr "modrm_class" "op0") (set_attr "use_carry" "1") (set_attr "pent_pair" "pu") (set_attr "mode" "") @@ -18244,7 +18224,6 @@ "" "sbb{}\t%0, %0" [(set_attr "type" "alu1") - (set_attr "modrm_class" "op0") (set_attr "use_carry" "1") (set_attr "pent_pair" "pu") (set_attr "mode" "") diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt index a34d4acf1a26..3724994760db 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -842,7 +842,7 @@ Target Report Mask(ISA_CLWB) Var(ix86_isa_flags) Save Support CLWB instruction. mpcommit -Target Undocumented Warn(%<-mpcommit%> was deprecated) +Target Ignore Warn(%qs was deprecated) ;; Deprecated mfxsr @@ -999,8 +999,8 @@ Target RejectNegative Joined Integer Var(ix86_stack_protector_guard_symbol_str) Use the given symbol for addressing the stack-protector guard. mmitigate-rop -Target Var(flag_mitigate_rop) -Attempt to avoid generating instruction sequences containing ret bytes. +Target Ignore Warn(%qs was deprecated) +;; Deprecated mgeneral-regs-only Target Report RejectNegative Mask(GENERAL_REGS_ONLY) Var(ix86_target_flags) Save diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 0e9a9c3e2f7f..3c211730b327 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -1292,7 +1292,7 @@ See RS/6000 and PowerPC Options. -malign-data=@var{type} -mstack-protector-guard=@var{guard} @gol -mstack-protector-guard-reg=@var{reg} @gol -mstack-protector-guard-offset=@var{offset} @gol --mstack-protector-guard-symbol=@var{symbol} -mmitigate-rop @gol +-mstack-protector-guard-symbol=@var{symbol} @gol -mgeneral-regs-only -mcall-ms2sysv-xlogues @gol -mindirect-branch=@var{choice} -mfunction-return=@var{choice} @gol -mindirect-branch-register} @@ -27983,13 +27983,6 @@ which segment register (@code{%fs} or @code{%gs}) to use as base register for reading the canary, and from what offset from that base register. The default for those is as specified in the relevant ABI. -@item -mmitigate-rop -@opindex mmitigate-rop -Try to avoid generating code sequences that contain unintended return -opcodes, to mitigate against certain forms of attack. At the moment, -this option is limited in what it can do and should not be relied -on to provide serious protection. - @item -mgeneral-regs-only @opindex mgeneral-regs-only Generate code that uses only the general-purpose registers. This