On Wed, Aug 2, 2017 at 7:10 PM, Jeff Law <l...@redhat.com> wrote: > On 08/01/2017 03:25 AM, Richard Biener wrote: >> On Tue, Aug 1, 2017 at 11:23 AM, Richard Biener >> <richard.guent...@gmail.com> wrote: >>> On Tue, Aug 1, 2017 at 4:27 AM, Martin Sebor <mse...@gmail.com> wrote: >>>> Richard, >>>> >>>> in discussing this work Jeff mentioned that your comments on >>>> the tree-ssa-alias.c parts would be helpful. When you have >>>> a chance could you please give it a once over and let me know >>>> if you have any suggestions or concerns? There are no visible >>>> changes to existing clients of the pass, just extensions that >>>> are relied on only by the new diagnostics. >>>> >>>> https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01264.html >>>> >>>> I expect to revisit the sprintf %s patch mentioned below and >>>> see how to simplify it by taking advantage of the changes >>>> implemented here. >>> >>> Your ptr_deref_alias_decl_p returns true when must_alias is true >>> but there is no must-alias relationship. >>> >>> The ao_ref_init_from_ptr_and_size doesn't belong there. >>> >>> I dislike all of the changes related to returning an alias "size". >>> >>> Please keep away from the alias machinery. >>> >>> Warning during folding is similarly bad design. Please don't add >>> more such things. >>> >>> Thanks for putting this on my radar though. >>> Richard. >> >> Oh, for a constructive comment this all feels like sth for either >> sanitizers or a proper static analysis tool. As I outlined repeatedly >> I belive GCC could host a static analysis tool but it surely should >> not be interwinded into it's optimization passes or tools. > I disagree strongly here. > > Sanitiers catch problems after the fact and only if you've compiled your > code with sanitization and manage to find a way to trigger the problem path. > > Other static analysis tools are useful, but they aren't an integral part > of the edit, compile, debug cycle and due to their cost are often run > long after code was committed. > > Finding useful warnings that can be issued as part of the compile, edit, > debug cycle is, IMHO, far more useful than sanitizers or independent > static checkers. > > -- > > > I think finding a way to exploit information that our various analyzers > can provide to give precise warnings is a good thing. So it seemed > natural that if we wanted to ask a must-alias question that we should be > querying the alias oracle rather than implementing that kind of query > within the sprintf warnings. I'm not sure why you'd say "Please keep > away from the alias machinery". > > I'm a little confused here...
Well, simply because the way as implemented isn't a must-alias query but maybe one that's good enough for warnings (reduces false positives but surely doesn't eliminate them). There's a must alias query already, in stmt_kills_ref_p. It's a matter of refactoring to make a refs_must_alias_p. Then propose that "overlap range" stuff separately. But I'm against lumping this all in as an innocent change suggesting the machinery can do sth (must alias) when it really can't. I also do not like adding a "must alias" bool to the may-alias APIs as the implementation is fundamentally may-alias, must-alias really is very different. And to repeat, no, I do not want a "good-enough-for-warnings" must-alias in an API that's supposed to be used by optimizations where "good enough" is not good enough. Richard. > > > Jeff
