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


On 11 Jul 2007 07:43:28 -0700, Ian Lance Taylor <iant@google.com> wrote:
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.

This patch breaks bootstrap on ia64. See PR32746.


Richard.


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