| Message ID | 526F4174.9080300@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/28/13 23:02, Jeff Law wrote: > > Based on a suggestion from Marc, I want to use infer_nonnull_range in > the erroneous path isolation optimization. > > In the process of doing that, I found a few deficiencies in > infer_nonnull_range that need to be addressed. > > > First, infer_nonnull_range_p doesn't infer ranges from a GIMPLE_RETURN > if the current function is marked as returns_nonnull. That's fixed in > the relatively obvious way. > > Second, infer_nonnull_range_p, when presented with an arglist where the > non-null attribute applies to all pointer arguments, it won't bother to > determine if OP is actually one of the arguments :( It just assumes > that OP is in the argument list. Opps. > > Third, I want to be able to call infer_nonnull_range with OP being 0B. > That lets me use infer_nonnull_range to look for explicit null pointer > dereferences, explicit uses of null in a return statement in functions > that can't return non-null and explicit uses of null arguments when > those arguments can't be null. Sadly, to make that work we need to use > operand_equal_p rather than simple pointer comparisons to see if OP > shows up in STMT. > > Finally, for detecting explicit null pointers, infer_nonnull_range calls > count_ptr_derefs. count_ptr_derefs counts things in two ways. One with > a FOR_EACH_SSA_TREE_OPERAND, then again with simple walks of the tree > structures. Not surprisingly if we're looking for an explicit 0B, then > the loop over the operands finds nothing, but walking the tree > structures does. And the checking assert triggers. This change removes > the assert and instead sets *num_uses_p to a sane value. > > I don't have testcases for this stuff that are independent of the > erroneous path isolation optimization. However, each is triggered by > tests I'll include in the the erroneous path isolation patch. > > Bootstrapped and regression tested on x86_64-unknown-linux-gnu. OK for > the trunk? I'm withdrawing this patch. While everything works exactly how I wanted, I think it's best to rework things a bit and totally eliminate count_ptr_derefs entirely. That change makes it significantly easier to move infer_nonnull_range out of tree-vrp.c and into gimple.c (or wherever Andrew wants it) without a significant modularity break. Jeff
On Thu, Oct 31, 2013 at 6:41 AM, Jeff Law <law@redhat.com> wrote: > On 10/28/13 23:02, Jeff Law wrote: >> >> >> Based on a suggestion from Marc, I want to use infer_nonnull_range in >> the erroneous path isolation optimization. >> >> In the process of doing that, I found a few deficiencies in >> infer_nonnull_range that need to be addressed. >> >> >> First, infer_nonnull_range_p doesn't infer ranges from a GIMPLE_RETURN >> if the current function is marked as returns_nonnull. That's fixed in >> the relatively obvious way. >> >> Second, infer_nonnull_range_p, when presented with an arglist where the >> non-null attribute applies to all pointer arguments, it won't bother to >> determine if OP is actually one of the arguments :( It just assumes >> that OP is in the argument list. Opps. >> >> Third, I want to be able to call infer_nonnull_range with OP being 0B. >> That lets me use infer_nonnull_range to look for explicit null pointer >> dereferences, explicit uses of null in a return statement in functions >> that can't return non-null and explicit uses of null arguments when >> those arguments can't be null. Sadly, to make that work we need to use >> operand_equal_p rather than simple pointer comparisons to see if OP >> shows up in STMT. >> >> Finally, for detecting explicit null pointers, infer_nonnull_range calls >> count_ptr_derefs. count_ptr_derefs counts things in two ways. One with >> a FOR_EACH_SSA_TREE_OPERAND, then again with simple walks of the tree >> structures. Not surprisingly if we're looking for an explicit 0B, then >> the loop over the operands finds nothing, but walking the tree >> structures does. And the checking assert triggers. This change removes >> the assert and instead sets *num_uses_p to a sane value. >> >> I don't have testcases for this stuff that are independent of the >> erroneous path isolation optimization. However, each is triggered by >> tests I'll include in the the erroneous path isolation patch. >> >> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. OK for >> the trunk? > > I'm withdrawing this patch. While everything works exactly how I wanted, I > think it's best to rework things a bit and totally eliminate > count_ptr_derefs entirely. Yes! And count_uses_and_derefs, too. See walk_stmt_load_store_addr_ops. Richard. > That change makes it significantly easier to move infer_nonnull_range out of > tree-vrp.c and into gimple.c (or wherever Andrew wants it) without a > significant modularity break. > Jeff >
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c index 0b743d1..d29665b 100644 --- a/gcc/tree-ssa.c +++ b/gcc/tree-ssa.c @@ -265,7 +265,8 @@ count_ptr_derefs (tree *tp, int *walk_subtrees, void *data) return NULL_TREE; } - if (TREE_CODE (*tp) == MEM_REF && TREE_OPERAND (*tp, 0) == count_p->ptr) + if (TREE_CODE (*tp) == MEM_REF + && operand_equal_p (TREE_OPERAND (*tp, 0), count_p->ptr, 0)) { if (wi_p->is_lhs) count_p->num_stores++; @@ -326,7 +327,11 @@ count_uses_and_derefs (tree ptr, gimple stmt, unsigned *num_uses_p, *num_loads_p = count.num_loads; } - gcc_assert (*num_uses_p >= *num_loads_p + *num_stores_p); + /* This can happen if PTR refers to something other than an SSA_NAME. + In that case we will find no uses as the FOR_EACH_SSA_TREE_OPERAND + only iterates over the SSA_NAMEs in a statement. */ + if (*num_uses_p < *num_loads_p + *num_stores_p) + *num_uses_p = *num_loads_p + *num_stores_p; } diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index d3a07f3..bbcb34c 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -4509,21 +4509,38 @@ infer_nonnull_range (gimple stmt, tree op) return false; /* If "nonnull" applies to all the arguments, then ARG - is non-null. */ + is non-null if it's in the argument list. */ if (TREE_VALUE (attrs) == NULL_TREE) - return true; + { + for (unsigned int i = 0; i < gimple_call_num_args (stmt); i++) + { + if (operand_equal_p (op, gimple_call_arg (stmt, i), 0) + && POINTER_TYPE_P (TREE_TYPE (gimple_call_arg (stmt, i)))) + return true; + } + return false; + } /* Now see if op appears in the nonnull list. */ for (tree t = TREE_VALUE (attrs); t; t = TREE_CHAIN (t)) { int idx = TREE_INT_CST_LOW (TREE_VALUE (t)) - 1; tree arg = gimple_call_arg (stmt, idx); - if (op == arg) + if (operand_equal_p (op, arg, 0)) return true; } } } + /* 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 + && gimple_return_retval (stmt) + && operand_equal_p (gimple_return_retval (stmt), op, 0) + && lookup_attribute ("returns_nonnull", + TYPE_ATTRIBUTES (TREE_TYPE (current_function_decl)))) + return true; + return false; }