On 10/30/2017 09:19 AM, Martin Sebor wrote: > On 10/30/2017 05:45 AM, Richard Biener wrote: >> On Sun, 29 Oct 2017, Martin Sebor wrote: >> >>> In my work on -Wrestrict, to issue meaningful warnings, I found >>> it important to detect both out of bounds array indices as well >>> as offsets in calls to restrict-qualified functions like strcpy. >>> GCC already detects some of these cases but my tests for >>> the enhanced warning exposed a few gaps. >>> >>> The attached patch enhances -Warray-bounds to detect more instances >>> out-of-bounds indices and offsets to member arrays and non-array >>> members. For example, it detects the out-of-bounds offset in the >>> call to strcpy below. >>> >>> The patch is meant to be applied on top posted here but not yet >>> committed: >>> https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01304.html >>> >>> Richard, since this also touches tree-vrp.c I look for your comments. >> >> You fail to tell what you are changing and why - I have to reverse >> engineer this from the patch which a) isn't easy in this case, b) feels >> like a waste of time. Esp. since the patch does many things. >> >> My first question is why do you add a warning from forwprop? It >> _feels_ like you're trying to warn about arbitrary out-of-bound >> addresses at the point they are folded to MEM_REFs. And it looks >> like you're warning about pointer arithmetic like &p->a + 6. >> That doesn't look correct to me. Pointer arithmetic in GIMPLE >> is not restricted to operate within fields that are appearantly >> accessed here - the only restriction is with respect to the >> whole underlying pointed-to-object. >> >> By doing the warning from forwprop you'll run into all such cases >> introduced by GCC itself during quite late optimization passes. > > I haven't run into any such cases. What would a more appropriate > place to detect out-of-bounds offsets? I'm having a hard time > distinguishing what is appropriate and what isn't. For instance, > if it's okay to detect some out of bounds offsets/indices in vrp > why is it wrong to do a better job of it in forwpropI think part of the > problem is there isn't a well defined place to do this kind of warning. I suspect it's currently in VRP simply because that is where we had range information in the past. It's still the location with the most accurate range information.
In a world where we have an embedded context sensitive range analysis engine, we should *really* look at pulling the out of bounds array warnings out of any optimization pass an have a distinct pass to deal with them. I guess in the immediate term the question I would ask Martin is what is it about forwprop that makes it interesting? Is it because of the lowering issues we touched on last week? If so I wonder if we could recreate an array form from a MEM_REF for the purposes of optimization. Or if we could just handle MEM_REFs better within the existing warning. > >> >> You're trying to re-do __builtin_object_size even when that wasn't >> used. > > That's not the quite my intent, although it is close. Wouldn't we be better off improving _b_o_s? > >> >> So it looks like you're on the wrong track. Yes, >> >> strcpy (p->a + 6, "y"); >> >> _may_ be "invalid" C (I'm not even sure about that!) but it >> is certainly not invalid GIMPLE. > > Adding (or subtracting) an integer to/from a pointer to an array > is defined in both C and C++ only if the result points to an element > of the array or just past the last element of the array. Otherwise > it's undefined. (A non-array object is considered an array of one > for this purpose.) I think Richi's argument is that gimple allows things that are not necessarily allowed by the C/C++ standard. For example we support virtual origins from Ada, which internally would look something like invalid C code OTOH, we currently have code in tree-vrp.c which warns if we compute the address of an out of bounds array index which is very C/C++ centric. jeff
