From patchwork Thu Dec 5 03:19:29 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 296703 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 9F7482C00AD for ; Thu, 5 Dec 2013 14:19:48 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=cwCBW9TNZToGdzt+vEk9jBCCe0XNRtb//abEik8oA3W3LH 7MfbvXwfv85F0/D1vP0opwMM69KTamLrA8oyUhhPui0VseoJoBpk5nguYQTGWl9X GvEuMR8rSk00AOb02VrWArdyvbLPOw2A2HjYjRQvnyf0Yi+FkKB93DXLwwr+g= 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 :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=UL+eY69ibF1/t7IM0L/iLimN3wo=; b=hQKpST8BxbX21IYk924i 7bH8A/hMa/iPv23jBK8gGyn6uBeR23Pq6klFuSyq2orpoL0CfyzgrTc8d0V/pyH0 wogHTUBRvCS0KwFbKARJkVmM1Jy2tFJpRASYitj9Nik7d/anQcGDDRDG5NRX5HCS sg4sqKdbPd+ML3UD5ahGfEU= Received: (qmail 23427 invoked by alias); 5 Dec 2013 03:19:40 -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 23417 invoked by uid 89); 5 Dec 2013 03:19:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.7 required=5.0 tests=AWL, BAYES_50, SPF_HELO_PASS, SPF_PASS, URIBL_BLOCKED autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Dec 2013 03:19:37 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rB53JUeJ019358 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 4 Dec 2013 22:19:30 -0500 Received: from stumpy.slc.redhat.com (ovpn-113-102.phx2.redhat.com [10.3.113.102]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rB53JTW4001783 for ; Wed, 4 Dec 2013 22:19:30 -0500 Message-ID: <529FF0C1.3090406@redhat.com> Date: Wed, 04 Dec 2013 20:19:29 -0700 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: gcc-patches Subject: [PATCH] Split -fisolate-erroneous-paths into two options X-IsSubscribed: yes As discussed late in this thread: http://gcc.gnu.org/ml/gcc/2013-11/msg00345.html This patch splits up the erroneous path optimization into two pieces. One which detects NULL dereferences and isolates those paths and a second which detects passing/returning a NULL pointer in cases where an attribute says a non-NULL value is required. The former is enabled by default at -O2, the latter is not enabled by default at any optimization level. Bootstrapped & regression tested on x86_64-unknown-linux-gnu. Installed on the trunk. The next cleanup will be to add the warning as discussed in the same thread. Jeff * common.opt: Split up -fisolate-erroneous-paths into -fisolate-erroneous-paths-dereference and -fisolate-erroneous-paths-attribute. * invoke.texi: Corresponding changes. * gimple.c (infer_nonnull_range): Add and use new arguments to control what kind of statements can be used to infer a non-null range. * gimple.h (infer_nonnull_range): Update prototype. * tree-vrp.c (infer_value_range): Corresponding changes. * opts.c (default_options_table): Update due to option split. * gimple-ssa-isolate-paths.c: Fix trailing whitespace. (find_implicit_erroneous_behaviour): Pass additional arguments to infer_nonnull_range. (find_explicit_erroneous_behaviour): Similarly. (gate_isolate_erroneous_paths): Check both of the new options. testsuite/ * gcc.dg/pr38984.c: Use -fno-isolate-erroneous-paths-dereference. * gcc.dg/tree-ssa/isolate-2.c: Explicitly turn on -fisolate-erroneous-paths-attribute. * gcc.dg/tree-ssa/isolate-4.c: Likewise. diff --git a/gcc/common.opt b/gcc/common.opt index 9ece683..0cd1fdd 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2112,11 +2112,18 @@ foptimize-strlen Common Report Var(flag_optimize_strlen) Optimization Enable string length optimizations on trees -fisolate-erroneous-paths -Common Report Var(flag_isolate_erroneous_paths) Optimization -Detect paths which trigger erroneous or undefined behaviour. Isolate those -paths from the main control flow and turn the statement with erroneous or -undefined behaviour into a trap. +fisolate-erroneous-paths-dereference +Common Report Var(flag_isolate_erroneous_paths_dereference) Optimization +Detect paths which trigger erroneous or undefined behaviour due to +dereferencing a NULL pointer. Isolate those paths from the main control +flow and turn the statement with erroneous or undefined behaviour into a trap. + +fisolate-erroneous-paths-attribute +Common Report Var(flag_isolate_erroneous_paths_attribute) Optimization +Detect paths which trigger erroneous or undefined behaviour due a NULL value +being used in a way which is forbidden by a returns_nonnull or nonnull +attribute. Isolate those paths from the main control flow and turn the +statement with erroneous or undefined behaviour into a trap. ftree-loop-distribution Common Report Var(flag_tree_loop_distribution) Optimization diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index b30e889..704d474 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -378,7 +378,7 @@ Objective-C and Objective-C++ Dialects}. -fira-region=@var{region} -fira-hoist-pressure @gol -fira-loop-pressure -fno-ira-share-save-slots @gol -fno-ira-share-spill-slots -fira-verbose=@var{n} @gol --fisolate-erroneous-paths +-fisolate-erroneous-paths-dereference -fisolate-erroneous-paths-attribute -fivopts -fkeep-inline-functions -fkeep-static-consts -flive-range-shrinkage @gol -floop-block -floop-interchange -floop-strip-mine -floop-nest-optimize @gol -floop-parallelize-all -flto -flto-compression-level @gol @@ -6848,7 +6848,7 @@ also turns on the following optimization flags: -finline-small-functions @gol -findirect-inlining @gol -fipa-sra @gol --fisolate-erroneous-paths @gol +-fisolate-erroneous-paths-dereference @gol -foptimize-sibling-calls @gol -fpartial-inlining @gol -fpeephole2 @gol @@ -7742,10 +7742,17 @@ it may significantly increase code size (see @option{--param ipcp-unit-growth=@var{value}}). This flag is enabled by default at @option{-O3}. -@item -fisolate-erroneous-paths -Detect paths which trigger erroneous or undefined behaviour. Isolate those -paths from the main control flow and turn the statement with erroneous or -undefined behaviour into a trap. +@item -fisolate-erroneous-paths-dereference +Detect paths which trigger erroneous or undefined behaviour due to +dereferencing a NULL pointer. Isolate those paths from the main control +flow and turn the statement with erroneous or undefined behaviour into a trap. + +@item -fisolate-erroneous-paths-attribute +Detect paths which trigger erroneous or undefined behaviour due a NULL value +being used in a way which is forbidden by a @code{returns_nonnull} or @code{nonnull} +attribute. Isolate those paths from the main control flow and turn the +statement with erroneous or undefined behaviour into a trap. This is not +currently enabled, but may be enabled by @code{-O2} in the future. @item -ftree-sink @opindex ftree-sink diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c index 440d2ed..052bf3f 100644 --- a/gcc/gimple-ssa-isolate-paths.c +++ b/gcc/gimple-ssa-isolate-paths.c @@ -48,7 +48,7 @@ along with GCC; see the file COPYING3. If not see static bool cfg_altered; /* Callback for walk_stmt_load_store_ops. - + Return TRUE if OP will dereference the tree stored in DATA, FALSE otherwise. @@ -144,7 +144,6 @@ isolate_path (basic_block bb, basic_block duplicate, gimple_stmt_iterator si, si2; edge_iterator ei; edge e2; - /* First duplicate BB if we have not done so already and remove all the duplicate's outgoing edges as duplicate is going to unconditionally @@ -171,7 +170,7 @@ isolate_path (basic_block bb, basic_block duplicate, the statement which triggers undefined behaviour. If found, then transform the statement into a trap and delete everything after the statement. If not found, then this particular instance was subsumed by - an earlier instance of undefined behaviour and there's nothing to do. + an earlier instance of undefined behaviour and there's nothing to do. This is made more complicated by the fact that we have STMT, which is in BB rather than in DUPLICATE. So we set up two iterators, one for each @@ -180,7 +179,7 @@ isolate_path (basic_block bb, basic_block duplicate, When we find STMT the second iterator should point to STMT's equivalent in duplicate. If DUPLICATE ends before STMT is found in BB, then there's - nothing to do. + nothing to do. Ignore labels and debug statements. */ si = gsi_start_nondebug_after_labels_bb (bb); @@ -247,7 +246,7 @@ find_implicit_erroneous_behaviour (void) continue; /* PHI produces a pointer result. See if any of the PHI's - arguments are NULL. + arguments are NULL. When we remove an edge, we want to reprocess the current index, hence the ugly way we update I for each iteration. */ @@ -259,7 +258,7 @@ find_implicit_erroneous_behaviour (void) tree op = gimple_phi_arg_def (phi, i); next_i = i + 1; - + if (!integer_zerop (op)) continue; @@ -277,7 +276,10 @@ find_implicit_erroneous_behaviour (void) if (gimple_bb (use_stmt) != bb) continue; - if (infer_nonnull_range (use_stmt, lhs)) + if (infer_nonnull_range (use_stmt, lhs, + flag_isolate_erroneous_paths_dereference, + flag_isolate_erroneous_paths_attribute)) + { duplicate = isolate_path (bb, duplicate, e, use_stmt, lhs); @@ -294,7 +296,7 @@ find_implicit_erroneous_behaviour (void) } /* Look for statements which exhibit erroneous behaviour. For example - a NULL pointer dereference. + a NULL pointer dereference. When found, optimize the block containing the erroneous behaviour. */ static void @@ -327,7 +329,9 @@ find_explicit_erroneous_behaviour (void) /* By passing null_pointer_node, we can use infer_nonnull_range to detect explicit NULL pointer dereferences and other uses where a non-NULL value is required. */ - if (infer_nonnull_range (stmt, null_pointer_node)) + if (infer_nonnull_range (stmt, null_pointer_node, + flag_isolate_erroneous_paths_dereference, + flag_isolate_erroneous_paths_attribute)) { insert_trap_and_remove_trailing_statements (&si, null_pointer_node); @@ -361,7 +365,7 @@ find_explicit_erroneous_behaviour (void) unconditional trap and eliminate the outgoing edges from the statement's basic block. This may expose secondary optimization opportunities. - In the latter case, we isolate the path(s) with the NULL PHI + In the latter case, we isolate the path(s) with the NULL PHI feeding the dereference. We can then replace the offending statement and eliminate the outgoing edges in the duplicate. Again, this may expose secondary optimization opportunities. @@ -398,7 +402,7 @@ gimple_ssa_isolate_erroneous_paths (void) free_original_copy_tables (); - /* We scramble the CFG and loop structures a bit, clean up + /* We scramble the CFG and loop structures a bit, clean up appropriately. We really should incrementally update the loop structures, in theory it shouldn't be that hard. */ if (cfg_altered) @@ -416,7 +420,8 @@ gate_isolate_erroneous_paths (void) { /* If we do not have a suitable builtin function for the trap statement, then do not perform the optimization. */ - return (flag_isolate_erroneous_paths != 0); + return (flag_isolate_erroneous_paths_dereference != 0 + || flag_isolate_erroneous_paths_attribute != 0); } namespace { diff --git a/gcc/gimple.c b/gcc/gimple.c index 7bc87bc..f11362a 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -2502,10 +2502,16 @@ check_loadstore (gimple stmt ATTRIBUTE_UNUSED, tree op, void *data) return false; } -/* If OP can be inferred to be non-zero after STMT executes, return true. */ +/* If OP can be inferred to be non-NULL after STMT executes, return true. + + DEREFERENCE is TRUE if we can use a pointer dereference to infer a + non-NULL range, FALSE otherwise. + + ATTRIBUTE is TRUE if we can use attributes to infer a non-NULL range + for function arguments and return values. FALSE otherwise. */ bool -infer_nonnull_range (gimple stmt, tree op) +infer_nonnull_range (gimple stmt, tree op, bool dereference, bool attribute) { /* We can only assume that a pointer dereference will yield non-NULL if -fdelete-null-pointer-checks is enabled. */ @@ -2514,11 +2520,13 @@ infer_nonnull_range (gimple stmt, tree op) || gimple_code (stmt) == GIMPLE_ASM) return false; - if (walk_stmt_load_store_ops (stmt, (void *)op, - check_loadstore, check_loadstore)) + if (dereference + && walk_stmt_load_store_ops (stmt, (void *)op, + check_loadstore, check_loadstore)) return true; - if (is_gimple_call (stmt) && !gimple_call_internal_p (stmt)) + if (attribute + && is_gimple_call (stmt) && !gimple_call_internal_p (stmt)) { tree fntype = gimple_call_fntype (stmt); tree attrs = TYPE_ATTRIBUTES (fntype); @@ -2557,7 +2565,8 @@ infer_nonnull_range (gimple stmt, tree op) /* If this function is marked as returning non-null, then we can infer OP is non-null if it is used in the return statement. */ - if (gimple_code (stmt) == GIMPLE_RETURN + if (attribute + && gimple_code (stmt) == GIMPLE_RETURN && gimple_return_retval (stmt) && operand_equal_p (gimple_return_retval (stmt), op, 0) && lookup_attribute ("returns_nonnull", diff --git a/gcc/gimple.h b/gcc/gimple.h index a97a5e8..c9d9a19 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -1259,7 +1259,7 @@ extern bool gimple_call_builtin_p (gimple, enum built_in_function); extern bool gimple_asm_clobbers_memory_p (const_gimple); extern void dump_decl_set (FILE *, bitmap); extern bool nonfreeing_call_p (gimple); -extern bool infer_nonnull_range (gimple, tree); +extern bool infer_nonnull_range (gimple, tree, bool, bool); extern void sort_case_labels (vec ); extern void preprocess_case_label_vec_for_gimple (vec , tree, tree *); extern void gimple_seq_set_location (gimple_seq , location_t); diff --git a/gcc/opts.c b/gcc/opts.c index a0a6c53..b26644d 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -494,7 +494,7 @@ static const struct default_options default_options_table[] = { OPT_LEVELS_2_PLUS_SPEED_ONLY, OPT_foptimize_strlen, NULL, 1 }, { OPT_LEVELS_2_PLUS, OPT_fhoist_adjacent_loads, NULL, 1 }, { OPT_LEVELS_2_PLUS, OPT_fipa_sem_equality, NULL, 1 }, - { OPT_LEVELS_2_PLUS, OPT_fisolate_erroneous_paths, NULL, 1 }, + { OPT_LEVELS_2_PLUS, OPT_fisolate_erroneous_paths_dereference, NULL, 1 }, /* -O3 optimizations. */ { OPT_LEVELS_3_PLUS, OPT_ftree_loop_distribute_patterns, NULL, 1 }, diff --git a/gcc/testsuite/gcc.dg/pr38984.c b/gcc/testsuite/gcc.dg/pr38984.c index 0c03180..3ccb0e4 100644 --- a/gcc/testsuite/gcc.dg/pr38984.c +++ b/gcc/testsuite/gcc.dg/pr38984.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fno-delete-null-pointer-checks -fdump-tree-optimized -fno-isolate-erroneous-paths" } +/* { dg-options "-O2 -fno-delete-null-pointer-checks -fdump-tree-optimized -fno-isolate-erroneous-paths-dereference" } * */ int f(int *p) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c index 290b44c..bfcaa2b 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */ +/* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */ int z; diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c index 6937d25..c9c074d 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */ +/* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */ extern void foo(void *) __attribute__ ((__nonnull__ (1))); diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 89f2ffd..8bcc160 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -4500,7 +4500,7 @@ infer_value_range (gimple stmt, tree op, enum tree_code *comp_code_p, tree *val_ if (stmt_ends_bb_p (stmt) && EDGE_COUNT (gimple_bb (stmt)->succs) == 0) return false; - if (infer_nonnull_range (stmt, op)) + if (infer_nonnull_range (stmt, op, true, true)) { *val_p = build_int_cst (TREE_TYPE (op), 0); *comp_code_p = NE_EXPR;