Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On 6/27/19 7:20 AM, Martin Liška wrote: > On 6/18/19 12:16 PM, Jakub Jelinek wrote: >> 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). > Ok, I'm sending updated version of the patch that addresses this. > I've been testing the patch. > > Ready to be installed then? > Thanks, > Martin > > > 0001-Handle-0-in-strcmp-in-RTL-expansion-PR-tree-optimiza.patch > > From 35043ab431c0dc1e8dcda484725a1f8875a4b95b Mon Sep 17 00:00:00 2001 > From: Martin Liska > 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 > > PR tree-optimization/90892 > * builtins.c (inline_expand_builtin_string_cmp): Handle '\0' > in string constants. > > gcc/testsuite/ChangeLog: > > 2019-06-17 Martin Liska > > PR tree-optimization/90892 > * gcc.dg/pr90892.c: New test. OK jeff > ---
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On 6/18/19 12:16 PM, Jakub Jelinek wrote:
> 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).
Ok, I'm sending updated version of the patch that addresses this.
I've been testing the patch.
Ready to be installed then?
Thanks,
Martin
>From 35043ab431c0dc1e8dcda484725a1f8875a4b95b Mon Sep 17 00:00:00 2001
From: Martin Liska
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
PR tree-optimization/90892
* builtins.c (inline_expand_builtin_string_cmp): Handle '\0'
in string constants.
gcc/testsuite/ChangeLog:
2019-06-17 Martin Liska
PR tree-optimization/90892
* gcc.dg/pr90892.c: New test.
---
gcc/builtins.c | 17 ++---
gcc/testsuite/gcc.dg/pr90892.c | 14 ++
2 files changed, 28 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr90892.c
diff --git a/gcc/builtins.c b/gcc/builtins.c
index c53afe8b033..db7939cfde8 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -7118,8 +7118,19 @@ inline_expand_builtin_string_cmp (tree exp, rtx target)
return NULL_RTX;
/* For strncmp, if the length is not a const, not qualify. */
- if (is_ncmp && !tree_fits_uhwi_p (len3_tree))
-return NULL_RTX;
+ if (is_ncmp)
+{
+ if (!tree_fits_uhwi_p (len3_tree))
+ return NULL_RTX;
+ else
+ len3 = tree_to_uhwi (len3_tree);
+}
+
+ if (src_str1 != NULL)
+len1 = strnlen (src_str1, len1) + 1;
+
+ if (src_str2 != NULL)
+len2 = strnlen (src_str2, len2) + 1;
int const_str_n = 0;
if (!len1)
@@ -7134,7 +7145,7 @@ inline_expand_builtin_string_cmp (tree exp, rtx target)
gcc_checking_assert (const_str_n > 0);
length = (const_str_n == 1) ? len1 : len2;
- if (is_ncmp && (len3 = tree_to_uhwi (len3_tree)) < length)
+ if (is_ncmp && len3 < length)
length = len3;
/* If the length of the comparision is larger than the threshold,
diff --git a/gcc/testsuite/gcc.dg/pr90892.c b/gcc/testsuite/gcc.dg/pr90892.c
new file mode 100644
index 000..e4b5310807a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr90892.c
@@ -0,0 +1,14 @@
+/* PR tree-optimization/90892 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+const char *a = "A\0b";
+
+int
+main()
+{
+ if (__builtin_strncmp(a, "A\0", 2) != 0)
+__builtin_abort ();
+
+ return 0;
+}
--
2.21.0
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On 6/18/19 11:56 AM, 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, );
const char *src_str2 = c_getstr (arg2, );
+ 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
>
Patch to this. I hope this version of the patch is correct.
Jakub?
Thanks,
Martin
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On 6/18/19 12:33 PM, Jakub Jelinek wrote:
> On Tue, Jun 18, 2019 at 12:27:31PM +0200, Martin Liška wrote:
>>> 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.
>>
>> The function always returns a null-terminated string:
>>
>> 14587 /* Return a pointer P to a NUL-terminated string representing the
>> sequence
>> 14588 of constant characters referred to by SRC (or a subsequence of
>> such
>> 14589 characters within it if SRC is a reference to a string plus some
>> 14590 constant offset). If STRLEN is non-null, store the number of
>> bytes
>> 14591 in the string constant including the terminating NUL char.
>> *STRLEN is
>> 14592 typically strlen(P) + 1 in the absence of embedded NUL
>> characters. */
>> 14593
>> 14594 const char *
>> 14595 c_getstr (tree src, unsigned HOST_WIDE_INT *strlen /* = NULL */)
>> 14596 {
>> 14597tree offset_node;
>> 14598tree mem_size;
>>
>> That said, the unconditional strnlen should be fine.
>
> But *strlen it sets might be smaller.
No, for "A\0" you'll get *strlen == 3, but strlen (returned value) == 1.
>
> I'd try say const char foo[5] = "foobar";
> or similar, or say stick gcc_assert in c_getstr where it is setting
> *strlen and gcc_assert (strnlen (to be returned value, *strlen) < *strlen);
> do a bootstrap/regtest with that and see if it ever triggers (or instead
> of assert failure log into a log file with "a" mode).
>
> If not, there is no point to pass non-NULL second argument to c_getstr,
> you'd always just use strlen on the returned string.
There might be consumers (like folding of memcmp) of c_getstr which want to
know how long is a string constant
even thought there's a null-terminating character in the middle of the constant.
Martin
>
> Jakub
>
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On Tue, Jun 18, 2019 at 12:27:31PM +0200, Martin Liška wrote:
> > 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.
>
> The function always returns a null-terminated string:
>
> 14587 /* Return a pointer P to a NUL-terminated string representing the
> sequence
> 14588 of constant characters referred to by SRC (or a subsequence of such
> 14589 characters within it if SRC is a reference to a string plus some
> 14590 constant offset). If STRLEN is non-null, store the number of bytes
> 14591 in the string constant including the terminating NUL char.
> *STRLEN is
> 14592 typically strlen(P) + 1 in the absence of embedded NUL characters.
> */
> 14593
> 14594 const char *
> 14595 c_getstr (tree src, unsigned HOST_WIDE_INT *strlen /* = NULL */)
> 14596 {
> 14597tree offset_node;
> 14598tree mem_size;
>
> That said, the unconditional strnlen should be fine.
But *strlen it sets might be smaller.
I'd try say const char foo[5] = "foobar";
or similar, or say stick gcc_assert in c_getstr where it is setting
*strlen and gcc_assert (strnlen (to be returned value, *strlen) < *strlen);
do a bootstrap/regtest with that and see if it ever triggers (or instead
of assert failure log into a log file with "a" mode).
If not, there is no point to pass non-NULL second argument to c_getstr,
you'd always just use strlen on the returned string.
Jakub
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On 6/18/19 12:16 PM, Jakub Jelinek wrote:
> 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, );
>>>>>const char *src_str2 = c_getstr (arg2, );
>>>>>
>>>>> + 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
>> 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
>>
>> 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.
The function always returns a null-terminated string:
14587 /* Return a pointer P to a NUL-terminated string representing the
sequence
14588 of constant characters referred to by SRC (or a subsequence of such
14589 characters within it if SRC is a reference to a string plus some
14590 constant offset). If STRLEN is non-null, store the number of bytes
14591 in the string constant including the terminating NUL char. *STRLEN
is
14592 typically strlen(P) + 1 in the absence of embedded NUL characters.
*/
14593
14594 const char *
14595 c_getstr (tree src, unsigned HOST_WIDE_INT *strlen /* = NULL */)
14596 {
14597tree offset_node;
14598tree mem_size;
That said, the unconditional strnlen should be fine.
Martin
> 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
>
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
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, );
> >>>const char *src_str2 = c_getstr (arg2, );
> >>>
> >>> + 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
> 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
>
> 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
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
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, );
>>>const char *src_str2 = c_getstr (arg2, );
>>>
>>> + 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
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
PR tree-optimization/90892
* builtins.c (inline_expand_builtin_string_cmp): Handle '\0'
in string constants.
gcc/testsuite/ChangeLog:
2019-06-17 Martin Liska
PR tree-optimization/90892
* gcc.dg/pr90892.c: New test.
---
gcc/builtins.c | 6 ++
gcc/testsuite/gcc.dg/pr90892.c | 14 ++
2 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr90892.c
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 3463ffb1539..78a4bec9bd0 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -7142,6 +7142,12 @@ inline_expand_builtin_string_cmp (tree exp, rtx target)
const char *src_str1 = c_getstr (arg1, );
const char *src_str2 = c_getstr (arg2, );
+ if (src_str1 != NULL)
+len1 = strnlen (src_str1, len1) + 1;
+
+ if (src_str2 != NULL)
+len2 = strnlen (src_str2, len2) + 1;
+
/* If neither strings is constant string, the call is not qualify. */
if (!src_str1 && !src_str2)
return NULL_RTX;
diff --git a/gcc/testsuite/gcc.dg/pr90892.c b/gcc/testsuite/gcc.dg/pr90892.c
new file mode 100644
index 000..e4b5310807a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr90892.c
@@ -0,0 +1,14 @@
+/* PR tree-optimization/90892 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+const char *a = "A\0b";
+
+int
+main()
+{
+ if (__builtin_strncmp(a, "A\0", 2) != 0)
+__builtin_abort ();
+
+ return 0;
+}
--
2.21.0
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
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, );
>>const char *src_str2 = c_getstr (arg2, );
>>
>> + 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
>From 5450be93ace37d93e700cb077aebe628a9fe0a0e Mon Sep 17 00:00:00 2001
From: Martin Liska
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
PR tree-optimization/90892
* builtins.c (inline_expand_builtin_string_cmp): Handle '\0'
in string constants.
gcc/testsuite/ChangeLog:
2019-06-17 Martin Liska
PR tree-optimization/90892
* gcc.dg/pr90892.c: New test.
---
gcc/builtins.c | 6 ++
gcc/testsuite/gcc.dg/pr90892.c | 14 ++
2 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr90892.c
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 3463ffb1539..24e92b7592b 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -7142,6 +7142,12 @@ inline_expand_builtin_string_cmp (tree exp, rtx target)
const char *src_str1 = c_getstr (arg1, );
const char *src_str2 = c_getstr (arg2, );
+ if (src_str1 != NULL)
+len1 = strnlen (src_str1, len1);
+
+ if (src_str2 != NULL)
+len2 = strnlen (src_str2, len2);
+
/* If neither strings is constant string, the call is not qualify. */
if (!src_str1 && !src_str2)
return NULL_RTX;
diff --git a/gcc/testsuite/gcc.dg/pr90892.c b/gcc/testsuite/gcc.dg/pr90892.c
new file mode 100644
index 000..e4b5310807a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr90892.c
@@ -0,0 +1,14 @@
+/* PR tree-optimization/90892 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+const char *a = "A\0b";
+
+int
+main()
+{
+ if (__builtin_strncmp(a, "A\0", 2) != 0)
+__builtin_abort ();
+
+ return 0;
+}
--
2.21.0
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
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, );
>const char *src_str2 = c_getstr (arg2, );
>
> + 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
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On 6/17/19 11:35 AM, Jakub Jelinek wrote:
> On Mon, Jun 17, 2019 at 11:26:03AM +0200, Martin Liška wrote:
>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>> index 3463ffb1539..917852071b9 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, );
>>const char *src_str2 = c_getstr (arg2, );
>>
>> + if (src_str1 != NULL)
>> +{
>> + unsigned HOST_WIDE_INT str_str1_strlen = strlen (src_str1);
>> + if (str_str1_strlen + 1 < len1)
>> +len1 = str_str1_strlen + 1;
>
> So use strnlen instead of strlen?
> if (src_str1 != NULL)
> len1 = strnlen (src_str1, len1);
> etc.?
Sure, done in attached patch.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
>
> Jakub
>
>From d3bb4fc58dc92057bdb3e0921d6c0ffce1e8d732 Mon Sep 17 00:00:00 2001
From: Martin Liska
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
PR tree-optimization/90892
* builtins.c (inline_expand_builtin_string_cmp): Handle '\0'
in string constants.
gcc/testsuite/ChangeLog:
2019-06-17 Martin Liska
PR tree-optimization/90892
* gcc.dg/pr90892.c: New test.
---
gcc/builtins.c | 14 ++
gcc/testsuite/gcc.dg/pr90892.c | 14 ++
2 files changed, 28 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr90892.c
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, );
const char *src_str2 = c_getstr (arg2, );
+ 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;
+}
+
+ if (src_str2 != NULL)
+{
+ unsigned HOST_WIDE_INT str_str2_strlen = strnlen (src_str2, len2);
+ if (str_str2_strlen + 1 < len2)
+ len2 = str_str2_strlen + 1;
+}
+
/* If neither strings is constant string, the call is not qualify. */
if (!src_str1 && !src_str2)
return NULL_RTX;
diff --git a/gcc/testsuite/gcc.dg/pr90892.c b/gcc/testsuite/gcc.dg/pr90892.c
new file mode 100644
index 000..e4b5310807a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr90892.c
@@ -0,0 +1,14 @@
+/* PR tree-optimization/90892 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+const char *a = "A\0b";
+
+int
+main()
+{
+ if (__builtin_strncmp(a, "A\0", 2) != 0)
+__builtin_abort ();
+
+ return 0;
+}
--
2.21.0
Re: [PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
On Mon, Jun 17, 2019 at 11:26:03AM +0200, Martin Liška wrote:
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 3463ffb1539..917852071b9 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, );
>const char *src_str2 = c_getstr (arg2, );
>
> + if (src_str1 != NULL)
> +{
> + unsigned HOST_WIDE_INT str_str1_strlen = strlen (src_str1);
> + if (str_str1_strlen + 1 < len1)
> + len1 = str_str1_strlen + 1;
So use strnlen instead of strlen?
if (src_str1 != NULL)
len1 = strnlen (src_str1, len1);
etc.?
Jakub
[PATCH] Handle '\0' in strcmp in RTL expansion (PR tree-optimization/90892).
Hi.
The function c_getstr returns string and length of the string.
In inline_expand_builtin_string_cmp, we should consider situations
where a string constant contains a zero character. In that case
we have to shorten len[12].
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
gcc/ChangeLog:
2019-06-17 Martin Liska
PR tree-optimization/90892
* builtins.c (inline_expand_builtin_string_cmp): Handle '\0'
in string constants.
gcc/testsuite/ChangeLog:
2019-06-17 Martin Liska
PR tree-optimization/90892
* gcc.dg/pr90892.c: New test.
---
gcc/builtins.c | 14 ++
gcc/testsuite/gcc.dg/pr90892.c | 14 ++
2 files changed, 28 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr90892.c
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 3463ffb1539..917852071b9 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, );
const char *src_str2 = c_getstr (arg2, );
+ if (src_str1 != NULL)
+{
+ unsigned HOST_WIDE_INT str_str1_strlen = strlen (src_str1);
+ if (str_str1_strlen + 1 < len1)
+ len1 = str_str1_strlen + 1;
+}
+
+ if (src_str2 != NULL)
+{
+ unsigned HOST_WIDE_INT str_str2_strlen = strlen (src_str2);
+ if (str_str2_strlen + 1 < len2)
+ len2 = str_str2_strlen + 1;
+}
+
/* If neither strings is constant string, the call is not qualify. */
if (!src_str1 && !src_str2)
return NULL_RTX;
diff --git a/gcc/testsuite/gcc.dg/pr90892.c b/gcc/testsuite/gcc.dg/pr90892.c
new file mode 100644
index 000..e4b5310807a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr90892.c
@@ -0,0 +1,14 @@
+/* PR tree-optimization/90892 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+const char *a = "A\0b";
+
+int
+main()
+{
+ if (__builtin_strncmp(a, "A\0", 2) != 0)
+__builtin_abort ();
+
+ return 0;
+}
