On 01/02/2018 10:27 AM, Martin Sebor wrote: > On 01/02/2018 09:44 AM, Jeff Law wrote: >> On 01/02/2018 02:57 AM, Jakub Jelinek wrote: >>> On Mon, Jan 01, 2018 at 03:31:46PM -0700, Martin Sebor wrote: >>>> The ICE in the test case submitted in PR tree-optimization/83640 >>>> is triggered by the tree-ssa-strlen pass transforming calls to >>>> strcat to strcpy with an offset pointing to the terminating NUL >>>> of the destination string, and allowing the upper bound of the >>>> offset's range to exceed PTRDIFF_MAX by the length of >>>> the appended constant string. Although the ICE itself is due >>>> to an unsafe assumption in the -Wrestrict checker, the excessive >>>> upper bound in the strcat case suggests an optimization opportunity >>>> that's missing from the tree-ssa-strlen pass: namely, to reduce >>>> the offset's upper bound by the length of the appended string. >>>> The attached patch adds this minor optimization. >>> >>> This is wrong for many reasons. >>> >>> The recorded range info is a property of the SSA_NAME, can't be >>> dependent on >>> where it is used, you are changing it globally. >> Right. And that's precisely the way to think about it -- does the range >> hold everywhere the SSA_NAME is or might be used. If so, then the range >> can be recorded via set_range_info. >> >> Otherwise the range is context sensitive. The best way to handle >> context sensitive ranges is discover them within the EVRP analyzer which >> has mechanisms to record temporary ranges and reset them to their prior >> values when leaving scope. > > I assumed the call to unshare_expr would somehow avoid this sharing > problem but it looks like I misunderstood the purpose of the function > and didn't do enough testing to find out. Yea. unshare_expr is more concerned with unsharing within the tree structures rather than creating a unique SSA_NAME within a context. unshare_rtl is similar to unshare_expr, except it works on RTL bits.
> >> tree-ssa-strlen is already a dominator walker, so embedding an EVRP >> range analyzer in there ought to be trivial. > > This might be a good opportunity for me to get some experience with > the analyzer. Let me look into it. Don't hesitate to pass along questions. Happy to help. I think I was supposed to pass you some skeleton code in this space, but haven't gotten around to it. jeff
