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] Improve checks in c_strlen (PR 87053)


On 08/22/2018 10:28 AM, Martin Sebor wrote:
> On 08/22/2018 08:41 AM, Bernd Edlinger wrote:
>> Hi!
>>
>>
>> This patch adds some more checks to c_getstr to fix PR middle-end/87053
>> wrong code bug.
>>
>> Unfortunately this patch alone is not sufficient to fix the problem,
>> but also the patch for PR 86714 that hardens c_getstr is necessary
>> to prevent the wrong folding.
>>
>>
>> Bootstrapped and reg-tested on top of my PR 86711/86714 patch.
>> Is it OK for trunk?
> 
> This case is also the subject of the patch I submitted back in
> July for 86711/86714 and 86552.  With it, GCC avoid folding
> the strlen call early and warns for the missing nul:
> 
> warning: ‘__builtin_strlen’ argument missing terminating nul
> [-Wstringop-overflow=]
>    if (__builtin_strlen (u.z) != 7)
>        ^~~~~~~~~~~~~~~~
> 
> The patch doesn't doesn't prevent all such strings from being
> folded and it eventually lets fold_builtin_strlen() do its thing:
> 
>       /* To avoid warning multiple times about unterminated
>          arrays only warn if its length has been determined
>          and is being folded to a constant.  */
>       if (nonstr)
>         warn_string_no_nul (loc, NULL_TREE, fndecl, nonstr);
> 
>       return fold_convert_loc (loc, type, len);
> 
> Handling this case is a matter of avoiding the folding here as
> well and moving the warning later.
> 
> Since my patch is still in the review queue and does much more
> than just prevent folding of non-nul terminated arrays it should
> be reviewed first.
I think we need to address 86711/86714 first.  However, neither approach
to 86711/86714 (Bernd's or yours) is sufficient alone to fix this bug.
This patch can be layered on top of either approach to 86711/86714 to
fix 87053 (I've actually tested that).

So let's table this, hopefully for just a day or so.  It's getting late
and I've got more tests to run on the 86714/86711 patches for comparison
purposes and some loose ends I want to look at in Martin's patch.  I'd
hoped to be finished already, but wasn't able to move things as fast as
I hoped.

jeff


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