public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix const pointer warning in gthr-win32.h
@ 2008-07-11  2:01 Aaron W. LaFramboise
  2008-07-11  4:29 ` Kaveh R. GHAZI
  2008-08-23 18:32 ` [PATCH] Make the pointer parameter to __gthread_setspecific non-const Aaron W. LaFramboise
  0 siblings, 2 replies; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-07-11  2:01 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

This patch fixes a warning about casting away pointer constness.

It was necessary to copy the CONST_CAST definition to tsystem.h since 
this file cannot include system.h.

This patch was tested by a bootstrap on i386-pc-mingw32.

OK to commit?


[-- Attachment #2: gcc-4.4.0-20080710-cast.patch --]
[-- Type: text/plain, Size: 1491 bytes --]

2008-07-10  Aaron W. LaFramboise  <aaronavay62@aaronwl.com>

	* gcc/tsystem.h (CONST_CAST, CONST_CAST2): Duplicate from system.h.
	* gcc/gthr-win32.h (__gthread_setspecific): Use CONST_CAST.

Index: gcc/gthr-win32.h
===================================================================
--- gcc/gthr-win32.h	(revision 137703)
+++ gcc/gthr-win32.h	(working copy)
@@ -612,7 +612,10 @@ __gthread_getspecific (__gthread_key_t k
 static inline int
 __gthread_setspecific (__gthread_key_t key, const void *ptr)
 {
-  return (TlsSetValue (key, (void*) ptr) != 0) ? 0 : (int) GetLastError ();
+  if (TlsSetValue (key, CONST_CAST(void *, ptr)) != 0)
+    return 0;
+  else
+    return GetLastError ();
 }
 
 static inline void
Index: gcc/tsystem.h
===================================================================
--- gcc/tsystem.h	(revision 137703)
+++ gcc/tsystem.h	(working copy)
@@ -131,6 +131,16 @@ extern int errno;
    unreachable default case of a switch.  Do not use gcc_assert(0).  */
 #define gcc_unreachable() (abort ())
 
+/* This is a copy of CONST_CAST from system.h */
+
+#if defined(__GNUC__) && GCC_VERSION != 4000
+/* GCC 4.0.x has a bug where it may ICE on this expression.  */
+#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((__extension__(union {FROMTYPE _q; TOTYPE _nq;})(X))._nq)
+#else
+#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((TOTYPE)(FROMTYPE)(X))
+#endif
+#define CONST_CAST(TYPE,X) CONST_CAST2(TYPE, const TYPE, (X))
+
 /* Filename handling macros.  */
 #include "filenames.h"
 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix const pointer warning in gthr-win32.h
  2008-07-11  2:01 [PATCH] Fix const pointer warning in gthr-win32.h Aaron W. LaFramboise
@ 2008-07-11  4:29 ` Kaveh R. GHAZI
  2008-07-28 23:33   ` Aaron W. LaFramboise
  2008-08-23 18:32 ` [PATCH] Make the pointer parameter to __gthread_setspecific non-const Aaron W. LaFramboise
  1 sibling, 1 reply; 18+ messages in thread
From: Kaveh R. GHAZI @ 2008-07-11  4:29 UTC (permalink / raw)
  To: Aaron W. LaFramboise; +Cc: gcc-patches

On Thu, 10 Jul 2008, Aaron W. LaFramboise wrote:

> --- gcc/tsystem.h	(revision 137703)
> +++ gcc/tsystem.h	(working copy)
> @@ -131,6 +131,16 @@ extern int errno;
>     unreachable default case of a switch.  Do not use gcc_assert(0).  */
>  #define gcc_unreachable() (abort ())
>
> +/* This is a copy of CONST_CAST from system.h */
> +
> +#if defined(__GNUC__) && GCC_VERSION != 4000
> +/* GCC 4.0.x has a bug where it may ICE on this expression.  */
> +#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((__extension__(union {FROMTYPE _q; TOTYPE _nq;})(X))._nq)
> +#else
> +#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((TOTYPE)(FROMTYPE)(X))
> +#endif
> +#define CONST_CAST(TYPE,X) CONST_CAST2(TYPE, const TYPE, (X))
> +
>  /* Filename handling macros.  */
>  #include "filenames.h"


Files including tsystem.h are always compiled with the latest GCC you just
built.  So only the first CONST_CAST2 is needed, you can get rid of the
#ifdef wrapping for version 4.0.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix const pointer warning in gthr-win32.h
  2008-07-11  4:29 ` Kaveh R. GHAZI
@ 2008-07-28 23:33   ` Aaron W. LaFramboise
  2008-07-29 17:21     ` Ian Lance Taylor
  0 siblings, 1 reply; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-07-28 23:33 UTC (permalink / raw)
  To: Kaveh R. GHAZI; +Cc: gcc-patches

Kaveh R. GHAZI wrote:
> On Thu, 10 Jul 2008, Aaron W. LaFramboise wrote:
> 
>> --- gcc/tsystem.h	(revision 137703)
>> +++ gcc/tsystem.h	(working copy)

>> +/* This is a copy of CONST_CAST from system.h */

> Files including tsystem.h are always compiled with the latest GCC you just
> built.  So only the first CONST_CAST2 is needed, you can get rid of the
> #ifdef wrapping for version 4.0.

OK, I discovered this file is also used in other places outside of GCC 
such as in libgfortran, so using tsystem.h is not OK there.  What is the  
proper way to make this error go away then, as I understand the union 
trick is the only way to silence it?


By the way, I think this prototype is actually incorrect.  The ptr
parameter really should not be const, because __gthread_getspecific
returns a non-const void *.  However, I'm not sure if there's some
other reason this should be const; does anyone know about this?


Is it OK just to define this macro right here and use it, like this?

Index: gcc/gthr-win32.h
===================================================================
--- gcc/gthr-win32.h    (revision 138215)
+++ gcc/gthr-win32.h    (working copy)
@@ -609,10 +609,17 @@ __gthread_getspecific (__gthread_key_t k
   return ptr;
 }
 
+/* This is a copy of CONST_CAST from system.h */
+#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((__extension__(union {FROMTYPE _q; \
+  TOTYPE _nq;})(X))._nq)
+
 static inline int
 __gthread_setspecific (__gthread_key_t key, const void *ptr)
 {
-  return (TlsSetValue (key, (void*) ptr) != 0) ? 0 : (int) GetLastError ();
+  if (TlsSetValue (key, CONST_CAST2(void *, const void *, ptr)) != 0)
+    return 0;
+  else
+    return GetLastError ();
 }
 
 static inline void

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix const pointer warning in gthr-win32.h
  2008-07-28 23:33   ` Aaron W. LaFramboise
@ 2008-07-29 17:21     ` Ian Lance Taylor
  2008-07-30 22:01       ` Kaveh R. Ghazi
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Lance Taylor @ 2008-07-29 17:21 UTC (permalink / raw)
  To: Aaron W. LaFramboise; +Cc: Kaveh R. GHAZI, gcc-patches

"Aaron W. LaFramboise" <aaronavay62@aaronwl.com> writes:

> OK, I discovered this file is also used in other places outside of GCC
> such as in libgfortran, so using tsystem.h is not OK there.  What is
> the  proper way to make this error go away then, as I understand the
> union trick is the only way to silence it?
>
>
> By the way, I think this prototype is actually incorrect.  The ptr
> parameter really should not be const, because __gthread_getspecific
> returns a non-const void *.  However, I'm not sure if there's some
> other reason this should be const; does anyone know about this?

What code are you compiling when you see this warning?  As far as I
know code that uses gthr-win32.h is mostly C code which should be
compiled with -Wc++-compat.

Ian

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix const pointer warning in gthr-win32.h
  2008-07-29 17:21     ` Ian Lance Taylor
@ 2008-07-30 22:01       ` Kaveh R. Ghazi
  2008-07-31  4:08         ` Aaron W. LaFramboise
  0 siblings, 1 reply; 18+ messages in thread
From: Kaveh R. Ghazi @ 2008-07-30 22:01 UTC (permalink / raw)
  To: Aaron W. LaFramboise, Ian Lance Taylor; +Cc: gcc-patches

From: "Ian Lance Taylor" <iant@google.com>

> "Aaron W. LaFramboise" <aaronavay62@aaronwl.com> writes:
>
>> OK, I discovered this file is also used in other places outside of GCC
>> such as in libgfortran, so using tsystem.h is not OK there.  What is
>> the  proper way to make this error go away then, as I understand the
>> union trick is the only way to silence it?

Is it possible to fix (i.e. const-ify) the relevant function prototypes, or
do you end up calling a system interface that's non-const and get stuck?


>> By the way, I think this prototype is actually incorrect.  The ptr
>> parameter really should not be const, because __gthread_getspecific
>> returns a non-const void *.  However, I'm not sure if there's some
>> other reason this should be const; does anyone know about this?
>
> What code are you compiling when you see this warning?  As far as I
> know code that uses gthr-win32.h is mostly C code which should be
> compiled with -Wc++-compat.
> Ian

I think the warning comes from -Wcast-qual, not -Wc++-compat.

        --Kaveh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix const pointer warning in gthr-win32.h
  2008-07-30 22:01       ` Kaveh R. Ghazi
@ 2008-07-31  4:08         ` Aaron W. LaFramboise
  0 siblings, 0 replies; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-07-31  4:08 UTC (permalink / raw)
  To: Kaveh R. Ghazi; +Cc: Ian Lance Taylor, gcc-patches

Kaveh R. Ghazi wrote:
> From: "Ian Lance Taylor" <iant@google.com>
> 
>> "Aaron W. LaFramboise" <aaronavay62@aaronwl.com> writes:
>>
>>> OK, I discovered this file is also used in other places outside of GCC
>>> such as in libgfortran, so using tsystem.h is not OK there.  What is
>>> the  proper way to make this error go away then, as I understand the
>>> union trick is the only way to silence it?
> 
> Is it possible to fix (i.e. const-ify) the relevant function prototypes, or
> do you end up calling a system interface that's non-const and get stuck?

No, the system interface is a void *.  And I think that is correct; it 
should not be const void *; the GTHR is incorrect in wanting it to be a 
const void *, because __gthread_getspecific is returning void *, and 
this is the same pointer.  If we're returning a non-const pointer from 
TLS, we need to be saving a non-const pointer, otherwise this doesn't 
make any sense.

I would try just taking out the 'const' but I think this will break a 
lot of platforms I have no way of testing.  But I suppose I could try 
anyway and just deal with the breakage.

So I'm not sure what the proper way to resolve this is.  Advice?

>>> By the way, I think this prototype is actually incorrect.  The ptr
>>> parameter really should not be const, because __gthread_getspecific
>>> returns a non-const void *.  However, I'm not sure if there's some
>>> other reason this should be const; does anyone know about this?
>>
>> What code are you compiling when you see this warning?  As far as I
>> know code that uses gthr-win32.h is mostly C code which should be
>> compiled with -Wc++-compat.

It comes from anything that includes gthr-win32.h.

> I think the warning comes from -Wcast-qual, not -Wc++-compat.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-07-11  2:01 [PATCH] Fix const pointer warning in gthr-win32.h Aaron W. LaFramboise
  2008-07-11  4:29 ` Kaveh R. GHAZI
@ 2008-08-23 18:32 ` Aaron W. LaFramboise
  2008-08-23 18:48   ` Richard Guenther
  1 sibling, 1 reply; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-08-23 18:32 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

Currently on i386-pc-mingw32, casting away constness in 
__gthread_setspecific causes a bootstrap failure with --enable-werror. 
This must be true for a number of other targets which do the same thing.

The ultimate problem here is that there's nothing about this parameter 
that is const: __gthread_getspecific returns a non-const pointer, not a 
const one.

This patch fixes this situation by making this parameter non-const in 
all instances of __gthread_setspecific.

I tested this by bootstrapping on i686-pc-linux and i386-pc-mingw32.

OK to commit?

[-- Attachment #2: gcc-4.4.0-20080823-gthr.patch --]
[-- Type: text/plain, Size: 6647 bytes --]

2008-08-23  Aaron W. LaFramboise  <aaronavay62@aaronwl.com>

	* gcc/gthr-single.h (__gthread_setspecific): Remove const.
	* gcc/gthr-rtems.h (__gthread_setspecific): Same
	* gcc/gthr-mipssde.h (__gthread_setspecific): Same
	* gcc/gthr-nks.h (__gthread_setspecific): Same
	* gcc/gthr.h (__gthread_setspecific): Same
	* gcc/gthr-dce.h (__gthread_setspecific): Same
	* gcc/gthr-tpf.h (__gthread_setspecific): Same
	* gcc/gthr-solaris.h (__gthread_setspecific): Same
	* gcc/gthr-posix.c (__gthread_setspecific): Same
	* gcc/gthr-posix.h (__gthread_setspecific): Same
	* gcc/gthr-win32.h (__gthread_setspecific): Same
	* gcc/gthr-posix95.h (__gthread_setspecific): Same

Index: gcc/gthr-single.h
===================================================================
--- gcc/gthr-single.h	(revision 139510)
+++ gcc/gthr-single.h	(working copy)
@@ -240,7 +240,7 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int 
-__gthread_setspecific (__gthread_key_t key UNUSED, const void *v UNUSED)
+__gthread_setspecific (__gthread_key_t key UNUSED, void *v UNUSED)
 {
   return 0;
 }
Index: gcc/gthr-rtems.h
===================================================================
--- gcc/gthr-rtems.h	(revision 139510)
+++ gcc/gthr-rtems.h	(working copy)
@@ -106,7 +106,7 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
   return rtems_gxx_setspecific (key, ptr);
 }
Index: gcc/gthr-mipssde.h
===================================================================
--- gcc/gthr-mipssde.h	(revision 139510)
+++ gcc/gthr-mipssde.h	(working copy)
@@ -129,7 +129,7 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
   return __gthrw_(__sdethread_setspecific) (key, ptr);
 }
Index: gcc/gthr-nks.h
===================================================================
--- gcc/gthr-nks.h	(revision 139510)
+++ gcc/gthr-nks.h	(working copy)
@@ -328,9 +328,9 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
-  return NXKeySetValue (key, (void *)ptr);
+  return NXKeySetValue (key, ptr);
 }
 
 static inline void
Index: gcc/gthr.h
===================================================================
--- gcc/gthr.h	(revision 139510)
+++ gcc/gthr.h	(working copy)
@@ -71,7 +71,7 @@ Software Foundation, 51 Franklin Street,
      int __gthread_key_delete (__gthread_key_t key)
 
      void *__gthread_getspecific (__gthread_key_t key)
-     int __gthread_setspecific (__gthread_key_t key, const void *ptr)
+     int __gthread_setspecific (__gthread_key_t key, void *ptr)
 
      int __gthread_mutex_destroy (__gthread_mutex_t *mutex);
 
Index: gcc/gthr-dce.h
===================================================================
--- gcc/gthr-dce.h	(revision 139510)
+++ gcc/gthr-dce.h	(working copy)
@@ -463,9 +463,9 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
-  return __gthrw_(pthread_setspecific) (key, (void *) ptr);
+  return __gthrw_(pthread_setspecific) (key, ptr);
 }
 
 static inline void
Index: gcc/gthr-tpf.h
===================================================================
--- gcc/gthr-tpf.h	(revision 139510)
+++ gcc/gthr-tpf.h	(working copy)
@@ -137,7 +137,7 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
   if (__tpf_pthread_active ())
     return __gthrw_(pthread_setspecific) (key, ptr);
Index: gcc/gthr-solaris.h
===================================================================
--- gcc/gthr-solaris.h	(revision 139510)
+++ gcc/gthr-solaris.h	(working copy)
@@ -456,9 +456,9 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
-  return __gthrw_(thr_setspecific) (key, (void *) ptr);
+  return __gthrw_(thr_setspecific) (key, ptr);
 }
 
 static inline int
Index: gcc/gthr-posix.c
===================================================================
--- gcc/gthr-posix.c	(revision 139510)
+++ gcc/gthr-posix.c	(working copy)
@@ -61,7 +61,7 @@ pthread_getspecific (pthread_key_t key A
 
 int
 pthread_setspecific (pthread_key_t key ATTRIBUTE_UNUSED,
-		     const void *ptr ATTRIBUTE_UNUSED)
+		     void *ptr ATTRIBUTE_UNUSED)
 {
   return 0;
 }
Index: gcc/gthr-posix.h
===================================================================
--- gcc/gthr-posix.h	(revision 139510)
+++ gcc/gthr-posix.h	(working copy)
@@ -674,7 +674,7 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
   return __gthrw_(pthread_setspecific) (key, ptr);
 }
Index: gcc/gthr-win32.h
===================================================================
--- gcc/gthr-win32.h	(revision 139510)
+++ gcc/gthr-win32.h	(working copy)
@@ -412,7 +392,7 @@ extern int __gthr_win32_once (__gthread_
 extern int __gthr_win32_key_create (__gthread_key_t *, void (*) (void*));
 extern int __gthr_win32_key_delete (__gthread_key_t);
 extern void * __gthr_win32_getspecific (__gthread_key_t);
-extern int __gthr_win32_setspecific (__gthread_key_t, const void *);
+extern int __gthr_win32_setspecific (__gthread_key_t, void *);
 extern void __gthr_win32_mutex_init_function (__gthread_mutex_t *);
 extern int __gthr_win32_mutex_lock (__gthread_mutex_t *);
 extern int __gthr_win32_mutex_trylock (__gthread_mutex_t *);
@@ -453,7 +433,7 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
   return __gthr_win32_setspecific (key, ptr);
 }
Index: gcc/gthr-posix95.h
===================================================================
--- gcc/gthr-posix95.h	(revision 139510)
+++ gcc/gthr-posix95.h	(working copy)
@@ -637,7 +637,7 @@ __gthread_getspecific (__gthread_key_t k
 }
 
 static inline int
-__gthread_setspecific (__gthread_key_t key, const void *ptr)
+__gthread_setspecific (__gthread_key_t key, void *ptr)
 {
   return __gthrw_(pthread_setspecific) (key, ptr);
 }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-23 18:32 ` [PATCH] Make the pointer parameter to __gthread_setspecific non-const Aaron W. LaFramboise
@ 2008-08-23 18:48   ` Richard Guenther
  2008-08-23 19:42     ` Aaron W. LaFramboise
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Guenther @ 2008-08-23 18:48 UTC (permalink / raw)
  To: Aaron W. LaFramboise; +Cc: gcc-patches

On Sat, Aug 23, 2008 at 7:14 PM, Aaron W. LaFramboise
<aaronavay62@aaronwl.com> wrote:
> Currently on i386-pc-mingw32, casting away constness in
> __gthread_setspecific causes a bootstrap failure with --enable-werror. This
> must be true for a number of other targets which do the same thing.
>
> The ultimate problem here is that there's nothing about this parameter that
> is const: __gthread_getspecific returns a non-const pointer, not a const
> one.
>
> This patch fixes this situation by making this parameter non-const in all
> instances of __gthread_setspecific.
>
> I tested this by bootstrapping on i686-pc-linux and i386-pc-mingw32.
>
> OK to commit?

But pthread_setspecific _does_ take a constant pointer as argument.  Why
not add an explicit cast for the mingw32 case?

Richard.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-23 18:48   ` Richard Guenther
@ 2008-08-23 19:42     ` Aaron W. LaFramboise
  2008-08-23 19:53       ` Richard Guenther
  0 siblings, 1 reply; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-08-23 19:42 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

Richard Guenther wrote:
> On Sat, Aug 23, 2008 at 7:14 PM, Aaron W. LaFramboise
> <aaronavay62@aaronwl.com> wrote:
>> Currently on i386-pc-mingw32, casting away constness in
>> __gthread_setspecific causes a bootstrap failure with --enable-werror. This
>> must be true for a number of other targets which do the same thing.
>>
>> The ultimate problem here is that there's nothing about this parameter that
>> is const: __gthread_getspecific returns a non-const pointer, not a const
>> one.

> But pthread_setspecific _does_ take a constant pointer as argument.

Oh, how unfortunate.

I think this is a defect in POSIX pthreads.  This means if you pass in a 
const void * to pthread_setspecific, you can get a void * back out of 
pthread_getspecific, losing the constness without ever casting.

> Why not add an explicit cast for the mingw32 case?

An explicit cast does not kill the warning.  The union trick is the only 
way to get rid of it, and the encapsulation for this trick, CONST_CAST, 
is not available to gthr.h.

*-mingw* is not the only target with this issue.  Pretty much all 
non-pthreads targets have to cast away constness in the same way, since 
most other threading APIs apparently get this right.

So, is it better here to deviate slightly from pthreads, to do what 
seems more correct, or to use the union trick to make this warning go 
away in all pthreads targets?

I'm happy either way; someone just let me know what I should do.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-23 19:42     ` Aaron W. LaFramboise
@ 2008-08-23 19:53       ` Richard Guenther
  2008-08-23 21:22         ` Aaron W. LaFramboise
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Guenther @ 2008-08-23 19:53 UTC (permalink / raw)
  To: Aaron W. LaFramboise; +Cc: gcc-patches

On Sat, Aug 23, 2008 at 8:46 PM, Aaron W. LaFramboise
<aaronavay62@aaronwl.com> wrote:
> Richard Guenther wrote:
>>
>> On Sat, Aug 23, 2008 at 7:14 PM, Aaron W. LaFramboise
>> <aaronavay62@aaronwl.com> wrote:
>>>
>>> Currently on i386-pc-mingw32, casting away constness in
>>> __gthread_setspecific causes a bootstrap failure with --enable-werror.
>>> This
>>> must be true for a number of other targets which do the same thing.
>>>
>>> The ultimate problem here is that there's nothing about this parameter
>>> that
>>> is const: __gthread_getspecific returns a non-const pointer, not a const
>>> one.
>
>> But pthread_setspecific _does_ take a constant pointer as argument.
>
> Oh, how unfortunate.
>
> I think this is a defect in POSIX pthreads.  This means if you pass in a
> const void * to pthread_setspecific, you can get a void * back out of
> pthread_getspecific, losing the constness without ever casting.
>
>> Why not add an explicit cast for the mingw32 case?
>
> An explicit cast does not kill the warning.  The union trick is the only way
> to get rid of it, and the encapsulation for this trick, CONST_CAST, is not
> available to gthr.h.

An explicit cast kills the warning for me for

void foo(void *);
void bar (const void *p)
{
  foo(p);
}

how is your situation different?

Richard.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-23 19:53       ` Richard Guenther
@ 2008-08-23 21:22         ` Aaron W. LaFramboise
  2008-08-23 21:31           ` Richard Guenther
  0 siblings, 1 reply; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-08-23 21:22 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

Richard Guenther wrote:
> On Sat, Aug 23, 2008 at 8:46 PM, Aaron W. LaFramboise
> <aaronavay62@aaronwl.com> wrote:
>> Richard Guenther wrote:
>>> On Sat, Aug 23, 2008 at 7:14 PM, Aaron W. LaFramboise
>>> <aaronavay62@aaronwl.com> wrote:
>>>> Currently on i386-pc-mingw32, casting away constness in
>>>> __gthread_setspecific causes a bootstrap failure with --enable-werror.
>>>> This
>>>> must be true for a number of other targets which do the same thing.

>>> Why not add an explicit cast for the mingw32 case?
>> An explicit cast does not kill the warning.  The union trick is the only way
>> to get rid of it, and the encapsulation for this trick, CONST_CAST, is not
>> available to gthr.h.
> 
> An explicit cast kills the warning for me for
> 
> void foo(void *);
> void bar (const void *p)
> {
>   foo(p);
> }
> 
> how is your situation different?

stage2 and stage3 of the bootstrap use -Wcast-qual which warns about 
specifically this case.  -Wcast-qual is not in -Wall or -Wextra.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-23 21:22         ` Aaron W. LaFramboise
@ 2008-08-23 21:31           ` Richard Guenther
  2008-08-23 22:12             ` Aaron W. LaFramboise
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Guenther @ 2008-08-23 21:31 UTC (permalink / raw)
  To: Aaron W. LaFramboise; +Cc: gcc-patches

On Sat, Aug 23, 2008 at 10:16 PM, Aaron W. LaFramboise
<aaronavay62@aaronwl.com> wrote:
> Richard Guenther wrote:
>>
>> On Sat, Aug 23, 2008 at 8:46 PM, Aaron W. LaFramboise
>> <aaronavay62@aaronwl.com> wrote:
>>>
>>> Richard Guenther wrote:
>>>>
>>>> On Sat, Aug 23, 2008 at 7:14 PM, Aaron W. LaFramboise
>>>> <aaronavay62@aaronwl.com> wrote:
>>>>>
>>>>> Currently on i386-pc-mingw32, casting away constness in
>>>>> __gthread_setspecific causes a bootstrap failure with --enable-werror.
>>>>> This
>>>>> must be true for a number of other targets which do the same thing.
>
>>>> Why not add an explicit cast for the mingw32 case?
>>>
>>> An explicit cast does not kill the warning.  The union trick is the only
>>> way
>>> to get rid of it, and the encapsulation for this trick, CONST_CAST, is
>>> not
>>> available to gthr.h.
>>
>> An explicit cast kills the warning for me for
>>
>> void foo(void *);
>> void bar (const void *p)
>> {
>>  foo(p);
>> }
>>
>> how is your situation different?
>
> stage2 and stage3 of the bootstrap use -Wcast-qual which warns about
> specifically this case.  -Wcast-qual is not in -Wall or -Wextra.

If this isn't a problem for regular bootstrap then I don't see why we
should fix it.
There are cast-qual warnings in GCC elsewhere.

Richard.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-23 21:31           ` Richard Guenther
@ 2008-08-23 22:12             ` Aaron W. LaFramboise
  2008-08-24  0:24               ` Richard Guenther
  0 siblings, 1 reply; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-08-23 22:12 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

Richard Guenther wrote:
> On Sat, Aug 23, 2008 at 10:16 PM, Aaron W. LaFramboise
> <aaronavay62@aaronwl.com> wrote:

>> stage2 and stage3 of the bootstrap use -Wcast-qual which warns about
>> specifically this case.  -Wcast-qual is not in -Wall or -Wextra.
> 
> If this isn't a problem for regular bootstrap then I don't see why we
> should fix it.
> There are cast-qual warnings in GCC elsewhere.

This is a problem for a regular bootstrap, because regular bootstraps 
use -Wcast-qual in stage2 and stage3 for gcc/.

If you do a clean bootstrap of i386-pc-mingw32 from svn or a snapshot, 
it will fail in stage2 in a file that includes gthr-win32.h.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-23 22:12             ` Aaron W. LaFramboise
@ 2008-08-24  0:24               ` Richard Guenther
  2008-08-24  4:23                 ` Aaron W. LaFramboise
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Guenther @ 2008-08-24  0:24 UTC (permalink / raw)
  To: Aaron W. LaFramboise; +Cc: gcc-patches

On Sat, Aug 23, 2008 at 10:58 PM, Aaron W. LaFramboise
<aaronavay62@aaronwl.com> wrote:
> Richard Guenther wrote:
>>
>> On Sat, Aug 23, 2008 at 10:16 PM, Aaron W. LaFramboise
>> <aaronavay62@aaronwl.com> wrote:
>
>>> stage2 and stage3 of the bootstrap use -Wcast-qual which warns about
>>> specifically this case.  -Wcast-qual is not in -Wall or -Wextra.
>>
>> If this isn't a problem for regular bootstrap then I don't see why we
>> should fix it.
>> There are cast-qual warnings in GCC elsewhere.
>
> This is a problem for a regular bootstrap, because regular bootstraps use
> -Wcast-qual in stage2 and stage3 for gcc/.
>
> If you do a clean bootstrap of i386-pc-mingw32 from svn or a snapshot, it
> will fail in stage2 in a file that includes gthr-win32.h.

Ok.  I'd rather use the union trick then in gthr-win32.h.

Richard.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-24  0:24               ` Richard Guenther
@ 2008-08-24  4:23                 ` Aaron W. LaFramboise
  2008-08-24 10:33                   ` Richard Guenther
  0 siblings, 1 reply; 18+ messages in thread
From: Aaron W. LaFramboise @ 2008-08-24  4:23 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

Richard Guenther wrote:
> On Sat, Aug 23, 2008 at 10:58 PM, Aaron W. LaFramboise
> <aaronavay62@aaronwl.com> wrote:

>> If you do a clean bootstrap of i386-pc-mingw32 from svn or a snapshot, it
>> will fail in stage2 in a file that includes gthr-win32.h.
> 
> Ok.  I'd rather use the union trick then in gthr-win32.h.

Alright.  In that case, is the attached patch OK?

As I mentioned, neither <system.h> nor <tsystem.h> are available here 
because other top-level libraries that don't have those use this, so we 
have to redefine this macro.

I tested this on i386-pc-mingw32 with a bootstrap.

[-- Attachment #2: gcc-4.4.0-20080823-gthr2.patch --]
[-- Type: text/plain, Size: 786 bytes --]

2008-08-23  Aaron W. LaFramboise  <aaronavay62@aaronwl.com>

	* gcc/gthr-win32.h (__gthread_setspecific): Use CONST_CAST2.

Index: gcc/gthr-win32.h
===================================================================
--- gcc/gthr-win32.h	(revision 139510)
+++ gcc/gthr-win32.h	(working copy)
+/* This is a copy of CONST_CAST2 from system.h */
+#ifndef CONST_CAST2
+#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((__extension__(union {FROMTYPE _q; \
+  TOTYPE _nq;})(X))._nq)
+#endif
+
 static inline int
 __gthread_setspecific (__gthread_key_t key, const void *ptr)
 {
-  return (TlsSetValue (key, (void*) ptr) != 0) ? 0 : (int) GetLastError ();
+  if (TlsSetValue (key, CONST_CAST2(void *, const void *, ptr)) != 0)
+    return 0;
+  else
+    return GetLastError ();
 }
 
 static inline void

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-24  4:23                 ` Aaron W. LaFramboise
@ 2008-08-24 10:33                   ` Richard Guenther
  2008-08-25  7:55                     ` Danny Smith
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Guenther @ 2008-08-24 10:33 UTC (permalink / raw)
  To: Aaron W. LaFramboise; +Cc: gcc-patches

On Sun, Aug 24, 2008 at 3:11 AM, Aaron W. LaFramboise
<aaronavay62@aaronwl.com> wrote:
> Richard Guenther wrote:
>>
>> On Sat, Aug 23, 2008 at 10:58 PM, Aaron W. LaFramboise
>> <aaronavay62@aaronwl.com> wrote:
>
>>> If you do a clean bootstrap of i386-pc-mingw32 from svn or a snapshot, it
>>> will fail in stage2 in a file that includes gthr-win32.h.
>>
>> Ok.  I'd rather use the union trick then in gthr-win32.h.
>
> Alright.  In that case, is the attached patch OK?
>
> As I mentioned, neither <system.h> nor <tsystem.h> are available here
> because other top-level libraries that don't have those use this, so we have
> to redefine this macro.
>
> I tested this on i386-pc-mingw32 with a bootstrap.

This is ok.

Thanks,
Richard.

> 2008-08-23  Aaron W. LaFramboise  <aaronavay62@aaronwl.com>
>
>        * gcc/gthr-win32.h (__gthread_setspecific): Use CONST_CAST2.
>
> Index: gcc/gthr-win32.h
> ===================================================================
> --- gcc/gthr-win32.h    (revision 139510)
> +++ gcc/gthr-win32.h    (working copy)
> +/* This is a copy of CONST_CAST2 from system.h */
> +#ifndef CONST_CAST2
> +#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((__extension__(union {FROMTYPE _q;
> \
> +  TOTYPE _nq;})(X))._nq)
> +#endif
> +
>  static inline int
>  __gthread_setspecific (__gthread_key_t key, const void *ptr)
>  {
> -  return (TlsSetValue (key, (void*) ptr) != 0) ? 0 : (int) GetLastError ();
> +  if (TlsSetValue (key, CONST_CAST2(void *, const void *, ptr)) != 0)
> +    return 0;
> +  else
> +    return GetLastError ();
>  }
>
>  static inline void
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-24 10:33                   ` Richard Guenther
@ 2008-08-25  7:55                     ` Danny Smith
  2008-09-03  8:31                       ` Danny Smith
  0 siblings, 1 reply; 18+ messages in thread
From: Danny Smith @ 2008-08-25  7:55 UTC (permalink / raw)
  To: Aaron W. LaFramboise, gcc-patches

On Sun, Aug 24, 2008 at 8:33 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Sun, Aug 24, 2008 at 3:11 AM, Aaron W. LaFramboise
> <aaronavay62@aaronwl.com> wrote:
>> Richard Guenther wrote:
>>>
>>> On Sat, Aug 23, 2008 at 10:58 PM, Aaron W. LaFramboise
>>> <aaronavay62@aaronwl.com> wrote:
>>
>>>> If you do a clean bootstrap of i386-pc-mingw32 from svn or a snapshot, it
>>>> will fail in stage2 in a file that includes gthr-win32.h.
>>>
>>> Ok.  I'd rather use the union trick then in gthr-win32.h.
>>
>> Alright.  In that case, is the attached patch OK?
>>
>> As I mentioned, neither <system.h> nor <tsystem.h> are available here
>> because other top-level libraries that don't have those use this, so we have
>> to redefine this macro.
>>
>> I tested this on i386-pc-mingw32 with a bootstrap.
>
> This is ok.
>
> Thanks,
> Richard.
>
>> 2008-08-23  Aaron W. LaFramboise  <aaronavay62@aaronwl.com>
>>
>>        * gcc/gthr-win32.h (__gthread_setspecific): Use CONST_CAST2.
>>
>> Index: gcc/gthr-win32.h

Also please make config/i386/gthr-win32.c consistent with this.
Thanks
Danny
>> ===================================================================
>> --- gcc/gthr-win32.h    (revision 139510)
>> +++ gcc/gthr-win32.h    (working copy)
>> +/* This is a copy of CONST_CAST2 from system.h */
>> +#ifndef CONST_CAST2
>> +#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((__extension__(union {FROMTYPE _q;
>> \
>> +  TOTYPE _nq;})(X))._nq)
>> +#endif
>> +
>>  static inline int
>>  __gthread_setspecific (__gthread_key_t key, const void *ptr)
>>  {
>> -  return (TlsSetValue (key, (void*) ptr) != 0) ? 0 : (int) GetLastError ();
>> +  if (TlsSetValue (key, CONST_CAST2(void *, const void *, ptr)) != 0)
>> +    return 0;
>> +  else
>> +    return GetLastError ();
>>  }
>>
>>  static inline void
>>
>>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make the pointer parameter to __gthread_setspecific non-const
  2008-08-25  7:55                     ` Danny Smith
@ 2008-09-03  8:31                       ` Danny Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Danny Smith @ 2008-09-03  8:31 UTC (permalink / raw)
  To: Aaron W. LaFramboise, gcc-patches

On Mon, Aug 25, 2008 at 5:43 PM, Danny Smith <dansmister@gmail.com> wrote:
> On Sun, Aug 24, 2008 at 8:33 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Sun, Aug 24, 2008 at 3:11 AM, Aaron W. LaFramboise
>> <aaronavay62@aaronwl.com> wrote:
>>> Richard Guenther wrote:
>>>>
>>>> On Sat, Aug 23, 2008 at 10:58 PM, Aaron W. LaFramboise
>>>> <aaronavay62@aaronwl.com> wrote:
>>>
>>>>> If you do a clean bootstrap of i386-pc-mingw32 from svn or a snapshot, it
>>>>> will fail in stage2 in a file that includes gthr-win32.h.
>>>>
>>>> Ok.  I'd rather use the union trick then in gthr-win32.h.
>>>
>>> Alright.  In that case, is the attached patch OK?
>>>
>>> As I mentioned, neither <system.h> nor <tsystem.h> are available here
>>> because other top-level libraries that don't have those use this, so we have
>>> to redefine this macro.
>>>
>>> I tested this on i386-pc-mingw32 with a bootstrap.
>>
>> This is ok.
>>
>> Thanks,
>> Richard.
>>
>>> 2008-08-23  Aaron W. LaFramboise  <aaronavay62@aaronwl.com>
>>>
>>>        * gcc/gthr-win32.h (__gthread_setspecific): Use CONST_CAST2.
>>>

I have committed the following patch as obvious to restore bootstrap on mingw32.

2008-09-03 Danny Smith  <dannysmith@usrs.sourceforge.net>

	* gthr-win32.h (CONST_CAST2): Really make sure CONST_CAST2 is
	defined.
	(__gthread_setspecific): Revert 2008-08-31 change to
	__GTHREAD_HIDE_W32API case.  Apply it to !__GTHREAD_HIDE_W32API
	case.

	
Index: gthr-win32.h
===================================================================
--- gthr-win32.h	(revision 139848)
+++ gthr-win32.h	(working copy)
@@ -32,6 +32,11 @@
 #ifndef GCC_GTHR_WIN32_H
 #define GCC_GTHR_WIN32_H

+/* Make sure CONST_CAST2 (origin in system.h) is declared.  */
+#ifndef CONST_CAST2
+#define CONST_CAST2(TOTYPE,FROMTYPE,X) ((__extension__(union
{FROMTYPE _q; TOTYPE _nq;})(X))._nq)
+#endif
+
 /* Windows32 threads specific definitions. The windows32 threading model
    does not map well into pthread-inspired gcc's threading model, and so
    there are caveats one needs to be aware of.
@@ -455,10 +460,7 @@
 static inline int
 __gthread_setspecific (__gthread_key_t key, const void *ptr)
 {
-  if (TlsSetValue (key, CONST_CAST2(void *, const void *, ptr)) != 0)
-    return 0;
-  else
-    return GetLastError ();
+  return __gthr_win32_setspecific (key, ptr);
 }

 static inline void
@@ -615,7 +617,10 @@
 static inline int
 __gthread_setspecific (__gthread_key_t key, const void *ptr)
 {
-  return (TlsSetValue (key, (void*) ptr) != 0) ? 0 : (int) GetLastError ();
+  if (TlsSetValue (key, CONST_CAST2(void *, const void *, ptr)) != 0)
+    return 0;
+  else
+    return GetLastError ();
 }

 static inline void

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2008-09-03  6:10 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-07-11  2:01 [PATCH] Fix const pointer warning in gthr-win32.h Aaron W. LaFramboise
2008-07-11  4:29 ` Kaveh R. GHAZI
2008-07-28 23:33   ` Aaron W. LaFramboise
2008-07-29 17:21     ` Ian Lance Taylor
2008-07-30 22:01       ` Kaveh R. Ghazi
2008-07-31  4:08         ` Aaron W. LaFramboise
2008-08-23 18:32 ` [PATCH] Make the pointer parameter to __gthread_setspecific non-const Aaron W. LaFramboise
2008-08-23 18:48   ` Richard Guenther
2008-08-23 19:42     ` Aaron W. LaFramboise
2008-08-23 19:53       ` Richard Guenther
2008-08-23 21:22         ` Aaron W. LaFramboise
2008-08-23 21:31           ` Richard Guenther
2008-08-23 22:12             ` Aaron W. LaFramboise
2008-08-24  0:24               ` Richard Guenther
2008-08-24  4:23                 ` Aaron W. LaFramboise
2008-08-24 10:33                   ` Richard Guenther
2008-08-25  7:55                     ` Danny Smith
2008-09-03  8:31                       ` Danny Smith

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