This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC] PR middle-end/179 gcc -O2 -Wuninitialized missing warning with &var
On Mon, Aug 11, 2008 at 5:24 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> 2008/8/11 Richard Guenther <richard.guenther@gmail.com>:
>> I think it is useful - however
>>
>> + if (TYPE_MODE (TREE_TYPE (var)) == BLKmode)
>> + return false;
>> +
>> + if (is_call_clobbered (var))
>> + {
>> + var_ann_t va = var_ann (var);
>> + unsigned int escape_mask = va->escape_mask;
>> + if (escape_mask & ESCAPE_TO_ASM)
>> + return false;
>> + if (escape_mask & ESCAPE_IS_GLOBAL)
>> + return false;
>> + if (escape_mask & ESCAPE_IS_PARM)
>> + return false;
>> + }
>>
>> this should be not necessary at all (in fact I believe just excluding
>> PARM_DECLs and is_global_var should be enough).
>
> GCC does not bootstrap without that. It gives false positives for
> really weird cases that are obviously correctly initialized before
> used. That whole function was designed by trial and error until GCC
> bootstrapped.
Huh. Ok. I think the patch should add testcases for (each) of them ;)
Still it doesn't make much sense to me to exclude aggregates
or call clobbered variables.
>> Also the following
>>
>> if (!ssa_undefined_value_p (t))
>> - return;
>> + {
>> + /*struct voptype_d *vdefs;*/
>> + struct voptype_d *vuses;
>> + /* Recurse into SSA virtual operands to check whether T is using
>> + a VDEF with default definition or it is VUSing another
>> + uninitialized variable. */
>> + gimple def = SSA_NAME_DEF_STMT (t);
>> + tree op;
>>
>> - /* TREE_NO_WARNING either means we already warned, or the front end
>> - wishes to suppress the warning. */
>> - if (TREE_NO_WARNING (var))
>> + if (!gimple_references_memory_p (def))
>> + return;
>> +
>> + vuses = gimple_vuse_ops (def);
>> + while (vuses)
>> + {
>> + int i, n;
>>
>> just should use
>>
>> def_stmt = SSA_NAME_DEF_STMT (t);
>> FOR_EACH_SSA_USE_OPERAND (..., def_stmt, ..., SSA_OP_VIRTUAL_USES)
>> if (ssa_maybe_uninitialized_var_p (SSA_NAME_VAR (USE_FROM_PTR (use)))
>> warn ...
>
> I cannot use that because I only warn if there is 1 VUSE, otherwise I
> get false positives in bootstrap. I mentioned that I am trying to warn
> for a very specific case. Further patches should enhance this
> iteratively.
I see. In this case just use SINGLE_SSA_USE_OPERAND (use_stmt,
SSA_OP_VIRTUAL_USES).
>> note that there may be false positives from like
>>
>> int *p, *q;
>>
>> int foo (int b)
>> {
>> int i, j = 0;
>> int *x;
>> p = &i;
>> q = &j;
>> if (b)
>> x = &p;
>> else
>> x = &q;
>> return *x;
>> }
>>
>> if you pass in false only it will warn because *x may access
>> uninitialized i.
>
> Sorry, I don't get what you mean. First, -Wuninitialized does not warn
> for that testcase but gcc says:
>
> 10: warning: assignment from incompatible pointer type
> 12: warning: assignment from incompatible pointer type
>
> Perhaps you mean x = p and x = q. In any case we do not warn nor we
> did before this patch.
It's probably optimized too much. But as you said you only warn
for a single VOP (and the above tried to construct a testcase with
multiple VOPs, one being an unintialized use and the other not),
my example is moot anyway ;)
> Second, if by false positive you mean that we fail to warn (I would
> call that a false negative), I don't care about that in this moment
> since this patch does not handle all cases anyway, the goal is to not
> warn in false. Looking at the dump of -fdump-tree-alias, this case is
> certainly not trivial so it is not a priority.
No, I meant false positives as we give a warning where there should
be none.
>> uninitialized i. So in the end you may want to omit warnings
>> from defining load statements that load via a pointer. Note
>> that using VOPs only works for the late uninitialized pass
>> (aliasing is not computed before). Note also that using VOPs
>> is unreliable due to partitioning, etc., a better way is to iterate
>> over the LOADED_SYMS bitmap of the defining statement
>> (doesn't work for calls!) - this would work for the early pass as well.
>
> I will check this alternative. However, such approach wastes the
> potential information given by the virtual SSA operators. And in any
Virtual SSA operators are exactly the same as LOADED_SYMS
(in fact, SSA VOPs are built walking over LOADED_SYMS).
Cheers,
Richard.