This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, revised]: Track uninitialized variables
- From: Ian Lance Taylor <iant at google dot com>
- To: Caroline Tice <ctice at apple dot com>
- Cc: "gcc-patches at gcc dot gnu dot org Patches" <gcc-patches at gcc dot gnu dot org>
- Date: 11 Jul 2007 07:43:28 -0700
- Subject: Re: [PATCH, revised]: Track uninitialized variables
- References: <F7F88B21-DF14-478D-A8C3-35CC1EF3C6C2@apple.com> <738B4C94-E6B1-4C3C-AEA6-42BD0C40C4CA@apple.com>
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