| Message ID | 20210512063345.2269779-1-stli@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix stringop-overflow warning in bug-regex19.c. | expand |
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 (®buf, 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; >
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 (®buf, 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; >> >
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
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 >
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;
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 (®buf, 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(+)