| Message ID | 20191121171159.21853-1-dimitar@dinux.eu |
|---|---|
| State | New |
| Headers | show |
| Series | testsuite: Fix array size in gcc.dg/strlenopt-66.c | expand |
On 11/21/19 10:11 AM, Dimitar Dimitrov wrote: > One of the passed arguments is actually a string with size 4 ("123"). > Adjust the destination buffer accordingly. > > gcc/testsuite/ChangeLog: > > 2019-11-21 Dimitar Dimitrov <dimitar@dinux.eu> > > * gcc.dg/strlenopt-66.c (test_strncmp_a4_cond_a5_a3_n): Fix array size. > > Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu> > --- > gcc/testsuite/gcc.dg/strlenopt-66.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.dg/strlenopt-66.c b/gcc/testsuite/gcc.dg/strlenopt-66.c > index 4ba31a845b0..3de22c18f4f 100644 > --- a/gcc/testsuite/gcc.dg/strlenopt-66.c > +++ b/gcc/testsuite/gcc.dg/strlenopt-66.c > @@ -88,7 +88,7 @@ __attribute__ ((noclone, noinline, noipa)) void > test_strncmp_a4_cond_a5_a3_n (const char *s1, const char *s2, const char *s3, > int i, unsigned n) > { > - char a3[3], a4[4], a5[5]; > + char a3[4], a4[4], a5[5]; That does look like a mistake. Thanks for bringing it up! The purpose of the test is to exercise strncmp calls whose first two arguments involve arrays of all different sizes (and strings of different lengths stored in them). In this function, the operands of the conditional expression should also be of different sizes than the first argument: one should be smaller and the other bigger. So to keep the test doing what it's meant to do I think we need to change lengths of the strings passed to the function to fit the arrays rather than the sizes of the locals. (Adding even more calls to cover all the permutations of lengths and sizes would be a further improvement.) If this sounds too elaborate let me know and I'll fix the test. Martin > strcpy (a3, s1); > strcpy (a4, s2); > strcpy (a5, s3); >
On Thu, 21 Nov 2019, 20:09:23 EET Martin Sebor wrote: > On 11/21/19 10:11 AM, Dimitar Dimitrov wrote: > > One of the passed arguments is actually a string with size 4 ("123"). > > Adjust the destination buffer accordingly. > > > > gcc/testsuite/ChangeLog: > > > > 2019-11-21 Dimitar Dimitrov <dimitar@dinux.eu> > > > > * gcc.dg/strlenopt-66.c (test_strncmp_a4_cond_a5_a3_n): Fix array size. > > > > Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu> > > --- > > > > gcc/testsuite/gcc.dg/strlenopt-66.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/gcc/testsuite/gcc.dg/strlenopt-66.c > > b/gcc/testsuite/gcc.dg/strlenopt-66.c index 4ba31a845b0..3de22c18f4f > > 100644 > > --- a/gcc/testsuite/gcc.dg/strlenopt-66.c > > +++ b/gcc/testsuite/gcc.dg/strlenopt-66.c > > @@ -88,7 +88,7 @@ __attribute__ ((noclone, noinline, noipa)) void > > > > test_strncmp_a4_cond_a5_a3_n (const char *s1, const char *s2, const char > > *s3,> > > int i, unsigned n) > > > > { > > > > - char a3[3], a4[4], a5[5]; > > + char a3[4], a4[4], a5[5]; > > That does look like a mistake. Thanks for bringing it up! > > The purpose of the test is to exercise strncmp calls whose first > two arguments involve arrays of all different sizes (and strings > of different lengths stored in them). In this function, > the operands of the conditional expression should also be of > different sizes than the first argument: one should be smaller > and the other bigger. > > So to keep the test doing what it's meant to do I think we need > to change lengths of the strings passed to the function to fit > the arrays rather than the sizes of the locals. (Adding even > more calls to cover all the permutations of lengths and sizes > would be a further improvement.) > > If this sounds too elaborate let me know and I'll fix the test. > > Martin Hi, I admit I'm a bit confused, so I'll let you fix the test as you see fit. With "noipa" function attribute the compiler should not be able to propagate the string constants, so a3/a4/a5 string lengths are unknown when strncmp is invoked. I don't understand how PR90626 would take effect in such case. Also, I fail to see how the different sizes of local arrays influence the test. Regards, Dimitar
On 11/21/19 2:24 PM, Dimitar Dimitrov wrote: > On Thu, 21 Nov 2019, 20:09:23 EET Martin Sebor wrote: >> On 11/21/19 10:11 AM, Dimitar Dimitrov wrote: >>> One of the passed arguments is actually a string with size 4 ("123"). >>> Adjust the destination buffer accordingly. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2019-11-21 Dimitar Dimitrov <dimitar@dinux.eu> >>> >>> * gcc.dg/strlenopt-66.c (test_strncmp_a4_cond_a5_a3_n): Fix array size. >>> >>> Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu> >>> --- >>> >>> gcc/testsuite/gcc.dg/strlenopt-66.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-66.c >>> b/gcc/testsuite/gcc.dg/strlenopt-66.c index 4ba31a845b0..3de22c18f4f >>> 100644 >>> --- a/gcc/testsuite/gcc.dg/strlenopt-66.c >>> +++ b/gcc/testsuite/gcc.dg/strlenopt-66.c >>> @@ -88,7 +88,7 @@ __attribute__ ((noclone, noinline, noipa)) void >>> >>> test_strncmp_a4_cond_a5_a3_n (const char *s1, const char *s2, const char >>> *s3,> >>> int i, unsigned n) >>> >>> { >>> >>> - char a3[3], a4[4], a5[5]; >>> + char a3[4], a4[4], a5[5]; >> >> That does look like a mistake. Thanks for bringing it up! >> >> The purpose of the test is to exercise strncmp calls whose first >> two arguments involve arrays of all different sizes (and strings >> of different lengths stored in them). In this function, >> the operands of the conditional expression should also be of >> different sizes than the first argument: one should be smaller >> and the other bigger. >> >> So to keep the test doing what it's meant to do I think we need >> to change lengths of the strings passed to the function to fit >> the arrays rather than the sizes of the locals. (Adding even >> more calls to cover all the permutations of lengths and sizes >> would be a further improvement.) >> >> If this sounds too elaborate let me know and I'll fix the test. >> >> Martin > Hi, > > I admit I'm a bit confused, so I'll let you fix the test as you see fit. > > With "noipa" function attribute the compiler should not be able to propagate > the string constants, so a3/a4/a5 string lengths are unknown when strncmp is > invoked. I don't understand how PR90626 would take effect in such case. The optimization doesn't take effect but the lengths can be assumed to be in the range of the arrays (e.g., the length of a3 can be assumed to be less than 3). The test tries to verify that the comparison of the strncmp call with zero doesn't get folded into false solely based on the ranges of the lengths. It's probably not a the best test. > Also, I fail to see how the different sizes of local arrays influence the > test. They shouldn't influence the outcome of the test but they are considered by the strncmp optimization (and others that deal with strings) because they determine the ranges of lengths of the strings that can be stored in them. I committed r278608 with the fix and added a few more cases. Martin
diff --git a/gcc/testsuite/gcc.dg/strlenopt-66.c b/gcc/testsuite/gcc.dg/strlenopt-66.c index 4ba31a845b0..3de22c18f4f 100644 --- a/gcc/testsuite/gcc.dg/strlenopt-66.c +++ b/gcc/testsuite/gcc.dg/strlenopt-66.c @@ -88,7 +88,7 @@ __attribute__ ((noclone, noinline, noipa)) void test_strncmp_a4_cond_a5_a3_n (const char *s1, const char *s2, const char *s3, int i, unsigned n) { - char a3[3], a4[4], a5[5]; + char a3[4], a4[4], a5[5]; strcpy (a3, s1); strcpy (a4, s2); strcpy (a5, s3);
One of the passed arguments is actually a string with size 4 ("123"). Adjust the destination buffer accordingly. gcc/testsuite/ChangeLog: 2019-11-21 Dimitar Dimitrov <dimitar@dinux.eu> * gcc.dg/strlenopt-66.c (test_strncmp_a4_cond_a5_a3_n): Fix array size. Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu> --- gcc/testsuite/gcc.dg/strlenopt-66.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)