Re: How to traverse all the local variables that declared in the current routine?

2020-12-09 Thread Qing Zhao via Gcc-patches



> On Dec 9, 2020, at 9:12 AM, Richard Biener  wrote:
> 
> On Wed, Dec 9, 2020 at 4:04 PM Qing Zhao  > wrote:
>> 
>> 
>> 
>> On Dec 9, 2020, at 2:23 AM, Richard Biener  
>> wrote:
>> 
>> On Tue, Dec 8, 2020 at 8:54 PM Qing Zhao  wrote:
>> 
>> 
>> 
>> 
>> On Dec 8, 2020, at 1:40 AM, Richard Biener  
>> wrote:
>> 
>> On Mon, Dec 7, 2020 at 5:20 PM Qing Zhao  wrote:
>> 
>> 
>> 
>> 
>> On Dec 7, 2020, at 1:12 AM, Richard Biener  
>> wrote:
>> 
>> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  wrote:
>> 
>> 
>> 
>> 
>> On Dec 4, 2020, at 2:50 AM, Richard Biener  
>> wrote:
>> 
>> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>>  wrote:
>> 
>> 
>> Richard Biener via Gcc-patches  writes:
>> 
>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
>> 
>> 
>> For locals it would be more reliable to set this flag during gimplification.
>> 
>> Do you have any comment and suggestions?
>> 
>> 
>> As said above - do you want to cover registers as well as locals?  I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> Note that optimization will already made have use of "uninitialized" state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
>> 
>> 
>> Haven't thought about this much, so it might be a daft idea, but would a
>> compromise be to use a const internal function:
>> 
>> X1 = .DEFERRED_INIT (X0, INIT)
>> 
>> where the X0 argument is an uninitialised value and the INIT argument
>> describes the initialisation pattern?  So for a decl we'd have:
>> 
>> X = .DEFERRED_INIT (X, INIT)
>> 
>> and for an SSA name we'd have:
>> 
>> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>> 
>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>> 
>> * Having the X0 argument would keep the uninitialised use of the
>> variable around for the later warning passes.
>> 
>> * Using a const function should still allow the UB to be deleted as dead
>> if X1 isn't needed.
>> 
>> * Having a function in the way should stop passes from taking advantage
>> of direct uninitialised uses for optimisation.
>> 
>> This means we won't be able to optimise based on the actual init
>> value at the gimple level, but that seems like a fair trade-off.
>> AIUI this is really a security feature or anti-UB hardening feature
>> (in the sense that users are more likely to see predictable behaviour
>> “in the field” even if the program has UB).
>> 
>> 
>> The question is whether it's in line of peoples expectation that
>> explicitely zero-initialized code behaves differently from
>> implicitely zero-initialized code with respect to optimization
>> and secondary side-effects (late diagnostics, latent bugs, etc.).
>> 
>> Introducing a new concept like .DEFERRED_INIT is much more
>> heavy-weight than an explicit zero initializer.
>> 
>> 
>> What exactly you mean by “heavy-weight”? More difficult to implement or much 
>> more run-time overhead or both? Or something else?
>> 
>> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
>> the current -Wuninitialized analysis untouched and also pass
>> the “uninitialized” info from source code level to “pass_expand”.
>> 
>> 
>> Well, "untouched" is a bit oversimplified.  You do need to handle
>> .DEFERRED_INIT as not
>> being an initialization which will definitely get interesting.
>> 
>> 
>> Yes, during uninitialized variable analysis pass, we should specially handle 
>> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>> 
>> If we want to keep the current -Wuninitialized analysis untouched, this is a 
>> quite reasonable approach.
>> 
>> However, if it’s not required to keep the current -Wuninitialized analysis 
>> untouched, adding zero-initializer directly during gimplification should
>> be much easier and simpler, and also smaller run-time overhead.
>> 
>> 
>> As for optimization I fear you'll get a load of redundant zero-init
>> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>> 
>> 
>> Runtime overhead for -fauto-init=zero is one important consideration for the 
>> whole feature, we should minimize the runtime overhead for zero
>> Initialization since it will be used in production build.
>> We can do some run-time performance evaluation when we have an 
>> implementation ready.
>> 
>> 
>> Note there will be other passes "confused" by 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-09 Thread Richard Biener via Gcc-patches
On Wed, Dec 9, 2020 at 4:04 PM Qing Zhao  wrote:
>
>
>
> On Dec 9, 2020, at 2:23 AM, Richard Biener  wrote:
>
> On Tue, Dec 8, 2020 at 8:54 PM Qing Zhao  wrote:
>
>
>
>
> On Dec 8, 2020, at 1:40 AM, Richard Biener  wrote:
>
> On Mon, Dec 7, 2020 at 5:20 PM Qing Zhao  wrote:
>
>
>
>
> On Dec 7, 2020, at 1:12 AM, Richard Biener  wrote:
>
> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  wrote:
>
>
>
>
> On Dec 4, 2020, at 2:50 AM, Richard Biener  wrote:
>
> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>  wrote:
>
>
> Richard Biener via Gcc-patches  writes:
>
> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>
> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
> unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.
>
>
> For locals it would be more reliable to set this flag during gimplification.
>
> Do you have any comment and suggestions?
>
>
> As said above - do you want to cover registers as well as locals?  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.
>
>
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
>
> X1 = .DEFERRED_INIT (X0, INIT)
>
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
>
> X = .DEFERRED_INIT (X, INIT)
>
> and for an SSA name we'd have:
>
> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>
> * Having the X0 argument would keep the uninitialised use of the
> variable around for the later warning passes.
>
> * Using a const function should still allow the UB to be deleted as dead
> if X1 isn't needed.
>
> * Having a function in the way should stop passes from taking advantage
> of direct uninitialised uses for optimisation.
>
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.
> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).
>
>
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).
>
> Introducing a new concept like .DEFERRED_INIT is much more
> heavy-weight than an explicit zero initializer.
>
>
> What exactly you mean by “heavy-weight”? More difficult to implement or much 
> more run-time overhead or both? Or something else?
>
> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
> the current -Wuninitialized analysis untouched and also pass
> the “uninitialized” info from source code level to “pass_expand”.
>
>
> Well, "untouched" is a bit oversimplified.  You do need to handle
> .DEFERRED_INIT as not
> being an initialization which will definitely get interesting.
>
>
> Yes, during uninitialized variable analysis pass, we should specially handle 
> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>
> If we want to keep the current -Wuninitialized analysis untouched, this is a 
> quite reasonable approach.
>
> However, if it’s not required to keep the current -Wuninitialized analysis 
> untouched, adding zero-initializer directly during gimplification should
> be much easier and simpler, and also smaller run-time overhead.
>
>
> As for optimization I fear you'll get a load of redundant zero-init
> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>
>
> Runtime overhead for -fauto-init=zero is one important consideration for the 
> whole feature, we should minimize the runtime overhead for zero
> Initialization since it will be used in production build.
> We can do some run-time performance evaluation when we have an implementation 
> ready.
>
>
> Note there will be other passes "confused" by .DEFERRED_INIT.  Note
> that there's going to be other
> considerations - namely where to emit the .DEFERRED_INIT - when
> emitting it during gimplification
> you can emit it at the start of the block of block-scope variables.
> When emitting after gimplification
> you have to emit at function start which will probably 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-09 Thread Qing Zhao via Gcc-patches



> On Dec 9, 2020, at 2:23 AM, Richard Biener  wrote:
> 
> On Tue, Dec 8, 2020 at 8:54 PM Qing Zhao  > wrote:
>> 
>> 
>> 
>> On Dec 8, 2020, at 1:40 AM, Richard Biener > > wrote:
>> 
>> On Mon, Dec 7, 2020 at 5:20 PM Qing Zhao > > wrote:
>> 
>> 
>> 
>> 
>> On Dec 7, 2020, at 1:12 AM, Richard Biener  
>> wrote:
>> 
>> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  wrote:
>> 
>> 
>> 
>> 
>> On Dec 4, 2020, at 2:50 AM, Richard Biener  
>> wrote:
>> 
>> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>>  wrote:
>> 
>> 
>> Richard Biener via Gcc-patches  writes:
>> 
>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
>> 
>> 
>> For locals it would be more reliable to set this flag during gimplification.
>> 
>> Do you have any comment and suggestions?
>> 
>> 
>> As said above - do you want to cover registers as well as locals?  I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> Note that optimization will already made have use of "uninitialized" state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
>> 
>> 
>> Haven't thought about this much, so it might be a daft idea, but would a
>> compromise be to use a const internal function:
>> 
>> X1 = .DEFERRED_INIT (X0, INIT)
>> 
>> where the X0 argument is an uninitialised value and the INIT argument
>> describes the initialisation pattern?  So for a decl we'd have:
>> 
>> X = .DEFERRED_INIT (X, INIT)
>> 
>> and for an SSA name we'd have:
>> 
>> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>> 
>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>> 
>> * Having the X0 argument would keep the uninitialised use of the
>> variable around for the later warning passes.
>> 
>> * Using a const function should still allow the UB to be deleted as dead
>> if X1 isn't needed.
>> 
>> * Having a function in the way should stop passes from taking advantage
>> of direct uninitialised uses for optimisation.
>> 
>> This means we won't be able to optimise based on the actual init
>> value at the gimple level, but that seems like a fair trade-off.
>> AIUI this is really a security feature or anti-UB hardening feature
>> (in the sense that users are more likely to see predictable behaviour
>> “in the field” even if the program has UB).
>> 
>> 
>> The question is whether it's in line of peoples expectation that
>> explicitely zero-initialized code behaves differently from
>> implicitely zero-initialized code with respect to optimization
>> and secondary side-effects (late diagnostics, latent bugs, etc.).
>> 
>> Introducing a new concept like .DEFERRED_INIT is much more
>> heavy-weight than an explicit zero initializer.
>> 
>> 
>> What exactly you mean by “heavy-weight”? More difficult to implement or much 
>> more run-time overhead or both? Or something else?
>> 
>> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
>> the current -Wuninitialized analysis untouched and also pass
>> the “uninitialized” info from source code level to “pass_expand”.
>> 
>> 
>> Well, "untouched" is a bit oversimplified.  You do need to handle
>> .DEFERRED_INIT as not
>> being an initialization which will definitely get interesting.
>> 
>> 
>> Yes, during uninitialized variable analysis pass, we should specially handle 
>> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>> 
>> If we want to keep the current -Wuninitialized analysis untouched, this is a 
>> quite reasonable approach.
>> 
>> However, if it’s not required to keep the current -Wuninitialized analysis 
>> untouched, adding zero-initializer directly during gimplification should
>> be much easier and simpler, and also smaller run-time overhead.
>> 
>> 
>> As for optimization I fear you'll get a load of redundant zero-init
>> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>> 
>> 
>> Runtime overhead for -fauto-init=zero is one important consideration for the 
>> whole feature, we should minimize the runtime overhead for zero
>> Initialization since it will be used in production build.
>> We can do some run-time performance evaluation when we have an 
>> implementation ready.
>> 
>> 
>> Note there will be other passes "confused" by .DEFERRED_INIT.  Note
>> that there's going to be other
>> considerations 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-09 Thread Richard Biener via Gcc-patches
On Tue, Dec 8, 2020 at 8:54 PM Qing Zhao  wrote:
>
>
>
> On Dec 8, 2020, at 1:40 AM, Richard Biener  wrote:
>
> On Mon, Dec 7, 2020 at 5:20 PM Qing Zhao  wrote:
>
>
>
>
> On Dec 7, 2020, at 1:12 AM, Richard Biener  wrote:
>
> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  wrote:
>
>
>
>
> On Dec 4, 2020, at 2:50 AM, Richard Biener  wrote:
>
> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>  wrote:
>
>
> Richard Biener via Gcc-patches  writes:
>
> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>
> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
> unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.
>
>
> For locals it would be more reliable to set this flag during gimplification.
>
> Do you have any comment and suggestions?
>
>
> As said above - do you want to cover registers as well as locals?  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.
>
>
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
>
> X1 = .DEFERRED_INIT (X0, INIT)
>
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
>
> X = .DEFERRED_INIT (X, INIT)
>
> and for an SSA name we'd have:
>
> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>
> * Having the X0 argument would keep the uninitialised use of the
> variable around for the later warning passes.
>
> * Using a const function should still allow the UB to be deleted as dead
> if X1 isn't needed.
>
> * Having a function in the way should stop passes from taking advantage
> of direct uninitialised uses for optimisation.
>
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.
> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).
>
>
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).
>
> Introducing a new concept like .DEFERRED_INIT is much more
> heavy-weight than an explicit zero initializer.
>
>
> What exactly you mean by “heavy-weight”? More difficult to implement or much 
> more run-time overhead or both? Or something else?
>
> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
> the current -Wuninitialized analysis untouched and also pass
> the “uninitialized” info from source code level to “pass_expand”.
>
>
> Well, "untouched" is a bit oversimplified.  You do need to handle
> .DEFERRED_INIT as not
> being an initialization which will definitely get interesting.
>
>
> Yes, during uninitialized variable analysis pass, we should specially handle 
> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>
> If we want to keep the current -Wuninitialized analysis untouched, this is a 
> quite reasonable approach.
>
> However, if it’s not required to keep the current -Wuninitialized analysis 
> untouched, adding zero-initializer directly during gimplification should
> be much easier and simpler, and also smaller run-time overhead.
>
>
> As for optimization I fear you'll get a load of redundant zero-init
> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>
>
> Runtime overhead for -fauto-init=zero is one important consideration for the 
> whole feature, we should minimize the runtime overhead for zero
> Initialization since it will be used in production build.
> We can do some run-time performance evaluation when we have an implementation 
> ready.
>
>
> Note there will be other passes "confused" by .DEFERRED_INIT.  Note
> that there's going to be other
> considerations - namely where to emit the .DEFERRED_INIT - when
> emitting it during gimplification
> you can emit it at the start of the block of block-scope variables.
> When emitting after gimplification
> you have to emit at function start which will probably make stack slot
> sharing inefficient because
> the deferred init will cause overlapping lifetimes.  With 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-08 Thread Qing Zhao via Gcc-patches



> On Dec 8, 2020, at 1:40 AM, Richard Biener  wrote:
> 
> On Mon, Dec 7, 2020 at 5:20 PM Qing Zhao  > wrote:
>> 
>> 
>> 
>> On Dec 7, 2020, at 1:12 AM, Richard Biener  
>> wrote:
>> 
>> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  wrote:
>> 
>> 
>> 
>> 
>> On Dec 4, 2020, at 2:50 AM, Richard Biener  
>> wrote:
>> 
>> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>>  wrote:
>> 
>> 
>> Richard Biener via Gcc-patches  writes:
>> 
>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
>> 
>> 
>> For locals it would be more reliable to set this flag during gimplification.
>> 
>> Do you have any comment and suggestions?
>> 
>> 
>> As said above - do you want to cover registers as well as locals?  I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> Note that optimization will already made have use of "uninitialized" state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
>> 
>> 
>> Haven't thought about this much, so it might be a daft idea, but would a
>> compromise be to use a const internal function:
>> 
>> X1 = .DEFERRED_INIT (X0, INIT)
>> 
>> where the X0 argument is an uninitialised value and the INIT argument
>> describes the initialisation pattern?  So for a decl we'd have:
>> 
>> X = .DEFERRED_INIT (X, INIT)
>> 
>> and for an SSA name we'd have:
>> 
>> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>> 
>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>> 
>> * Having the X0 argument would keep the uninitialised use of the
>> variable around for the later warning passes.
>> 
>> * Using a const function should still allow the UB to be deleted as dead
>> if X1 isn't needed.
>> 
>> * Having a function in the way should stop passes from taking advantage
>> of direct uninitialised uses for optimisation.
>> 
>> This means we won't be able to optimise based on the actual init
>> value at the gimple level, but that seems like a fair trade-off.
>> AIUI this is really a security feature or anti-UB hardening feature
>> (in the sense that users are more likely to see predictable behaviour
>> “in the field” even if the program has UB).
>> 
>> 
>> The question is whether it's in line of peoples expectation that
>> explicitely zero-initialized code behaves differently from
>> implicitely zero-initialized code with respect to optimization
>> and secondary side-effects (late diagnostics, latent bugs, etc.).
>> 
>> Introducing a new concept like .DEFERRED_INIT is much more
>> heavy-weight than an explicit zero initializer.
>> 
>> 
>> What exactly you mean by “heavy-weight”? More difficult to implement or much 
>> more run-time overhead or both? Or something else?
>> 
>> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
>> the current -Wuninitialized analysis untouched and also pass
>> the “uninitialized” info from source code level to “pass_expand”.
>> 
>> 
>> Well, "untouched" is a bit oversimplified.  You do need to handle
>> .DEFERRED_INIT as not
>> being an initialization which will definitely get interesting.
>> 
>> 
>> Yes, during uninitialized variable analysis pass, we should specially handle 
>> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>> 
>> If we want to keep the current -Wuninitialized analysis untouched, this is a 
>> quite reasonable approach.
>> 
>> However, if it’s not required to keep the current -Wuninitialized analysis 
>> untouched, adding zero-initializer directly during gimplification should
>> be much easier and simpler, and also smaller run-time overhead.
>> 
>> 
>> As for optimization I fear you'll get a load of redundant zero-init
>> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>> 
>> 
>> Runtime overhead for -fauto-init=zero is one important consideration for the 
>> whole feature, we should minimize the runtime overhead for zero
>> Initialization since it will be used in production build.
>> We can do some run-time performance evaluation when we have an 
>> implementation ready.
>> 
>> 
>> Note there will be other passes "confused" by .DEFERRED_INIT.  Note
>> that there's going to be other
>> considerations - namely where to emit the .DEFERRED_INIT - when
>> emitting it during gimplification
>> you can emit it at the start of the block of block-scope variables.
>> When emitting after gimplification

Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Richard Biener via Gcc-patches
On Mon, Dec 7, 2020 at 5:20 PM Qing Zhao  wrote:
>
>
>
> On Dec 7, 2020, at 1:12 AM, Richard Biener  wrote:
>
> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  wrote:
>
>
>
>
> On Dec 4, 2020, at 2:50 AM, Richard Biener  wrote:
>
> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>  wrote:
>
>
> Richard Biener via Gcc-patches  writes:
>
> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>
> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
> unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.
>
>
> For locals it would be more reliable to set this flag during gimplification.
>
> Do you have any comment and suggestions?
>
>
> As said above - do you want to cover registers as well as locals?  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.
>
>
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
>
> X1 = .DEFERRED_INIT (X0, INIT)
>
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
>
> X = .DEFERRED_INIT (X, INIT)
>
> and for an SSA name we'd have:
>
> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>
> * Having the X0 argument would keep the uninitialised use of the
> variable around for the later warning passes.
>
> * Using a const function should still allow the UB to be deleted as dead
> if X1 isn't needed.
>
> * Having a function in the way should stop passes from taking advantage
> of direct uninitialised uses for optimisation.
>
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.
> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).
>
>
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).
>
> Introducing a new concept like .DEFERRED_INIT is much more
> heavy-weight than an explicit zero initializer.
>
>
> What exactly you mean by “heavy-weight”? More difficult to implement or much 
> more run-time overhead or both? Or something else?
>
> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
> the current -Wuninitialized analysis untouched and also pass
> the “uninitialized” info from source code level to “pass_expand”.
>
>
> Well, "untouched" is a bit oversimplified.  You do need to handle
> .DEFERRED_INIT as not
> being an initialization which will definitely get interesting.
>
>
> Yes, during uninitialized variable analysis pass, we should specially handle 
> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>
> If we want to keep the current -Wuninitialized analysis untouched, this is a 
> quite reasonable approach.
>
> However, if it’s not required to keep the current -Wuninitialized analysis 
> untouched, adding zero-initializer directly during gimplification should
> be much easier and simpler, and also smaller run-time overhead.
>
>
> As for optimization I fear you'll get a load of redundant zero-init
> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>
>
> Runtime overhead for -fauto-init=zero is one important consideration for the 
> whole feature, we should minimize the runtime overhead for zero
> Initialization since it will be used in production build.
> We can do some run-time performance evaluation when we have an implementation 
> ready.
>
>
> Note there will be other passes "confused" by .DEFERRED_INIT.  Note
> that there's going to be other
> considerations - namely where to emit the .DEFERRED_INIT - when
> emitting it during gimplification
> you can emit it at the start of the block of block-scope variables.
> When emitting after gimplification
> you have to emit at function start which will probably make stack slot
> sharing inefficient because
> the deferred init will cause overlapping lifetimes.  With emitting at
> block boundary the .DEFERRED_INIT
> will act as code-motion barrier (and it itself likely cannot be moved)

Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Richard Biener via Gcc-patches
On Mon, Dec 7, 2020 at 7:34 PM Qing Zhao  wrote:
>
>
>
> On Dec 7, 2020, at 12:05 PM, Richard Sandiford  
> wrote:
>
> Qing Zhao  writes:
>
> On Dec 7, 2020, at 11:10 AM, Richard Sandiford  
> wrote:
>
>
> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
> unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.
>
>
> For locals it would be more reliable to set this flag during gimplification.
>
> Do you have any comment and suggestions?
>
>
> As said above - do you want to cover registers as well as locals?  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.
>
>
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
>
> X1 = .DEFERRED_INIT (X0, INIT)
>
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
>
> X = .DEFERRED_INIT (X, INIT)
>
> and for an SSA name we'd have:
>
> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>
> * Having the X0 argument would keep the uninitialised use of the
> variable around for the later warning passes.
>
> * Using a const function should still allow the UB to be deleted as dead
> if X1 isn't needed.
>
> * Having a function in the way should stop passes from taking advantage
> of direct uninitialised uses for optimisation.
>
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.
> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).
>
>
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).
>
> Introducing a new concept like .DEFERRED_INIT is much more
> heavy-weight than an explicit zero initializer.
>
>
> What exactly you mean by “heavy-weight”? More difficult to implement or much 
> more run-time overhead or both? Or something else?
>
> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
> the current -Wuninitialized analysis untouched and also pass
> the “uninitialized” info from source code level to “pass_expand”.
>
>
> Well, "untouched" is a bit oversimplified.  You do need to handle
> .DEFERRED_INIT as not
> being an initialization which will definitely get interesting.
>
>
> Yes, during uninitialized variable analysis pass, we should specially handle 
> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>
>
> Are you sure we need to do that?  The point of having the first argument
> to .DEFERRED_INIT was that that argument would still provide an
> uninitialised use of the variable.  And the values are passed and
> returned by value, so the lack of initialisation is explicit in
> the gcall itself, without knowing what the target function does.
>
> The idea is that we can essentially treat .DEFERRED_INIT as a normal
> (const) function call.  I'd be surprised if many passes needed to
> handle it specially.
>
>
> Just checked with a small testing case (to emulate the .DEFERRED_INIT 
> approach):
>
> qinzhao@gcc10:~/Bugs/auto-init$ cat t.c
> extern int DEFFERED_INIT (int, int) __attribute__ ((const));
>
> int foo (int n, int r)
> {
>  int v;
>
>  v = DEFFERED_INIT (v, 0);
>  if (n < 10)
>v = r;
>
>  return v;
> }
> qinzhao@gcc10:~/Bugs/auto-init$ sh t
> /home/qinzhao/Install/latest_write/bin/gcc -O -Wuninitialized -fdump-tree-all 
> -S t.c
> t.c: In function ‘foo’:
> t.c:7:7: warning: ‘v’ is used uninitialized [-Wuninitialized]
>7 |   v = DEFFERED_INIT (v, 0);
>  |   ^~~~
>
> We can see that the current uninitialized variable analysis treats the new 
> added artificial initialization as the first use of the uninialized variable. 
>  Therefore report the warning there.
> However, we should report warning at “return v”.
>
>
> Ah, OK, so this is about the quality of the warning, rather than about
> whether we report a warning or not?
>
> So, I think that we still need to specifically handle the new added 
> 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Qing Zhao via Gcc-patches



> On Dec 7, 2020, at 12:05 PM, Richard Sandiford  
> wrote:
> 
> Qing Zhao mailto:qing.z...@oracle.com>> writes:
>>> On Dec 7, 2020, at 11:10 AM, Richard Sandiford  
>>> wrote:
>> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then 
>> keep it
>> even though DECL_INITIAL might be NULLed.
>> 
>> 
>> For locals it would be more reliable to set this flag during 
>> gimplification.
>> 
>> Do you have any comment and suggestions?
>> 
>> 
>> As said above - do you want to cover registers as well as locals?  I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> Note that optimization will already made have use of "uninitialized" 
>> state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
>> 
>> 
>> Haven't thought about this much, so it might be a daft idea, but would a
>> compromise be to use a const internal function:
>> 
>> X1 = .DEFERRED_INIT (X0, INIT)
>> 
>> where the X0 argument is an uninitialised value and the INIT argument
>> describes the initialisation pattern?  So for a decl we'd have:
>> 
>> X = .DEFERRED_INIT (X, INIT)
>> 
>> and for an SSA name we'd have:
>> 
>> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>> 
>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>> 
>> * Having the X0 argument would keep the uninitialised use of the
>> variable around for the later warning passes.
>> 
>> * Using a const function should still allow the UB to be deleted as dead
>> if X1 isn't needed.
>> 
>> * Having a function in the way should stop passes from taking advantage
>> of direct uninitialised uses for optimisation.
>> 
>> This means we won't be able to optimise based on the actual init
>> value at the gimple level, but that seems like a fair trade-off.
>> AIUI this is really a security feature or anti-UB hardening feature
>> (in the sense that users are more likely to see predictable behaviour
>> “in the field” even if the program has UB).
>> 
>> 
>> The question is whether it's in line of peoples expectation that
>> explicitely zero-initialized code behaves differently from
>> implicitely zero-initialized code with respect to optimization
>> and secondary side-effects (late diagnostics, latent bugs, etc.).
>> 
>> Introducing a new concept like .DEFERRED_INIT is much more
>> heavy-weight than an explicit zero initializer.
>> 
>> 
>> What exactly you mean by “heavy-weight”? More difficult to implement or 
>> much more run-time overhead or both? Or something else?
>> 
>> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us 
>> keep the current -Wuninitialized analysis untouched and also pass
>> the “uninitialized” info from source code level to “pass_expand”.
> 
> Well, "untouched" is a bit oversimplified.  You do need to handle
> .DEFERRED_INIT as not
> being an initialization which will definitely get interesting.
 
 Yes, during uninitialized variable analysis pass, we should specially 
 handle the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>>> 
>>> Are you sure we need to do that?  The point of having the first argument
>>> to .DEFERRED_INIT was that that argument would still provide an
>>> uninitialised use of the variable.  And the values are passed and
>>> returned by value, so the lack of initialisation is explicit in
>>> the gcall itself, without knowing what the target function does.
>>> 
>>> The idea is that we can essentially treat .DEFERRED_INIT as a normal
>>> (const) function call.  I'd be surprised if many passes needed to
>>> handle it specially.
>>> 
>> 
>> Just checked with a small testing case (to emulate the .DEFERRED_INIT 
>> approach):
>> 
>> qinzhao@gcc10:~/Bugs/auto-init$ cat t.c
>> extern int DEFFERED_INIT (int, int) __attribute__ ((const));
>> 
>> int foo (int n, int r)
>> {
>>  int v;
>> 
>>  v = DEFFERED_INIT (v, 0);
>>  if (n < 10) 
>>v = r;
>> 
>>  return v;
>> }
>> qinzhao@gcc10:~/Bugs/auto-init$ sh t
>> /home/qinzhao/Install/latest_write/bin/gcc -O -Wuninitialized 
>> -fdump-tree-all -S t.c
>> t.c: In function ‘foo’:
>> t.c:7:7: warning: ‘v’ is used uninitialized [-Wuninitialized]
>>7 |   v = 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Richard Sandiford via Gcc-patches
Qing Zhao  writes:
>> On Dec 7, 2020, at 11:10 AM, Richard Sandiford  
>> wrote:
> 
> Another issue is, in order to check whether an auto-variable has 
> initializer, I plan to add a new bit in “decl_common” as:
> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
> unsigned decl_is_initialized :1;
> 
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
> 
> set this bit when setting DECL_INITIAL for the variables in FE. then keep 
> it
> even though DECL_INITIAL might be NULLed.
> 
> 
> For locals it would be more reliable to set this flag during 
> gimplification.
> 
> Do you have any comment and suggestions?
> 
> 
> As said above - do you want to cover registers as well as locals?  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
> 
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too 
> late.
> 
> 
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
> 
> X1 = .DEFERRED_INIT (X0, INIT)
> 
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
> 
> X = .DEFERRED_INIT (X, INIT)
> 
> and for an SSA name we'd have:
> 
> X_2 = .DEFERRED_INIT (X_1(D), INIT)
> 
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
> 
> * Having the X0 argument would keep the uninitialised use of the
> variable around for the later warning passes.
> 
> * Using a const function should still allow the UB to be deleted as dead
> if X1 isn't needed.
> 
> * Having a function in the way should stop passes from taking advantage
> of direct uninitialised uses for optimisation.
> 
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.
> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).
> 
> 
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).
> 
> Introducing a new concept like .DEFERRED_INIT is much more
> heavy-weight than an explicit zero initializer.
> 
> 
> What exactly you mean by “heavy-weight”? More difficult to implement or 
> much more run-time overhead or both? Or something else?
> 
> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us 
> keep the current -Wuninitialized analysis untouched and also pass
> the “uninitialized” info from source code level to “pass_expand”.
 
 Well, "untouched" is a bit oversimplified.  You do need to handle
 .DEFERRED_INIT as not
 being an initialization which will definitely get interesting.
>>> 
>>> Yes, during uninitialized variable analysis pass, we should specially 
>>> handle the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
>> 
>> Are you sure we need to do that?  The point of having the first argument
>> to .DEFERRED_INIT was that that argument would still provide an
>> uninitialised use of the variable.  And the values are passed and
>> returned by value, so the lack of initialisation is explicit in
>> the gcall itself, without knowing what the target function does.
>> 
>> The idea is that we can essentially treat .DEFERRED_INIT as a normal
>> (const) function call.  I'd be surprised if many passes needed to
>> handle it specially.
>> 
>
> Just checked with a small testing case (to emulate the .DEFERRED_INIT 
> approach):
>
> qinzhao@gcc10:~/Bugs/auto-init$ cat t.c
> extern int DEFFERED_INIT (int, int) __attribute__ ((const));
>
> int foo (int n, int r)
> {
>   int v;
>
>   v = DEFFERED_INIT (v, 0);
>   if (n < 10) 
> v = r;
>
>   return v;
> }
> qinzhao@gcc10:~/Bugs/auto-init$ sh t
> /home/qinzhao/Install/latest_write/bin/gcc -O -Wuninitialized -fdump-tree-all 
> -S t.c
> t.c: In function ‘foo’:
> t.c:7:7: warning: ‘v’ is used uninitialized [-Wuninitialized]
> 7 |   v = DEFFERED_INIT (v, 0);
>   |   ^~~~
>
> We can see that the current uninitialized variable analysis treats the new 
> added artificial initialization as the first use of the uninialized variable. 
>  Therefore 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Qing Zhao via Gcc-patches



> On Dec 7, 2020, at 11:10 AM, Richard Sandiford  
> wrote:
 
 Another issue is, in order to check whether an auto-variable has 
 initializer, I plan to add a new bit in “decl_common” as:
 /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
 unsigned decl_is_initialized :1;
 
 /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
 #define DECL_IS_INITIALIZED(NODE) \
 (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
 
 set this bit when setting DECL_INITIAL for the variables in FE. then keep 
 it
 even though DECL_INITIAL might be NULLed.
 
 
 For locals it would be more reliable to set this flag during 
 gimplification.
 
 Do you have any comment and suggestions?
 
 
 As said above - do you want to cover registers as well as locals?  I'd do
 the actual zeroing during RTL expansion instead since otherwise you
 have to figure youself whether a local is actually used (see 
 expand_stack_vars)
 
 Note that optimization will already made have use of "uninitialized" state
 of locals so depending on what the actual goal is here "late" may be too 
 late.
 
 
 Haven't thought about this much, so it might be a daft idea, but would a
 compromise be to use a const internal function:
 
 X1 = .DEFERRED_INIT (X0, INIT)
 
 where the X0 argument is an uninitialised value and the INIT argument
 describes the initialisation pattern?  So for a decl we'd have:
 
 X = .DEFERRED_INIT (X, INIT)
 
 and for an SSA name we'd have:
 
 X_2 = .DEFERRED_INIT (X_1(D), INIT)
 
 with all other uses of X_1(D) being replaced by X_2.  The idea is that:
 
 * Having the X0 argument would keep the uninitialised use of the
 variable around for the later warning passes.
 
 * Using a const function should still allow the UB to be deleted as dead
 if X1 isn't needed.
 
 * Having a function in the way should stop passes from taking advantage
 of direct uninitialised uses for optimisation.
 
 This means we won't be able to optimise based on the actual init
 value at the gimple level, but that seems like a fair trade-off.
 AIUI this is really a security feature or anti-UB hardening feature
 (in the sense that users are more likely to see predictable behaviour
 “in the field” even if the program has UB).
 
 
 The question is whether it's in line of peoples expectation that
 explicitely zero-initialized code behaves differently from
 implicitely zero-initialized code with respect to optimization
 and secondary side-effects (late diagnostics, latent bugs, etc.).
 
 Introducing a new concept like .DEFERRED_INIT is much more
 heavy-weight than an explicit zero initializer.
 
 
 What exactly you mean by “heavy-weight”? More difficult to implement or 
 much more run-time overhead or both? Or something else?
 
 The major benefit of the approach of “.DEFERRED_INIT”  is to enable us 
 keep the current -Wuninitialized analysis untouched and also pass
 the “uninitialized” info from source code level to “pass_expand”.
>>> 
>>> Well, "untouched" is a bit oversimplified.  You do need to handle
>>> .DEFERRED_INIT as not
>>> being an initialization which will definitely get interesting.
>> 
>> Yes, during uninitialized variable analysis pass, we should specially handle 
>> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.
> 
> Are you sure we need to do that?  The point of having the first argument
> to .DEFERRED_INIT was that that argument would still provide an
> uninitialised use of the variable.  And the values are passed and
> returned by value, so the lack of initialisation is explicit in
> the gcall itself, without knowing what the target function does.
> 
> The idea is that we can essentially treat .DEFERRED_INIT as a normal
> (const) function call.  I'd be surprised if many passes needed to
> handle it specially.
> 

Just checked with a small testing case (to emulate the .DEFERRED_INIT approach):

qinzhao@gcc10:~/Bugs/auto-init$ cat t.c
extern int DEFFERED_INIT (int, int) __attribute__ ((const));

int foo (int n, int r)
{
  int v;

  v = DEFFERED_INIT (v, 0);
  if (n < 10) 
v = r;

  return v;
}
qinzhao@gcc10:~/Bugs/auto-init$ sh t
/home/qinzhao/Install/latest_write/bin/gcc -O -Wuninitialized -fdump-tree-all 
-S t.c
t.c: In function ‘foo’:
t.c:7:7: warning: ‘v’ is used uninitialized [-Wuninitialized]
7 |   v = DEFFERED_INIT (v, 0);
  |   ^~~~

We can see that the current uninitialized variable analysis treats the new 
added artificial initialization as the first use of the uninialized variable.  
Therefore report the warning there.
However, we should report warning at “return v”. 
So, I think that we still need to specifically handle the new added artificial 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Richard Sandiford via Gcc-patches
Richard Biener  writes:
> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>  wrote:
>>
>> Richard Biener via Gcc-patches  writes:
>> > On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>> >> Another issue is, in order to check whether an auto-variable has 
>> >> initializer, I plan to add a new bit in “decl_common” as:
>> >>   /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> >>   unsigned decl_is_initialized :1;
>> >>
>> >> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> >> #define DECL_IS_INITIALIZED(NODE) \
>> >>   (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> >>
>> >> set this bit when setting DECL_INITIAL for the variables in FE. then keep 
>> >> it
>> >> even though DECL_INITIAL might be NULLed.
>> >
>> > For locals it would be more reliable to set this flag during 
>> > gimplification.
>> >
>> >> Do you have any comment and suggestions?
>> >
>> > As said above - do you want to cover registers as well as locals?  I'd do
>> > the actual zeroing during RTL expansion instead since otherwise you
>> > have to figure youself whether a local is actually used (see 
>> > expand_stack_vars)
>> >
>> > Note that optimization will already made have use of "uninitialized" state
>> > of locals so depending on what the actual goal is here "late" may be too 
>> > late.
>>
>> Haven't thought about this much, so it might be a daft idea, but would a
>> compromise be to use a const internal function:
>>
>>   X1 = .DEFERRED_INIT (X0, INIT)
>>
>> where the X0 argument is an uninitialised value and the INIT argument
>> describes the initialisation pattern?  So for a decl we'd have:
>>
>>   X = .DEFERRED_INIT (X, INIT)
>>
>> and for an SSA name we'd have:
>>
>>   X_2 = .DEFERRED_INIT (X_1(D), INIT)
>>
>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>>
>> * Having the X0 argument would keep the uninitialised use of the
>>   variable around for the later warning passes.
>>
>> * Using a const function should still allow the UB to be deleted as dead
>>   if X1 isn't needed.
>>
>> * Having a function in the way should stop passes from taking advantage
>>   of direct uninitialised uses for optimisation.
>>
>> This means we won't be able to optimise based on the actual init
>> value at the gimple level, but that seems like a fair trade-off.
>> AIUI this is really a security feature or anti-UB hardening feature
>> (in the sense that users are more likely to see predictable behaviour
>> “in the field” even if the program has UB).
>
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).

>From my understanding, that's OK.  I don't think this option is like -g,
which is supposed to have no observable effect other than adding or
removing debug info.

It's OK for implicit zero initialisation to be slower than explicit
zero initialisation.  After all, if someone actively wants something
to be initialised to zero, they're still expected to do it in the
source code.  The implicit initalisation is just a safety net.

Similarly, I think it's OK that code won't be optimised identically
with and without .DEFERRED_INIT (or whatever other mechanism we use),
and so won't provide identical late warnings.  In both cases we should
just do our best to diagnose what we can.

> Btw, I don't think theres any reason to cling onto clangs semantics
> for a particular switch.  We'll never be able to emulate 1:1 behavior
> and our -Wuninit behavior is probably wastly different already.

Yeah, this isn't about trying to match compilers diagnostic-for-diagnostic.
It's more about matching them principle-for-principle.

Thanks,
Richard


Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Richard Sandiford via Gcc-patches
Qing Zhao via Gcc-patches  writes:
>> On Dec 7, 2020, at 1:12 AM, Richard Biener  
>> wrote:
>> 
>> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao > > wrote:
>>> 
>>> 
>>> 
>>> On Dec 4, 2020, at 2:50 AM, Richard Biener  
>>> wrote:
>>> 
>>> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>>>  wrote:
>>> 
>>> 
>>> Richard Biener via Gcc-patches  writes:
>>> 
>>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>>> 
>>> Another issue is, in order to check whether an auto-variable has 
>>> initializer, I plan to add a new bit in “decl_common” as:
>>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>>> unsigned decl_is_initialized :1;
>>> 
>>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>>> #define DECL_IS_INITIALIZED(NODE) \
>>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>>> 
>>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>>> even though DECL_INITIAL might be NULLed.
>>> 
>>> 
>>> For locals it would be more reliable to set this flag during gimplification.
>>> 
>>> Do you have any comment and suggestions?
>>> 
>>> 
>>> As said above - do you want to cover registers as well as locals?  I'd do
>>> the actual zeroing during RTL expansion instead since otherwise you
>>> have to figure youself whether a local is actually used (see 
>>> expand_stack_vars)
>>> 
>>> Note that optimization will already made have use of "uninitialized" state
>>> of locals so depending on what the actual goal is here "late" may be too 
>>> late.
>>> 
>>> 
>>> Haven't thought about this much, so it might be a daft idea, but would a
>>> compromise be to use a const internal function:
>>> 
>>> X1 = .DEFERRED_INIT (X0, INIT)
>>> 
>>> where the X0 argument is an uninitialised value and the INIT argument
>>> describes the initialisation pattern?  So for a decl we'd have:
>>> 
>>> X = .DEFERRED_INIT (X, INIT)
>>> 
>>> and for an SSA name we'd have:
>>> 
>>> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>>> 
>>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>>> 
>>> * Having the X0 argument would keep the uninitialised use of the
>>> variable around for the later warning passes.
>>> 
>>> * Using a const function should still allow the UB to be deleted as dead
>>> if X1 isn't needed.
>>> 
>>> * Having a function in the way should stop passes from taking advantage
>>> of direct uninitialised uses for optimisation.
>>> 
>>> This means we won't be able to optimise based on the actual init
>>> value at the gimple level, but that seems like a fair trade-off.
>>> AIUI this is really a security feature or anti-UB hardening feature
>>> (in the sense that users are more likely to see predictable behaviour
>>> “in the field” even if the program has UB).
>>> 
>>> 
>>> The question is whether it's in line of peoples expectation that
>>> explicitely zero-initialized code behaves differently from
>>> implicitely zero-initialized code with respect to optimization
>>> and secondary side-effects (late diagnostics, latent bugs, etc.).
>>> 
>>> Introducing a new concept like .DEFERRED_INIT is much more
>>> heavy-weight than an explicit zero initializer.
>>> 
>>> 
>>> What exactly you mean by “heavy-weight”? More difficult to implement or 
>>> much more run-time overhead or both? Or something else?
>>> 
>>> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
>>> the current -Wuninitialized analysis untouched and also pass
>>> the “uninitialized” info from source code level to “pass_expand”.
>> 
>> Well, "untouched" is a bit oversimplified.  You do need to handle
>> .DEFERRED_INIT as not
>> being an initialization which will definitely get interesting.
>
> Yes, during uninitialized variable analysis pass, we should specially handle 
> the defs with “.DEFERRED_INIT”, to treat them as uninitializations.

Are you sure we need to do that?  The point of having the first argument
to .DEFERRED_INIT was that that argument would still provide an
uninitialised use of the variable.  And the values are passed and
returned by value, so the lack of initialisation is explicit in
the gcall itself, without knowing what the target function does.

The idea is that we can essentially treat .DEFERRED_INIT as a normal
(const) function call.  I'd be surprised if many passes needed to
handle it specially.

Thanks,
Richard


Re: How to traverse all the local variables that declared in the current routine?

2020-12-07 Thread Qing Zhao via Gcc-patches



> On Dec 7, 2020, at 1:12 AM, Richard Biener  wrote:
> 
> On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  > wrote:
>> 
>> 
>> 
>> On Dec 4, 2020, at 2:50 AM, Richard Biener  
>> wrote:
>> 
>> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>>  wrote:
>> 
>> 
>> Richard Biener via Gcc-patches  writes:
>> 
>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
>> 
>> 
>> For locals it would be more reliable to set this flag during gimplification.
>> 
>> Do you have any comment and suggestions?
>> 
>> 
>> As said above - do you want to cover registers as well as locals?  I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> Note that optimization will already made have use of "uninitialized" state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
>> 
>> 
>> Haven't thought about this much, so it might be a daft idea, but would a
>> compromise be to use a const internal function:
>> 
>> X1 = .DEFERRED_INIT (X0, INIT)
>> 
>> where the X0 argument is an uninitialised value and the INIT argument
>> describes the initialisation pattern?  So for a decl we'd have:
>> 
>> X = .DEFERRED_INIT (X, INIT)
>> 
>> and for an SSA name we'd have:
>> 
>> X_2 = .DEFERRED_INIT (X_1(D), INIT)
>> 
>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>> 
>> * Having the X0 argument would keep the uninitialised use of the
>> variable around for the later warning passes.
>> 
>> * Using a const function should still allow the UB to be deleted as dead
>> if X1 isn't needed.
>> 
>> * Having a function in the way should stop passes from taking advantage
>> of direct uninitialised uses for optimisation.
>> 
>> This means we won't be able to optimise based on the actual init
>> value at the gimple level, but that seems like a fair trade-off.
>> AIUI this is really a security feature or anti-UB hardening feature
>> (in the sense that users are more likely to see predictable behaviour
>> “in the field” even if the program has UB).
>> 
>> 
>> The question is whether it's in line of peoples expectation that
>> explicitely zero-initialized code behaves differently from
>> implicitely zero-initialized code with respect to optimization
>> and secondary side-effects (late diagnostics, latent bugs, etc.).
>> 
>> Introducing a new concept like .DEFERRED_INIT is much more
>> heavy-weight than an explicit zero initializer.
>> 
>> 
>> What exactly you mean by “heavy-weight”? More difficult to implement or much 
>> more run-time overhead or both? Or something else?
>> 
>> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
>> the current -Wuninitialized analysis untouched and also pass
>> the “uninitialized” info from source code level to “pass_expand”.
> 
> Well, "untouched" is a bit oversimplified.  You do need to handle
> .DEFERRED_INIT as not
> being an initialization which will definitely get interesting.

Yes, during uninitialized variable analysis pass, we should specially handle 
the defs with “.DEFERRED_INIT”, to treat them as uninitializations.

>> If we want to keep the current -Wuninitialized analysis untouched, this is a 
>> quite reasonable approach.
>> 
>> However, if it’s not required to keep the current -Wuninitialized analysis 
>> untouched, adding zero-initializer directly during gimplification should
>> be much easier and simpler, and also smaller run-time overhead.
>> 
>> 
>> As for optimization I fear you'll get a load of redundant zero-init
>> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>> 
>> 
>> Runtime overhead for -fauto-init=zero is one important consideration for the 
>> whole feature, we should minimize the runtime overhead for zero
>> Initialization since it will be used in production build.
>> We can do some run-time performance evaluation when we have an 
>> implementation ready.
> 
> Note there will be other passes "confused" by .DEFERRED_INIT.  Note
> that there's going to be other
> considerations - namely where to emit the .DEFERRED_INIT - when
> emitting it during gimplification
> you can emit it at the start of the block of block-scope variables.
> When emitting after gimplification
> you have to emit at function start which will probably make stack slot
> sharing inefficient because
> the deferred init will cause overlapping lifetimes.  With 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-06 Thread Richard Biener via Gcc-patches
On Fri, Dec 4, 2020 at 5:19 PM Qing Zhao  wrote:
>
>
>
> On Dec 4, 2020, at 2:50 AM, Richard Biener  wrote:
>
> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
>  wrote:
>
>
> Richard Biener via Gcc-patches  writes:
>
> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>
> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
>  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>  unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
>  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.
>
>
> For locals it would be more reliable to set this flag during gimplification.
>
> Do you have any comment and suggestions?
>
>
> As said above - do you want to cover registers as well as locals?  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.
>
>
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
>
>  X1 = .DEFERRED_INIT (X0, INIT)
>
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
>
>  X = .DEFERRED_INIT (X, INIT)
>
> and for an SSA name we'd have:
>
>  X_2 = .DEFERRED_INIT (X_1(D), INIT)
>
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>
> * Having the X0 argument would keep the uninitialised use of the
>  variable around for the later warning passes.
>
> * Using a const function should still allow the UB to be deleted as dead
>  if X1 isn't needed.
>
> * Having a function in the way should stop passes from taking advantage
>  of direct uninitialised uses for optimisation.
>
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.
> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).
>
>
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).
>
> Introducing a new concept like .DEFERRED_INIT is much more
> heavy-weight than an explicit zero initializer.
>
>
> What exactly you mean by “heavy-weight”? More difficult to implement or much 
> more run-time overhead or both? Or something else?
>
> The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep 
> the current -Wuninitialized analysis untouched and also pass
> the “uninitialized” info from source code level to “pass_expand”.

Well, "untouched" is a bit oversimplified.  You do need to handle
.DEFERRED_INIT as not
being an initialization which will definitely get interesting.

> If we want to keep the current -Wuninitialized analysis untouched, this is a 
> quite reasonable approach.
>
> However, if it’s not required to keep the current -Wuninitialized analysis 
> untouched, adding zero-initializer directly during gimplification should
> be much easier and simpler, and also smaller run-time overhead.
>
>
> As for optimization I fear you'll get a load of redundant zero-init
> actually emitted if you can just rely on RTL DSE/DCE to remove it.
>
>
> Runtime overhead for -fauto-init=zero is one important consideration for the 
> whole feature, we should minimize the runtime overhead for zero
> Initialization since it will be used in production build.
> We can do some run-time performance evaluation when we have an implementation 
> ready.

Note there will be other passes "confused" by .DEFERRED_INIT.  Note
that there's going to be other
considerations - namely where to emit the .DEFERRED_INIT - when
emitting it during gimplification
you can emit it at the start of the block of block-scope variables.
When emitting after gimplification
you have to emit at function start which will probably make stack slot
sharing inefficient because
the deferred init will cause overlapping lifetimes.  With emitting at
block boundary the .DEFERRED_INIT
will act as code-motion barrier (and it itself likely cannot be moved)
so for example invariant motion
will no longer happen.  Likewise optimizations like SRA will be
confused by .DEFERRED_INIT which
again will lead to bigger stack usage (and less optimization).

But sure, you can try implement a few variants but definitely
.DEFERRED_INIT will be the most
work.


Re: How to traverse all the local variables that declared in the current routine?

2020-12-04 Thread Qing Zhao via Gcc-patches



> On Dec 4, 2020, at 2:50 AM, Richard Biener  wrote:
> 
> On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
> mailto:richard.sandif...@arm.com>> wrote:
>> 
>> Richard Biener via Gcc-patches  writes:
>>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
 Another issue is, in order to check whether an auto-variable has 
 initializer, I plan to add a new bit in “decl_common” as:
  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
  unsigned decl_is_initialized :1;
 
 /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
 #define DECL_IS_INITIALIZED(NODE) \
  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
 
 set this bit when setting DECL_INITIAL for the variables in FE. then keep 
 it
 even though DECL_INITIAL might be NULLed.
>>> 
>>> For locals it would be more reliable to set this flag during gimplification.
>>> 
 Do you have any comment and suggestions?
>>> 
>>> As said above - do you want to cover registers as well as locals?  I'd do
>>> the actual zeroing during RTL expansion instead since otherwise you
>>> have to figure youself whether a local is actually used (see 
>>> expand_stack_vars)
>>> 
>>> Note that optimization will already made have use of "uninitialized" state
>>> of locals so depending on what the actual goal is here "late" may be too 
>>> late.
>> 
>> Haven't thought about this much, so it might be a daft idea, but would a
>> compromise be to use a const internal function:
>> 
>>  X1 = .DEFERRED_INIT (X0, INIT)
>> 
>> where the X0 argument is an uninitialised value and the INIT argument
>> describes the initialisation pattern?  So for a decl we'd have:
>> 
>>  X = .DEFERRED_INIT (X, INIT)
>> 
>> and for an SSA name we'd have:
>> 
>>  X_2 = .DEFERRED_INIT (X_1(D), INIT)
>> 
>> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>> 
>> * Having the X0 argument would keep the uninitialised use of the
>>  variable around for the later warning passes.
>> 
>> * Using a const function should still allow the UB to be deleted as dead
>>  if X1 isn't needed.
>> 
>> * Having a function in the way should stop passes from taking advantage
>>  of direct uninitialised uses for optimisation.
>> 
>> This means we won't be able to optimise based on the actual init
>> value at the gimple level, but that seems like a fair trade-off.
>> AIUI this is really a security feature or anti-UB hardening feature
>> (in the sense that users are more likely to see predictable behaviour
>> “in the field” even if the program has UB).
> 
> The question is whether it's in line of peoples expectation that
> explicitely zero-initialized code behaves differently from
> implicitely zero-initialized code with respect to optimization
> and secondary side-effects (late diagnostics, latent bugs, etc.).
> 
> Introducing a new concept like .DEFERRED_INIT is much more
> heavy-weight than an explicit zero initializer.

What exactly you mean by “heavy-weight”? More difficult to implement or much 
more run-time overhead or both? Or something else?

The major benefit of the approach of “.DEFERRED_INIT”  is to enable us keep the 
current -Wuninitialized analysis untouched and also pass
the “uninitialized” info from source code level to “pass_expand”. 

If we want to keep the current -Wuninitialized analysis untouched, this is a 
quite reasonable approach. 

However, if it’s not required to keep the current -Wuninitialized analysis 
untouched, adding zero-initializer directly during gimplification should
be much easier and simpler, and also smaller run-time overhead.

> 
> As for optimization I fear you'll get a load of redundant zero-init
> actually emitted if you can just rely on RTL DSE/DCE to remove it.

Runtime overhead for -fauto-init=zero is one important consideration for the 
whole feature, we should minimize the runtime overhead for zero
Initialization since it will be used in production build. 
We can do some run-time performance evaluation when we have an implementation 
ready. 

> 
> Btw, I don't think theres any reason to cling onto clangs semantics
> for a particular switch.  We'll never be able to emulate 1:1 behavior
> and our -Wuninit behavior is probably wastly different already.

From my study so far, yes, the currently behavior of -Wunit for Clang and GCC 
is not exactly the same. 

For example, for the following small testing case:
void blah(int);

int foo_2 (int n, int l, int m, int r)
{
  int v;

  if ( (n > 10) && (m != 100)  && (r < 20) )
v = r;

  if (l > 100)
if ( (n <= 8) &&  (m < 102)  && (r < 19) )
  blah(v); /* { dg-warning "uninitialized" "real warning" } */

  return 0;
}

GCC is able to report maybe uninitialized warning, but Clang cannot. 
Looks like that GCC’s uninitialized analysis relies on more analysis and 
optimization information than CLANG. 

Really curious on how clang implement its uninitialized analysis?

Qing



> 
> Richard.
> 
>> Thanks,
>> Richard



Re: How to traverse all the local variables that declared in the current routine?

2020-12-04 Thread Richard Biener via Gcc-patches
On Thu, Dec 3, 2020 at 6:33 PM Richard Sandiford
 wrote:
>
> Richard Biener via Gcc-patches  writes:
> > On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
> >> Another issue is, in order to check whether an auto-variable has 
> >> initializer, I plan to add a new bit in “decl_common” as:
> >>   /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
> >>   unsigned decl_is_initialized :1;
> >>
> >> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> >> #define DECL_IS_INITIALIZED(NODE) \
> >>   (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
> >>
> >> set this bit when setting DECL_INITIAL for the variables in FE. then keep 
> >> it
> >> even though DECL_INITIAL might be NULLed.
> >
> > For locals it would be more reliable to set this flag during gimplification.
> >
> >> Do you have any comment and suggestions?
> >
> > As said above - do you want to cover registers as well as locals?  I'd do
> > the actual zeroing during RTL expansion instead since otherwise you
> > have to figure youself whether a local is actually used (see 
> > expand_stack_vars)
> >
> > Note that optimization will already made have use of "uninitialized" state
> > of locals so depending on what the actual goal is here "late" may be too 
> > late.
>
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
>
>   X1 = .DEFERRED_INIT (X0, INIT)
>
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
>
>   X = .DEFERRED_INIT (X, INIT)
>
> and for an SSA name we'd have:
>
>   X_2 = .DEFERRED_INIT (X_1(D), INIT)
>
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
>
> * Having the X0 argument would keep the uninitialised use of the
>   variable around for the later warning passes.
>
> * Using a const function should still allow the UB to be deleted as dead
>   if X1 isn't needed.
>
> * Having a function in the way should stop passes from taking advantage
>   of direct uninitialised uses for optimisation.
>
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.
> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).

The question is whether it's in line of peoples expectation that
explicitely zero-initialized code behaves differently from
implicitely zero-initialized code with respect to optimization
and secondary side-effects (late diagnostics, latent bugs, etc.).

Introducing a new concept like .DEFERRED_INIT is much more
heavy-weight than an explicit zero initializer.

As for optimization I fear you'll get a load of redundant zero-init
actually emitted if you can just rely on RTL DSE/DCE to remove it.

Btw, I don't think theres any reason to cling onto clangs semantics
for a particular switch.  We'll never be able to emulate 1:1 behavior
and our -Wuninit behavior is probably wastly different already.

Richard.

> Thanks,
> Richard


Re: How to traverse all the local variables that declared in the current routine?

2020-12-03 Thread Qing Zhao via Gcc-patches
Hi, Richard,

Thanks a lot for your suggestion.

Actually, I like this idea. 

My understanding of your suggestion is:

1. During gimplification phase:

For each auto-variable that does not have an explicit initializer, insert the 
following initializer for it:

X = DEFERRED_INIT (X, INIT)

In which, DEFERRED_INIT is an internal const function, which can be defined as:

DEF_INTERNAL_FN (DEFERRED_INIT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)

It’s two arguments are:

1st argument:   this uninitialized auto-variable;
2nd argument:  initialized pattern (zero | pattern);

2.  During tree to SSA phase:  

No change, the current tree to SSA phase should automatically change the above 
new inserted statement as

X_2 = DEFERRED_INIT (X_1(D), INIT);
And all other uses of X-1(D) being replaced by X_2. 

3. During expanding phase:

Expand each call to “DEFERRED_INIT (X, INIT)” to zero or pattern depends on 
“INIT”. 

Is the above understanding correct? Do I miss anything? 

More comments and questions are embedded below:


> On Dec 3, 2020, at 11:32 AM, Richard Sandiford  
> wrote:
> 
> Richard Biener via Gcc-patches  writes:
>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>>> Another issue is, in order to check whether an auto-variable has 
>>> initializer, I plan to add a new bit in “decl_common” as:
>>>  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>>>  unsigned decl_is_initialized :1;
>>> 
>>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>>> #define DECL_IS_INITIALIZED(NODE) \
>>>  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>>> 
>>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>>> even though DECL_INITIAL might be NULLed.
>> 
>> For locals it would be more reliable to set this flag during gimplification.
>> 
>>> Do you have any comment and suggestions?
>> 
>> As said above - do you want to cover registers as well as locals?  I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> Note that optimization will already made have use of "uninitialized" state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
> 
> Haven't thought about this much, so it might be a daft idea, but would a
> compromise be to use a const internal function:
> 
>  X1 = .DEFERRED_INIT (X0, INIT)
> 
> where the X0 argument is an uninitialised value and the INIT argument
> describes the initialisation pattern?  So for a decl we'd have:
> 
>  X = .DEFERRED_INIT (X, INIT)
> 
> and for an SSA name we'd have:
> 
>  X_2 = .DEFERRED_INIT (X_1(D), INIT)
> 
> with all other uses of X_1(D) being replaced by X_2.  The idea is that:
> 
> * Having the X0 argument would keep the uninitialised use of the
>  variable around for the later warning passes.
> 
> * Using a const function should still allow the UB to be deleted as dead
>  if X1 isn't needed.

So, current GCC will delete the UB as dead code when X1 is not needed, with
The new option, we should keep this behavior? 

> 
> * Having a function in the way should stop passes from taking advantage
>  of direct uninitialised uses for optimisation.

This will resolve the issue we raised before with directly adding “artificial” 
zero-initializer 
during gimplification. 

However, I am wondering whether the new added const internal functions will 
impact the 
optimization and then change the uninitialized analysis behavior? 
> 
> This means we won't be able to optimise based on the actual init
> value at the gimple level, but that seems like a fair trade-off.

Yes, with this approach: 

At gimple level, we will not be able to optimize on the new added init values;
At RTL level, we will optimize on the new added init values;
RTL optimizations will be able to eliminate any redundancy introduced by this 
new
Initializations to reduce the cost of this options. 



> AIUI this is really a security feature or anti-UB hardening feature
> (in the sense that users are more likely to see predictable behaviour
> “in the field” even if the program has UB).

Yes, this option is for security purpose, and currently have been used in 
productions by Microsoft, 
Apple and google, etc. 

Qing
> 
> Thanks,
> Richard



Re: How to traverse all the local variables that declared in the current routine?

2020-12-03 Thread Richard Sandiford via Gcc-patches
Richard Biener via Gcc-patches  writes:
> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>>   /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>>   unsigned decl_is_initialized :1;
>>
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>>   (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>>
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
>
> For locals it would be more reliable to set this flag during gimplification.
>
>> Do you have any comment and suggestions?
>
> As said above - do you want to cover registers as well as locals?  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.

Haven't thought about this much, so it might be a daft idea, but would a
compromise be to use a const internal function:

  X1 = .DEFERRED_INIT (X0, INIT)

where the X0 argument is an uninitialised value and the INIT argument
describes the initialisation pattern?  So for a decl we'd have:

  X = .DEFERRED_INIT (X, INIT)

and for an SSA name we'd have:

  X_2 = .DEFERRED_INIT (X_1(D), INIT)

with all other uses of X_1(D) being replaced by X_2.  The idea is that:

* Having the X0 argument would keep the uninitialised use of the
  variable around for the later warning passes.

* Using a const function should still allow the UB to be deleted as dead
  if X1 isn't needed.

* Having a function in the way should stop passes from taking advantage
  of direct uninitialised uses for optimisation.

This means we won't be able to optimise based on the actual init
value at the gimple level, but that seems like a fair trade-off.
AIUI this is really a security feature or anti-UB hardening feature
(in the sense that users are more likely to see predictable behaviour
“in the field” even if the program has UB).

Thanks,
Richard


Re: How to traverse all the local variables that declared in the current routine?

2020-12-03 Thread Richard Sandiford via Gcc-patches
Richard Biener via Gcc-patches  writes:
> On December 3, 2020 5:07:28 PM GMT+01:00, Qing Zhao  
> wrote:
>>
>>
>>> On Dec 3, 2020, at 2:45 AM, Richard Biener
>> wrote:
>>> 
>>> On Wed, Dec 2, 2020 at 4:36 PM Qing Zhao >> wrote:
 
 
 
 On Dec 2, 2020, at 2:45 AM, Richard Biener
>> wrote:
 
 On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao 
>>wrote:
 
 
 Hi, Richard,
 
 Could you please comment on the following approach:
 
 Instead of adding the zero-initializer quite late at the pass
>>“pass_expand”, we can add it as early as during gimplification.
 However, we will mark these new added zero-initializers as
>>“artificial”. And passing this “artificial” information to
 “pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”,
>>in these two uninitialized variable analysis passes,
 (i.e., in tree-sea-uninit.c) We will update the checking on
>>“ssa_undefined_value_p”  to consider “artificial” zero-initializers.
 (i.e, if the def_stmt is marked with “artificial”, then it’s a
>>undefined value).
 
 With such approach, we should be able to address all those
>>conflicts.
 
 Do you see any obvious issue with this approach?
 
 
 Yes, DSE will happily elide an explicit zero-init following the
 artificial one leading to false uninit diagnostics.
 
 
 Indeed.  This is a big issue. And other optimizations might also be
>>impacted by the new zero-init, resulting changed behavior
 of uninitialized analysis in the later stage.
>>> 
>>> I don't see how the issue can be resolved, you can't get both, uninit
>>> warnings and no uninitialized memory.
>>> People can compile twice, once without -fzero-init to get uninit
>>> warnings and once with -fzero-init to get
>>> the extra "security".
>>
>>So, for GCC, you think that it’s okay to get rid of the following
>>requirement:
>>
>>C. The implementation needs to keep the current static warning on
>>uninitialized
>>variables untouched in order to avoid "forking the language”.
>>
>>Then, we can add explanation in the user documentation of the new
>>-fzero-init and also 
>>that of the -Wuninitialized to inform users that -fzero-init will
>>change the behavior of -Wuninitialized.
>>In order to get the warnings, -fzero-init should not be added at the
>>same time?
>>
>>With this requirement being eliminated, implementation will be much
>>easier. 
>>
>>We can add the new initialization during simplification phase. Then
>>this new option will work
>>for all languages.  Is this reasonable?
>
> I think that's reasonable indeed. Eventually doing the init after the early 
> uninit pass is possible as well.

Sorry to be awkward, but I kind-of disagree.  IIRC, clang was able to
give uninit warnings while implementing the initialisation as expected,
so I think this is a GCC restriction rather than a fundamental
incompatibility.

I don't think it's reasonable to expect people to read the documentation
of -ffoo for Clang and separately read the documentation of -ffoo for
GCC.  They'll at best read the documentation for one and (rightly)
expect the other compiler to behave in a compatible way.  I'm also not
sure people would build twice in practice.

I remember the issue of forking the language was discussed at length on
the Clang dev list at the time (but I haven't gone back and re-read the
thread, so I'm relying on memory here).  Not forking the language was an
important goal/requirement of the option and I don't think we should
drop it when implementing the option in GCC.

IMO, if we want to define a dialect of C/C++ in which uninitialised uses
are always well defined rather than UB, we should do that as a separate
option.  If we're implementing the Clang options, we should continue
to treat uninitialised uses as UB that triggers the same warnings as
if the option wasn't passed.

So TBH I'd rather not add the option until it can be implemented in a
way that is compatible with Clang.

Thanks,
Richard


Re: How to traverse all the local variables that declared in the current routine?

2020-12-03 Thread Qing Zhao via Gcc-patches



> On Dec 3, 2020, at 10:36 AM, Richard Biener  
> wrote:
> 
> On December 3, 2020 5:07:28 PM GMT+01:00, Qing Zhao  > wrote:
>> 
>> 
 of uninitialized analysis in the later stage.
>>> 
>>> I don't see how the issue can be resolved, you can't get both, uninit
>>> warnings and no uninitialized memory.
>>> People can compile twice, once without -fzero-init to get uninit
>>> warnings and once with -fzero-init to get
>>> the extra "security".
>> 
>> So, for GCC, you think that it’s okay to get rid of the following
>> requirement:
>> 
>> C. The implementation needs to keep the current static warning on
>> uninitialized
>> variables untouched in order to avoid "forking the language”.
>> 
>> Then, we can add explanation in the user documentation of the new
>> -fzero-init and also 
>> that of the -Wuninitialized to inform users that -fzero-init will
>> change the behavior of -Wuninitialized.
>> In order to get the warnings, -fzero-init should not be added at the
>> same time?
>> 
>> With this requirement being eliminated, implementation will be much
>> easier. 
>> 
>> We can add the new initialization during simplification phase. Then
>> this new option will work
>> for all languages.  Is this reasonable?
> 
> I think that's reasonable indeed. Eventually doing the init after the early 
> uninit pass is possible as well.

You suggested to put the new pass after the early uninit pass? Why?

Qing
> 
> Richard. 
> 
>> thanks.
>> 
>> Qing
>> 
>> 
>> 
>>> 
>>> 



Re: How to traverse all the local variables that declared in the current routine?

2020-12-03 Thread Richard Biener via Gcc-patches
On December 3, 2020 5:07:28 PM GMT+01:00, Qing Zhao  
wrote:
>
>
>> On Dec 3, 2020, at 2:45 AM, Richard Biener
> wrote:
>> 
>> On Wed, Dec 2, 2020 at 4:36 PM Qing Zhao > wrote:
>>> 
>>> 
>>> 
>>> On Dec 2, 2020, at 2:45 AM, Richard Biener
> wrote:
>>> 
>>> On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao 
>wrote:
>>> 
>>> 
>>> Hi, Richard,
>>> 
>>> Could you please comment on the following approach:
>>> 
>>> Instead of adding the zero-initializer quite late at the pass
>“pass_expand”, we can add it as early as during gimplification.
>>> However, we will mark these new added zero-initializers as
>“artificial”. And passing this “artificial” information to
>>> “pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”,
>in these two uninitialized variable analysis passes,
>>> (i.e., in tree-sea-uninit.c) We will update the checking on
>“ssa_undefined_value_p”  to consider “artificial” zero-initializers.
>>> (i.e, if the def_stmt is marked with “artificial”, then it’s a
>undefined value).
>>> 
>>> With such approach, we should be able to address all those
>conflicts.
>>> 
>>> Do you see any obvious issue with this approach?
>>> 
>>> 
>>> Yes, DSE will happily elide an explicit zero-init following the
>>> artificial one leading to false uninit diagnostics.
>>> 
>>> 
>>> Indeed.  This is a big issue. And other optimizations might also be
>impacted by the new zero-init, resulting changed behavior
>>> of uninitialized analysis in the later stage.
>> 
>> I don't see how the issue can be resolved, you can't get both, uninit
>> warnings and no uninitialized memory.
>> People can compile twice, once without -fzero-init to get uninit
>> warnings and once with -fzero-init to get
>> the extra "security".
>
>So, for GCC, you think that it’s okay to get rid of the following
>requirement:
>
>C. The implementation needs to keep the current static warning on
>uninitialized
>variables untouched in order to avoid "forking the language”.
>
>Then, we can add explanation in the user documentation of the new
>-fzero-init and also 
>that of the -Wuninitialized to inform users that -fzero-init will
>change the behavior of -Wuninitialized.
>In order to get the warnings, -fzero-init should not be added at the
>same time?
>
>With this requirement being eliminated, implementation will be much
>easier. 
>
>We can add the new initialization during simplification phase. Then
>this new option will work
>for all languages.  Is this reasonable?

I think that's reasonable indeed. Eventually doing the init after the early 
uninit pass is possible as well.

Richard. 

>thanks.
>
>Qing
>
>
>
>> 
>> Richard.
>> 
>>> 
>>> What's the intended purpose of the zero-init?
>>> 
>>> 
>>> 
>>> The purpose of this new option is: (from the original LLVM patch
>submission):
>>> 
>>> "Add an option to initialize automatic variables with either a
>pattern or with
>>> zeroes. The default is still that automatic variables are
>uninitialized. Also
>>> add attributes to request uninitialized on a per-variable basis,
>mainly to disable
>>> initialization of large stack arrays when deemed too expensive.
>>> 
>>> This isn't meant to change the semantics of C and C++. Rather, it's
>meant to be
>>> a last-resort when programmers inadvertently have some undefined
>behavior in
>>> their code. This patch aims to make undefined behavior hurt less,
>which
>>> security-minded people will be very happy about. Notably, this means
>that
>>> there's no inadvertent information leak when:
>>> 
>>> • The compiler re-uses stack slots, and a value is used
>uninitialized.
>>> • The compiler re-uses a register, and a value is used
>uninitialized.
>>> • Stack structs / arrays / unions with padding are copied.
>>> This patch only addresses stack and register information leaks.
>There's many
>>> more infoleaks that we could address, and much more undefined
>behavior that
>>> could be tamed. Let's keep this patch focused, and I'm happy to
>address related
>>> issues elsewhere."
>>> 
>>> For more details, please refer to the LLVM code review discussion on
>this patch:
>>> https://reviews.llvm.org/D54604
>>> 
>>> 
>>> I also wrote a simple writeup for this task based on my study and
>discussion with
>>> Kees Cook (cc’ing him) as following:
>>> 
>>> 
>>> thanks.
>>> 
>>> Qing
>>> 
>>> Support stack variables auto-initialization in GCC
>>> 
>>> 11/19/2020
>>> 
>>> Qing Zhao
>>> 
>>> ===
>>> 
>>> 
>>> ** Background of the task:
>>> 
>>> The correponding GCC bugzilla RFE was created on 9/3/2018:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210
>>> 
>>> A similar option for LLVM (around Nov, 2018)
>>> https://lists.llvm.org/pipermail/cfe-dev/2018-November/060172.html
>>> had invoked a lot of discussion before committed.
>>> 
>>> (The following are quoted from the comments of Alexander Potapenko
>in
>>> GCC bug 87210):
>>> 
>>> Finally, on Oct, 2019, upstream Clang supports force initialization
>>> of stack 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-03 Thread Qing Zhao via Gcc-patches



> On Dec 3, 2020, at 2:45 AM, Richard Biener  wrote:
> 
> On Wed, Dec 2, 2020 at 4:36 PM Qing Zhao  > wrote:
>> 
>> 
>> 
>> On Dec 2, 2020, at 2:45 AM, Richard Biener  
>> wrote:
>> 
>> On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao  wrote:
>> 
>> 
>> Hi, Richard,
>> 
>> Could you please comment on the following approach:
>> 
>> Instead of adding the zero-initializer quite late at the pass “pass_expand”, 
>> we can add it as early as during gimplification.
>> However, we will mark these new added zero-initializers as “artificial”. And 
>> passing this “artificial” information to
>> “pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”, in these 
>> two uninitialized variable analysis passes,
>> (i.e., in tree-sea-uninit.c) We will update the checking on 
>> “ssa_undefined_value_p”  to consider “artificial” zero-initializers.
>> (i.e, if the def_stmt is marked with “artificial”, then it’s a undefined 
>> value).
>> 
>> With such approach, we should be able to address all those conflicts.
>> 
>> Do you see any obvious issue with this approach?
>> 
>> 
>> Yes, DSE will happily elide an explicit zero-init following the
>> artificial one leading to false uninit diagnostics.
>> 
>> 
>> Indeed.  This is a big issue. And other optimizations might also be impacted 
>> by the new zero-init, resulting changed behavior
>> of uninitialized analysis in the later stage.
> 
> I don't see how the issue can be resolved, you can't get both, uninit
> warnings and no uninitialized memory.
> People can compile twice, once without -fzero-init to get uninit
> warnings and once with -fzero-init to get
> the extra "security".

So, for GCC, you think that it’s okay to get rid of the following requirement:

C. The implementation needs to keep the current static warning on uninitialized
variables untouched in order to avoid "forking the language”.

Then, we can add explanation in the user documentation of the new -fzero-init 
and also 
that of the -Wuninitialized to inform users that -fzero-init will change the 
behavior of -Wuninitialized.
In order to get the warnings, -fzero-init should not be added at the same time?

With this requirement being eliminated, implementation will be much easier. 

We can add the new initialization during simplification phase. Then this new 
option will work
for all languages.  Is this reasonable?

thanks.

Qing



> 
> Richard.
> 
>> 
>> What's the intended purpose of the zero-init?
>> 
>> 
>> 
>> The purpose of this new option is: (from the original LLVM patch submission):
>> 
>> "Add an option to initialize automatic variables with either a pattern or 
>> with
>> zeroes. The default is still that automatic variables are uninitialized. Also
>> add attributes to request uninitialized on a per-variable basis, mainly to 
>> disable
>> initialization of large stack arrays when deemed too expensive.
>> 
>> This isn't meant to change the semantics of C and C++. Rather, it's meant to 
>> be
>> a last-resort when programmers inadvertently have some undefined behavior in
>> their code. This patch aims to make undefined behavior hurt less, which
>> security-minded people will be very happy about. Notably, this means that
>> there's no inadvertent information leak when:
>> 
>> • The compiler re-uses stack slots, and a value is used uninitialized.
>> • The compiler re-uses a register, and a value is used uninitialized.
>> • Stack structs / arrays / unions with padding are copied.
>> This patch only addresses stack and register information leaks. There's many
>> more infoleaks that we could address, and much more undefined behavior that
>> could be tamed. Let's keep this patch focused, and I'm happy to address 
>> related
>> issues elsewhere."
>> 
>> For more details, please refer to the LLVM code review discussion on this 
>> patch:
>> https://reviews.llvm.org/D54604
>> 
>> 
>> I also wrote a simple writeup for this task based on my study and discussion 
>> with
>> Kees Cook (cc’ing him) as following:
>> 
>> 
>> thanks.
>> 
>> Qing
>> 
>> Support stack variables auto-initialization in GCC
>> 
>> 11/19/2020
>> 
>> Qing Zhao
>> 
>> ===
>> 
>> 
>> ** Background of the task:
>> 
>> The correponding GCC bugzilla RFE was created on 9/3/2018:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210
>> 
>> A similar option for LLVM (around Nov, 2018)
>> https://lists.llvm.org/pipermail/cfe-dev/2018-November/060172.html
>> had invoked a lot of discussion before committed.
>> 
>> (The following are quoted from the comments of Alexander Potapenko in
>> GCC bug 87210):
>> 
>> Finally, on Oct, 2019, upstream Clang supports force initialization
>> of stack variables under the -ftrivial-auto-var-init flag.
>> 
>> -ftrivial-auto-var-init=pattern initializes local variables with a 0xAA 
>> pattern
>> (actually it's more complicated, see https://reviews.llvm.org/D54604)
>> 
>> -ftrivial-auto-var-init=zero provides zero-initialization of 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-03 Thread Richard Biener via Gcc-patches
On Wed, Dec 2, 2020 at 4:36 PM Qing Zhao  wrote:
>
>
>
> On Dec 2, 2020, at 2:45 AM, Richard Biener  wrote:
>
> On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao  wrote:
>
>
> Hi, Richard,
>
> Could you please comment on the following approach:
>
> Instead of adding the zero-initializer quite late at the pass “pass_expand”, 
> we can add it as early as during gimplification.
> However, we will mark these new added zero-initializers as “artificial”. And 
> passing this “artificial” information to
> “pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”, in these 
> two uninitialized variable analysis passes,
> (i.e., in tree-sea-uninit.c) We will update the checking on 
> “ssa_undefined_value_p”  to consider “artificial” zero-initializers.
> (i.e, if the def_stmt is marked with “artificial”, then it’s a undefined 
> value).
>
> With such approach, we should be able to address all those conflicts.
>
> Do you see any obvious issue with this approach?
>
>
> Yes, DSE will happily elide an explicit zero-init following the
> artificial one leading to false uninit diagnostics.
>
>
> Indeed.  This is a big issue. And other optimizations might also be impacted 
> by the new zero-init, resulting changed behavior
> of uninitialized analysis in the later stage.

I don't see how the issue can be resolved, you can't get both, uninit
warnings and no uninitialized memory.
People can compile twice, once without -fzero-init to get uninit
warnings and once with -fzero-init to get
the extra "security".

Richard.

>
> What's the intended purpose of the zero-init?
>
>
>
> The purpose of this new option is: (from the original LLVM patch submission):
>
> "Add an option to initialize automatic variables with either a pattern or with
> zeroes. The default is still that automatic variables are uninitialized. Also
> add attributes to request uninitialized on a per-variable basis, mainly to 
> disable
> initialization of large stack arrays when deemed too expensive.
>
> This isn't meant to change the semantics of C and C++. Rather, it's meant to 
> be
> a last-resort when programmers inadvertently have some undefined behavior in
> their code. This patch aims to make undefined behavior hurt less, which
> security-minded people will be very happy about. Notably, this means that
> there's no inadvertent information leak when:
>
> • The compiler re-uses stack slots, and a value is used uninitialized.
> • The compiler re-uses a register, and a value is used uninitialized.
> • Stack structs / arrays / unions with padding are copied.
> This patch only addresses stack and register information leaks. There's many
> more infoleaks that we could address, and much more undefined behavior that
> could be tamed. Let's keep this patch focused, and I'm happy to address 
> related
> issues elsewhere."
>
> For more details, please refer to the LLVM code review discussion on this 
> patch:
> https://reviews.llvm.org/D54604
>
>
> I also wrote a simple writeup for this task based on my study and discussion 
> with
> Kees Cook (cc’ing him) as following:
>
>
> thanks.
>
> Qing
>
> Support stack variables auto-initialization in GCC
>
> 11/19/2020
>
> Qing Zhao
>
> ===
>
>
> ** Background of the task:
>
> The correponding GCC bugzilla RFE was created on 9/3/2018:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210
>
> A similar option for LLVM (around Nov, 2018)
> https://lists.llvm.org/pipermail/cfe-dev/2018-November/060172.html
> had invoked a lot of discussion before committed.
>
> (The following are quoted from the comments of Alexander Potapenko in
> GCC bug 87210):
>
> Finally, on Oct, 2019, upstream Clang supports force initialization
> of stack variables under the -ftrivial-auto-var-init flag.
>
> -ftrivial-auto-var-init=pattern initializes local variables with a 0xAA 
> pattern
> (actually it's more complicated, see https://reviews.llvm.org/D54604)
>
> -ftrivial-auto-var-init=zero provides zero-initialization of locals.
> This mode isn't officially supported yet and is hidden behind an additional
> -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang flag.
> This is done to avoid creating a C++ dialect where all variables are
> zero-initialized.
>
> Starting v5.2, Linux kernel has a CONFIG_INIT_STACK_ALL config that performs
> the build  with -ftrivial-auto-var-init=pattern. This one isn't widely adopted
> yet, partially because initializing locals with 0xAA isn't fast enough.
>
> Linus Torvalds is quite positive about zero-initializing the locals though,
> see https://lkml.org/lkml/2019/7/30/1303:
>
> "when a compiler has an option to initialize stack variables, it
> would probably _also_ be a very good idea for that compiler to then
> support a variable attribute that says "don't initialize _this_
> variable, I will do that manually".
> I also think that the "initialize with poison" is
> pointless and wrong. Yes, it can find bugs, but it doesn't really help
> improve the 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-02 Thread Qing Zhao via Gcc-patches



> On Dec 2, 2020, at 2:45 AM, Richard Biener  wrote:
> 
> On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao  wrote:
>> 
>> Hi, Richard,
>> 
>> Could you please comment on the following approach:
>> 
>> Instead of adding the zero-initializer quite late at the pass “pass_expand”, 
>> we can add it as early as during gimplification.
>> However, we will mark these new added zero-initializers as “artificial”. And 
>> passing this “artificial” information to
>> “pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”, in these 
>> two uninitialized variable analysis passes,
>> (i.e., in tree-sea-uninit.c) We will update the checking on 
>> “ssa_undefined_value_p”  to consider “artificial” zero-initializers.
>> (i.e, if the def_stmt is marked with “artificial”, then it’s a undefined 
>> value).
>> 
>> With such approach, we should be able to address all those conflicts.
>> 
>> Do you see any obvious issue with this approach?
> 
> Yes, DSE will happily elide an explicit zero-init following the
> artificial one leading to false uninit diagnostics.

Indeed.  This is a big issue. And other optimizations might also be impacted by 
the new zero-init, resulting changed behavior
of uninitialized analysis in the later stage.

> 
> What's the intended purpose of the zero-init?


The purpose of this new option is: (from the original LLVM patch submission):

"Add an option to initialize automatic variables with either a pattern or with
zeroes. The default is still that automatic variables are uninitialized. Also
add attributes to request uninitialized on a per-variable basis, mainly to 
disable
initialization of large stack arrays when deemed too expensive.

This isn't meant to change the semantics of C and C++. Rather, it's meant to be
a last-resort when programmers inadvertently have some undefined behavior in
their code. This patch aims to make undefined behavior hurt less, which
security-minded people will be very happy about. Notably, this means that
there's no inadvertent information leak when:

• The compiler re-uses stack slots, and a value is used uninitialized.
• The compiler re-uses a register, and a value is used uninitialized.
• Stack structs / arrays / unions with padding are copied.
This patch only addresses stack and register information leaks. There's many
more infoleaks that we could address, and much more undefined behavior that
could be tamed. Let's keep this patch focused, and I'm happy to address related
issues elsewhere."

For more details, please refer to the LLVM code review discussion on this patch:
https://reviews.llvm.org/D54604


I also wrote a simple writeup for this task based on my study and discussion 
with
Kees Cook (cc’ing him) as following:


thanks.

Qing

Support stack variables auto-initialization in GCC

11/19/2020

Qing Zhao

===


** Background of the task:

The correponding GCC bugzilla RFE was created on 9/3/2018:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210

A similar option for LLVM (around Nov, 2018)
https://lists.llvm.org/pipermail/cfe-dev/2018-November/060172.html
had invoked a lot of discussion before committed.

(The following are quoted from the comments of Alexander Potapenko in
GCC bug 87210):

Finally, on Oct, 2019, upstream Clang supports force initialization
of stack variables under the -ftrivial-auto-var-init flag.

-ftrivial-auto-var-init=pattern initializes local variables with a 0xAA pattern
(actually it's more complicated, see https://reviews.llvm.org/D54604)

-ftrivial-auto-var-init=zero provides zero-initialization of locals.
This mode isn't officially supported yet and is hidden behind an additional
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang flag.
This is done to avoid creating a C++ dialect where all variables are
zero-initialized.

Starting v5.2, Linux kernel has a CONFIG_INIT_STACK_ALL config that performs
the build  with -ftrivial-auto-var-init=pattern. This one isn't widely adopted
yet, partially because initializing locals with 0xAA isn't fast enough.

Linus Torvalds is quite positive about zero-initializing the locals though,
see https://lkml.org/lkml/2019/7/30/1303:

"when a compiler has an option to initialize stack variables, it
would probably _also_ be a very good idea for that compiler to then
support a variable attribute that says "don't initialize _this_
variable, I will do that manually".
I also think that the "initialize with poison" is
pointless and wrong. Yes, it can find bugs, but it doesn't really help
improve the general situation, and people see it as a debugging tool,
not a "improve code quality and improve the life of kernel developers"
tool.

So having a flag similar to -ftrivial-auto-var-init=zero in GCC will be
appreciated by the Linux kernel community.

currently, kernel is using a gcc plugin to support stack variables
auto-initialization:

Re: How to traverse all the local variables that declared in the current routine?

2020-12-02 Thread Richard Biener via Gcc-patches
On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao  wrote:
>
> Hi, Richard,
>
> Could you please comment on the following approach:
>
> Instead of adding the zero-initializer quite late at the pass “pass_expand”, 
> we can add it as early as during gimplification.
> However, we will mark these new added zero-initializers as “artificial”. And 
> passing this “artificial” information to
> “pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”, in these 
> two uninitialized variable analysis passes,
> (i.e., in tree-sea-uninit.c) We will update the checking on 
> “ssa_undefined_value_p”  to consider “artificial” zero-initializers.
> (i.e, if the def_stmt is marked with “artificial”, then it’s a undefined 
> value).
>
> With such approach, we should be able to address all those conflicts.
>
> Do you see any obvious issue with this approach?

Yes, DSE will happily elide an explicit zero-init following the
artificial one leading to false uninit diagnostics.

What's the intended purpose of the zero-init?

Richard.

> Thanks a lot for your help.
>
> Qing
>
>
> On Nov 25, 2020, at 3:11 AM, Richard Biener  
> wrote:
>
>
>
> I am planing to add a new phase immediately after 
> “pass_late_warn_uninitialized” to initialize all auto-variables that are
> not explicitly initialized in the declaration, the basic idea is following:
>
> ** The proposal:
>
> A. add a new GCC option: (same name and meaning as CLANG)
> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>
> B. add a new attribute for variable:
> __attribute((uninitialized)
> the marked variable is uninitialized intentionaly for performance purpose.
>
> C. The implementation needs to keep the current static warning on 
> uninitialized
> variables untouched in order to avoid "forking the language".
>
>
> ** The implementation:
>
> There are two major requirements for the implementation:
>
> 1. all auto-variables that do not have an explicit initializer should be 
> initialized to
> zero by this option.  (Same behavior as CLANG)
>
> 2. keep the current static warning on uninitialized variables untouched.
>
> In order to satisfy 1, we should check whether an auto-variable has 
> initializer
> or not;
> In order to satisfy 2, we should add this new transformation after
> "pass_late_warn_uninitialized".
>
> So, we should be able to check whether an auto-variable has initializer or 
> not after “pass_late_warn_uninitialized”,
> If Not, then insert an initialization for it.
>
> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
>
>
> I think both as long as they are source-level auto-variables. Then which one 
> is better?
>
>
> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
> unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.
>
>
> For locals it would be more reliable to set this flag-Wmaybe-uninitialized.
>
>
>
> You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
> “gimpley_decl_expr” (gimplify.c) as following:
>
>  if (VAR_P (decl) && !DECL_EXTERNAL (decl))
>{
>  tree init = DECL_INITIAL (decl);
> ...
>  if (init && init != error_mark_node)
>{
>  if (!TREE_STATIC (decl))
>{
>  DECL_IS_INITIALIZED(decl) = 1;
>}
>
> Is this enough for all Frontends? Are there other places that I need to 
> maintain this bit?
>
>
>
> Do you have any comment and suggestions?
>
>
> As said above - do you want to cover registers as well as locals?
>
>
> All the locals from the source-code point of view should be covered.   (From 
> my study so far,  looks like that Clang adds that phase in FE).
> If GCC adds this phase in FE, then the following design requirement
>
> C. The implementation needs to keep the current static warning on 
> uninitialized
> variables untouched in order to avoid "forking the language”.
>
> cannot be satisfied.  Since gcc’s uninitialized variables analysis is applied 
> quite late.
>
> So, we have to add this new phase after “pass_late_warn_uninitialized”.
>
> I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
>
> Adding  this new transformation during RTL expansion is okay.  I will check 
> on this in more details to see how to add it to RTL expansion phase.
>
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.
>
>
> This is a really good point…
>
> In order to avoid optimization  to use the “uninitialized” state of locals, 
> we 

Re: How to traverse all the local variables that declared in the current routine?

2020-12-01 Thread Qing Zhao via Gcc-patches
Hi, Richard, 

Could you please comment on the following approach:

Instead of adding the zero-initializer quite late at the pass “pass_expand”, we 
can add it as early as during gimplification. 
However, we will mark these new added zero-initializers as “artificial”. And 
passing this “artificial” information to 
“pass_early_warn_uninitialized” and “pass_late_warn_uninitialized”, in these 
two uninitialized variable analysis passes, 
(i.e., in tree-sea-uninit.c) We will update the checking on 
“ssa_undefined_value_p”  to consider “artificial” zero-initializers. 
(i.e, if the def_stmt is marked with “artificial”, then it’s a undefined 
value). 

With such approach, we should be able to address all those conflicts. 

Do you see any obvious issue with this approach?

Thanks a lot for your help.

Qing


> On Nov 25, 2020, at 3:11 AM, Richard Biener  
> wrote:
>> 
>> 
>> I am planing to add a new phase immediately after 
>> “pass_late_warn_uninitialized” to initialize all auto-variables that are
>> not explicitly initialized in the declaration, the basic idea is following:
>> 
>> ** The proposal:
>> 
>> A. add a new GCC option: (same name and meaning as CLANG)
>> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>> 
>> B. add a new attribute for variable:
>> __attribute((uninitialized)
>> the marked variable is uninitialized intentionaly for performance purpose.
>> 
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language".
>> 
>> 
>> ** The implementation:
>> 
>> There are two major requirements for the implementation:
>> 
>> 1. all auto-variables that do not have an explicit initializer should be 
>> initialized to
>> zero by this option.  (Same behavior as CLANG)
>> 
>> 2. keep the current static warning on uninitialized variables untouched.
>> 
>> In order to satisfy 1, we should check whether an auto-variable has 
>> initializer
>> or not;
>> In order to satisfy 2, we should add this new transformation after
>> "pass_late_warn_uninitialized".
>> 
>> So, we should be able to check whether an auto-variable has initializer or 
>> not after “pass_late_warn_uninitialized”,
>> If Not, then insert an initialization for it.
>> 
>> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
>> 
>> 
>> I think both as long as they are source-level auto-variables. Then which one 
>> is better?
>> 
>> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
>> 
>> 
>> For locals it would be more reliable to set this flag-Wmaybe-uninitialized.
>> 
>> 
>> You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
>> “gimpley_decl_expr” (gimplify.c) as following:
>> 
>>  if (VAR_P (decl) && !DECL_EXTERNAL (decl))
>>{
>>  tree init = DECL_INITIAL (decl);
>> ...
>>  if (init && init != error_mark_node)
>>{
>>  if (!TREE_STATIC (decl))
>>{
>>  DECL_IS_INITIALIZED(decl) = 1;
>>}
>> 
>> Is this enough for all Frontends? Are there other places that I need to 
>> maintain this bit?
>> 
>> 
>> 
>> Do you have any comment and suggestions?
>> 
>> 
>> As said above - do you want to cover registers as well as locals?
>> 
>> 
>> All the locals from the source-code point of view should be covered.   (From 
>> my study so far,  looks like that Clang adds that phase in FE).
>> If GCC adds this phase in FE, then the following design requirement
>> 
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language”.
>> 
>> cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
>> applied quite late.
>> 
>> So, we have to add this new phase after “pass_late_warn_uninitialized”.
>> 
>> I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> 
>> Adding  this new transformation during RTL expansion is okay.  I will check 
>> on this in more details to see how to add it to RTL expansion phase.
>> 
>> 
>> Note that optimization will already made have use of "uninitialized" state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
>> 
>> 
>> This is a really good point…
>> 
>> In order to avoid optimization  to use the “uninitialized” state of locals, 
>> we should add the zeroing phase as early as possible (adding it in FE might 
>> be best
>> for this 

Re: How to traverse all the local variables that declared in the current routine?

2020-11-30 Thread Qing Zhao via Gcc-patches
On Nov 30, 2020, at 11:18 AM, Martin Sebor  wrote:
 Does gcc provide an iterator to traverse all the local variables that 
 are declared in the current routine?
 
 If not, what’s the best way to traverse the local variables?
>>> 
>>> Depends on what for.  There's the source level view you get by walking
>>> BLOCK_VARS of the
>>> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
>>> there's SSA names
>>> (FOR_EACH_SSA_NAME).
>> 
>> I am planing to add a new phase immediately after 
>> “pass_late_warn_uninitialized” to initialize all auto-variables that are
>> not explicitly initialized in the declaration, the basic idea is 
>> following:
>> 
>> ** The proposal:
>> 
>> A. add a new GCC option: (same name and meaning as CLANG)
>> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>> 
>> B. add a new attribute for variable:
>> __attribute((uninitialized)
>> the marked variable is uninitialized intentionaly for performance 
>> purpose.
>> 
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language".
>> 
>> 
>> ** The implementation:
>> 
>> There are two major requirements for the implementation:
>> 
>> 1. all auto-variables that do not have an explicit initializer should be 
>> initialized to
>> zero by this option.  (Same behavior as CLANG)
>> 
>> 2. keep the current static warning on uninitialized variables untouched.
>> 
>> In order to satisfy 1, we should check whether an auto-variable has 
>> initializer
>> or not;
>> In order to satisfy 2, we should add this new transformation after
>> "pass_late_warn_uninitialized".
>> 
>> So, we should be able to check whether an auto-variable has initializer 
>> or not after “pass_late_warn_uninitialized”,
>> If Not, then insert an initialization for it.
>> 
>> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
> 
> Yes, but do you want to catch variables promoted to register as well
> or just variables
> on the stack?
 I think both as long as they are source-level auto-variables. Then which 
 one is better?
> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>>  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>>  unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>>  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then 
>> keep it
>> even though DECL_INITIAL might be NULLed.
> 
> For locals it would be more reliable to set this flag during 
> gimplification.
 You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the 
 routine “gimpley_decl_expr” (gimplify.c) as following:
   if (VAR_P (decl) && !DECL_EXTERNAL (decl))
 {
   tree init = DECL_INITIAL (decl);
 ...
   if (init && init != error_mark_node)
 {
   if (!TREE_STATIC (decl))
 {
   DECL_IS_INITIALIZED(decl) = 1;
 }
 Is this enough for all Frontends? Are there other places that I need to 
 maintain this bit?
> 
>> Do you have any comment and suggestions?
> 
> As said above - do you want to cover registers as well as locals?
 All the locals from the source-code point of view should be covered.   
 (From my study so far,  looks like that Clang adds that phase in FE).
 If GCC adds this phase in FE, then the following design requirement
 C. The implementation needs to keep the current static warning on 
 uninitialized
 variables untouched in order to avoid "forking the language”.
 cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
 applied quite late.
 So, we have to add this new phase after “pass_late_warn_uninitialized”.
>  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
 Adding  this new transformation during RTL expansion is okay.  I will 
 check on this in more details to see how to add it to RTL expansion phase.
> 
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too 
> late.
 This is a really good point…
 In order to avoid optimization  to use the “uninitialized” state of 
 locals, we should add the zeroing phase as early as possible (adding it in 
 FE might 

Re: How to traverse all the local variables that declared in the current routine?

2020-11-30 Thread Martin Sebor via Gcc-patches

On 11/30/20 9:23 AM, Qing Zhao wrote:

Hi, Martin,

Thanks a lot for your suggestion.

On Nov 25, 2020, at 6:08 PM, Martin Sebor > wrote:


On 11/24/20 9:54 AM, Qing Zhao via Gcc-patches wrote:
On Nov 24, 2020, at 9:55 AM, Richard Biener 
mailto:richard.guent...@gmail.com>> wrote:


On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao > wrote:




On Nov 24, 2020, at 1:32 AM, Richard Biener 
mailto:richard.guent...@gmail.com>> 
wrote:


On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
mailto:gcc-patches@gcc.gnu.org>> wrote:


Hi,

Does gcc provide an iterator to traverse all the local variables 
that are declared in the current routine?


If not, what’s the best way to traverse the local variables?


Depends on what for.  There's the source level view you get by walking
BLOCK_VARS of the
scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
there's SSA names
(FOR_EACH_SSA_NAME).


I am planing to add a new phase immediately after 
“pass_late_warn_uninitialized” to initialize all auto-variables 
that are
not explicitly initialized in the declaration, the basic idea is 
following:


** The proposal:

A. add a new GCC option: (same name and meaning as CLANG)
-ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;

B. add a new attribute for variable:
__attribute((uninitialized)
the marked variable is uninitialized intentionaly for performance 
purpose.


C. The implementation needs to keep the current static warning on 
uninitialized

variables untouched in order to avoid "forking the language".


** The implementation:

There are two major requirements for the implementation:

1. all auto-variables that do not have an explicit initializer 
should be initialized to

zero by this option.  (Same behavior as CLANG)

2. keep the current static warning on uninitialized variables 
untouched.


In order to satisfy 1, we should check whether an auto-variable has 
initializer

or not;
In order to satisfy 2, we should add this new transformation after
"pass_late_warn_uninitialized".

So, we should be able to check whether an auto-variable has 
initializer or not after “pass_late_warn_uninitialized”,

If Not, then insert an initialization for it.

For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?


Yes, but do you want to catch variables promoted to register as well
or just variables
on the stack?
I think both as long as they are source-level auto-variables. Then 
which one is better?


Another issue is, in order to check whether an auto-variable has 
initializer, I plan to add a new bit in “decl_common” as:

 /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
 unsigned decl_is_initialized :1;

/* IN VAR_DECL, set when the decl is initialized at the 
declaration.  */

#define DECL_IS_INITIALIZED(NODE) \
 (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)

set this bit when setting DECL_INITIAL for the variables in FE. 
then keep it

even though DECL_INITIAL might be NULLed.


For locals it would be more reliable to set this flag during 
gimplification.
You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the 
routine “gimpley_decl_expr” (gimplify.c) as following:

  if (VAR_P (decl) && !DECL_EXTERNAL (decl))
    {
      tree init = DECL_INITIAL (decl);
...
      if (init && init != error_mark_node)
        {
          if (!TREE_STATIC (decl))
    {
      DECL_IS_INITIALIZED(decl) = 1;
    }
Is this enough for all Frontends? Are there other places that I need 
to maintain this bit?



Do you have any comment and suggestions?


As said above - do you want to cover registers as well as locals?
All the locals from the source-code point of view should be covered. 
  (From my study so far,  looks like that Clang adds that phase in FE).

If GCC adds this phase in FE, then the following design requirement
C. The implementation needs to keep the current static warning on 
uninitialized

variables untouched in order to avoid "forking the language”.
cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
applied quite late.

So, we have to add this new phase after “pass_late_warn_uninitialized”.

 I'd do
the actual zeroing during RTL expansion instead since otherwise you
have to figure youself whether a local is actually used (see 
expand_stack_vars)
Adding  this new transformation during RTL expansion is okay.  I will 
check on this in more details to see how to add it to RTL expansion 
phase.


Note that optimization will already made have use of "uninitialized" 
state
of locals so depending on what the actual goal is here "late" may be 
too late.

This is a really good point…
In order to avoid optimization  to use the “uninitialized” state of 
locals, we should add the zeroing phase as early as possible (adding 
it in FE might be best

for this issue). However, if we have to met the following requirement:
C. The implementation needs to keep the current static warning on 
uninitialized


Re: How to traverse all the local variables that declared in the current routine?

2020-11-30 Thread Qing Zhao via Gcc-patches
Hi, Martin,

Thanks a lot for your suggestion.

> On Nov 25, 2020, at 6:08 PM, Martin Sebor  wrote:
> 
> On 11/24/20 9:54 AM, Qing Zhao via Gcc-patches wrote:
>>> On Nov 24, 2020, at 9:55 AM, Richard Biener  
>>> wrote:
>>> 
>>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
 
 
 
> On Nov 24, 2020, at 1:32 AM, Richard Biener  
> wrote:
> 
> On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
>  wrote:
>> 
>> Hi,
>> 
>> Does gcc provide an iterator to traverse all the local variables that 
>> are declared in the current routine?
>> 
>> If not, what’s the best way to traverse the local variables?
> 
> Depends on what for.  There's the source level view you get by walking
> BLOCK_VARS of the
> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
> there's SSA names
> (FOR_EACH_SSA_NAME).
 
 I am planing to add a new phase immediately after 
 “pass_late_warn_uninitialized” to initialize all auto-variables that are
 not explicitly initialized in the declaration, the basic idea is following:
 
 ** The proposal:
 
 A. add a new GCC option: (same name and meaning as CLANG)
 -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
 
 B. add a new attribute for variable:
 __attribute((uninitialized)
 the marked variable is uninitialized intentionaly for performance purpose.
 
 C. The implementation needs to keep the current static warning on 
 uninitialized
 variables untouched in order to avoid "forking the language".
 
 
 ** The implementation:
 
 There are two major requirements for the implementation:
 
 1. all auto-variables that do not have an explicit initializer should be 
 initialized to
 zero by this option.  (Same behavior as CLANG)
 
 2. keep the current static warning on uninitialized variables untouched.
 
 In order to satisfy 1, we should check whether an auto-variable has 
 initializer
 or not;
 In order to satisfy 2, we should add this new transformation after
 "pass_late_warn_uninitialized".
 
 So, we should be able to check whether an auto-variable has initializer or 
 not after “pass_late_warn_uninitialized”,
 If Not, then insert an initialization for it.
 
 For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
>>> 
>>> Yes, but do you want to catch variables promoted to register as well
>>> or just variables
>>> on the stack?
>> I think both as long as they are source-level auto-variables. Then which one 
>> is better?
>>> 
 Another issue is, in order to check whether an auto-variable has 
 initializer, I plan to add a new bit in “decl_common” as:
  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
  unsigned decl_is_initialized :1;
 
 /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
 #define DECL_IS_INITIALIZED(NODE) \
  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
 
 set this bit when setting DECL_INITIAL for the variables in FE. then keep 
 it
 even though DECL_INITIAL might be NULLed.
>>> 
>>> For locals it would be more reliable to set this flag during gimplification.
>> You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
>> “gimpley_decl_expr” (gimplify.c) as following:
>>   if (VAR_P (decl) && !DECL_EXTERNAL (decl))
>> {
>>   tree init = DECL_INITIAL (decl);
>> ...
>>   if (init && init != error_mark_node)
>> {
>>   if (!TREE_STATIC (decl))
>>  {
>>DECL_IS_INITIALIZED(decl) = 1;
>>  }
>> Is this enough for all Frontends? Are there other places that I need to 
>> maintain this bit?
>>> 
 Do you have any comment and suggestions?
>>> 
>>> As said above - do you want to cover registers as well as locals?
>> All the locals from the source-code point of view should be covered.   (From 
>> my study so far,  looks like that Clang adds that phase in FE).
>> If GCC adds this phase in FE, then the following design requirement
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language”.
>> cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
>> applied quite late.
>> So, we have to add this new phase after “pass_late_warn_uninitialized”.
>>>  I'd do
>>> the actual zeroing during RTL expansion instead since otherwise you
>>> have to figure youself whether a local is actually used (see 
>>> expand_stack_vars)
>> Adding  this new transformation during RTL expansion is okay.  I will check 
>> on this in more details to see how to add it to RTL expansion phase.
>>> 
>>> Note that optimization will already made have use of "uninitialized" state
>>> of locals so depending on what the actual goal is here "late" may be too 
>>> late.
>> 

Re: How to traverse all the local variables that declared in the current routine?

2020-11-25 Thread Martin Sebor via Gcc-patches

On 11/24/20 9:54 AM, Qing Zhao via Gcc-patches wrote:




On Nov 24, 2020, at 9:55 AM, Richard Biener  wrote:

On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:





On Nov 24, 2020, at 1:32 AM, Richard Biener  wrote:

On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
 wrote:


Hi,

Does gcc provide an iterator to traverse all the local variables that are 
declared in the current routine?

If not, what’s the best way to traverse the local variables?


Depends on what for.  There's the source level view you get by walking
BLOCK_VARS of the
scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
there's SSA names
(FOR_EACH_SSA_NAME).


I am planing to add a new phase immediately after 
“pass_late_warn_uninitialized” to initialize all auto-variables that are
not explicitly initialized in the declaration, the basic idea is following:

** The proposal:

A. add a new GCC option: (same name and meaning as CLANG)
-ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;

B. add a new attribute for variable:
__attribute((uninitialized)
the marked variable is uninitialized intentionaly for performance purpose.

C. The implementation needs to keep the current static warning on uninitialized
variables untouched in order to avoid "forking the language".


** The implementation:

There are two major requirements for the implementation:

1. all auto-variables that do not have an explicit initializer should be 
initialized to
zero by this option.  (Same behavior as CLANG)

2. keep the current static warning on uninitialized variables untouched.

In order to satisfy 1, we should check whether an auto-variable has initializer
or not;
In order to satisfy 2, we should add this new transformation after
"pass_late_warn_uninitialized".

So, we should be able to check whether an auto-variable has initializer or not 
after “pass_late_warn_uninitialized”,
If Not, then insert an initialization for it.

For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?


Yes, but do you want to catch variables promoted to register as well
or just variables
on the stack?


I think both as long as they are source-level auto-variables. Then which one is 
better?




Another issue is, in order to check whether an auto-variable has initializer, I 
plan to add a new bit in “decl_common” as:
  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
  unsigned decl_is_initialized :1;

/* IN VAR_DECL, set when the decl is initialized at the declaration.  */
#define DECL_IS_INITIALIZED(NODE) \
  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)

set this bit when setting DECL_INITIAL for the variables in FE. then keep it
even though DECL_INITIAL might be NULLed.


For locals it would be more reliable to set this flag during gimplification.


You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
“gimpley_decl_expr” (gimplify.c) as following:

   if (VAR_P (decl) && !DECL_EXTERNAL (decl))
 {
   tree init = DECL_INITIAL (decl);
...
   if (init && init != error_mark_node)
 {
   if (!TREE_STATIC (decl))
{
  DECL_IS_INITIALIZED(decl) = 1;
}

Is this enough for all Frontends? Are there other places that I need to 
maintain this bit?





Do you have any comment and suggestions?


As said above - do you want to cover registers as well as locals?


All the locals from the source-code point of view should be covered.   (From my 
study so far,  looks like that Clang adds that phase in FE).
If GCC adds this phase in FE, then the following design requirement

C. The implementation needs to keep the current static warning on uninitialized
variables untouched in order to avoid "forking the language”.

cannot be satisfied.  Since gcc’s uninitialized variables analysis is applied 
quite late.

So, we have to add this new phase after “pass_late_warn_uninitialized”.


  I'd do
the actual zeroing during RTL expansion instead since otherwise you
have to figure youself whether a local is actually used (see expand_stack_vars)


Adding  this new transformation during RTL expansion is okay.  I will check on 
this in more details to see how to add it to RTL expansion phase.


Note that optimization will already made have use of "uninitialized" state
of locals so depending on what the actual goal is here "late" may be too late.


This is a really good point…

In order to avoid optimization  to use the “uninitialized” state of locals, we 
should add the zeroing phase as early as possible (adding it in FE might be best
for this issue). However, if we have to met the following requirement:

C. The implementation needs to keep the current static warning on uninitialized
variables untouched in order to avoid "forking the language”.

We have to move the new phase after all the uninitialized analysis is done in 
order to avoid “forking the language”.

So, this is a problem that is not easy to resolve.

Do you have suggestion on this?


Not having 

Re: How to traverse all the local variables that declared in the current routine?

2020-11-25 Thread Qing Zhao via Gcc-patches



> On Nov 25, 2020, at 3:11 AM, Richard Biener  
> wrote:
>> 
>> 
>> Hi,
>> 
>> Does gcc provide an iterator to traverse all the local variables that are 
>> declared in the current routine?
>> 
>> If not, what’s the best way to traverse the local variables?
>> 
>> 
>> Depends on what for.  There's the source level view you get by walking
>> BLOCK_VARS of the
>> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
>> there's SSA names
>> (FOR_EACH_SSA_NAME).
>> 
>> 
>> I am planing to add a new phase immediately after 
>> “pass_late_warn_uninitialized” to initialize all auto-variables that are
>> not explicitly initialized in the declaration, the basic idea is following:
>> 
>> ** The proposal:
>> 
>> A. add a new GCC option: (same name and meaning as CLANG)
>> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>> 
>> B. add a new attribute for variable:
>> __attribute((uninitialized)
>> the marked variable is uninitialized intentionaly for performance purpose.
>> 
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language".
>> 
>> 
>> ** The implementation:
>> 
>> There are two major requirements for the implementation:
>> 
>> 1. all auto-variables that do not have an explicit initializer should be 
>> initialized to
>> zero by this option.  (Same behavior as CLANG)
>> 
>> 2. keep the current static warning on uninitialized variables untouched.
>> 
>> In order to satisfy 1, we should check whether an auto-variable has 
>> initializer
>> or not;
>> In order to satisfy 2, we should add this new transformation after
>> "pass_late_warn_uninitialized".
>> 
>> So, we should be able to check whether an auto-variable has initializer or 
>> not after “pass_late_warn_uninitialized”,
>> If Not, then insert an initialization for it.
>> 
>> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
>> 
>> 
>> Yes, but do you want to catch variables promoted to register as well
>> or just variables
>> on the stack?
>> 
>> 
>> I think both as long as they are source-level auto-variables. Then which one 
>> is better?
>> 
>> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>> /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>> unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>> (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
>> 
>> 
>> For locals it would be more reliable to set this flag during gimplification.
>> 
>> 
>> You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
>> “gimpley_decl_expr” (gimplify.c) as following:
>> 
>>  if (VAR_P (decl) && !DECL_EXTERNAL (decl))
>>{
>>  tree init = DECL_INITIAL (decl);
>> ...
>>  if (init && init != error_mark_node)
>>{
>>  if (!TREE_STATIC (decl))
>>{
>>  DECL_IS_INITIALIZED(decl) = 1;
>>}
>> 
>> Is this enough for all Frontends? Are there other places that I need to 
>> maintain this bit?
>> 
>> 
>> 
>> Do you have any comment and suggestions?
>> 
>> 
>> As said above - do you want to cover registers as well as locals?
>> 
>> 
>> All the locals from the source-code point of view should be covered.   (From 
>> my study so far,  looks like that Clang adds that phase in FE).
>> If GCC adds this phase in FE, then the following design requirement
>> 
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language”.
>> 
>> cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
>> applied quite late.
>> 
>> So, we have to add this new phase after “pass_late_warn_uninitialized”.
>> 
>> I'd do
>> the actual zeroing during RTL expansion instead since otherwise you
>> have to figure youself whether a local is actually used (see 
>> expand_stack_vars)
>> 
>> 
>> Adding  this new transformation during RTL expansion is okay.  I will check 
>> on this in more details to see how to add it to RTL expansion phase.
>> 
>> 
>> Note that optimization will already made have use of "uninitialized" state
>> of locals so depending on what the actual goal is here "late" may be too 
>> late.
>> 
>> 
>> This is a really good point…
>> 
>> In order to avoid optimization  to use the “uninitialized” state of locals, 
>> we should add the zeroing phase as early as possible (adding it in FE might 
>> be best
>> for this issue). However, if we have to met the following requirement:
> 
> So is optimization supposed to pick up zero or is it supposed to act
> as if the initializer
> is unknown?

Good question!

Theoretically,  the new option -ftrivial-auto-var-init=zero is supposed to add 

Re: How to traverse all the local variables that declared in the current routine?

2020-11-25 Thread Richard Biener via Gcc-patches
On Tue, Nov 24, 2020 at 5:54 PM Qing Zhao  wrote:
>
>
>
> On Nov 24, 2020, at 9:55 AM, Richard Biener  
> wrote:
>
> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>
>
>
>
> On Nov 24, 2020, at 1:32 AM, Richard Biener  
> wrote:
>
> On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
>  wrote:
>
>
> Hi,
>
> Does gcc provide an iterator to traverse all the local variables that are 
> declared in the current routine?
>
> If not, what’s the best way to traverse the local variables?
>
>
> Depends on what for.  There's the source level view you get by walking
> BLOCK_VARS of the
> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
> there's SSA names
> (FOR_EACH_SSA_NAME).
>
>
> I am planing to add a new phase immediately after 
> “pass_late_warn_uninitialized” to initialize all auto-variables that are
> not explicitly initialized in the declaration, the basic idea is following:
>
> ** The proposal:
>
> A. add a new GCC option: (same name and meaning as CLANG)
> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>
> B. add a new attribute for variable:
> __attribute((uninitialized)
> the marked variable is uninitialized intentionaly for performance purpose.
>
> C. The implementation needs to keep the current static warning on 
> uninitialized
> variables untouched in order to avoid "forking the language".
>
>
> ** The implementation:
>
> There are two major requirements for the implementation:
>
> 1. all auto-variables that do not have an explicit initializer should be 
> initialized to
> zero by this option.  (Same behavior as CLANG)
>
> 2. keep the current static warning on uninitialized variables untouched.
>
> In order to satisfy 1, we should check whether an auto-variable has 
> initializer
> or not;
> In order to satisfy 2, we should add this new transformation after
> "pass_late_warn_uninitialized".
>
> So, we should be able to check whether an auto-variable has initializer or 
> not after “pass_late_warn_uninitialized”,
> If Not, then insert an initialization for it.
>
> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
>
>
> Yes, but do you want to catch variables promoted to register as well
> or just variables
> on the stack?
>
>
> I think both as long as they are source-level auto-variables. Then which one 
> is better?
>
>
> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
>  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>  unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
>  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.
>
>
> For locals it would be more reliable to set this flag during gimplification.
>
>
> You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
> “gimpley_decl_expr” (gimplify.c) as following:
>
>   if (VAR_P (decl) && !DECL_EXTERNAL (decl))
> {
>   tree init = DECL_INITIAL (decl);
> ...
>   if (init && init != error_mark_node)
> {
>   if (!TREE_STATIC (decl))
> {
>   DECL_IS_INITIALIZED(decl) = 1;
> }
>
> Is this enough for all Frontends? Are there other places that I need to 
> maintain this bit?
>
>
>
> Do you have any comment and suggestions?
>
>
> As said above - do you want to cover registers as well as locals?
>
>
> All the locals from the source-code point of view should be covered.   (From 
> my study so far,  looks like that Clang adds that phase in FE).
> If GCC adds this phase in FE, then the following design requirement
>
> C. The implementation needs to keep the current static warning on 
> uninitialized
> variables untouched in order to avoid "forking the language”.
>
> cannot be satisfied.  Since gcc’s uninitialized variables analysis is applied 
> quite late.
>
> So, we have to add this new phase after “pass_late_warn_uninitialized”.
>
>  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
>
>
> Adding  this new transformation during RTL expansion is okay.  I will check 
> on this in more details to see how to add it to RTL expansion phase.
>
>
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.
>
>
> This is a really good point…
>
> In order to avoid optimization  to use the “uninitialized” state of locals, 
> we should add the zeroing phase as early as possible (adding it in FE might 
> be best
> for this issue). However, if we have to met the following requirement:

So is optimization supposed to pick up zero or is it supposed to act
as if the initializer
is unknown?

> C. The implementation needs to keep the 

Re: How to traverse all the local variables that declared in the current routine?

2020-11-24 Thread Qing Zhao via Gcc-patches



> On Nov 24, 2020, at 9:55 AM, Richard Biener  
> wrote:
> 
> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>> 
>> 
>> 
>>> On Nov 24, 2020, at 1:32 AM, Richard Biener  
>>> wrote:
>>> 
>>> On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
>>>  wrote:
 
 Hi,
 
 Does gcc provide an iterator to traverse all the local variables that are 
 declared in the current routine?
 
 If not, what’s the best way to traverse the local variables?
>>> 
>>> Depends on what for.  There's the source level view you get by walking
>>> BLOCK_VARS of the
>>> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
>>> there's SSA names
>>> (FOR_EACH_SSA_NAME).
>> 
>> I am planing to add a new phase immediately after 
>> “pass_late_warn_uninitialized” to initialize all auto-variables that are
>> not explicitly initialized in the declaration, the basic idea is following:
>> 
>> ** The proposal:
>> 
>> A. add a new GCC option: (same name and meaning as CLANG)
>> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>> 
>> B. add a new attribute for variable:
>> __attribute((uninitialized)
>> the marked variable is uninitialized intentionaly for performance purpose.
>> 
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language".
>> 
>> 
>> ** The implementation:
>> 
>> There are two major requirements for the implementation:
>> 
>> 1. all auto-variables that do not have an explicit initializer should be 
>> initialized to
>> zero by this option.  (Same behavior as CLANG)
>> 
>> 2. keep the current static warning on uninitialized variables untouched.
>> 
>> In order to satisfy 1, we should check whether an auto-variable has 
>> initializer
>> or not;
>> In order to satisfy 2, we should add this new transformation after
>> "pass_late_warn_uninitialized".
>> 
>> So, we should be able to check whether an auto-variable has initializer or 
>> not after “pass_late_warn_uninitialized”,
>> If Not, then insert an initialization for it.
>> 
>> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
> 
> Yes, but do you want to catch variables promoted to register as well
> or just variables
> on the stack?

I think both as long as they are source-level auto-variables. Then which one is 
better?

> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>>  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>>  unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>>  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
>> even though DECL_INITIAL might be NULLed.
> 
> For locals it would be more reliable to set this flag during gimplification.

You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
“gimpley_decl_expr” (gimplify.c) as following:

  if (VAR_P (decl) && !DECL_EXTERNAL (decl))
{
  tree init = DECL_INITIAL (decl);
...
  if (init && init != error_mark_node)
{
  if (!TREE_STATIC (decl))
{
  DECL_IS_INITIALIZED(decl) = 1;
}

Is this enough for all Frontends? Are there other places that I need to 
maintain this bit? 


> 
>> Do you have any comment and suggestions?
> 
> As said above - do you want to cover registers as well as locals?

All the locals from the source-code point of view should be covered.   (From my 
study so far,  looks like that Clang adds that phase in FE). 
If GCC adds this phase in FE, then the following design requirement

C. The implementation needs to keep the current static warning on uninitialized
variables untouched in order to avoid "forking the language”.

cannot be satisfied.  Since gcc’s uninitialized variables analysis is applied 
quite late. 

So, we have to add this new phase after “pass_late_warn_uninitialized”. 

>  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)

Adding  this new transformation during RTL expansion is okay.  I will check on 
this in more details to see how to add it to RTL expansion phase. 
> 
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too late.

This is a really good point… 

In order to avoid optimization  to use the “uninitialized” state of locals, we 
should add the zeroing phase as early as possible (adding it in FE might be 
best 
for this issue). However, if we have to met the following requirement:

C. The implementation needs to keep the current static warning on uninitialized
variables untouched in order to avoid "forking the language”.

We 

Re: How to traverse all the local variables that declared in the current routine?

2020-11-24 Thread Richard Biener via Gcc-patches
On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
>
>
>
> > On Nov 24, 2020, at 1:32 AM, Richard Biener  
> > wrote:
> >
> > On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
> >  wrote:
> >>
> >> Hi,
> >>
> >> Does gcc provide an iterator to traverse all the local variables that are 
> >> declared in the current routine?
> >>
> >> If not, what’s the best way to traverse the local variables?
> >
> > Depends on what for.  There's the source level view you get by walking
> > BLOCK_VARS of the
> > scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
> > there's SSA names
> > (FOR_EACH_SSA_NAME).
>
> I am planing to add a new phase immediately after 
> “pass_late_warn_uninitialized” to initialize all auto-variables that are
> not explicitly initialized in the declaration, the basic idea is following:
>
> ** The proposal:
>
> A. add a new GCC option: (same name and meaning as CLANG)
> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>
> B. add a new attribute for variable:
> __attribute((uninitialized)
> the marked variable is uninitialized intentionaly for performance purpose.
>
> C. The implementation needs to keep the current static warning on 
> uninitialized
> variables untouched in order to avoid "forking the language".
>
>
> ** The implementation:
>
> There are two major requirements for the implementation:
>
> 1. all auto-variables that do not have an explicit initializer should be 
> initialized to
> zero by this option.  (Same behavior as CLANG)
>
> 2. keep the current static warning on uninitialized variables untouched.
>
> In order to satisfy 1, we should check whether an auto-variable has 
> initializer
> or not;
> In order to satisfy 2, we should add this new transformation after
> "pass_late_warn_uninitialized".
>
> So, we should be able to check whether an auto-variable has initializer or 
> not after “pass_late_warn_uninitialized”,
> If Not, then insert an initialization for it.
>
> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?

Yes, but do you want to catch variables promoted to register as well
or just variables
on the stack?

> Another issue is, in order to check whether an auto-variable has initializer, 
> I plan to add a new bit in “decl_common” as:
>   /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>   unsigned decl_is_initialized :1;
>
> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
> #define DECL_IS_INITIALIZED(NODE) \
>   (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>
> set this bit when setting DECL_INITIAL for the variables in FE. then keep it
> even though DECL_INITIAL might be NULLed.

For locals it would be more reliable to set this flag during gimplification.

> Do you have any comment and suggestions?

As said above - do you want to cover registers as well as locals?  I'd do
the actual zeroing during RTL expansion instead since otherwise you
have to figure youself whether a local is actually used (see expand_stack_vars)

Note that optimization will already made have use of "uninitialized" state
of locals so depending on what the actual goal is here "late" may be too late.

Richard.

>
> Thanks a lot for the help.
>
> Qing
>
> > Richard.
> >
> >>
> >> Thanks.
> >>
> >> Qing
>


Re: How to traverse all the local variables that declared in the current routine?

2020-11-24 Thread Qing Zhao via Gcc-patches



> On Nov 24, 2020, at 1:32 AM, Richard Biener  
> wrote:
> 
> On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
>  wrote:
>> 
>> Hi,
>> 
>> Does gcc provide an iterator to traverse all the local variables that are 
>> declared in the current routine?
>> 
>> If not, what’s the best way to traverse the local variables?
> 
> Depends on what for.  There's the source level view you get by walking
> BLOCK_VARS of the
> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
> there's SSA names
> (FOR_EACH_SSA_NAME).

I am planing to add a new phase immediately after 
“pass_late_warn_uninitialized” to initialize all auto-variables that are
not explicitly initialized in the declaration, the basic idea is following:

** The proposal:

A. add a new GCC option: (same name and meaning as CLANG)
-ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;

B. add a new attribute for variable:
__attribute((uninitialized)
the marked variable is uninitialized intentionaly for performance purpose.

C. The implementation needs to keep the current static warning on uninitialized
variables untouched in order to avoid "forking the language".


** The implementation:

There are two major requirements for the implementation:

1. all auto-variables that do not have an explicit initializer should be 
initialized to
zero by this option.  (Same behavior as CLANG)

2. keep the current static warning on uninitialized variables untouched.

In order to satisfy 1, we should check whether an auto-variable has initializer
or not;
In order to satisfy 2, we should add this new transformation after
"pass_late_warn_uninitialized".

So, we should be able to check whether an auto-variable has initializer or not 
after “pass_late_warn_uninitialized”, 
If Not, then insert an initialization for it. 

For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?

Another issue is, in order to check whether an auto-variable has initializer, I 
plan to add a new bit in “decl_common” as:
  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
  unsigned decl_is_initialized :1;

/* IN VAR_DECL, set when the decl is initialized at the declaration.  */
#define DECL_IS_INITIALIZED(NODE) \
  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)

set this bit when setting DECL_INITIAL for the variables in FE. then keep it
even though DECL_INITIAL might be NULLed.

Do you have any comment and suggestions?

Thanks a lot for the help.

Qing

> Richard.
> 
>> 
>> Thanks.
>> 
>> Qing



Re: How to traverse all the local variables that declared in the current routine?

2020-11-23 Thread Richard Biener via Gcc-patches
On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
 wrote:
>
> Hi,
>
> Does gcc provide an iterator to traverse all the local variables that are 
> declared in the current routine?
>
> If not, what’s the best way to traverse the local variables?

Depends on what for.  There's the source level view you get by walking
BLOCK_VARS of the
scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
there's SSA names
(FOR_EACH_SSA_NAME).

Richard.

>
> Thanks.
>
> Qing


How to traverse all the local variables that declared in the current routine?

2020-11-23 Thread Qing Zhao via Gcc-patches
Hi, 

Does gcc provide an iterator to traverse all the local variables that are 
declared in the current routine? 

If not, what’s the best way to traverse the local variables?

Thanks.

Qing