On 01/16/2018 05:35 PM, Martin Sebor wrote: > On 01/16/2018 02:32 PM, Jakub Jelinek wrote: >> On Tue, Jan 16, 2018 at 01:36:26PM -0700, Martin Sebor wrote: >>> --- gcc/gimple-ssa-warn-restrict.c (revision 256752) >>> +++ gcc/gimple-ssa-warn-restrict.c (working copy) >>> @@ -384,6 +384,12 @@ builtin_memref::builtin_memref (tree expr, tree si >>> base = SSA_NAME_VAR (base); >>> } >>> >>> + if (DECL_P (base) && TREE_CODE (TREE_TYPE (base)) == ARRAY_TYPE) >>> + { >>> + if (offrange[0] < 0 && offrange[1] > 0) >>> + offrange[0] = 0; >>> + } >> >> Why the 2 nested ifs? > > No particular reason. There may have been more code in there > that I ended up removing. Or a comment. I can remove the > extra braces when the patch is approved. > >> >>> @@ -1079,14 +1085,35 @@ builtin_access::strcat_overlap () >>> return false; >>> >>> /* When strcat overlap is certain it is always a single byte: >>> - the terminatinn NUL, regardless of offsets and sizes. When >>> + the terminating NUL, regardless of offsets and sizes. When >>> overlap is only possible its range is [0, 1]. */ >>> acs.ovlsiz[0] = dstref->sizrange[0] == dstref->sizrange[1] ? 1 : 0; >>> acs.ovlsiz[1] = 1; >>> - acs.ovloff[0] = (dstref->sizrange[0] + >>> dstref->offrange[0]).to_shwi (); >>> - acs.ovloff[1] = (dstref->sizrange[1] + >>> dstref->offrange[1]).to_shwi (); >> >> You use to_shwi many times in the patch, do the callers or something >> earlier >> in this function guarantee that you aren't throwing away any bits (unlike >> tree_to_shwi, to_shwi method doesn't ICE, just throws away upper bits). >> Especially when you perform additions like here, even if both >> wide_ints fit >> into a shwi, the result might not. > > No, I'm not sure. In fact, it wouldn't surprise me if it did > happen. It doesn't cause false positives or negatives but it > can make the offsets less than meaningful in cases where they > are within valid bounds. There are also cases where they are > meaningless to begin with and there is little the pass can do > about that. I was kind of expecting an update to try and address some of these issues. Though after re-reading your response the consequence of throwing away bits here is just the diagnostic is not as precise as it could be, right? ie, it doesn't change when we issue a diagnostic, just the contents of the diagnostic.
I filed this into my gcc9 bucket because it doesn't fix a regression, but it appears that a regression fix does depend on this stuff to some degree (84095). So I'll try to take a look at this shortly so that we can unblock 84095. Jeff