This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, revised]: Track uninitialized variables


Caroline Tice <ctice@apple.com> writes:

>   static dw_loc_descr_ref
> ! one_reg_loc_descriptor (unsigned int regno, enum var_init_status initialized)
>   {
> +   dw_loc_descr_ref reg_loc_descr;
>     if (regno <= 31)
> !     reg_loc_descr = new_loc_descr (DW_OP_reg0 + regno, 0, 0);
>     else
> !     reg_loc_descr =  new_loc_descr (DW_OP_regx, regno, 0);

Extraneous space after '='.


> *************** multiple_reg_loc_descriptor (rtx rtl, rt
> *** 8707,8713 ****
>   	{
>   	  dw_loc_descr_ref t;
>   
> ! 	  t = one_reg_loc_descriptor (DBX_REGISTER_NUMBER (reg));
>   	  add_loc_descr (&loc_result, t);
>   	  add_loc_descr_op_piece (&loc_result, size);
>   	  ++reg;
> --- 8731,8738 ----
>   	{
>   	  dw_loc_descr_ref t;
>   
> ! 	  t = one_reg_loc_descriptor (DBX_REGISTER_NUMBER (reg),
> ! 				      STATUS_UNINITIALIZED);
>   	  add_loc_descr (&loc_result, t);
>   	  add_loc_descr_op_piece (&loc_result, size);
>   	  ++reg;

You should be passing STATUS_INITIALIZED here, not
STATUS_UNINITIALIZED.


> + /* Possible initialization status of a variable.   When requested
> +    by the user, this information is tracked and recorded in the DWARF
> +    debug information, along with the variable's location.  */
> + enum var_init_status
> + {
> +   STATUS_UNKNOWN,
> +   STATUS_UNINITIALIZED,
> +   STATUS_INITIALIZED
> + };

I'm sorry to do this to you, but these enum constant names are too
generic.  Please use something like VAR_INIT_STATUS_UNKNOWN, etc.


> +   if (!(flag_var_tracking_uninit))
> +     initialized = STATUS_INITIALIZED;

Remove the extraneous parentheses.


> *************** emit_note_insn_var_location (void **varp
> *** 2472,2480 ****
>         parallel = gen_rtx_PARALLEL (VOIDmode,
>   				   gen_rtvec_v (n_var_parts, loc));
>         NOTE_VAR_LOCATION (note) = gen_rtx_VAR_LOCATION (VOIDmode, var->decl,
> ! 						       parallel);
>       }
>   
>     htab_clear_slot (changed_variables, varp);
>   
>     /* When there are no location parts the variable has been already
> --- 2724,2735 ----
>         parallel = gen_rtx_PARALLEL (VOIDmode,
>   				   gen_rtvec_v (n_var_parts, loc));
>         NOTE_VAR_LOCATION (note) = gen_rtx_VAR_LOCATION (VOIDmode, var->decl,
> ! 						       parallel, 
> ! 						       (int) initialized);
>       }
>   
> +   NOTE_VAR_LOCATION_STATUS (note) = (int) initialized;

Setting NOTE_VAR_LOCATION_STATUS here is redundant with passing
initialized to gen_rtx_VAR_LOCATION.  You should either remove this
and fix the one case of gen_rtx_VAR_LOCATION to which you pass 0
rather than (int) initialized, or you should pass 0 in all cases to
gen_rtx_VAR_LOCATION.


The patch is OK with those changes.

Thanks, and sorry for the slow review.

Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]