On 09/17/2018 11:35 AM, Richard Biener wrote:
On September 17, 2018 7:24:16 PM GMT+02:00, Jeff Law <law@redhat.com>
wrote:
On 9/15/18 2:14 AM, Bernd Edlinger wrote:
On 9/14/18, Martin Sebor wrote:
As I said above, this happens during the dom walk in the ccp
pass:
substitute_and_fold_dom_walker walker (CDI_DOMINATORS, this);
walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun));
The warning is issued during the walker.walk() call as
strncpy is being folded into memcpy. The prior assignments are
only propagated later, when the next statement after the strncpy
call is reached. It happens in
substitute_and_fold_dom_walker::before_dom_children(). So during
the strncpy folding we see the next statement as:
MEM[(struct S *)_1].a[n_7] = 0;
After the strncpy call is transformed to memcpy, the assignment
above is transformed to
MEM[(struct S *)_8].a[3] = 0;
If they're only discovered as copies within the pass where
you're trying
to issue the diagnostic, then you'd want to see if the pass has
any
internal structures that tell you about equivalences.
I don't know if this is possible. I don't see any APIs in
tree-ssa-propagate.h that would let me query the internal data
somehow to find out during folding (when the warning is issued).
Well,
if I see this right, the CCP is doing tree transformations
while from the folding of strncpy the predicate
maybe_diag_stxncpy_trunc
is called, and sees inconsistent information, in the tree,
and therefore it issues a warning.
I understand that walking the references is fragile at least
in this state.
But why not just prevent warnings when this is called from CCP?
Like this?
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
No. That's just hacking around the real problem.
The real problem is emitting diagnostics from folding code.
Strncpy is a particularly dangerous function that's often
misunderstood and misused. IMO, it would be a worthwhile
tradeoff to move the strncpy to memcpy transformation to
the strlen pass where these bugs could be detected more
reliably, and with fewer false positives. I would not
expect it to have a noticeable impact on code efficiency.
I'd be happy to modify the patch to do that if you find it
acceptable.