Tcl Source Code

View Ticket
Login
Ticket UUID: 1592791
Title: Fix 64bit pointer cast warnings
Type: Patch Version: None
Submitter: das Created on: 2006-11-08 17:25:27
Subsystem: 53. Configure and Build Tools Assigned To: das
Priority: 6 Severity:
Status: Closed Last Modified: 2006-11-16 21:23:33
Resolution: Fixed Closed By: dgp
    Closed on: 2006-11-16 14:23:33
Description:
Building 64bit tcl on LP64 platforms with gcc4 (and probably earlier) gives 
many 'cast to/from pointer from/to integer of different size' warnings 
about (safe) casts to/from ClientData and other void* types

The recommended way to avoid these warnings is to add casts to the 
intermediate types intptr_t/uintptr_t.

The recently released autoconf 2.60 has AC_TYPE_INTPTR_T resp 
AC_TYPE_UINTPTR_T macros  to check if these C99 types are available (if 
an upgrade to autoconf 2.60 is not possible, it should be possible to write 
equivalent macros for tcl.m4).

The attached patch adds casts to (INTPTR_T) resp (UINTPTR_T) where 
needed, those are macros in tclInt.h that fall back to void* if the intptr_t /
uintptr_t types are not available.

I have not touched the windows configure.in, 64bit win will thus fallback 
as above and may still have those warnings, if so it is probably sufficient 
to make the same changes as in unix/configure.in but I cannot test this.
User Comments: dgp added on 2006-11-16 21:23:33:
Logged In: YES 
user_id=80530
Originator: NO


Looks like I guessed wrong:

checking for intptr_t... yes
checking for uintptr_t... yes

Can't reproduce today, so best 
guess is I overlooked a `make distclean`
when testing the first time.

das added on 2006-11-16 07:11:42:
Logged In: YES 
user_id=90580
Originator: YES

Don,
for compilers without intptr_t, the check below in configure.in is supposed to find an integer type as wide as a pointer and define intptr_t to that, can you investigate why this doesn't work for you, or send me the part of config.log showing what fails in that test ?

    AC_CACHE_CHECK([for pointer-size signed integer type], tcl_cv_intptr_t, [
    for tcl_cv_intptr_t in "int" "long" "long long" none; do
if test "$tcl_cv_intptr_t" != none; then
    AC_COMPILE_IFELSE([AC_LANG_BOOL_COMPILE_TRY([AC_INCLUDES_DEFAULT],
    [[sizeof (void *) <= sizeof ($tcl_cv_intptr_t)]])], 
[tcl_ok=yes], [tcl_ok=no])
    test "$tcl_ok" = yes && break; fi
    done])
    if test "$tcl_cv_intptr_t" != none; then
AC_DEFINE_UNQUOTED([intptr_t], [$tcl_cv_intptr_t], [Signed integer
   type wide enough to hold a pointer.])
    fi

dgp added on 2006-11-14 23:37:06:
Logged In: YES 
user_id=80530


My compiler doesn't have (intptr_t),
so in that case I shouldn't expect
any warnings to be silenced?

I guess I was just imagining that
the build seemed quieter.

das added on 2006-11-14 05:14:49:
Logged In: YES 
user_id=90580

Don,
that is peculiar. I now get a warning-free 64-bit build on
the platforms I have access to...

tclCompExpr.c:1249 is
    Tcl_SetHashValue(hPtr, (ClientData) INT2PTR(i));
i.e. it uses the new macro to cast the int i to (intpt_t)
before casting to (void*), which should prevent the warning.
Can you investigate why it still occurs on your platform?
maybe the configure script does not detect intptr_t
correctly? what are the values of the defines HAVE_INTPTR_T
and intptr_t ?

dgp added on 2006-11-13 21:38:31:
Logged In: YES 
user_id=80530


Noise is much reduce on a 64-bit
build, but I still see warnings
from the following files:

tclCompExpr.c
tclEncoding.c
tclEvent.c
tclExecute.c
tclHash.c
tclIO.c
tclProc.c
tclTimer.c
tclUtil.c
tclUnixChan.c
tclUnixPipe.c

Here's the first set of warnings
as an example:

/local/src/tcl/unix/../generic/tclCompExpr.c: In function
`TclCompileExpr':
/local/src/tcl/unix/../generic/tclCompExpr.c:1249: warning:
cast to pointer from integer of different size
/local/src/tcl/unix/../generic/tclCompExpr.c: In function
`CompileSubExpr':
/local/src/tcl/unix/../generic/tclCompExpr.c:1390: warning:
cast from pointer to integer of different size

das added on 2006-11-13 16:05:14:
Logged In: YES 
user_id=90580

verified that HEAD with this patch builds on all the available SF compilefarm hosts, i.e.
alpha-linux1 amd64-linux1 openpower-linux1 sparc-solaris1 x86-freebsd1 x86-linux1 x86-linux2 x86-netbsd1 x86-
openbsd1 x86-solaris1 
as well as on macosx 10.4 (ppc, ppc64, i386 & x86_64), on macosx 10.3 (ppc) and macosx 10.2 (ppc)

das added on 2006-11-13 15:23:30:
Logged In: YES 
user_id=90580

committed to HEAD

das added on 2006-11-10 11:24:40:

File Deleted - 202174: 



File Added - 202176: 1592791.diff

das added on 2006-11-10 11:16:27:

File Deleted - 202079: 



File Added - 202174: 1592791.diff

Logged In: YES 
user_id=90580

attached revised patch that no longer depends on autoconf 2.60, instead it implements checks for intptr_t/uintptr_t 
explicitly in configure.in.

like the previous patch, uses PTR2INT(), INT2PTR(), PTR2UINT(), UINT2PTR() macros.

if there are no further objections, I'll commit this tomorrow

das added on 2006-11-09 20:23:43:
Logged In: YES 
user_id=90580

come on...
$ curl -O http://ftp.gnu.org/gnu/autoconf/autoconf-2.60.tar.gz
$ tar zxf autoconf-2.60.tar.gz
$ cd autoconf-2.60
$ ./configure --prefix=$HOME/ac --program-suffix=-2.60
$ make install
$ ~/ac/bin/autoconf-2.60 --version

dkf added on 2006-11-09 20:14:16:
Logged In: YES 
user_id=79902

"it's a matter of 5 minutes to install a new version of
autoconf, and it's trivial to have several versions installed"

Umm, not round here. Different local policies (and I don't
want to do local admin when there are others to do that for
me; why keep a dog and bark yourself?)

das added on 2006-11-09 19:37:47:

File Added - 202079: tcl-1592791-INT2PTR.diff

Logged In: YES 
user_id=90580

attached updated patch that uses PTR2INT(), INT2PTR(), PTR2UINT(), UINT2PTR() as proposed earlier.

das added on 2006-11-09 19:04:02:
Logged In: YES 
user_id=90580

Donal: it's a matter of 5 minutes to install a new version of autoconf, and it's trivial to have several versions installed 
in parallel (I have 2.13, 2.52, 2.57, 2.59 and 2.60 here...), so I don't think that's a valid objection.
In any case, returning to 2.13 is certainly not an option, it is broken in so many ways, and we use numerous ac 2.5x-
only features in HEAD configure.in & tcl.m4 now; moreover, parity of HEAD's tcl.m4 with tclconfig/tcl.m4 needs to be 
maintained (i.e. all TEA3 extension would have to go back to 2.13, which isn't going to happen).

Jan: I'm don't think size_t would work, it is defined to be the type returned by sizeof, which does not need to be the 
same size as that of a pointer (admittedly this would only happen on exotic platforms...)
Also, in C89 there is no standard signed equivalent of size_t, and in pre-C89, size_t is not even guaranteed to be an 
unsigned type...

nijtmans added on 2006-11-09 18:26:14:
Logged In: YES 
user_id=61031

Suggestion: why not use an intermediate cast to size_t. This is a type that
exists in C89 as well, and it works the same. No change to autoconf is
needed.

Regards,
        Jan Nijtmans

dkf added on 2006-11-09 18:13:38:
Logged In: YES 
user_id=79902

Please, no autoconf version changes. Or, if they have to
change, we should go back to using autoconf 2.13 (since that
produced much faster configure scripts) or 2.57 (which is
the version I have. :-))

das added on 2006-11-09 01:49:10:
Logged In: YES 
user_id=90580

while intptr_t/uintptr_t are C99isms, gcc makes them available in C89 mode as well; in any case, ac checks if they are 
available in <inttypes.h> or <stdint.h>,  otherwise it finds a scalar type of the same size as s a pointer and defines 
intptr_t/uintptr_t  to that. If this fails (and on win currently), my INTPTR_T/UINTPTR_T macros fall back to do a nop 
cast to (void*).

I agree about the ugliness, while standard, INTPTR_T is not a very nice name, but I couldn't come up with anything 
memnonic to designate a cast to an integer type defined to be the same size as a pointer...

I think UINT() would be confusing if you mean it to be just a cast to (intptr_t), but it might make sense for a cast of a 
pointer to (int)(intptr_t). We would then also need a name for the other direction, i.e. cast of an int to (void*)(intptr_t), 
as well as (unsigned int) variants of both, e.g. UINT(), UPTR(), UUINT(), UUPTR() ?
not sure that's all that clear, maybe PTR2INT(), INT2PTR, PTR2UINT, UINT2PTR() ?

hobbs added on 2006-11-09 01:01:40:
Logged In: YES 
user_id=72656

Tcl is still stuck in pre-C89 code compatibility.  I believe
the head should be able to move to C89, but /not/ C99, so
lets keep that in mind for whatever changes are required.

As far as the patching goes, I'd like to minimize the
ugliness of these casts, and preferably give them some form
that is clear in use, like the UCHAR() macro (something like
UINT()).

Otherwise I guess I'm ok with requiring ac 2.60 on the head,
although I'll have to upgrade myself (only 2.59 on all my
machines).

das added on 2006-11-09 00:42:04:
Logged In: YES 
user_id=90580

Jeff,
any objections to the autoconf 2.60 upgrade ?

das added on 2006-11-09 00:40:43:

File Added - 201952: tk-1592791.diff

das added on 2006-11-09 00:39:43:

File Deleted - 201945: 



File Added - 201951: tcl-1592791.diff

das added on 2006-11-09 00:26:54:

File Added - 201945: tcl-1592791.diff

Attachments: