On 10/9/21 9:04 AM, Aldy Hernandez wrote:
On Fri, Oct 8, 2021 at 6:52 PM Martin Sebor <mse...@gmail.com> wrote:
On 10/8/21 9:12 AM, Aldy Hernandez via Gcc-patches wrote:
The following patch converts the strlen pass from evrp to ranger,
leaving DOM as the last remaining user.
Thanks for doing this. I know I said I'd work on it but I'm still
bogged down in my stage 1 work that's not going so great :( I just
have a few minor comments/questions on the strlen change (inline)
but I am a bit concerned about the test failure.
No additional cleanups have been done. For example, the strlen pass
still has uses of VR_ANTI_RANGE, and the sprintf still passes around
pairs of integers instead of using a proper range. Fixing this
could further improve these passes.
As a further enhancement, if the relevant maintainers deem useful,
the domwalk could be removed from strlen. That is, unless the pass
needs it for something else.
With ranger we are now able to remove the range calculation from
before_dom_children entirely. Just working with the ranger on-demand
catches all the strlen and sprintf testcases with the exception of
builtin-sprintf-warn-22.c which is due to a limitation of the sprintf
code. I have XFAILed the test and documented what the problem is.
builtin-sprintf-warn-22.c is a regression test for a false positive
in Glibc. If it fails we'll have to deal with the Glibc failure
again, which I would rather avoid. Have you checked to see if
Glibc is affected by the change?
Have you tested Glibc with the patch to see if the warning comes
back? If it does there are other ways to suppress it, and I can
take care of it. I just want us to avoid reintroducing it without
doing our due diligence first.
...
I think there still are quite a few calls to get_addr_stridx() and
get_addr() that don't pass in rvals. I started doing this back in
the GCC 11 cycle but didn't finish the job. Eventually, rvals
should be passed to all their callers (or they made class members
with a ranger member). I mention this in case it has an effect on
the range propagation (since the pass also sets ranges).
Definitely. You'll get even better ranges, though perhaps more false
positives as discussed above :-/.
We want better ranges and better analysis. Ultimately, it will
lead to better quality warnings, as has been one of the goals
behind Ranger. If along the way it means a few false positives
in some edge cases, we'll deal with them. I don't see this one
as a significant problem.
Martin
