* [PATCH] Suppress -Wcast-qual warnings in bsearch
@ 2021-05-19 14:14 Jonathan Wakely
2021-05-19 15:37 ` Joseph Myers
0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Wakely @ 2021-05-19 14:14 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 555 bytes --]
This fixes these GCC warnings when -Wsystem-headers is used:
/usr/include/bits/stdlib-bsearch.h: In function ‘bsearch’:
/usr/include/bits/stdlib-bsearch.h:32:13: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
32 | __p = (void *) (((const char *) __base) + (__idx * __size));
| ^
/usr/include/bits/stdlib-bsearch.h:39:16: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
39 | return (void *) __p;
| ^
OK to push?
[-- Attachment #2: bsearch.patch --]
[-- Type: text/plain, Size: 1208 bytes --]
commit 2e988862a07b75b433468cfd591a8acb1ab4f0af
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Wed May 19 14:58:50 2021 +0100
Suppress -Wcast-qual warnings in bsearch
The first cast to (void *) is redundant but should be (const void *)
anyway, because that's the type of the lvalue being assigned to.
The second cast is necessary and intentionally not const-correct, so
tell the compiler not to warn about it.
diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h
index 4132dc6af0..a08ad34ed6 100644
--- a/bits/stdlib-bsearch.h
+++ b/bits/stdlib-bsearch.h
@@ -29,14 +29,17 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size,
while (__l < __u)
{
__idx = (__l + __u) / 2;
- __p = (void *) (((const char *) __base) + (__idx * __size));
+ __p = (const void *) (((const char *) __base) + (__idx * __size));
__comparison = (*__compar) (__key, __p);
if (__comparison < 0)
__u = __idx;
else if (__comparison > 0)
__l = __idx + 1;
else
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-qual"
return (void *) __p;
+#pragma GCC diagnostic pop
}
return NULL;
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] Suppress -Wcast-qual warnings in bsearch 2021-05-19 14:14 [PATCH] Suppress -Wcast-qual warnings in bsearch Jonathan Wakely @ 2021-05-19 15:37 ` Joseph Myers 2021-05-19 15:50 ` Jonathan Wakely 0 siblings, 1 reply; 16+ messages in thread From: Joseph Myers @ 2021-05-19 15:37 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libc-alpha On Wed, 19 May 2021, Jonathan Wakely via Libc-alpha wrote: > +#pragma GCC diagnostic push > +#pragma GCC diagnostic ignored "-Wcast-qual" > return (void *) __p; > +#pragma GCC diagnostic pop I think such pragma uses in installed headers should be conditional on __GNUC_PREREQ (4, 6) (either directly or via conditionally defining a macro in sys/cdefs.h). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Suppress -Wcast-qual warnings in bsearch 2021-05-19 15:37 ` Joseph Myers @ 2021-05-19 15:50 ` Jonathan Wakely 2021-06-01 10:09 ` [PATCH v3] " Jonathan Wakely 2021-09-30 9:22 ` [PATCH v2] " Jonathan Wakely 0 siblings, 2 replies; 16+ messages in thread From: Jonathan Wakely @ 2021-05-19 15:50 UTC (permalink / raw) To: Joseph Myers; +Cc: libc-alpha [-- Attachment #1: Type: text/plain, Size: 691 bytes --] On 19/05/21 15:37 +0000, Joseph Myers wrote: >On Wed, 19 May 2021, Jonathan Wakely via Libc-alpha wrote: > >> +#pragma GCC diagnostic push >> +#pragma GCC diagnostic ignored "-Wcast-qual" >> return (void *) __p; >> +#pragma GCC diagnostic pop > >I think such pragma uses in installed headers should be conditional on >__GNUC_PREREQ (4, 6) (either directly or via conditionally defining a >macro in sys/cdefs.h). Good point. I spent about two minutes trying to do something with _Pragma in sys/cdefs.h to allow: __GLIBC_IGNORE_WARNING("-Wcast-qual") return (void *) __p; __GLIBC_UNIGNORE_WARNING but didn't get it working, so here's a patch that just tests __GNUC_PREREQ directly. [-- Attachment #2: bsearch.patch --] [-- Type: text/plain, Size: 1277 bytes --] commit ee7c2451a102b4cd3c87a31b3156ad4458729840 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed May 19 16:48:19 2021 +0100 Suppress -Wcast-qual warnings in bsearch The first cast to (void *) is redundant but should be (const void *) anyway, because that's the type of the lvalue being assigned to. The second cast is necessary and intentionally not const-correct, so tell the compiler not to warn about it. diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h index 4132dc6af0..d688ed2e15 100644 --- a/bits/stdlib-bsearch.h +++ b/bits/stdlib-bsearch.h @@ -29,14 +29,21 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size, while (__l < __u) { __idx = (__l + __u) / 2; - __p = (void *) (((const char *) __base) + (__idx * __size)); + __p = (const void *) (((const char *) __base) + (__idx * __size)); __comparison = (*__compar) (__key, __p); if (__comparison < 0) __u = __idx; else if (__comparison > 0) __l = __idx + 1; else +#if __GNUC_PREREQ(4, 6) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wcast-qual" +#endif return (void *) __p; +#if __GNUC_PREREQ(4, 6) +# pragma GCC diagnostic pop +#endif } return NULL; ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] Suppress -Wcast-qual warnings in bsearch 2021-05-19 15:50 ` Jonathan Wakely @ 2021-06-01 10:09 ` Jonathan Wakely 2021-09-30 9:22 ` [PATCH v2] " Jonathan Wakely 1 sibling, 0 replies; 16+ messages in thread From: Jonathan Wakely @ 2021-06-01 10:09 UTC (permalink / raw) To: libc-alpha; +Cc: Joseph Myers, carlos, dj [-- Attachment #1: Type: text/plain, Size: 835 bytes --] On 19/05/21 16:50 +0100, Jonathan Wakely wrote: >On 19/05/21 15:37 +0000, Joseph Myers wrote: >>On Wed, 19 May 2021, Jonathan Wakely via Libc-alpha wrote: >> >>>+#pragma GCC diagnostic push >>>+#pragma GCC diagnostic ignored "-Wcast-qual" >>> return (void *) __p; >>>+#pragma GCC diagnostic pop >> >>I think such pragma uses in installed headers should be conditional on >>__GNUC_PREREQ (4, 6) (either directly or via conditionally defining a >>macro in sys/cdefs.h). > >Good point. > >I spent about two minutes trying to do something with _Pragma in >sys/cdefs.h to allow: > >__GLIBC_IGNORE_WARNING("-Wcast-qual") > return (void *) __p; >__GLIBC_UNIGNORE_WARNING > >but didn't get it working, so here's a patch that just tests >__GNUC_PREREQ directly. Resending patch v2 with a modified subject, to see if patchwork picks it up. [-- Attachment #2: bsearch.patch --] [-- Type: text/plain, Size: 1277 bytes --] commit ee7c2451a102b4cd3c87a31b3156ad4458729840 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed May 19 16:48:19 2021 +0100 Suppress -Wcast-qual warnings in bsearch The first cast to (void *) is redundant but should be (const void *) anyway, because that's the type of the lvalue being assigned to. The second cast is necessary and intentionally not const-correct, so tell the compiler not to warn about it. diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h index 4132dc6af0..d688ed2e15 100644 --- a/bits/stdlib-bsearch.h +++ b/bits/stdlib-bsearch.h @@ -29,14 +29,21 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size, while (__l < __u) { __idx = (__l + __u) / 2; - __p = (void *) (((const char *) __base) + (__idx * __size)); + __p = (const void *) (((const char *) __base) + (__idx * __size)); __comparison = (*__compar) (__key, __p); if (__comparison < 0) __u = __idx; else if (__comparison > 0) __l = __idx + 1; else +#if __GNUC_PREREQ(4, 6) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wcast-qual" +#endif return (void *) __p; +#if __GNUC_PREREQ(4, 6) +# pragma GCC diagnostic pop +#endif } return NULL; ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-05-19 15:50 ` Jonathan Wakely 2021-06-01 10:09 ` [PATCH v3] " Jonathan Wakely @ 2021-09-30 9:22 ` Jonathan Wakely 2021-09-30 10:43 ` Florian Weimer 1 sibling, 1 reply; 16+ messages in thread From: Jonathan Wakely @ 2021-09-30 9:22 UTC (permalink / raw) To: Joseph Myers; +Cc: GNU C Library [-- Attachment #1: Type: text/plain, Size: 946 bytes --] Ping for patch originally sent in https://sourceware.org/pipermail/libc-alpha/2021-May/126572.html The patch still applies cleanly to current master. On Wed, 19 May 2021 at 16:50, Jonathan Wakely wrote: > > On 19/05/21 15:37 +0000, Joseph Myers wrote: > >On Wed, 19 May 2021, Jonathan Wakely via Libc-alpha wrote: > > > >> +#pragma GCC diagnostic push > >> +#pragma GCC diagnostic ignored "-Wcast-qual" > >> return (void *) __p; > >> +#pragma GCC diagnostic pop > > > >I think such pragma uses in installed headers should be conditional on > >__GNUC_PREREQ (4, 6) (either directly or via conditionally defining a > >macro in sys/cdefs.h). > > Good point. > > I spent about two minutes trying to do something with _Pragma in > sys/cdefs.h to allow: > > __GLIBC_IGNORE_WARNING("-Wcast-qual") > return (void *) __p; > __GLIBC_UNIGNORE_WARNING > > but didn't get it working, so here's a patch that just tests > __GNUC_PREREQ directly. > > [-- Attachment #2: bsearch.patch --] [-- Type: application/x-patch, Size: 1316 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 9:22 ` [PATCH v2] " Jonathan Wakely @ 2021-09-30 10:43 ` Florian Weimer 2021-09-30 14:49 ` Adhemerval Zanella 0 siblings, 1 reply; 16+ messages in thread From: Florian Weimer @ 2021-09-30 10:43 UTC (permalink / raw) To: Jonathan Wakely via Libc-alpha; +Cc: Joseph Myers, Jonathan Wakely * Jonathan Wakely via Libc-alpha: > diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h > index 4132dc6af0..d688ed2e15 100644 > --- a/bits/stdlib-bsearch.h > +++ b/bits/stdlib-bsearch.h > @@ -29,14 +29,21 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size, > while (__l < __u) > { > __idx = (__l + __u) / 2; > - __p = (void *) (((const char *) __base) + (__idx * __size)); > + __p = (const void *) (((const char *) __base) + (__idx * __size)); > __comparison = (*__compar) (__key, __p); > if (__comparison < 0) > __u = __idx; > else if (__comparison > 0) > __l = __idx + 1; > else > +#if __GNUC_PREREQ(4, 6) > +# pragma GCC diagnostic push > +# pragma GCC diagnostic ignored "-Wcast-qual" > +#endif > return (void *) __p; > +#if __GNUC_PREREQ(4, 6) > +# pragma GCC diagnostic pop > +#endif > } > > return NULL; Patch looks okay, thanks. Reviewed-by: Florian Weimer <fweimer@redhat.com> Florian ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 10:43 ` Florian Weimer @ 2021-09-30 14:49 ` Adhemerval Zanella 2021-09-30 15:07 ` Joseph Myers ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Adhemerval Zanella @ 2021-09-30 14:49 UTC (permalink / raw) To: libc-alpha, Jonathan Wakely, Joseph Myers On 30/09/2021 07:43, Florian Weimer via Libc-alpha wrote: > * Jonathan Wakely via Libc-alpha: > >> diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h >> index 4132dc6af0..d688ed2e15 100644 >> --- a/bits/stdlib-bsearch.h >> +++ b/bits/stdlib-bsearch.h >> @@ -29,14 +29,21 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size, >> while (__l < __u) >> { >> __idx = (__l + __u) / 2; >> - __p = (void *) (((const char *) __base) + (__idx * __size)); >> + __p = (const void *) (((const char *) __base) + (__idx * __size)); >> __comparison = (*__compar) (__key, __p); >> if (__comparison < 0) >> __u = __idx; >> else if (__comparison > 0) >> __l = __idx + 1; >> else >> +#if __GNUC_PREREQ(4, 6) >> +# pragma GCC diagnostic push >> +# pragma GCC diagnostic ignored "-Wcast-qual" >> +#endif >> return (void *) __p; >> +#if __GNUC_PREREQ(4, 6) >> +# pragma GCC diagnostic pop >> +#endif >> } >> >> return NULL; > > Patch looks okay, thanks. > > Reviewed-by: Florian Weimer <fweimer@redhat.com> > > Florian > I am seeing a lot of failure on x86_64 with gcc 11.1 after this patch landed: x86_64-linux-gnu$ grep ^FAIL tests.sum FAIL: catgets/de/libc.cat FAIL: catgets/test-gencat FAIL: catgets/test1.cat FAIL: catgets/tst-catgets FAIL: debug/tst-chk1 FAIL: debug/tst-chk2 FAIL: debug/tst-chk3 FAIL: debug/tst-chk4 FAIL: debug/tst-chk5 FAIL: debug/tst-chk6 FAIL: debug/tst-lfschk1 FAIL: debug/tst-lfschk2 FAIL: debug/tst-lfschk3 FAIL: debug/tst-lfschk4 FAIL: debug/tst-lfschk5 FAIL: debug/tst-lfschk6 [...] For instance some math tests shows ulps failures that does not make much sense: $ ./testrun.sh math/test-double-cacos testing double (without inline functions) Failure: Test: Imaginary part of: cacos_downward (-0x1p-52 - 0x1.0000000000001p+0 i) Result: is: 8.8137358701954271e-01 0x1.c34366179d424p-1 should be: 8.8137358701954315e-01 0x1.c34366179d428p-1 difference: 4.4408920985006261e-16 0x1.0000000000000p-51 ulp : 4.0000 max.ulp : 3.0000 Failure: Test: Imaginary part of: cacos_downward (-0x1p-52 - 0x1.000002p+0 i) Result: is: 8.8137367131323707e-01 0x1.c34368ebb10d9p-1 should be: 8.8137367131323751e-01 0x1.c34368ebb10ddp-1 difference: 4.4408920985006261e-16 0x1.0000000000000p-51 ulp : 4.0000 max.ulp : 3.0000 [...] Reverting fixes it. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 14:49 ` Adhemerval Zanella @ 2021-09-30 15:07 ` Joseph Myers 2021-09-30 15:19 ` Adhemerval Zanella 2021-09-30 16:52 ` Jonathan Wakely 2021-09-30 17:54 ` H.J. Lu 2 siblings, 1 reply; 16+ messages in thread From: Joseph Myers @ 2021-09-30 15:07 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha, Jonathan Wakely On Thu, 30 Sep 2021, Adhemerval Zanella via Libc-alpha wrote: > >> else > >> +#if __GNUC_PREREQ(4, 6) > >> +# pragma GCC diagnostic push > >> +# pragma GCC diagnostic ignored "-Wcast-qual" > >> +#endif > >> return (void *) __p; > >> +#if __GNUC_PREREQ(4, 6) > >> +# pragma GCC diagnostic pop > >> +#endif > >> } I think braces may need adding around those pragmas to avoid a pragma being considered the body of the else in some cases. > For instance some math tests shows ulps failures that does not > make much sense: libm-test-support.c does use bsearch (in order to look up ulps by name in a table). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 15:07 ` Joseph Myers @ 2021-09-30 15:19 ` Adhemerval Zanella 2021-09-30 15:30 ` Andreas Schwab ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Adhemerval Zanella @ 2021-09-30 15:19 UTC (permalink / raw) To: Joseph Myers; +Cc: libc-alpha, Jonathan Wakely On 30/09/2021 12:07, Joseph Myers wrote: > On Thu, 30 Sep 2021, Adhemerval Zanella via Libc-alpha wrote: > >>>> else >>>> +#if __GNUC_PREREQ(4, 6) >>>> +# pragma GCC diagnostic push >>>> +# pragma GCC diagnostic ignored "-Wcast-qual" >>>> +#endif >>>> return (void *) __p; >>>> +#if __GNUC_PREREQ(4, 6) >>>> +# pragma GCC diagnostic pop >>>> +#endif >>>> } > > I think braces may need adding around those pragmas to avoid a pragma > being considered the body of the else in some cases. But how exactly the pragma is changing the code semantic in this case? Would it be safe for all supported gcc (since it is an installed header)? I wonder if you would be better to follow what we did for inline string micro-optimizations and just remove this header. > >> For instance some math tests shows ulps failures that does not >> make much sense: > > libm-test-support.c does use bsearch (in order to look up ulps by name in > a table). > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 15:19 ` Adhemerval Zanella @ 2021-09-30 15:30 ` Andreas Schwab 2021-09-30 16:41 ` Florian Weimer 2021-09-30 16:42 ` Joseph Myers 2 siblings, 0 replies; 16+ messages in thread From: Andreas Schwab @ 2021-09-30 15:30 UTC (permalink / raw) To: Adhemerval Zanella via Libc-alpha Cc: Joseph Myers, Adhemerval Zanella, Jonathan Wakely On Sep 30 2021, Adhemerval Zanella via Libc-alpha wrote: > I wonder if you would be better to follow what we did for inline string > micro-optimizations and just remove this header. See https://sourceware.org/pipermail/libc-alpha/2013-January/037308.html for the motivation. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 15:19 ` Adhemerval Zanella 2021-09-30 15:30 ` Andreas Schwab @ 2021-09-30 16:41 ` Florian Weimer 2021-09-30 16:42 ` Joseph Myers 2 siblings, 0 replies; 16+ messages in thread From: Florian Weimer @ 2021-09-30 16:41 UTC (permalink / raw) To: Adhemerval Zanella via Libc-alpha Cc: Joseph Myers, Adhemerval Zanella, Jonathan Wakely * Adhemerval Zanella via Libc-alpha: > On 30/09/2021 12:07, Joseph Myers wrote: >> On Thu, 30 Sep 2021, Adhemerval Zanella via Libc-alpha wrote: >> >>>>> else >>>>> +#if __GNUC_PREREQ(4, 6) >>>>> +# pragma GCC diagnostic push >>>>> +# pragma GCC diagnostic ignored "-Wcast-qual" >>>>> +#endif >>>>> return (void *) __p; >>>>> +#if __GNUC_PREREQ(4, 6) >>>>> +# pragma GCC diagnostic pop >>>>> +#endif >>>>> } >> >> I think braces may need adding around those pragmas to avoid a pragma >> being considered the body of the else in some cases. > > But how exactly the pragma is changing the code semantic in this case? > Would it be safe for all supported gcc (since it is an installed header)? It's parsed as: while (__l < __u) { __idx = (__l + __u) / 2; __p = (const void *) (((const char *) __base) + (__idx * __size)); __comparison = (*__compar) (__key, __p); if (__comparison < 0) __u = __idx; else if (__comparison > 0) __l = __idx + 1; else #if __GNUC_PREREQ(4, 6) # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wcast-qual" #endif return (void *) __p; #if __GNUC_PREREQ(4, 6) # pragma GCC diagnostic pop #endif } See (GCC-pragma) - whether a #pragma is a statement depends on the type of pra <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63326> and: -Wmisleading-indentation should also detect missing indentation <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66298#c3> Sorry, I should have caught this during review (and actually tested the patch). Joseph is right, we need braces here. Thanks, Florian ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 15:19 ` Adhemerval Zanella 2021-09-30 15:30 ` Andreas Schwab 2021-09-30 16:41 ` Florian Weimer @ 2021-09-30 16:42 ` Joseph Myers 2 siblings, 0 replies; 16+ messages in thread From: Joseph Myers @ 2021-09-30 16:42 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: Jonathan Wakely, libc-alpha On Thu, 30 Sep 2021, Adhemerval Zanella via Libc-alpha wrote: > On 30/09/2021 12:07, Joseph Myers wrote: > > On Thu, 30 Sep 2021, Adhemerval Zanella via Libc-alpha wrote: > > > >>>> else > >>>> +#if __GNUC_PREREQ(4, 6) > >>>> +# pragma GCC diagnostic push > >>>> +# pragma GCC diagnostic ignored "-Wcast-qual" > >>>> +#endif > >>>> return (void *) __p; > >>>> +#if __GNUC_PREREQ(4, 6) > >>>> +# pragma GCC diagnostic pop > >>>> +#endif > >>>> } > > > > I think braces may need adding around those pragmas to avoid a pragma > > being considered the body of the else in some cases. > > But how exactly the pragma is changing the code semantic in this case? The effect, when the #pragma is treated as a statement by the C parser, is that the prior cases of the "if" fall through to the "return", which becomes unconditional, when otherwise they would not have returned. > Would it be safe for all supported gcc (since it is an installed header)? The #if conditionals are still needed. It's just that braces should be added before the first #if and after the last #endif, to avoid a single #pragma being considered as the else body (see GCC bug 41517 and other bugs related to #pragma parsing). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 14:49 ` Adhemerval Zanella 2021-09-30 15:07 ` Joseph Myers @ 2021-09-30 16:52 ` Jonathan Wakely 2021-09-30 17:54 ` H.J. Lu 2 siblings, 0 replies; 16+ messages in thread From: Jonathan Wakely @ 2021-09-30 16:52 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: GNU C Library, Joseph Myers [-- Attachment #1: Type: text/plain, Size: 2792 bytes --] On Thu, 30 Sept 2021 at 15:49, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > > On 30/09/2021 07:43, Florian Weimer via Libc-alpha wrote: > > * Jonathan Wakely via Libc-alpha: > > > >> diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h > >> index 4132dc6af0..d688ed2e15 100644 > >> --- a/bits/stdlib-bsearch.h > >> +++ b/bits/stdlib-bsearch.h > >> @@ -29,14 +29,21 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size, > >> while (__l < __u) > >> { > >> __idx = (__l + __u) / 2; > >> - __p = (void *) (((const char *) __base) + (__idx * __size)); > >> + __p = (const void *) (((const char *) __base) + (__idx * __size)); > >> __comparison = (*__compar) (__key, __p); > >> if (__comparison < 0) > >> __u = __idx; > >> else if (__comparison > 0) > >> __l = __idx + 1; > >> else > >> +#if __GNUC_PREREQ(4, 6) > >> +# pragma GCC diagnostic push > >> +# pragma GCC diagnostic ignored "-Wcast-qual" > >> +#endif > >> return (void *) __p; > >> +#if __GNUC_PREREQ(4, 6) > >> +# pragma GCC diagnostic pop > >> +#endif > >> } > >> > >> return NULL; > > > > Patch looks okay, thanks. > > > > Reviewed-by: Florian Weimer <fweimer@redhat.com> > > > > Florian > > > > I am seeing a lot of failure on x86_64 with gcc 11.1 after this > patch landed: > > x86_64-linux-gnu$ grep ^FAIL tests.sum > FAIL: catgets/de/libc.cat > FAIL: catgets/test-gencat > FAIL: catgets/test1.cat > FAIL: catgets/tst-catgets > FAIL: debug/tst-chk1 > FAIL: debug/tst-chk2 > FAIL: debug/tst-chk3 > FAIL: debug/tst-chk4 > FAIL: debug/tst-chk5 > FAIL: debug/tst-chk6 > FAIL: debug/tst-lfschk1 > FAIL: debug/tst-lfschk2 > FAIL: debug/tst-lfschk3 > FAIL: debug/tst-lfschk4 > FAIL: debug/tst-lfschk5 > FAIL: debug/tst-lfschk6 > [...] > > For instance some math tests shows ulps failures that does not > make much sense: > > $ ./testrun.sh math/test-double-cacos > testing double (without inline functions) > Failure: Test: Imaginary part of: cacos_downward (-0x1p-52 - 0x1.0000000000001p+0 i) > Result: > is: 8.8137358701954271e-01 0x1.c34366179d424p-1 > should be: 8.8137358701954315e-01 0x1.c34366179d428p-1 > difference: 4.4408920985006261e-16 0x1.0000000000000p-51 > ulp : 4.0000 > max.ulp : 3.0000 > Failure: Test: Imaginary part of: cacos_downward (-0x1p-52 - 0x1.000002p+0 i) > Result: > is: 8.8137367131323707e-01 0x1.c34368ebb10d9p-1 > should be: 8.8137367131323751e-01 0x1.c34368ebb10ddp-1 > difference: 4.4408920985006261e-16 0x1.0000000000000p-51 > ulp : 4.0000 > max.ulp : 3.0000 > [...] > > Reverting fixes it. Huh, I wonder why I didn't see these. Sorry. I'll see if Joseph's suggestion helps (as attached) or I'll revert it. [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 1049 bytes --] commit f1763cd2d3b77582309288198e56de5d883b425a Author: Jonathan Wakely <jwakely@redhat.com> Date: Thu Sep 30 17:24:54 2021 +0100 stdlib: Add braces around return statement in bsearch The diagnostic pragmas are apparently interpreted as the statement following the else, so add braces around them to create a block statement containing the pragmas and the return statement. Signed-off-by: Jonathan Wakely <jwakely@redhat.com> diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h index d688ed2e15..c4b485edf8 100644 --- a/bits/stdlib-bsearch.h +++ b/bits/stdlib-bsearch.h @@ -36,14 +36,16 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size, else if (__comparison > 0) __l = __idx + 1; else + { #if __GNUC_PREREQ(4, 6) # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wcast-qual" #endif - return (void *) __p; + return (void *) __p; #if __GNUC_PREREQ(4, 6) # pragma GCC diagnostic pop #endif + } } return NULL; ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 14:49 ` Adhemerval Zanella 2021-09-30 15:07 ` Joseph Myers 2021-09-30 16:52 ` Jonathan Wakely @ 2021-09-30 17:54 ` H.J. Lu 2021-09-30 18:04 ` Florian Weimer 2 siblings, 1 reply; 16+ messages in thread From: H.J. Lu @ 2021-09-30 17:54 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: GNU C Library, Jonathan Wakely, Joseph Myers On Thu, Sep 30, 2021 at 7:49 AM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > > On 30/09/2021 07:43, Florian Weimer via Libc-alpha wrote: > > * Jonathan Wakely via Libc-alpha: > > > >> diff --git a/bits/stdlib-bsearch.h b/bits/stdlib-bsearch.h > >> index 4132dc6af0..d688ed2e15 100644 > >> --- a/bits/stdlib-bsearch.h > >> +++ b/bits/stdlib-bsearch.h > >> @@ -29,14 +29,21 @@ bsearch (const void *__key, const void *__base, size_t __nmemb, size_t __size, > >> while (__l < __u) > >> { > >> __idx = (__l + __u) / 2; > >> - __p = (void *) (((const char *) __base) + (__idx * __size)); > >> + __p = (const void *) (((const char *) __base) + (__idx * __size)); > >> __comparison = (*__compar) (__key, __p); > >> if (__comparison < 0) > >> __u = __idx; > >> else if (__comparison > 0) > >> __l = __idx + 1; > >> else > >> +#if __GNUC_PREREQ(4, 6) > >> +# pragma GCC diagnostic push > >> +# pragma GCC diagnostic ignored "-Wcast-qual" > >> +#endif > >> return (void *) __p; > >> +#if __GNUC_PREREQ(4, 6) > >> +# pragma GCC diagnostic pop > >> +#endif > >> } > >> > >> return NULL; > > > > Patch looks okay, thanks. > > > > Reviewed-by: Florian Weimer <fweimer@redhat.com> > > > > Florian > > > > I am seeing a lot of failure on x86_64 with gcc 11.1 after this > patch landed: > > x86_64-linux-gnu$ grep ^FAIL tests.sum > FAIL: catgets/de/libc.cat > FAIL: catgets/test-gencat > FAIL: catgets/test1.cat > FAIL: catgets/tst-catgets > FAIL: debug/tst-chk1 > FAIL: debug/tst-chk2 > FAIL: debug/tst-chk3 > FAIL: debug/tst-chk4 > FAIL: debug/tst-chk5 > FAIL: debug/tst-chk6 > FAIL: debug/tst-lfschk1 > FAIL: debug/tst-lfschk2 > FAIL: debug/tst-lfschk3 > FAIL: debug/tst-lfschk4 > FAIL: debug/tst-lfschk5 > FAIL: debug/tst-lfschk6 > [...] > > For instance some math tests shows ulps failures that does not > make much sense: > > $ ./testrun.sh math/test-double-cacos > testing double (without inline functions) > Failure: Test: Imaginary part of: cacos_downward (-0x1p-52 - 0x1.0000000000001p+0 i) > Result: > is: 8.8137358701954271e-01 0x1.c34366179d424p-1 > should be: 8.8137358701954315e-01 0x1.c34366179d428p-1 > difference: 4.4408920985006261e-16 0x1.0000000000000p-51 > ulp : 4.0000 > max.ulp : 3.0000 > Failure: Test: Imaginary part of: cacos_downward (-0x1p-52 - 0x1.000002p+0 i) > Result: > is: 8.8137367131323707e-01 0x1.c34368ebb10d9p-1 > should be: 8.8137367131323751e-01 0x1.c34368ebb10ddp-1 > difference: 4.4408920985006261e-16 0x1.0000000000000p-51 > ulp : 4.0000 > max.ulp : 3.0000 > [...] > > Reverting fixes it. I opened: https://sourceware.org/bugzilla/show_bug.cgi?id=28400 I think the patch should be reverted. -- H.J. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 17:54 ` H.J. Lu @ 2021-09-30 18:04 ` Florian Weimer 2021-09-30 18:09 ` H.J. Lu 0 siblings, 1 reply; 16+ messages in thread From: Florian Weimer @ 2021-09-30 18:04 UTC (permalink / raw) To: H.J. Lu via Libc-alpha Cc: Adhemerval Zanella, H.J. Lu, Jonathan Wakely, Joseph Myers * H. J. Lu via Libc-alpha: > I opened: > > https://sourceware.org/bugzilla/show_bug.cgi?id=28400 > > I think the patch should be reverted. Both Jonathan and I have posted the same fix (with a tabs-vs-spaces) difference. One needs to be reviewed. I can push my variant if someone acks it. Thanks, Florian ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Suppress -Wcast-qual warnings in bsearch 2021-09-30 18:04 ` Florian Weimer @ 2021-09-30 18:09 ` H.J. Lu 0 siblings, 0 replies; 16+ messages in thread From: H.J. Lu @ 2021-09-30 18:09 UTC (permalink / raw) To: Florian Weimer Cc: H.J. Lu via Libc-alpha, Adhemerval Zanella, Jonathan Wakely, Joseph Myers On Thu, Sep 30, 2021 at 11:04 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu via Libc-alpha: > > > I opened: > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=28400 > > > > I think the patch should be reverted. > > Both Jonathan and I have posted the same fix (with a tabs-vs-spaces) > difference. One needs to be reviewed. I can push my variant if someone > acks it. > Please repost your patch with BZ #28400. Thanks. -- H.J. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-09-30 18:10 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-19 14:14 [PATCH] Suppress -Wcast-qual warnings in bsearch Jonathan Wakely 2021-05-19 15:37 ` Joseph Myers 2021-05-19 15:50 ` Jonathan Wakely 2021-06-01 10:09 ` [PATCH v3] " Jonathan Wakely 2021-09-30 9:22 ` [PATCH v2] " Jonathan Wakely 2021-09-30 10:43 ` Florian Weimer 2021-09-30 14:49 ` Adhemerval Zanella 2021-09-30 15:07 ` Joseph Myers 2021-09-30 15:19 ` Adhemerval Zanella 2021-09-30 15:30 ` Andreas Schwab 2021-09-30 16:41 ` Florian Weimer 2021-09-30 16:42 ` Joseph Myers 2021-09-30 16:52 ` Jonathan Wakely 2021-09-30 17:54 ` H.J. Lu 2021-09-30 18:04 ` Florian Weimer 2021-09-30 18:09 ` H.J. Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).