* [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).