From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2140) id E8E1F384B126; Wed, 11 May 2022 07:32:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E8E1F384B126 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Alexandre Oliva To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/users/aoliva/heads/testme)] Avoid visiting newly-created blocks in harden-conditionals X-Act-Checkin: gcc X-Git-Author: Alexandre Oliva X-Git-Refname: refs/users/aoliva/heads/testme X-Git-Oldrev: f7a3ab2c6a7b81883e14b6e2604ecb39838597a8 X-Git-Newrev: 91a0ae071116f11a8430b8434fdb5dd8d1e6d605 Message-Id: <20220511073224.E8E1F384B126@sourceware.org> Date: Wed, 11 May 2022 07:32:24 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 May 2022 07:32:25 -0000 https://gcc.gnu.org/g:91a0ae071116f11a8430b8434fdb5dd8d1e6d605 commit 91a0ae071116f11a8430b8434fdb5dd8d1e6d605 Author: Alexandre Oliva Date: Wed May 11 03:34:58 2022 -0300 Avoid visiting newly-created blocks in harden-conditionals Diff: --- gcc/gimple-harden-conditionals.cc | 401 ++++++++++++++++++++------------------ 1 file changed, 211 insertions(+), 190 deletions(-) diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc index c7e5e077a74..28c4810f0a7 100644 --- a/gcc/gimple-harden-conditionals.cc +++ b/gcc/gimple-harden-conditionals.cc @@ -301,9 +301,18 @@ insert_edge_check_and_trap (location_t loc, edge e, unsigned int pass_harden_conditional_branches::execute (function *fun) { + int orig_last_block = last_basic_block_for_fn (fun); + basic_block bb; FOR_EACH_BB_REVERSE_FN (bb, fun) { + /* Despite our backwards iteration on basic blocks, sometimes + split_edge will insert the new block before the block we're + hardening, and then we'd harden the hardening block. Skip + newly-created blocks to avoid that. */ + if (bb->index >= orig_last_block) + continue; + gimple_stmt_iterator gsi = gsi_last_bb (bb); if (gsi_end_p (gsi)) @@ -383,6 +392,8 @@ non_eh_succ_edge (basic_block bb, edge *ehp = NULL) unsigned int pass_harden_compares::execute (function *fun) { + int orig_last_block = last_basic_block_for_fn (fun); + basic_block bb; /* Go backwards over BBs and stmts, so that, even if we split the block multiple times to insert a cond_expr after each compare we @@ -390,198 +401,208 @@ pass_harden_compares::execute (function *fun) stmt exactly once, and not visiting newly-added blocks or stmts. */ FOR_EACH_BB_REVERSE_FN (bb, fun) - for (gimple_stmt_iterator gsi = gsi_last_bb (bb); - !gsi_end_p (gsi); gsi_prev (&gsi)) - { - gassign *asgn = dyn_cast (gsi_stmt (gsi)); - if (!asgn) - continue; - - /* Turn: - - z = x op y; - - into: - - z = x op y; - z' = x' cop y'; - if (z == z') __builtin_trap (); - - where cop is a complementary boolean operation to op; and x' - and y' hold the same value as x and y, but in a way that does - not enable the compiler to optimize the redundant compare - away. - */ - - enum tree_code op = gimple_assign_rhs_code (asgn); - - enum tree_code cop; - - switch (op) - { - case EQ_EXPR: - case NE_EXPR: - case GT_EXPR: - case GE_EXPR: - case LT_EXPR: - case LE_EXPR: - case LTGT_EXPR: - case UNEQ_EXPR: - case UNGT_EXPR: - case UNGE_EXPR: - case UNLT_EXPR: - case UNLE_EXPR: - case ORDERED_EXPR: - case UNORDERED_EXPR: - cop = invert_tree_comparison (op, - HONOR_NANS - (gimple_assign_rhs1 (asgn))); - - if (cop == ERROR_MARK) - /* ??? Can we do better? */ - continue; + { + /* Despite our backwards iteration on basic blocks, sometimes + split_edge will insert the new block before the block we're + hardening, and then we'd harden the hardening block. Skip + newly-created blocks to avoid that. */ + if (bb->index >= orig_last_block) + continue; - break; - - /* ??? Maybe handle these too? */ - case TRUTH_NOT_EXPR: - /* ??? The code below assumes binary ops, it would have to - be adjusted for TRUTH_NOT_EXPR, since it's unary. */ - case TRUTH_ANDIF_EXPR: - case TRUTH_ORIF_EXPR: - case TRUTH_AND_EXPR: - case TRUTH_OR_EXPR: - case TRUTH_XOR_EXPR: - default: + for (gimple_stmt_iterator gsi = gsi_last_bb (bb); + !gsi_end_p (gsi); gsi_prev (&gsi)) + { + gassign *asgn = dyn_cast (gsi_stmt (gsi)); + if (!asgn) + continue; + + /* Turn: + + z = x op y; + + into: + + z = x op y; + z' = x' cop y'; + if (z == z') __builtin_trap (); + + where cop is a complementary boolean operation to op; and x' + and y' hold the same value as x and y, but in a way that does + not enable the compiler to optimize the redundant compare + away. + */ + + enum tree_code op = gimple_assign_rhs_code (asgn); + + enum tree_code cop; + + switch (op) + { + case EQ_EXPR: + case NE_EXPR: + case GT_EXPR: + case GE_EXPR: + case LT_EXPR: + case LE_EXPR: + case LTGT_EXPR: + case UNEQ_EXPR: + case UNGT_EXPR: + case UNGE_EXPR: + case UNLT_EXPR: + case UNLE_EXPR: + case ORDERED_EXPR: + case UNORDERED_EXPR: + cop = invert_tree_comparison (op, + HONOR_NANS + (gimple_assign_rhs1 (asgn))); + + if (cop == ERROR_MARK) + /* ??? Can we do better? */ + continue; + + break; + + /* ??? Maybe handle these too? */ + case TRUTH_NOT_EXPR: + /* ??? The code below assumes binary ops, it would have to + be adjusted for TRUTH_NOT_EXPR, since it's unary. */ + case TRUTH_ANDIF_EXPR: + case TRUTH_ORIF_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_OR_EXPR: + case TRUTH_XOR_EXPR: + default: + continue; + } + + /* These are the operands for the verification. */ + tree lhs = gimple_assign_lhs (asgn); + tree op1 = gimple_assign_rhs1 (asgn); + tree op2 = gimple_assign_rhs2 (asgn); + location_t loc = gimple_location (asgn); + + /* Vector booleans can't be used in conditional branches. ??? + Can we do better? How to reduce compare and + reversed-compare result vectors to a single boolean? */ + if (VECTOR_TYPE_P (TREE_TYPE (op1))) continue; - } - - /* These are the operands for the verification. */ - tree lhs = gimple_assign_lhs (asgn); - tree op1 = gimple_assign_rhs1 (asgn); - tree op2 = gimple_assign_rhs2 (asgn); - location_t loc = gimple_location (asgn); - - /* Vector booleans can't be used in conditional branches. ??? - Can we do better? How to reduce compare and - reversed-compare result vectors to a single boolean? */ - if (VECTOR_TYPE_P (TREE_TYPE (op1))) - continue; - - /* useless_type_conversion_p enables conversions from 1-bit - integer types to boolean to be discarded. */ - gcc_checking_assert (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE - || (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) - && TYPE_PRECISION (TREE_TYPE (lhs)) == 1)); - - tree rhs = copy_ssa_name (lhs); - - gimple_stmt_iterator gsi_split = gsi; - /* Don't separate the original assignment from debug stmts - that might be associated with it, and arrange to split the - block after debug stmts, so as to make sure the split block - won't be debug stmts only. */ - gsi_next_nondebug (&gsi_split); - - bool throwing_compare_p = stmt_ends_bb_p (asgn); - if (throwing_compare_p) - { - basic_block nbb = split_edge (non_eh_succ_edge - (gimple_bb (asgn))); - gsi_split = gsi_start_bb (nbb); - - if (dump_file) - fprintf (dump_file, - "Splitting non-EH edge from block %i into %i" - " after a throwing compare\n", - gimple_bb (asgn)->index, nbb->index); - } - - bool same_p = (op1 == op2); - op1 = detach_value (loc, &gsi_split, op1); - op2 = same_p ? op1 : detach_value (loc, &gsi_split, op2); - - gassign *asgnck = gimple_build_assign (rhs, cop, op1, op2); - gimple_set_location (asgnck, loc); - gsi_insert_before (&gsi_split, asgnck, GSI_SAME_STMT); - - /* We wish to insert a cond_expr after the compare, so arrange - for it to be at the end of a block if it isn't, and for it - to have a single successor in case there's more than - one, as in PR104975. */ - if (!gsi_end_p (gsi_split) - || !single_succ_p (gsi_bb (gsi_split))) - { - if (!gsi_end_p (gsi_split)) - gsi_prev (&gsi_split); - else - gsi_split = gsi_last_bb (gsi_bb (gsi_split)); - basic_block obb = gsi_bb (gsi_split); - basic_block nbb = split_block (obb, gsi_stmt (gsi_split))->dest; - gsi_next (&gsi_split); - gcc_checking_assert (gsi_end_p (gsi_split)); - - single_succ_edge (bb)->goto_locus = loc; - - if (dump_file) - fprintf (dump_file, - "Splitting block %i into %i" - " before the conditional trap branch\n", - obb->index, nbb->index); - } - - /* If the check assignment must end a basic block, we can't - insert the conditional branch in the same block, so split - the block again, and prepare to insert the conditional - branch in the new block. - - Also assign an EH region to the compare. Even though it's - unlikely that the hardening compare will throw after the - original compare didn't, the compiler won't even know that - it's the same compare operands, so add the EH edge anyway. */ - if (throwing_compare_p) - { - add_stmt_to_eh_lp (asgnck, lookup_stmt_eh_lp (asgn)); - make_eh_edges (asgnck); - - edge ckeh; - basic_block nbb = split_edge (non_eh_succ_edge - (gimple_bb (asgnck), &ckeh)); - gsi_split = gsi_start_bb (nbb); - - if (dump_file) - fprintf (dump_file, - "Splitting non-EH edge from block %i into %i after" - " the newly-inserted reversed throwing compare\n", - gimple_bb (asgnck)->index, nbb->index); - - if (!gimple_seq_empty_p (phi_nodes (ckeh->dest))) - { - edge aseh; - non_eh_succ_edge (gimple_bb (asgn), &aseh); - - gcc_checking_assert (aseh->dest == ckeh->dest); - - for (gphi_iterator psi = gsi_start_phis (ckeh->dest); - !gsi_end_p (psi); gsi_next (&psi)) - { - gphi *phi = psi.phi (); - add_phi_arg (phi, PHI_ARG_DEF_FROM_EDGE (phi, aseh), ckeh, - gimple_phi_arg_location_from_edge (phi, aseh)); - } - - if (dump_file) - fprintf (dump_file, - "Copying PHI args in EH block %i from %i to %i\n", - aseh->dest->index, aseh->src->index, ckeh->src->index); - } - } - - gcc_checking_assert (single_succ_p (gsi_bb (gsi_split))); - - insert_check_and_trap (loc, &gsi_split, EDGE_TRUE_VALUE, - EQ_EXPR, lhs, rhs); - } + + /* useless_type_conversion_p enables conversions from 1-bit + integer types to boolean to be discarded. */ + gcc_checking_assert (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE + || (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) + && TYPE_PRECISION (TREE_TYPE (lhs)) == 1)); + + tree rhs = copy_ssa_name (lhs); + + gimple_stmt_iterator gsi_split = gsi; + /* Don't separate the original assignment from debug stmts + that might be associated with it, and arrange to split the + block after debug stmts, so as to make sure the split block + won't be debug stmts only. */ + gsi_next_nondebug (&gsi_split); + + bool throwing_compare_p = stmt_ends_bb_p (asgn); + if (throwing_compare_p) + { + basic_block nbb = split_edge (non_eh_succ_edge + (gimple_bb (asgn))); + gsi_split = gsi_start_bb (nbb); + + if (dump_file) + fprintf (dump_file, + "Splitting non-EH edge from block %i into %i" + " after a throwing compare\n", + gimple_bb (asgn)->index, nbb->index); + } + + bool same_p = (op1 == op2); + op1 = detach_value (loc, &gsi_split, op1); + op2 = same_p ? op1 : detach_value (loc, &gsi_split, op2); + + gassign *asgnck = gimple_build_assign (rhs, cop, op1, op2); + gimple_set_location (asgnck, loc); + gsi_insert_before (&gsi_split, asgnck, GSI_SAME_STMT); + + /* We wish to insert a cond_expr after the compare, so arrange + for it to be at the end of a block if it isn't, and for it + to have a single successor in case there's more than + one, as in PR104975. */ + if (!gsi_end_p (gsi_split) + || !single_succ_p (gsi_bb (gsi_split))) + { + if (!gsi_end_p (gsi_split)) + gsi_prev (&gsi_split); + else + gsi_split = gsi_last_bb (gsi_bb (gsi_split)); + basic_block obb = gsi_bb (gsi_split); + basic_block nbb = split_block (obb, gsi_stmt (gsi_split))->dest; + gsi_next (&gsi_split); + gcc_checking_assert (gsi_end_p (gsi_split)); + + single_succ_edge (bb)->goto_locus = loc; + + if (dump_file) + fprintf (dump_file, + "Splitting block %i into %i" + " before the conditional trap branch\n", + obb->index, nbb->index); + } + + /* If the check assignment must end a basic block, we can't + insert the conditional branch in the same block, so split + the block again, and prepare to insert the conditional + branch in the new block. + + Also assign an EH region to the compare. Even though it's + unlikely that the hardening compare will throw after the + original compare didn't, the compiler won't even know that + it's the same compare operands, so add the EH edge anyway. */ + if (throwing_compare_p) + { + add_stmt_to_eh_lp (asgnck, lookup_stmt_eh_lp (asgn)); + make_eh_edges (asgnck); + + edge ckeh; + basic_block nbb = split_edge (non_eh_succ_edge + (gimple_bb (asgnck), &ckeh)); + gsi_split = gsi_start_bb (nbb); + + if (dump_file) + fprintf (dump_file, + "Splitting non-EH edge from block %i into %i after" + " the newly-inserted reversed throwing compare\n", + gimple_bb (asgnck)->index, nbb->index); + + if (!gimple_seq_empty_p (phi_nodes (ckeh->dest))) + { + edge aseh; + non_eh_succ_edge (gimple_bb (asgn), &aseh); + + gcc_checking_assert (aseh->dest == ckeh->dest); + + for (gphi_iterator psi = gsi_start_phis (ckeh->dest); + !gsi_end_p (psi); gsi_next (&psi)) + { + gphi *phi = psi.phi (); + add_phi_arg (phi, PHI_ARG_DEF_FROM_EDGE (phi, aseh), ckeh, + gimple_phi_arg_location_from_edge (phi, aseh)); + } + + if (dump_file) + fprintf (dump_file, + "Copying PHI args in EH block %i from %i to %i\n", + aseh->dest->index, aseh->src->index, + ckeh->src->index); + } + } + + gcc_checking_assert (single_succ_p (gsi_bb (gsi_split))); + + insert_check_and_trap (loc, &gsi_split, EDGE_TRUE_VALUE, + EQ_EXPR, lhs, rhs); + } + } return 0; }