| Message ID | 59A68270.7010101@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [TESTSUITE] Use strncpy instead of strcpy in testsuite/gcc.dg/memcmp-1.c | expand |
On Wed, 2017-08-30 at 10:16 +0100, Renlin Li wrote: > Hi, > > In test_driver_memcmp function, I found buf1 and buf2 is not properly > terminated with null character. > > In lib_strncmp, strcpy will be called with buf1 and buf2. > The normal implementation of strcpy function has a loop to copy > character from source > to destination one by one until a null character is encountered. > > If the string is not properly terminated, this will cause the strcpy > read/write > memory beyond the boundary. > > Here I changed the strcpy into strncpy to constraint the function to > visit > legal memory only. Hi, Renlin you are correct that it shouldn't be using strcpy because the string may not be null terminated. However I would suggest we use memcpy instead of strncpy. The reason is that cases where there is a null char in the middle of the string test whether the strncmp is properly ignoring what comes after. So how about using this: memcpy(a,str1,SZ); \ memcpy(b,str2,SZ); \ as in the test_memcmp_ part of the macro? Aaron > > Test Okay without any problem. Okay to commit? > > Regard, > Renlin > > > gcc/testsuite/ChangeLog: > > 2017-08-30 Renlin Li <renlin.li@arm.com> > > * gcc.dg/memcmp-1.c (test_strncmp): Use strncpy instead of > strcpy.
Hi Aaron, On 30/08/17 15:37, Aaron Sawdey wrote: > On Wed, 2017-08-30 at 10:16 +0100, Renlin Li wrote: >> Hi, >> > > Hi, > Renlin you are correct that it shouldn't be using strcpy because the > string may not be null terminated. However I would suggest we use > memcpy instead of strncpy. The reason is that cases where there is a > null char in the middle of the string test whether the strncmp is > properly ignoring what comes after. So how about using this: > > memcpy(a,str1,SZ); \ > memcpy(b,str2,SZ); \ > > as in the test_memcmp_ part of the macro? You are right. For strncpy, if there is a null-character before size, the destination will be padded with zeros. memcpy is better than strncpy in this case. Here is the updated patch. Regards, Renlin diff --git a/gcc/testsuite/gcc.dg/memcmp-1.c b/gcc/testsuite/gcc.dg/memcmp-1.c index 828a0ca..d258354 100644 --- a/gcc/testsuite/gcc.dg/memcmp-1.c +++ b/gcc/testsuite/gcc.dg/memcmp-1.c @@ -110,8 +110,8 @@ static void test_strncmp_ ## SZ ## _ ## ALIGN (const char *str1, const char *str { \ a = three+i*ALIGN+j*(4096-2*i*ALIGN); \ b = four+i*ALIGN+j*(4096-2*i*ALIGN); \ - strcpy(a,str1); \ - strcpy(b,str2); \ + memcpy(a,str1,SZ); \ + memcpy(b,str2,SZ); \ r = strncmp(a,b,SZ); \ if ( r < 0 && !(expect < 0) ) abort(); \ if ( r > 0 && !(expect > 0) ) abort(); \
On Aug 30, 2017, at 8:31 AM, Renlin Li <renlin.li@foss.arm.com> wrote: > memcpy is better than strncpy in this case. > Here is the updated patch. Ok.
diff --git a/gcc/testsuite/gcc.dg/memcmp-1.c b/gcc/testsuite/gcc.dg/memcmp-1.c index 828a0ca..d258354 100644 --- a/gcc/testsuite/gcc.dg/memcmp-1.c +++ b/gcc/testsuite/gcc.dg/memcmp-1.c @@ -110,8 +110,8 @@ static void test_strncmp_ ## SZ ## _ ## ALIGN (const char *str1, const char *str { \ a = three+i*ALIGN+j*(4096-2*i*ALIGN); \ b = four+i*ALIGN+j*(4096-2*i*ALIGN); \ - strcpy(a,str1); \ - strcpy(b,str2); \ + strncpy(a,str1,SZ); \ + strncpy(b,str2,SZ); \ r = strncmp(a,b,SZ); \ if ( r < 0 && !(expect < 0) ) abort(); \ if ( r > 0 && !(expect > 0) ) abort(); \