Jeff Law <l...@redhat.com> writes:
> On 08/05/2015 08:18 AM, Richard Sandiford wrote:
>> Building some targets results in a warning about orig_dup[i] potentially
>> being used uninitialised. I think the warning is fair, since it isn't
>> obvious that the reog_data-based loop bound remains unchanged between:
>>
>> for (i = 0; i < recog_data.n_dups; i++)
>> orig_dup[i] = *recog_data.dup_loc[i];
>>
>> and:
>>
>> for (i = 0; i < recog_data.n_dups; i++)
>> *recog_data.dup_loc[i] = orig_dup[i];
>>
>> Tested on x86_64-linux-gnu. OK to install?
>>
>> Thanks,
>> Richard
>>
>> gcc/
>> * reload1.c (elimination_costs_in_insn): Make it obvious to the
>> compiler that the n_dups and n_operands loop bounds are invariant.
> There's a BZ about this issue. 55035.
>
> What I want is to make sure we don't lose track of the false positive in
> 55035 (caused by a miss jump thread due to aliasing issues).
>
> So perhaps the way forward is to install your change and twiddle the
> summary of 55035 in some way that makes it more obvious the bz tracks a
> false positive from -Wuninitialized and attach 55035 to the
> -Wuninitialized meta bug (24639).
Is it really a false positive in this case though? We have:
for (i = 0; i < recog_data.n_dups; i++)
orig_dup[i] = *recog_data.dup_loc[i];
for (i = 0; i < recog_data.n_operands; i++)
{
orig_operand[i] = recog_data.operand[i];
/* For an asm statement, every operand is eliminable. */
if (insn_is_asm || insn_data[icode].operand[i].eliminable)
{
bool is_set_src, in_plus;
/* Check for setting a register that we know about. */
if (recog_data.operand_type[i] != OP_IN
&& REG_P (orig_operand[i]))
{
/* If we are assigning to a register that can be eliminated, it
must be as part of a PARALLEL, since the code above handles
single SETs. We must indicate that we can no longer
eliminate this reg. */
for (ep = reg_eliminate; ep < ®_eliminate[NUM_ELIMINABLE_REGS];
ep++)
if (ep->from_rtx == orig_operand[i])
ep->can_eliminate = 0;
}
/* Companion to the above plus substitution, we can allow
invariants as the source of a plain move. */
is_set_src = false;
if (old_set && recog_data.operand_loc[i] == &SET_SRC (old_set))
is_set_src = true;
if (is_set_src && !sets_reg_p)
note_reg_elim_costly (SET_SRC (old_set), insn);
in_plus = false;
if (plus_src && sets_reg_p
&& (recog_data.operand_loc[i] == &XEXP (plus_src, 0)
|| recog_data.operand_loc[i] == &XEXP (plus_src, 1)))
in_plus = true;
eliminate_regs_1 (recog_data.operand[i], VOIDmode,
NULL_RTX,
is_set_src || in_plus, true);
/* Terminate the search in check_eliminable_occurrences at
this point. */
*recog_data.operand_loc[i] = 0;
}
}
for (i = 0; i < recog_data.n_dups; i++)
*recog_data.dup_loc[i]
= *recog_data.operand_loc[(int) recog_data.dup_num[i]];
/* If any eliminable remain, they aren't eliminable anymore. */
check_eliminable_occurrences (old_body);
/* Restore the old body. */
for (i = 0; i < recog_data.n_operands; i++)
*recog_data.operand_loc[i] = orig_operand[i];
for (i = 0; i < recog_data.n_dups; i++)
*recog_data.dup_loc[i] = orig_dup[i];
and I don't see how GCC could prove that eliminate_regs_1 doesn't
modify the value of recog_data.n_dups between the two loops.
eliminate_regs_1 calls functions like plus_constant that are defined
outside the TU and that certainly aren't pure/const.
So I think c#5 (marked as a bogus reduction) is an accurate reflection
of what reload1.c does. c#4 looks like a genuine bug but seems different
from the reload1.c case. If we still warn for c#4 then I think we
should keep the bugzilla entry open for that, but the warning for the
reload1.c code seems justified. Perhaps the question is why it doesn't
trigger on more targets :-)
Thanks,
Richard
