On Tue, Jun 18, 2019 at 11:56:41AM +0200, Martin Liška wrote:
> On 6/18/19 10:23 AM, Martin Liška wrote:
> > On 6/18/19 10:11 AM, Jakub Jelinek wrote:
> >> On Tue, Jun 18, 2019 at 10:07:50AM +0200, Martin Liška wrote:
> >>> diff --git a/gcc/builtins.c b/gcc/builtins.c
> >>> index 3463ffb1539..b58e1e58d4d 100644
> >>> --- a/gcc/builtins.c
> >>> +++ b/gcc/builtins.c
> >>> @@ -7142,6 +7142,20 @@ inline_expand_builtin_string_cmp (tree exp, rtx
> >>> target)
> >>> const char *src_str1 = c_getstr (arg1, &len1);
> >>> const char *src_str2 = c_getstr (arg2, &len2);
> >>>
> >>> + if (src_str1 != NULL)
> >>> + {
> >>> + unsigned HOST_WIDE_INT str_str1_strlen = strnlen (src_str1, len1);
> >>> + if (str_str1_strlen + 1 < len1)
> >>> + len1 = str_str1_strlen + 1;
> >>
> >> You really don't need any of this after strnlen. strnlen is already
> >> guaranteed to return a number from 0 to len1 inclusive, so you can really
> >> just do:
> >> if (src_str1 != NULL)
> >> len1 = strnlen (src_str1, len1);
> >>
> >> Jakub
> >>
> >
> > Got it, I'm testing that.
> >
> > Martin
> >
>
> Ok, there's an off-by-one error in the previous patch candidate.
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
> >From fe4ef7d43c506f450de802a7d8a602fab7da4545 Mon Sep 17 00:00:00 2001
> From: Martin Liska <mli...@suse.cz>
> Date: Mon, 17 Jun 2019 10:39:15 +0200
> Subject: [PATCH] Handle '\0' in strcmp in RTL expansion (PR
> tree-optimization/90892).
>
> gcc/ChangeLog:
>
> 2019-06-17 Martin Liska <mli...@suse.cz>
>
> PR tree-optimization/90892
> * builtins.c (inline_expand_builtin_string_cmp): Handle '\0'
> in string constants.
Oops. The problematic case is then if the STRING_CST c_getstr finds
is not NUL terminated (dunno if we ever construct that) or if
string_size is smaller than string_length and there are no NULs in that
size.
With your patch that would mean setting len1 or len2 one larger than needed.
Looking at the function, I think we want to do more changes.
I think any such length changes should be moved after the two punt checks.
Move also the len3 setting before the new checks (of course conditional on
is_ncmp).
Sorry for the earlier advice, you indeed should store the strnlen result
into a different variable, and through that you can easily differentiate between
when the string is NUL terminated and when it is not (if strnlen < lenN,
then NUL terminated).
If !is_ncmp, I think you should punt for not NUL terminated strings
(return NULL_RTX).
If is_ncmp and not NUL terminated, you should punt if len3 is bigger than
len{1,2}.
If NUL terminated, you should set lenN to the strnlen returned value + 1.
Does this sound reasonable?
Jakub
