On 9/17/18 3:15 PM, Martin Sebor wrote: > On 09/17/2018 11:35 AM, Richard Biener wrote: >> On September 17, 2018 7:24:16 PM GMT+02:00, Jeff Law <l...@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. That natural question is what is the immediate (ie testsuite) fallout?
Jeff
