[perl #16019] [PATCH] Silence warnings on pointer/integer casts

# New Ticket Created by  Andy Dougherty 
# Please include the string:  [perl #16019]
# in the subject line of all future correspondence about this issue. 
# <URL: http://rt.perl.org/rt2/Ticket/Display.html?id=16019 >


The following patch eliminates 147 warnings on my system, and puts
in place the infrastructure to get rid of even more.

Specifically, on Solaris 8 with gcc-2.8.1, and perl-5.8.0 with
64bit ints, sizeof(INTVAL) == 8, while sizeof(void *) == 4.  Hence all
casts between pointers and integers raise a warning.

The following patch expands the INTVAL2PTR and PTR2INTVAL macros in
include/parrot/parrot.h to silence the warnings in such cases.  (The
macros do not check for out-of-range errors -- they assume that the
structure of the code is such that everything is guaranteed to fit in
either a pointer or an integer.)  I also introduced a parallel set of
macros to use opcode_t (If I understand the current state of affairs,
we currently don't officially require sizeof(opcode_t) == sizeof(intval)).

I then used these macros in core.ops to eliminate a bunch of warnings.
Mostly, these are simple things like

    -  fclose((FILE *)$1);
    +  fclose(OPCODE_T2PTR(FILE *,$1));

However, in the jsr ops, where we have

    goto ADDRESS($1);

I found that if I replaced it with
    
    goto ADDRESS(INTVAL2PTR(opcode_t *, $1));

then core_ops_cg.c was generated with invalid code.  I think that's a
flaw in whatever generates core_ops_cg.c, but I didn't pursue that
much further.  Instead, I used a temporary variable to simplify the
expressions.  Alternatively, the generator could be changed to get the
casts right.

I can't test this too well -- parrot currently fails all tests either
with or without my patch :-).

At this point, I'm down to 74 warnings, falling into the following
general categories:
   
   2 cast from pointer to integer of different size
  10 cast to pointer from integer of different size

These can probably be eliminated by use of the new macros.
I just haven't gotten to them yet.  Volunteers welcome.

  48 cast increases required alignment of target type

These might each need thought.

  14 long int format, different type arg

These should go away when we do our own printf processing; in the
meantime a bandaid might be appropriate.  Volunteers welcome.

diff -r -u parrot-orig/config/gen/config_h/config_h.in parrot-andy/config/gen/config_h/config_h.in
--- parrot-orig/config/gen/config_h/config_h.in	Fri Jul 19 11:42:17 2002
+++ parrot-andy/config/gen/config_h/config_h.in	Tue Aug  6 12:41:38 2002
@@ -49,6 +49,8 @@
 #define INTVAL_SIZE ${intvalsize}
 #define NUMVAL_SIZE ${nvsize}
 #define OPCODE_T_SIZE ${opcode_t_size}
+#define PTR_SIZE ${ptrsize}
+#define LONG_SIZE ${longsize}
 
 typedef Parrot_Opcode opcode_t;
 typedef size_t ptrcast_t;
diff -r -u parrot-orig/core.ops parrot-andy/core.ops
--- parrot-orig/core.ops	Tue Aug  6 10:51:14 2002
+++ parrot-andy/core.ops	Tue Aug  6 12:57:19 2002
@@ -80,7 +80,7 @@
 =cut
 
 inline op close(inout INT) {
-  fclose((FILE *)$1);
+  fclose(OPCODE_T2PTR(FILE *,$1));
   goto NEXT();
 }
 
@@ -123,7 +123,7 @@
 =cut
 
 inline op open(out INT, in STR) {
-  $1 = (INTVAL)fopen(string_to_cstring(interpreter, ($2)), "r+");
+  $1 = PTR2OPCODE_T(fopen(string_to_cstring(interpreter, ($2)), "r+"));
   if (!$1) {
     perror("Can't open");
     exit(1);
@@ -133,7 +133,7 @@
 }
 
 inline op open(out INT, in STR, in STR) {
-  $1 = (INTVAL)fopen(string_to_cstring(interpreter, ($2)), string_to_cstring(interpreter, ($3)));
+  $1 = PTR2OPCODE_T(fopen(string_to_cstring(interpreter, ($2)), string_to_cstring(interpreter, ($3))));
   goto NEXT();
 }
 
@@ -161,7 +161,7 @@
                 break;
 	case 2: file = stderr; 
                 break;
-	default: file = (FILE *)$2;
+	default: file = OPCODE_T2PTR(FILE *, $2);
   }
 
   $1 = string_make(interpreter, NULL, 65535, NULL, 0, NULL);
@@ -263,7 +263,7 @@
                 break;
 	case 2: file = stderr; 
                 break;
-	default: file = (FILE *)$1;
+	default: file = OPCODE_T2PTR(FILE *, $1);
   }
   fprintf(file, INTVAL_FMT, (INTVAL)$2);
   goto NEXT();
@@ -278,7 +278,7 @@
                 break;
 	case 2: file = stderr; 
                 break;
-	default: file = (FILE *)$1;
+	default: file = OPCODE_T2PTR(FILE *, $1);
   }
   fprintf(file, FLOATVAL_FMT, $2);
   goto NEXT();
@@ -294,7 +294,7 @@
                 break;
 	case 2: file = stderr; 
                 break;
-	default: file = (FILE *)$1;
+	default: file = OPCODE_T2PTR(FILE *, $1);
   }
   if (s && string_length(s)) {
     fprintf(file, "%s", string_to_cstring(interpreter, (s)));
@@ -313,7 +313,7 @@
                 break;
 	case 2: file = stderr; 
                 break;
-	default: file = (FILE *)$1;
+	default: file = OPCODE_T2PTR(FILE *, $1);
   }
   if (s) {
     fprintf(file, "%s", string_to_cstring(interpreter, (s)));
@@ -461,7 +461,7 @@
 =cut
 
 inline op set_addr(out INT, in INT) {
-  $1 = (INTVAL) (CUR_OPCODE + $2);
+  $1 = PTR2OPCODE_T(CUR_OPCODE + $2);
   goto NEXT();
 }
 
@@ -3597,8 +3597,10 @@
 =cut
 
 inline op jsr(in INT) {
+  opcode_t * loc;
   stack_push(interpreter, &interpreter->ctx.control_stack, expr NEXT(),  STACK_ENTRY_DESTINATION, STACK_CLEANUP_NULL);
-  goto ADDRESS($1);
+  loc = INTVAL2PTR(opcode_t *, $1);
+  goto ADDRESS(loc);
 }
 
 
@@ -3611,7 +3613,9 @@
 =cut
 
 inline op jump(in INT) {
-  goto ADDRESS($1);
+  opcode_t * loc;
+  loc = INTVAL2PTR(opcode_t *, $1);
+  goto ADDRESS(loc);
 }
 
 =back
@@ -3886,9 +3890,9 @@
 
 inline op stringinfo(out INT, in STR, in INT) {
   switch ($3) {
-    case STRINGINFO_HEADER:   $1 = (UINTVAL)$2;
+    case STRINGINFO_HEADER:   $1 = PTR2UINTVAL($2);
                               break;
-    case STRINGINFO_BUFSTART: $1 = (UINTVAL)$2->bufstart;
+    case STRINGINFO_BUFSTART: $1 = PTR2UINTVAL($2->bufstart);
                               break;
     case STRINGINFO_BUFLEN:   $1 = $2->buflen;
                               break;
@@ -4194,7 +4198,6 @@
   const char * name = string_to_cstring(interpreter, ($3));
   extern PMC * Parrot_new_csub(struct Parrot_Interp * ,
 			       void (*p)(Interp *, PMC *));
-
   void (*p)(Interp *, PMC *) = (void (*)(Interp *, PMC *))D2FPTR(Parrot_dlsym(
                                                 ($2)->data, name));
   if(p == NULL) {
diff -r -u parrot-orig/include/parrot/parrot.h parrot-andy/include/parrot/parrot.h
--- parrot-orig/include/parrot/parrot.h	Tue Aug  6 10:51:19 2002
+++ parrot-andy/include/parrot/parrot.h	Tue Aug  6 12:30:28 2002
@@ -78,6 +78,46 @@
 typedef void STRING_FUNCS;
 typedef void BIGNUM;
 
+/*  Casting between pointers and integers:  If pointers and integers
+    are the same size, then direct casting is fine.  If pointers and
+    integers are not the same size, then the compiler might complain.
+    Also, there's a possible loss of information in going from (for
+    example) a 64-bit integer to a 32-bit pointer.
+
+    These casts silence the warnings but do no limits checks.
+    Perhaps a different set should be defined (and only compiled if
+    explicitly Configured in) which do limits checks?
+    A. D. Aug. 6, 2002.
+*/
+#if (INTVAL_SIZE == PTR_SIZE) && (UINTVAL_SIZE == PTR_SIZE)
+#  define INTVAL2PTR(any,d)	(any)(d)
+#  define UINTVAL2PTR(any,d)	(any)(d)
+#else
+#  if PTR_SIZE == LONG_SIZE
+#    define INTVAL2PTR(any,d)	(any)(unsigned long)(d)
+#    define UINTVAL2PTR(any,d)	(any)(unsigned long)(d)
+#  else
+#    define INTVAL2PTR(any,d)	(any)(unsigned int)(d)
+#    define UINTVAL2PTR(any,d)	(any)(unsigned int)(d)
+#  endif
+#endif
+#define PTR2INTVAL(p)	INTVAL2PTR(INTVAL,p)
+#define PTR2UINTVAL(p)	UINTVAL2PTR(UINTVAL,p)
+
+/* Use similar macros for casting between pointers and opcode_t.
+   (We can't assume that sizeof(opcode_t) == sizeof(intval).
+*/
+#if (OPCODE_T_SIZE == PTR_SIZE)
+#  define OPCODE_T2PTR(any,d)	(any)(d)
+#else
+#  if PTR_SIZE == LONG_SIZE
+#    define OPCODE_T2PTR(any,d)	(any)(unsigned long)(d)
+#  else
+#    define OPCODE_T2PTR(any,d)	(any)(unsigned int)(d)
+#  endif
+#endif
+#define PTR2OPCODE_T(p)	OPCODE_T2PTR(opcode_t,p)
+
 /* define some shortcuts for dealing with function pointers */
 /* according to ANSI C, casting between function and non-function pointers is
  * no good.  So we should use "funcptr_t" in place of void* when dealing with
@@ -94,14 +134,8 @@
  * that. The equivalent of C99's uintptr_t- a non-pointer data type that can 
  * hold a pointer.
  */
-#define D2FPTR(x) (funcptr_t)(UINTVAL) x
-#define F2DPTR(x) (void*)(UINTVAL)(funcptr_t) x
-
-/*
- * Macroized in case more complex casts required in some platforms.
- */
-#define PTR2INTVAL(p)   (INTVAL)(p)
-#define INTVAL2PTR(i,p) (p*)(i)
+#define D2FPTR(x) UINTVAL2PTR(funcptr_t, PTR2UINTVAL(x))
+#define F2DPTR(x) UINTVAL2PTR(void *, PTR2UINTVAL((funcptr_t) x))
 
 /* On Win32 we need the constant O_BINARY for open() (at least for Borland C), 
    but on UNIX it doesn't exist, so set it to 0 if it's not defined

-- 
    Andy Dougherty		doughera@lafayette.edu
    Dept. of Physics
    Lafayette College, Easton PA 18042

• [perl #16019] [PATCH] Silence warnings on pointer/integer casts by Andy Dougherty
• Re: [perl #16019] [PATCH] Silence warnings on pointer/integer casts by Mike Lambert
• Re: [perl #16019] [PATCH] Silence warnings on pointer/integer casts by Andy Dougherty