This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] set range for strlen(array) to avoid spurious -Wstringop-overflow (PR 83373 , PR 78450)


On 12/12/2017 05:35 PM, Jeff Law wrote:
On 12/12/2017 01:15 PM, Martin Sebor wrote:
Bug 83373 - False positive reported by -Wstringop-overflow, is
another example of warning triggered by a missed optimization
opportunity, this time in the strlen pass.  The optimization
is discussed in pr78450 - strlen(s) return value can be assumed
to be less than the size of s.  The gist of it is that the result
of strlen(array) can be assumed to be less than the size of
the array (except in the corner case of last struct members).

To avoid the false positive the attached patch adds this
optimization to the strlen pass.  Although the patch passes
bootstrap and regression tests for all front-ends I'm not sure
the way it determines the upper bound of the range is 100%
correct for languages with arrays with a non-zero lower bound.
Maybe it's just not as tight as it could be.
What about something hideous like

struct fu {
  char x1[10];
  char x2[10];
  int avoid_trailing_array;
}

Where objects stored in x1 are not null terminated.  Are we in the realm
of undefined behavior at that point (I hope so)?

Yes, this is undefined.  Pointer arithmetic (either direct or
via standard library functions) is only defined for pointers
to the same object or subobject.  So even something like

 memcpy (pfu->x1, pfu->x1 + 10, 10);

is undefined.  _FORTIFY_SOURCE allows it for raw memory functions
because some low level code copies regions of the same object that
span two or more members (I've seen an example or two in the Linux
kernel but, IIRC, nowhere else).  With the patch, using memchr
would be the only way to get away with it.

Other than that, the new -Wstringop-truncation warning is designed
to prevent creating unterminated character arrays and the manual
suggests using attribute nonstring when it's necessary.  The
-Wrestrict warning I'm about to check in also complains about
forming invalid pointers by built-ins with restrict-qualified
arguments (although it won't diagnose the above).

Although I would prefer not to, I suppose if letting strlen cross
the boundaries of subobjects was considered an important use to
accommodate in limited cases the optimization could be disabled
for member arrays declared with the new nonstring attribute (while
still issuing a warning for it as GCC does today).

Another alternative (if the above use case is considered important
enough) might be to suppress the optimization for member character
arrays that are immediately followed by another such array.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]