> On Aug 17, 2021, at 10:04 AM, Qing Zhao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>
>
>
>> On Aug 16, 2021, at 11:48 AM, Qing Zhao via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>
>>>> From the above IR file after “FRE”, we can see that the major issue with
>>>> this IR is:
>>>>
>>>> The address taken auto variable “alt_reloc” has been completely replaced
>>>> by the temporary variable “_1” in all
>>>> the uses of the original “alt_reloc”.
>>>
>>> Well, this can happen with regular code as well, there's no need for
>>> .DEFERRED_INIT. This is the usual problem with reporting uninitialized
>>> uses late.
>>>
>>> IMHO this shouldn't be a blocker. The goal of zero "regressions" wrt
>>> -Wuninitialized isn't really achievable.
>>
>> Okay. Sounds reasonable to me too.
>>
>>>
>>>> The major problem with such IR is, during uninitialized analysis phase,
>>>> the original use of “alt_reloc” disappeared completely.
>>>> So, the warning cannot be reported.
>>>>
>>>>
>>>> My questions:
>>>>
>>>> 1. Is it possible to get the original “alt_reloc” through the temporary
>>>> variable “_1” with some available information recorded in the IR?
>>>> 2. If not, then we have to record the relationship between “alt_reloc” and
>>>> “_1” when the original “alt_reloc” is replaced by “_1” and get such
>>>> relationship during
>>>> Uninitialized analysis phase. Is this doable?
>>>
>>> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
>>> diagnostics. The difficulty is to avoid tracking it as actual use so
>>> you could for example pass a string with the declarations name though
>>> this wouldn't give the association with the actual decl.
>> Good suggestion, I can try this a little bit.
>
> I tried this yesterday, added the 4th argument to .DEFERRED_INIT as:
>
> 1st argument: SIZE of the DECL;
> 2nd argument: INIT_TYPE;
> 3rd argument: IS_VLA, 0 NO, 1 YES;
> + 4th argument: The NAME for the DECL;
>
> - as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
> + as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA, NAME)
>
> + tree name_node
> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)),
> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>
> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
> IFN_DEFERRED_INIT,
> - TREE_TYPE (decl), 3,
> + TREE_TYPE (decl), 4,
> decl_size, init_type_node,
> - is_vla_node);
> + is_vla_node, name_node);
>
>
> And got the following IR in .uninit1 dump:
>
>
> ….
>
> _1 = .DEFERRED_INIT (4, 2, 0, &"alt_reloc"[0]);
> if (_1 != 0)
> ….
>
>
> My questions:
>
> 1. Is “build_string_literal” the correct utility routine to use for this new
> argument?
> 2. Will Such string literal nodes have potential other impact?
I tried to get the 4th argument from the call to .DEFERED_INIT during
uninitialized variable analysis in tree-ssa-uninit.c:
@@ -197,18 +197,25 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree
var,
the COMPLEX_EXPRs real part in that case. See PR71581. */
if (expr == NULL_TREE
&& var == NULL_TREE
- && SSA_NAME_VAR (t) == NULL_TREE
- && is_gimple_assign (SSA_NAME_DEF_STMT (t))
- && gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR)
- {
- tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
- if (TREE_CODE (v) == SSA_NAME
- && has_undefined_value_p (v)
- && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t))))
+ && SSA_NAME_VAR (t) == NULL_TREE)
+ {
+ if (is_gimple_assign (SSA_NAME_DEF_STMT (t))
+ && (gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR))
{
- expr = SSA_NAME_VAR (v);
- var = expr;
+ tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
+ if (TREE_CODE (v) == SSA_NAME
+ && has_undefined_value_p (v)
+ && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t))))
+ {
+ expr = SSA_NAME_VAR (v);
+ var = expr;
+ }
}
+ else if (gimple_call_internal_p (SSA_NAME_DEF_STMT (t),
IFN_DEFERRED_INIT))
+ {
+ expr = gimple_call_arg (SSA_NAME_DEF_STMT (t), 3);
+ var = expr;
+ }
}
However, this 4th argument is not a regular variable, it’s just an ADDR_EXPR
that includes the constant string for the name of
the deleted variable.
If we’d like to report the warning based on this ADDR_EXPR, a complete new code
to report the warnings other than the current one that based on
“Variables” need to be added, this might make the code very ugly.
My questions:
1. Is there better way to do this?
1. As you mentioned before, it’s very unrealistic to meet the goal of “zero
regression” for -Wuninitialized, can we leave this part of work in a later
patch to improve
The warning for “address taken” auto variables?
Thanks.
Qing
>
> Qing
>
