Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
> On Jan 14, 2022, at 12:58 PM, Martin Sebor wrote: > > On 1/14/22 11:29, Qing Zhao wrote: >>> On Jan 14, 2022, at 12:11 PM, Martin Sebor wrote: >>> >>> On 1/14/22 09:30, Qing Zhao wrote: > On Jan 14, 2022, at 6:45 AM, Richard Biener > wrote: > > On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao wrote: >> >> Hi, Richard, >> >> This is the updated version for the second patch, which is mainly the >> change for "Enable -Wuninitialized + -ftrivial-auto-var-init for >> address taken variables”. >> >> In this update, I mainly made the following change: >> >> 1. Delete “repl_var”, use the var_def_stmt, i.e, the call to >> .DEFERRED_INIT to record the warning suppressed info. >> 2. Add and change the comments in multiple places to make the change >> more readable. >> >> Now, for the deleted variable, we will get the necessary info to report >> warning from the call to .DEFERRED_INIT. >> >>A. the name string of DECL from the 3rd parameter of the call; >>B. the location of the DECL from the location of the call; >>C. the call can also be used to hold the information on whether the >> warning >> has been issued or not to suppress warning messages when needed; >> >> Please see the detailed description below for the problem and solution >> of this patch. >> >> This patch has been bootstrapped and regressing tested on both X86 and >> aarch64. >> >> Okay for GCC12? > > I think the change to split "%qD is used uninitialized" is bad for i8n > though it seems > like none of the strings passed to warn_uninit are currently marked > for localization. > I suppose there's lazy matching with the exact same strings passed to > warning_at around like 641 but after your change those will no longer > match up, Silly question: What does the above “641” mean? >>> >>> At around line 641 :) >> I thought of this, but I didn’t find a meaningful statement around line 641 >> in tree-ssa-uninit.c… >> 636 found_alloc = true; >> 637 break; >> 638 } >> 639 >> 640 if (!is_gimple_assign (def_stmt)) >> 641 break; >> 642 >> 643 tree_code code = gimple_assign_rhs_code (def_stmt); >> 644 if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR) >> 645 break; >> >>> > at least for the "%qs ..." case constructed. I think a better way > (for i8n) would be > to pass down a flag whether it is "may" or "is" and have the full > translatable > strings literally passed to warning_at at the expense of some code > duplication. > Actually the extra flag is unnecessary, the OPT_W... we pass is already > fully > specifying that. The only issue with the above change is: among the 4 calls to “warn_uninit” as following: if (use_stmt) warn_uninit (OPT_Wuninitialized, def, SSA_NAME_VAR (def), -"%qD is used uninitialized", use_stmt); +"is used uninitialized", use_stmt); } } @@ -932,10 +1015,10 @@ warn_uninitialized_vars (bool wmaybe_uninit) tree use = USE_FROM_PTR (use_p); if (wlims.always_executed) warn_uninit (OPT_Wuninitialized, use, SSA_NAME_VAR (use), -"%qD is used uninitialized", stmt); +"is used uninitialized", stmt); else if (wmaybe_uninit) warn_uninit (OPT_Wmaybe_uninitialized, use, SSA_NAME_VAR (use), -"%qD may be used uninitialized", stmt); +"may be used uninitialized", stmt); } /* For limiting the alias walk below we count all @@ -1182,7 +1265,7 @@ warn_uninitialized_phi (gphi *phi, vec *worklist, warn_uninit (OPT_Wmaybe_uninitialized, uninit_op, SSA_NAME_VAR (uninit_op), - "%qD may be used uninitialized in this function", + "may be used uninitialized in this function", uninit_use_stmt, loc); All the strings passed map well with the OPT_W… except the last one, since the last one has an additional string “in this function” at the end. However, since the last call has the last argument “loc” been NULL, maybe we can use this to distinguish. >>> >>> Now that diagnostics are prefixed by something like "In function 'foo': >>> the "in this function" part is superfluous and could be removed from >>> all warning messages. >> Okay, so, you mean that it’s safe to just remove “in this function” from the >> last callsite? > > Yes, I would remove it. It might require some cleanup in the test > suite but I wouldn't expect it to be
Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
On 1/14/22 11:29, Qing Zhao wrote: On Jan 14, 2022, at 12:11 PM, Martin Sebor wrote: On 1/14/22 09:30, Qing Zhao wrote: On Jan 14, 2022, at 6:45 AM, Richard Biener wrote: On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao wrote: Hi, Richard, This is the updated version for the second patch, which is mainly the change for "Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables”. In this update, I mainly made the following change: 1. Delete “repl_var”, use the var_def_stmt, i.e, the call to .DEFERRED_INIT to record the warning suppressed info. 2. Add and change the comments in multiple places to make the change more readable. Now, for the deleted variable, we will get the necessary info to report warning from the call to .DEFERRED_INIT. A. the name string of DECL from the 3rd parameter of the call; B. the location of the DECL from the location of the call; C. the call can also be used to hold the information on whether the warning has been issued or not to suppress warning messages when needed; Please see the detailed description below for the problem and solution of this patch. This patch has been bootstrapped and regressing tested on both X86 and aarch64. Okay for GCC12? I think the change to split "%qD is used uninitialized" is bad for i8n though it seems like none of the strings passed to warn_uninit are currently marked for localization. I suppose there's lazy matching with the exact same strings passed to warning_at around like 641 but after your change those will no longer match up, Silly question: What does the above “641” mean? At around line 641 :) I thought of this, but I didn’t find a meaningful statement around line 641 in tree-ssa-uninit.c… 636 found_alloc = true; 637 break; 638 } 639 640 if (!is_gimple_assign (def_stmt)) 641 break; 642 643 tree_code code = gimple_assign_rhs_code (def_stmt); 644 if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR) 645 break; at least for the "%qs ..." case constructed. I think a better way (for i8n) would be to pass down a flag whether it is "may" or "is" and have the full translatable strings literally passed to warning_at at the expense of some code duplication. Actually the extra flag is unnecessary, the OPT_W... we pass is already fully specifying that. The only issue with the above change is: among the 4 calls to “warn_uninit” as following: if (use_stmt) warn_uninit (OPT_Wuninitialized, def, SSA_NAME_VAR (def), -"%qD is used uninitialized", use_stmt); +"is used uninitialized", use_stmt); } } @@ -932,10 +1015,10 @@ warn_uninitialized_vars (bool wmaybe_uninit) tree use = USE_FROM_PTR (use_p); if (wlims.always_executed) warn_uninit (OPT_Wuninitialized, use, SSA_NAME_VAR (use), -"%qD is used uninitialized", stmt); +"is used uninitialized", stmt); else if (wmaybe_uninit) warn_uninit (OPT_Wmaybe_uninitialized, use, SSA_NAME_VAR (use), -"%qD may be used uninitialized", stmt); +"may be used uninitialized", stmt); } /* For limiting the alias walk below we count all @@ -1182,7 +1265,7 @@ warn_uninitialized_phi (gphi *phi, vec *worklist, warn_uninit (OPT_Wmaybe_uninitialized, uninit_op, SSA_NAME_VAR (uninit_op), - "%qD may be used uninitialized in this function", + "may be used uninitialized in this function", uninit_use_stmt, loc); All the strings passed map well with the OPT_W… except the last one, since the last one has an additional string “in this function” at the end. However, since the last call has the last argument “loc” been NULL, maybe we can use this to distinguish. Now that diagnostics are prefixed by something like "In function 'foo': the "in this function" part is superfluous and could be removed from all warning messages. Okay, so, you mean that it’s safe to just remove “in this function” from the last callsite? Yes, I would remove it. It might require some cleanup in the test suite but I wouldn't expect it to be too bad. When there's no location (i.e., loc is UNKNOWN_LOCATION) the called code tries to derive a location from the context. When it can't, it won't point anywhere useful, so if that case ever comes up here it should probably be handled by using the location of the curly brace closing the function definition. You mean the following in the routine warn_uninit: /* Use either the location of the read statement or that of the PHI argument, or that of the uninitialized variable, in that order, whichever is valid. */ location_t location = UNKNOWN_LOCATION; if (gimple_has_location
Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
> On Jan 14, 2022, at 12:11 PM, Martin Sebor wrote: > > On 1/14/22 09:30, Qing Zhao wrote: >>> On Jan 14, 2022, at 6:45 AM, Richard Biener >>> wrote: >>> >>> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao wrote: Hi, Richard, This is the updated version for the second patch, which is mainly the change for "Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables”. In this update, I mainly made the following change: 1. Delete “repl_var”, use the var_def_stmt, i.e, the call to .DEFERRED_INIT to record the warning suppressed info. 2. Add and change the comments in multiple places to make the change more readable. Now, for the deleted variable, we will get the necessary info to report warning from the call to .DEFERRED_INIT. A. the name string of DECL from the 3rd parameter of the call; B. the location of the DECL from the location of the call; C. the call can also be used to hold the information on whether the warning has been issued or not to suppress warning messages when needed; Please see the detailed description below for the problem and solution of this patch. This patch has been bootstrapped and regressing tested on both X86 and aarch64. Okay for GCC12? >>> >>> I think the change to split "%qD is used uninitialized" is bad for i8n >>> though it seems >>> like none of the strings passed to warn_uninit are currently marked >>> for localization. >>> I suppose there's lazy matching with the exact same strings passed to >>> warning_at around like 641 but after your change those will no longer match >>> up, >> Silly question: What does the above “641” mean? > > At around line 641 :) I thought of this, but I didn’t find a meaningful statement around line 641 in tree-ssa-uninit.c… 636 found_alloc = true; 637 break; 638 } 639 640 if (!is_gimple_assign (def_stmt)) 641 break; 642 643 tree_code code = gimple_assign_rhs_code (def_stmt); 644 if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR) 645 break; > >>> at least for the "%qs ..." case constructed. I think a better way >>> (for i8n) would be >>> to pass down a flag whether it is "may" or "is" and have the full >>> translatable >>> strings literally passed to warning_at at the expense of some code >>> duplication. >>> Actually the extra flag is unnecessary, the OPT_W... we pass is already >>> fully >>> specifying that. >> The only issue with the above change is: among the 4 calls to >> “warn_uninit” as following: >>if (use_stmt) >> warn_uninit (OPT_Wuninitialized, def, SSA_NAME_VAR (def), >> -"%qD is used uninitialized", use_stmt); >> +"is used uninitialized", use_stmt); >> } >> } >> @@ -932,10 +1015,10 @@ warn_uninitialized_vars (bool wmaybe_uninit) >> tree use = USE_FROM_PTR (use_p); >> if (wlims.always_executed) >> warn_uninit (OPT_Wuninitialized, use, SSA_NAME_VAR (use), >> -"%qD is used uninitialized", stmt); >> +"is used uninitialized", stmt); >> else if (wmaybe_uninit) >> warn_uninit (OPT_Wmaybe_uninitialized, use, SSA_NAME_VAR >> (use), >> -"%qD may be used uninitialized", stmt); >> +"may be used uninitialized", stmt); >> } >> /* For limiting the alias walk below we count all >> @@ -1182,7 +1265,7 @@ warn_uninitialized_phi (gphi *phi, vec >> *worklist, >>warn_uninit (OPT_Wmaybe_uninitialized, uninit_op, >>SSA_NAME_VAR (uninit_op), >> - "%qD may be used uninitialized in this function", >> + "may be used uninitialized in this function", >>uninit_use_stmt, loc); >> All the strings passed map well with the OPT_W… except the last one, since >> the last one has an additional string “in this function” at the end. >> However, since the last call has the last argument “loc” been NULL, maybe we >> can use this to distinguish. > > Now that diagnostics are prefixed by something like "In function 'foo': > the "in this function" part is superfluous and could be removed from > all warning messages. Okay, so, you mean that it’s safe to just remove “in this function” from the last callsite? > > When there's no location (i.e., loc is UNKNOWN_LOCATION) the called > code tries to derive a location from the context. When it can't, it > won't point anywhere useful, so if that case ever comes up here it > should probably be handled by using the location of the curly brace > closing the function definition. You mean the following in the routine warn_uninit: /* Use either the location of the read statement or that
Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
On 1/14/22 09:30, Qing Zhao wrote:
On Jan 14, 2022, at 6:45 AM, Richard Biener wrote:
On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao wrote:
Hi, Richard,
This is the updated version for the second patch, which is mainly the change for
"Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables”.
In this update, I mainly made the following change:
1. Delete “repl_var”, use the var_def_stmt, i.e, the call to .DEFERRED_INIT to
record the warning suppressed info.
2. Add and change the comments in multiple places to make the change more
readable.
Now, for the deleted variable, we will get the necessary info to report warning
from the call to .DEFERRED_INIT.
A. the name string of DECL from the 3rd parameter of the call;
B. the location of the DECL from the location of the call;
C. the call can also be used to hold the information on whether the warning
has been issued or not to suppress warning messages when needed;
Please see the detailed description below for the problem and solution of this
patch.
This patch has been bootstrapped and regressing tested on both X86 and aarch64.
Okay for GCC12?
I think the change to split "%qD is used uninitialized" is bad for i8n
though it seems
like none of the strings passed to warn_uninit are currently marked
for localization.
I suppose there's lazy matching with the exact same strings passed to
warning_at around like 641 but after your change those will no longer match up,
Silly question: What does the above “641” mean?
At around line 641 :)
at least for the "%qs ..." case constructed. I think a better way
(for i8n) would be
to pass down a flag whether it is "may" or "is" and have the full translatable
strings literally passed to warning_at at the expense of some code duplication.
Actually the extra flag is unnecessary, the OPT_W... we pass is already fully
specifying that.
The only issue with the above change is: among the 4 calls to “warn_uninit”
as following:
if (use_stmt)
warn_uninit (OPT_Wuninitialized, def, SSA_NAME_VAR (def),
-"%qD is used uninitialized", use_stmt);
+"is used uninitialized", use_stmt);
}
}
@@ -932,10 +1015,10 @@ warn_uninitialized_vars (bool wmaybe_uninit)
tree use = USE_FROM_PTR (use_p);
if (wlims.always_executed)
warn_uninit (OPT_Wuninitialized, use, SSA_NAME_VAR (use),
-"%qD is used uninitialized", stmt);
+"is used uninitialized", stmt);
else if (wmaybe_uninit)
warn_uninit (OPT_Wmaybe_uninitialized, use, SSA_NAME_VAR (use),
-"%qD may be used uninitialized", stmt);
+"may be used uninitialized", stmt);
}
/* For limiting the alias walk below we count all
@@ -1182,7 +1265,7 @@ warn_uninitialized_phi (gphi *phi, vec *worklist,
warn_uninit (OPT_Wmaybe_uninitialized, uninit_op,
SSA_NAME_VAR (uninit_op),
- "%qD may be used uninitialized in this function",
+ "may be used uninitialized in this function",
uninit_use_stmt, loc);
All the strings passed map well with the OPT_W… except the last one, since the
last one has an additional string “in this function” at the end.
However, since the last call has the last argument “loc” been NULL, maybe we
can use this to distinguish.
Now that diagnostics are prefixed by something like "In function 'foo':
the "in this function" part is superfluous and could be removed from
all warning messages.
When there's no location (i.e., loc is UNKNOWN_LOCATION) the called
code tries to derive a location from the context. When it can't, it
won't point anywhere useful, so if that case ever comes up here it
should probably be handled by using the location of the curly brace
closing the function definition.
Martin
Instead of doing
+ if (gimple_call_internal_p (var_def_stmt, IFN_DEFERRED_INIT))
+ {
+ /* Ignore the call to .DEFERRED_INIT that define the original
+var itself as the following case:
+ temp = .DEFERRED_INIT (4, 2, â~@~\alt_reloc");
+ alt_reloc = temp;
+In order to avoid generating warning for the fake usage
+at alt_reloc = temp.
...
I thought of many options, none really very appealing so I guess we have to
go with this for now.
I agree with you, this is really ugly, I am not very comfortable myself either.
But looks like no better way to resolve it….
So OK with the i8n thing sorted out - please post one hopefully last update
for the patch.
Will do it.
Thanks for your patience,
Thank you!
Qing
Richard.
thanks.
Qing.
Enable -Wuninitialized + -ftrivial-auto-var-init for address
taken variables.
With -ftrivial-auto-var-init, the address taken auto variable is replaced with
a
Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
> On Jan 14, 2022, at 6:45 AM, Richard Biener
> wrote:
>
> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao wrote:
>>
>> Hi, Richard,
>>
>> This is the updated version for the second patch, which is mainly the change
>> for "Enable -Wuninitialized + -ftrivial-auto-var-init for address taken
>> variables”.
>>
>> In this update, I mainly made the following change:
>>
>> 1. Delete “repl_var”, use the var_def_stmt, i.e, the call to .DEFERRED_INIT
>> to record the warning suppressed info.
>> 2. Add and change the comments in multiple places to make the change more
>> readable.
>>
>> Now, for the deleted variable, we will get the necessary info to report
>> warning from the call to .DEFERRED_INIT.
>>
>>A. the name string of DECL from the 3rd parameter of the call;
>>B. the location of the DECL from the location of the call;
>>C. the call can also be used to hold the information on whether the
>> warning
>> has been issued or not to suppress warning messages when needed;
>>
>> Please see the detailed description below for the problem and solution of
>> this patch.
>>
>> This patch has been bootstrapped and regressing tested on both X86 and
>> aarch64.
>>
>> Okay for GCC12?
>
> I think the change to split "%qD is used uninitialized" is bad for i8n
> though it seems
> like none of the strings passed to warn_uninit are currently marked
> for localization.
> I suppose there's lazy matching with the exact same strings passed to
> warning_at around like 641 but after your change those will no longer match
> up,
Silly question: What does the above “641” mean?
> at least for the "%qs ..." case constructed. I think a better way
> (for i8n) would be
> to pass down a flag whether it is "may" or "is" and have the full translatable
> strings literally passed to warning_at at the expense of some code
> duplication.
> Actually the extra flag is unnecessary, the OPT_W... we pass is already fully
> specifying that.
The only issue with the above change is: among the 4 calls to “warn_uninit”
as following:
if (use_stmt)
warn_uninit (OPT_Wuninitialized, def, SSA_NAME_VAR (def),
-"%qD is used uninitialized", use_stmt);
+"is used uninitialized", use_stmt);
}
}
@@ -932,10 +1015,10 @@ warn_uninitialized_vars (bool wmaybe_uninit)
tree use = USE_FROM_PTR (use_p);
if (wlims.always_executed)
warn_uninit (OPT_Wuninitialized, use, SSA_NAME_VAR (use),
-"%qD is used uninitialized", stmt);
+"is used uninitialized", stmt);
else if (wmaybe_uninit)
warn_uninit (OPT_Wmaybe_uninitialized, use, SSA_NAME_VAR (use),
-"%qD may be used uninitialized", stmt);
+"may be used uninitialized", stmt);
}
/* For limiting the alias walk below we count all
@@ -1182,7 +1265,7 @@ warn_uninitialized_phi (gphi *phi, vec *worklist,
warn_uninit (OPT_Wmaybe_uninitialized, uninit_op,
SSA_NAME_VAR (uninit_op),
- "%qD may be used uninitialized in this function",
+ "may be used uninitialized in this function",
uninit_use_stmt, loc);
All the strings passed map well with the OPT_W… except the last one, since the
last one has an additional string “in this function” at the end.
However, since the last call has the last argument “loc” been NULL, maybe we
can use this to distinguish.
>
> Instead of doing
>
> + if (gimple_call_internal_p (var_def_stmt, IFN_DEFERRED_INIT))
> + {
> + /* Ignore the call to .DEFERRED_INIT that define the original
> +var itself as the following case:
> + temp = .DEFERRED_INIT (4, 2, â~@~\alt_reloc");
> + alt_reloc = temp;
> +In order to avoid generating warning for the fake usage
> +at alt_reloc = temp.
> ...
>
> I thought of many options, none really very appealing so I guess we have to
> go with this for now.
I agree with you, this is really ugly, I am not very comfortable myself either.
But looks like no better way to resolve it….
>
> So OK with the i8n thing sorted out - please post one hopefully last update
> for the patch.
Will do it.
>
> Thanks for your patience,
Thank you!
Qing
> Richard.
>
>> thanks.
>>
>> Qing.
>>
>>
>> Enable -Wuninitialized + -ftrivial-auto-var-init for address
>> taken variables.
>>
>> With -ftrivial-auto-var-init, the address taken auto variable is replaced
>> with
>> a temporary variable during gimplification, and the original auto variable
>> might
>> be eliminated by compiler optimization completely. As a result, the current
>> uninitialized warning analysis cannot get enough information from the IR,
>> therefore the uninitialized warnings for address taken variable cannot be
>> issued based on the current implemenation of
Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao wrote:
>
> Hi, Richard,
>
> This is the updated version for the second patch, which is mainly the change
> for "Enable -Wuninitialized + -ftrivial-auto-var-init for address taken
> variables”.
>
> In this update, I mainly made the following change:
>
> 1. Delete “repl_var”, use the var_def_stmt, i.e, the call to .DEFERRED_INIT
> to record the warning suppressed info.
> 2. Add and change the comments in multiple places to make the change more
> readable.
>
> Now, for the deleted variable, we will get the necessary info to report
> warning from the call to .DEFERRED_INIT.
>
> A. the name string of DECL from the 3rd parameter of the call;
> B. the location of the DECL from the location of the call;
> C. the call can also be used to hold the information on whether the
> warning
>has been issued or not to suppress warning messages when needed;
>
> Please see the detailed description below for the problem and solution of
> this patch.
>
> This patch has been bootstrapped and regressing tested on both X86 and
> aarch64.
>
> Okay for GCC12?
I think the change to split "%qD is used uninitialized" is bad for i8n
though it seems
like none of the strings passed to warn_uninit are currently marked
for localization.
I suppose there's lazy matching with the exact same strings passed to
warning_at around like 641 but after your change those will no longer match up,
at least for the "%qs ..." case constructed. I think a better way
(for i8n) would be
to pass down a flag whether it is "may" or "is" and have the full translatable
strings literally passed to warning_at at the expense of some code duplication.
Actually the extra flag is unnecessary, the OPT_W... we pass is already fully
specifying that.
Instead of doing
+ if (gimple_call_internal_p (var_def_stmt, IFN_DEFERRED_INIT))
+ {
+ /* Ignore the call to .DEFERRED_INIT that define the original
+var itself as the following case:
+ temp = .DEFERRED_INIT (4, 2, â~@~\alt_reloc");
+ alt_reloc = temp;
+In order to avoid generating warning for the fake usage
+at alt_reloc = temp.
...
I thought of many options, none really very appealing so I guess we have to
go with this for now.
So OK with the i8n thing sorted out - please post one hopefully last update
for the patch.
Thanks for your patience,
Richard.
> thanks.
>
> Qing.
>
>
> Enable -Wuninitialized + -ftrivial-auto-var-init for address
> taken variables.
>
> With -ftrivial-auto-var-init, the address taken auto variable is replaced with
> a temporary variable during gimplification, and the original auto variable
> might
> be eliminated by compiler optimization completely. As a result, the current
> uninitialized warning analysis cannot get enough information from the IR,
> therefore the uninitialized warnings for address taken variable cannot be
> issued based on the current implemenation of -ftrival-auto-var-init.
>
> For more info please refer to:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>
> In order to improve this situation, we can improve uninitialized analysis
> for address taken auto variables with -ftrivial-auto-var-init as following:
>
> for the following stmt:
>
> _1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]);
> if (_1 != 0)
>
> The original variable DECL has been eliminated from the IR, all the necessary
> information that is needed for reporting the warnings for DECL can be acquired
> from the call to .DEFERRED_INIT.
>
> A. the name string of DECL from the 3rd parameter of the call;
> B. the location of the DECL from the location of the call;
> C. the call can also be used to hold the information on whether the
> warning
>has been issued or not to suppress warning messages when needed;
>
> The current testing cases for uninitialized warnings + -ftrivial-auto-var-init
> are adjusted to reflect the fact that we can issue warnings for address taken
> variables.
>
> gcc/ChangeLog:
>
> 2022-01-12 qing zhao
>
> * tree-ssa-uninit.c (warn_uninit): Handle .DEFERRED_INIT call with an
> anonymous SSA_NAME specially.
> (check_defs): Likewise.
> (warn_uninit_phi_uses): Adjust the message format for warn_uninit.
> (warn_uninitialized_vars): Likewise.
> (warn_uninitialized_phi): Likewise
>
> gcc/testsuite/ChangeLog:
>
> 2022-01-12 qing zhao
>
> * gcc.dg/auto-init-uninit-16.c (testfunc): Delete xfail to reflect
> the fact that address taken variable can be warned.
> * gcc.dg/auto-init-uninit-34.c (warn_scalar_1): Likewise.
> (warn_scalar_2): Likewise.
> * gcc.dg/auto-init-uninit-37.c (T1): Likewise.
> (T2): Likewise.
> * gcc.dg/auto-init-uninit-B.c (baz): Likewise.
>
> The complete patch is attached:
>
[Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
Hi, Richard, This is the updated version for the second patch, which is mainly the change for "Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables”. In this update, I mainly made the following change: 1. Delete “repl_var”, use the var_def_stmt, i.e, the call to .DEFERRED_INIT to record the warning suppressed info. 2. Add and change the comments in multiple places to make the change more readable. Now, for the deleted variable, we will get the necessary info to report warning from the call to .DEFERRED_INIT. A. the name string of DECL from the 3rd parameter of the call; B. the location of the DECL from the location of the call; C. the call can also be used to hold the information on whether the warning has been issued or not to suppress warning messages when needed; Please see the detailed description below for the problem and solution of this patch. This patch has been bootstrapped and regressing tested on both X86 and aarch64. Okay for GCC12? thanks. Qing. Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables. With -ftrivial-auto-var-init, the address taken auto variable is replaced with a temporary variable during gimplification, and the original auto variable might be eliminated by compiler optimization completely. As a result, the current uninitialized warning analysis cannot get enough information from the IR, therefore the uninitialized warnings for address taken variable cannot be issued based on the current implemenation of -ftrival-auto-var-init. For more info please refer to: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html In order to improve this situation, we can improve uninitialized analysis for address taken auto variables with -ftrivial-auto-var-init as following: for the following stmt: _1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]); if (_1 != 0) The original variable DECL has been eliminated from the IR, all the necessary information that is needed for reporting the warnings for DECL can be acquired from the call to .DEFERRED_INIT. A. the name string of DECL from the 3rd parameter of the call; B. the location of the DECL from the location of the call; C. the call can also be used to hold the information on whether the warning has been issued or not to suppress warning messages when needed; The current testing cases for uninitialized warnings + -ftrivial-auto-var-init are adjusted to reflect the fact that we can issue warnings for address taken variables. gcc/ChangeLog: 2022-01-12 qing zhao * tree-ssa-uninit.c (warn_uninit): Handle .DEFERRED_INIT call with an anonymous SSA_NAME specially. (check_defs): Likewise. (warn_uninit_phi_uses): Adjust the message format for warn_uninit. (warn_uninitialized_vars): Likewise. (warn_uninitialized_phi): Likewise gcc/testsuite/ChangeLog: 2022-01-12 qing zhao * gcc.dg/auto-init-uninit-16.c (testfunc): Delete xfail to reflect the fact that address taken variable can be warned. * gcc.dg/auto-init-uninit-34.c (warn_scalar_1): Likewise. (warn_scalar_2): Likewise. * gcc.dg/auto-init-uninit-37.c (T1): Likewise. (T2): Likewise. * gcc.dg/auto-init-uninit-B.c (baz): Likewise. The complete patch is attached: 0001-Enable-Wuninitialized-ftrivial-auto-var-init-for-add.patch Description: 0001-Enable-Wuninitialized-ftrivial-auto-var-init-for-add.patch
