Re: [PATCH 2/6] detect unterminated const arrays in strlen calls (PR 86552)

[Martin Sebor <msebor@gmail.com>]

On 08/30/2018 04:15 PM, Jeff Law wrote:
> On 08/13/2018 09:21 PM, Martin Sebor wrote:
>> [PATCH 2/6] detect unterminated const arrays in strlen calls (PR 86552)
>>
>> The attached changes implement the detection of past-the-end reads
>> by strlen due to unterminated arguments.
>>
>> gcc-86552-2.diff
>>
>>
>> PR tree-optimization/86552 - missing warning for reading past the end
>>
>> gcc/ChangeLog:
>>
>> 	* builtins.c (warn_string_no_nul): New function.
>> 	(expand_builtin_strlen): Warn for unterminated arrays.
>> 	(fold_builtin_strlen): Add argument.  Warn for unterminated arrays.
>> 	(fold_builtin_1): Adjust call to fold_builtin_strlen.
>> 	* builtins.h (warn_string_no_nul): New function.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.dg/warn-strlen-no-nul.c: New test.
> So this has a dependency on parts of the 1/6 patch that haven't been
> committed yet.
>
> Ignoring that for the moment (since I have those parts in my tree :-)...
>
> There are minor API changes to functions we need to use.  Those are
> trivially fixed up.
>
> With that taken care of I get one XPASS from the new test:
>
>
>> +T (v0 ? &b[3][v0] : &b[3][v1]);   /* { dg-warning "nul" "bug" { xfail *-*-* } }  */
> I haven't dug into why this now passes.  It could well be the various
> refinements we've made over the last couple weeks.

I'm not sure what's letting it succeed.  get_range_strlen() can
tell the array isn't nul-terminated but it's only called from
gimple_fold_builtin_strlen() which doesn't warn.   The next
chance to warn is handle_builtin_strlen() but it doesn't call
get_range_strlen().  The next opportunity to warn after that
is expand_builtin_strlen() and it doesn't call get_range_strlen()
either.  There definitely are more opportunities to warn as
the many xfails in the warn-strlen-no-nul.c test indicate.
I didn't want to make the initial patch too bin and intrusive
by handling all those cases but it's something I'd like to do
in a followup.

>
> Given that I've got the patch in my tree I'll take care of posting the
> final version of the patch and committing it once I've committed the
> prereqs.

Sounds good.  Thanks for handling that!

Martin