Fix stringop-overflow warning in bug-regex19.c.

Message ID 20210512063345.2269779-1-stli@linux.ibm.com
State New
Headers show
Series
  • Fix stringop-overflow warning in bug-regex19.c.
Related show

Commit Message

Fangrui Song via Libc-alpha May 12, 2021, 6:33 a.m.
Starting with commit
26492c0a14966c32c43cd6ca1d0dca5e62c6cfef
"Annotate additional APIs with GCC attribute access.",
gcc emits this warning on s390x:
In function ‘do_one_test’,
    inlined from ‘do_mb_tests’ at bug-regex19.c:385:11:
bug-regex19.c:271:9: error: ‘re_search’ specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  271 |   res = re_search (&regbuf, test->string, strlen (test->string),
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  272 |      test->start, strlen (test->string) - test->start, NULL);
      |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../include/regex.h:2,
                 from bug-regex19.c:22:
bug-regex19.c: In function ‘do_mb_tests’:
../posix/regex.h:554:17: note: in a call to function ‘re_search’ declared with attribute ‘read_only (2, 3)’
  554 | extern regoff_t re_search (struct re_pattern_buffer *__buffer,
      |                 ^~~~~~~~~
...

The function do_one_test is inlined into do_mb_tests on s390x (at least with
gcc 10).  If do_one_test is marked with __attribute__ ((noinline)), there are
no warnings on s390x. If do_one_test is marked with
__attribute__ ((always_inline)), there are the same warnings on x86_64.

test->string points to a variable length array on stack of do_mb_tests
and the content is generated based on the passed test struct.
---
 posix/bug-regex19.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.30.2

Comments

Fangrui Song via Libc-alpha May 12, 2021, 3:56 p.m. | #1
On 5/12/21 12:33 AM, Stefan Liebler via Libc-alpha wrote:
> Starting with commit

> 26492c0a14966c32c43cd6ca1d0dca5e62c6cfef

> "Annotate additional APIs with GCC attribute access.",

> gcc emits this warning on s390x:

> In function ‘do_one_test’,

>      inlined from ‘do_mb_tests’ at bug-regex19.c:385:11:

> bug-regex19.c:271:9: error: ‘re_search’ specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]

>    271 |   res = re_search (&regbuf, test->string, strlen (test->string),

>        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>    272 |      test->start, strlen (test->string) - test->start, NULL);

>        |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> In file included from ../include/regex.h:2,

>                   from bug-regex19.c:22:

> bug-regex19.c: In function ‘do_mb_tests’:

> ../posix/regex.h:554:17: note: in a call to function ‘re_search’ declared with attribute ‘read_only (2, 3)’

>    554 | extern regoff_t re_search (struct re_pattern_buffer *__buffer,

>        |                 ^~~~~~~~~

> ...

> 

> The function do_one_test is inlined into do_mb_tests on s390x (at least with

> gcc 10).  If do_one_test is marked with __attribute__ ((noinline)), there are

> no warnings on s390x. If do_one_test is marked with

> __attribute__ ((always_inline)), there are the same warnings on x86_64.

> 

> test->string points to a variable length array on stack of do_mb_tests

> and the content is generated based on the passed test struct.


This is a false positive caused by the same bug as the one in
nss/makedb.c.  It's fixed in GCC 11 but the whole change is too
intrusive to backport to 10.  I'll see if I can extract just
the fix itself and backport it.

Disabling inlining for the test function as a workaround seems
reasonable to me (a comment should probably be added mentioning
why it's being done).  An alternative is to suppress the warning
using #pragma GCC diagnostic (as was done in nss/makedb.c).

Martin


> ---

>   posix/bug-regex19.c | 1 +

>   1 file changed, 1 insertion(+)

> 

> diff --git a/posix/bug-regex19.c b/posix/bug-regex19.c

> index 9bbffb17e3..fcae533762 100644

> --- a/posix/bug-regex19.c

> +++ b/posix/bug-regex19.c

> @@ -251,6 +251,7 @@ static struct test_s

>   };

>   

>   int

> +__attribute__ ((noinline))

>   do_one_test (const struct test_s *test, const char *fail)

>   {

>     int res;

>
Fangrui Song via Libc-alpha May 13, 2021, 5:04 p.m. | #2
On 5/12/21 9:56 AM, Martin Sebor wrote:
> On 5/12/21 12:33 AM, Stefan Liebler via Libc-alpha wrote:

>> Starting with commit

>> 26492c0a14966c32c43cd6ca1d0dca5e62c6cfef

>> "Annotate additional APIs with GCC attribute access.",

>> gcc emits this warning on s390x:

>> In function ‘do_one_test’,

>>      inlined from ‘do_mb_tests’ at bug-regex19.c:385:11:

>> bug-regex19.c:271:9: error: ‘re_search’ specified size 

>> 18446744073709551615 exceeds maximum object size 9223372036854775807 

>> [-Werror=stringop-overflow=]

>>    271 |   res = re_search (&regbuf, test->string, strlen (test->string),

>>        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>>    272 |      test->start, strlen (test->string) - test->start, NULL);

>>        |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>> In file included from ../include/regex.h:2,

>>                   from bug-regex19.c:22:

>> bug-regex19.c: In function ‘do_mb_tests’:

>> ../posix/regex.h:554:17: note: in a call to function ‘re_search’ 

>> declared with attribute ‘read_only (2, 3)’

>>    554 | extern regoff_t re_search (struct re_pattern_buffer *__buffer,

>>        |                 ^~~~~~~~~

>> ...

>>

>> The function do_one_test is inlined into do_mb_tests on s390x (at 

>> least with

>> gcc 10).  If do_one_test is marked with __attribute__ ((noinline)), 

>> there are

>> no warnings on s390x. If do_one_test is marked with

>> __attribute__ ((always_inline)), there are the same warnings on x86_64.

>>

>> test->string points to a variable length array on stack of do_mb_tests

>> and the content is generated based on the passed test struct.

> 

> This is a false positive caused by the same bug as the one in

> nss/makedb.c.  It's fixed in GCC 11 but the whole change is too

> intrusive to backport to 10.  I'll see if I can extract just

> the fix itself and backport it.


I did that in r10-9819.  The fix should be in GCC 10.4 if/when it's
released.

Martin

> 

> Disabling inlining for the test function as a workaround seems

> reasonable to me (a comment should probably be added mentioning

> why it's being done).  An alternative is to suppress the warning

> using #pragma GCC diagnostic (as was done in nss/makedb.c).

> 

> Martin

> 

> 

>> ---

>>   posix/bug-regex19.c | 1 +

>>   1 file changed, 1 insertion(+)

>>

>> diff --git a/posix/bug-regex19.c b/posix/bug-regex19.c

>> index 9bbffb17e3..fcae533762 100644

>> --- a/posix/bug-regex19.c

>> +++ b/posix/bug-regex19.c

>> @@ -251,6 +251,7 @@ static struct test_s

>>   };

>>   int

>> +__attribute__ ((noinline))

>>   do_one_test (const struct test_s *test, const char *fail)

>>   {

>>     int res;

>>

>
Fangrui Song via Libc-alpha May 17, 2021, 2:23 p.m. | #3
On 12/05/2021 17:56, Martin Sebor wrote:
> 

> This is a false positive caused by the same bug as the one in

> nss/makedb.c.  It's fixed in GCC 11 but the whole change is too

> intrusive to backport to 10.  I'll see if I can extract just

> the fix itself and backport it.

> 

> Disabling inlining for the test function as a workaround seems

> reasonable to me (a comment should probably be added mentioning

> why it's being done).  An alternative is to suppress the warning

> using #pragma GCC diagnostic (as was done in nss/makedb.c).

> 

> Martin


Thanks for the hint. Then I prefer to use the same approach as for
nss/makedb.c.
Please have a look at V2:
https://sourceware.org/pipermail/libc-alpha/2021-May/126413.html

Thanks,
Stefan
Fangrui Song via Libc-alpha May 17, 2021, 3:22 p.m. | #4
On 5/17/21 8:23 AM, Stefan Liebler wrote:
> On 12/05/2021 17:56, Martin Sebor wrote:

>>

>> This is a false positive caused by the same bug as the one in

>> nss/makedb.c.  It's fixed in GCC 11 but the whole change is too

>> intrusive to backport to 10.  I'll see if I can extract just

>> the fix itself and backport it.

>>

>> Disabling inlining for the test function as a workaround seems

>> reasonable to me (a comment should probably be added mentioning

>> why it's being done).  An alternative is to suppress the warning

>> using #pragma GCC diagnostic (as was done in nss/makedb.c).

>>

>> Martin

> 

> Thanks for the hint. Then I prefer to use the same approach as for

> nss/makedb.c.

> Please have a look at V2:

> https://sourceware.org/pipermail/libc-alpha/2021-May/126413.html


That looks good to me.

Martin

> 

> Thanks,

> Stefan

>

Patch

diff --git a/posix/bug-regex19.c b/posix/bug-regex19.c
index 9bbffb17e3..fcae533762 100644
--- a/posix/bug-regex19.c
+++ b/posix/bug-regex19.c
@@ -251,6 +251,7 @@  static struct test_s
 };
 
 int
+__attribute__ ((noinline))
 do_one_test (const struct test_s *test, const char *fail)
 {
   int res;