On Mon, Jan 24, 2022 at 11:55 PM Martin Sebor <mse...@gmail.com> wrote: > > The pointer_query algorithm fails when it's unable to determine > the provenance of an pointer SSA variable. A failure prevents it > from making a record of the variable in the cache. Although this > doesn't happen often, one common cause of failures is PHI nodes: > e.g., because one of its arguments references the PHI itself, or > an undefined variable. Because a failed SSA_NAME isn't added to > the cache, the computation that led up to its failure is repeated > on each interesting use of the variable. This is computationally > wasteful and can cause excessive CPU usage in pathological cases > like in the test case in PR 104203. > > To avoid the problem the attached patch changes the logic for PHI > arguments and nodes to cache the most conservative result on such > a failure. It treats such a pointer as pointing into the middle > of an object of maximum size (same as under some other conditions). > > In addition, the patch also adds a new timer variable to > the warn_access pass to make these problems easier in the future > to track down.
LGTM. > There are a number of other conditions under which point_query > fails. Even though the same approach could be used to replace > all those, I did not make the change in this patch. It's probably worth fixing them. Btw, I wonder whether caching fails as true fails would work as well though caching a conservative answer of course is fine. > Tested on x86_64-linux. > > Martin > > PS This solution relies on the pointer_query instance having caching > enabled. Two warning passes run without a cache: -Warray-bounds in > VRP and -Wrestrict in the wrestrict pass. I can look into enabling > it for GCC 12 if there's concern that they might be impacted. Please. There is IMHO no reason to not use the cache - for PHIs you can run into the same PHI arg def from multiple edges and thus you'll do unnecessary work even when you just do a single query for the toplevel API. Richard.
