Re: [PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664)

[Jeff Law <law at redhat dot com>]

On 3/19/19 12:01 PM, Martin Sebor wrote:
> On 3/19/19 8:33 AM, Jeff Law wrote:
>> On 3/11/19 8:27 PM, Martin Sebor wrote:
>>> The -Wstringop-truncation handling for strncpy/stpncpy neglects
>>> to consider that character arrays tracked by the strlen pass
>>> are not necessarily nul-terminated.  It unconditionally adds
>>> one when computing the size of each sequence to account for
>>> the nul.  This leads to false positive warnings when checking
>>> the validity of indices/pointers computed by the built-ins.
>>>
>>> The attached patch corrects this by adding one for the nul
>>> only when the character array is known to be nul-terminated.
>>>
>>> Since GCC 7 does not issue the warning this is a 8/9 regression
>>> that I would like to fix in both releases.  Is the patch okay
>>> for trunk/gcc-8-branch?
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>>
>>> gcc-89644.diff
>>>
>>> PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic
>>> on strncpy
>>>
>>> gcc/ChangeLog:
>>>
>>>     PR tree-optimization/89644
>>>     * tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated
>>>     arrays in determining sequence sizes in strncpy and stpncpy.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     PR tree-optimization/89644
>>>     * gcc.dg/Wstringop-truncation-8.c: New test.
>> OK for the trunk as well as the affected release branches.
> 
> I just noticed some some -Wrestrict test failures that I had missed
> in the test report.  The patch wasn't quite right.  Sorry about that.
> I'll fix it shortly.
Something like this perhaps?

> # Comparing 4 common sum files
> ## /bin/sh gcc/contrib/compare_tests  /tmp/gxx-sum1.6751 /tmp/gxx-sum2.6751
> Tests that now fail, but worked before (15 tests):
> 
> c-c++-common/Wrestrict.c  -Wc++-compat  (test for excess errors)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 811)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 812)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 813)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 819)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 820)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 827)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 828)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 867)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 875)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 876)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 884)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 885)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 893)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 894)

My testers just spit this out on a couple of targets in the last half
hour or so.

jeff