* Re: RFA: libjava seems to miss some files for win32 [not found] <90baa01f0907180506h1a58152du5d45d66628043ad9@mail.gmail.com> @ 2009-07-18 13:53 ` Dave Korn 2009-07-18 16:01 ` Kai Tietz 0 siblings, 1 reply; 20+ messages in thread From: Dave Korn @ 2009-07-18 13:53 UTC (permalink / raw) To: Kai Tietz; +Cc: gcc, java Kai Tietz wrote: > Hello, > > I noticed, while trying to build libjava for x64 windows, that the > configure script fails to generate link to > 'libjava/gnu/java/security/jce/prng/natVMSecureRandomWin32.cc'. This > file isn't existing. Is there a fix for this? > > Thanks in advance for the answer, > Kai > You probably want to post this to the main list rather than -patches! Also the java list, I would suppose. The bug is strange. I get nothing from "grep -R SecureRandomWin32 libjava/*" in my sandbox (but I'm still on r.149334 from 07/07). cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 13:53 ` RFA: libjava seems to miss some files for win32 Dave Korn @ 2009-07-18 16:01 ` Kai Tietz 2009-07-18 17:09 ` Dave Korn 0 siblings, 1 reply; 20+ messages in thread From: Kai Tietz @ 2009-07-18 16:01 UTC (permalink / raw) To: Andrew Haley; +Cc: gcc, java, Dave Korn [-- Attachment #1: Type: text/plain, Size: 1422 bytes --] 2009/7/18 Dave Korn <dave.korn.cygwin@googlemail.com>: > Kai Tietz wrote: >> Hello, >> >> I noticed, while trying to build libjava for x64 windows, that the >> configure script fails to generate link to >> 'libjava/gnu/java/security/jce/prng/natVMSecureRandomWin32.cc'. This >> file isn't existing. Is there a fix for this? >> >> Thanks in advance for the answer, >> Kai >> > > You probably want to post this to the main list rather than -patches! Also > the java list, I would suppose. The bug is strange. I get nothing from "grep > -R SecureRandomWin32 libjava/*" in my sandbox (but I'm still on r.149334 from > 07/07). Yes, I missed to add java maintainer and the patch here. I noticed, while trying to build libjava for x64 windows, that the configure script fails to generate link to 'libjava/gnu/java/security/jce/prng/natVMSecureRandomWin32.cc'. This file isn't existing. The attached patch fixes this. The implementation is straight forward, but works for win32 api. The random value generation could be improved here. ChangeLog 2009-07-18 Kai Tietz <kai.tietz@onevision.com> * gnu/java/security/jce/prng/natVMSecureRandomWin32.cc: Implementation for native win32. Tested for x86 and x64 mingw targets. Ok for apply? Cheers, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination [-- Attachment #2: libjave_win.diff --] [-- Type: text/x-c, Size: 1337 bytes --] Index: gcc/libjava/gnu/java/security/jce/prng/natVMSecureRandomWin32.cc =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/libjava/gnu/java/security/jce/prng/natVMSecureRandomWin32.cc 2009-07-18 14:35:14.102884300 +0200 @@ -0,0 +1,44 @@ +// natVMSecureRandomPosix.cc - Native part of VMSecureRandom class for POSIX. + +/* Copyright (C) 2009 Free Software Foundation + + This file is part of libgcj. + +This software is copyrighted work licensed under the terms of the +Libgcj License. Please consult the file "LIBGCJ_LICENSE" for +details. */ + +#include <config.h> + +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> +#include <string.h> +#include <errno.h> + +#include <gcj/cni.h> +#include <java/lang/InternalError.h> +#include <gnu/java/security/jce/prng/VMSecureRandom.h> + +jint +gnu::java::security::jce::prng::VMSecureRandom::natGenerateSeed(jbyteArray byte_array, jint offset, jint length) +{ + static int was_init = 0; + int a; + jbyte *bytes = elements (byte_array); + ssize_t count = 0; + + if (!was_init) + { + srand (256); + was_init = 1; + } + for (a = 0; a < offset; ++a) + bytes++; + for (a = 0; a < length; a++, count++) + *bytes++= (jbyte) rand (); + + return count; +} + ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 16:01 ` Kai Tietz @ 2009-07-18 17:09 ` Dave Korn 2009-07-18 17:27 ` Kai Tietz 0 siblings, 1 reply; 20+ messages in thread From: Dave Korn @ 2009-07-18 17:09 UTC (permalink / raw) To: Kai Tietz; +Cc: Andrew Haley, gcc, java, Dave Korn Kai Tietz wrote: > * gnu/java/security/jce/prng/natVMSecureRandomWin32.cc: Implementation > for native win32. > > Tested for x86 and x64 mingw targets. Ok for apply? + for (a = 0; a < length; a++, count++) + *bytes++= (jbyte) rand (); Surely not, the standard C library rand() function is completely unsuitable for security purposes. It should use the win32 crypto api to get real high-quality random data I think. cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 17:09 ` Dave Korn @ 2009-07-18 17:27 ` Kai Tietz 2009-07-18 18:15 ` Dave Korn 2009-07-18 18:27 ` Dave Korn 0 siblings, 2 replies; 20+ messages in thread From: Kai Tietz @ 2009-07-18 17:27 UTC (permalink / raw) To: Dave Korn; +Cc: Andrew Haley, gcc, java 2009/7/18 Dave Korn <dave.korn.cygwin@googlemail.com>: > Kai Tietz wrote: > >> * gnu/java/security/jce/prng/natVMSecureRandomWin32.cc: Implementation >> for native win32. >> >> Tested for x86 and x64 mingw targets. Ok for apply? > > + for (a = 0; a < length; a++, count++) > + *bytes++= (jbyte) rand (); > > Surely not, the standard C library rand() function is completely unsuitable > for security purposes. It should use the win32 crypto api to get real > high-quality random data I think. > > cheers, > DaveK > > Yes, I agree to this as I said in the patch post. Can we assume that any win32 target has a working wincrypt.h file? I just suggested this patch, to have at least an implementation here for win32 for further improvement (Btw I missed in my initial patch to include explicit <stdlib.h> here, too). I am just running through libjava for an initial port for x64 windows. There are a lot of assumptions about sizeof (long) == sizeof (void*), but the worse thing I see is the casting of HANDLE values to jint. For x86 this is fine, but for x64 this can lead to serious troubles. Cheers, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 17:27 ` Kai Tietz @ 2009-07-18 18:15 ` Dave Korn 2009-07-18 19:09 ` Andrew Haley 2009-07-18 18:27 ` Dave Korn 1 sibling, 1 reply; 20+ messages in thread From: Dave Korn @ 2009-07-18 18:15 UTC (permalink / raw) To: Kai Tietz; +Cc: Dave Korn, Andrew Haley, gcc, java Kai Tietz wrote: > Yes, I agree to this as I said in the patch post. Can we assume that > any win32 target has a working wincrypt.h file? Hmmm... it is supported since win2k. I imagine DGJPP runs on 9x targets, and believe it or not there are still some Cygwin users on NT4. I would think it needs an autoconf test, and on platforms that don't support CryptoAPI it should probably throw an exception to indicate 'not implemented' rather than (e.g.) fall back to rand(), but that's a policy decision for the Java maintainers really. > I just suggested this patch, to have at least an implementation here > for win32 for further improvement (Btw I missed in my initial patch to > include explicit <stdlib.h> here, too). > I am just running through libjava for an initial port for x64 windows. > There are a lot of assumptions about sizeof (long) == sizeof (void*), > but the worse thing I see is the casting of HANDLE values to jint. For > x86 this is fine, but for x64 this can lead to serious troubles. Ouch, yes! You'll probably be best off replacing those with something based on uintptr_t, which may or may not have a j* equivalent already. cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 18:15 ` Dave Korn @ 2009-07-18 19:09 ` Andrew Haley 2009-07-18 19:19 ` Andrew Pinski 0 siblings, 1 reply; 20+ messages in thread From: Andrew Haley @ 2009-07-18 19:09 UTC (permalink / raw) To: Dave Korn; +Cc: Kai Tietz, gcc, java On 07/18/2009 07:27 PM, Dave Korn wrote: > Kai Tietz wrote: > >> Yes, I agree to this as I said in the patch post. Can we assume that >> any win32 target has a working wincrypt.h file? > > Hmmm... it is supported since win2k. I imagine DGJPP runs on 9x targets, > and believe it or not there are still some Cygwin users on NT4. I would think > it needs an autoconf test, and on platforms that don't support CryptoAPI it > should probably throw an exception to indicate 'not implemented' rather than > (e.g.) fall back to rand(), but that's a policy decision for the Java > maintainers really. Either this is a cryptographically strong random number or it must throw NotImplemented. >> I just suggested this patch, to have at least an implementation here >> for win32 for further improvement (Btw I missed in my initial patch to >> include explicit <stdlib.h> here, too). >> I am just running through libjava for an initial port for x64 windows. >> There are a lot of assumptions about sizeof (long) == sizeof (void*), >> but the worse thing I see is the casting of HANDLE values to jint. For >> x86 this is fine, but for x64 this can lead to serious troubles. > > Ouch, yes! You'll probably be best off replacing those with something based > on uintptr_t, which may or may not have a j* equivalent already. Don't use uintptr_t, use unsigned int __attribute__((mode(PTR)). This is built-in to gcc, not a dependency on the host libc which might not be c99.. Andrew. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 19:09 ` Andrew Haley @ 2009-07-18 19:19 ` Andrew Pinski 2009-07-18 19:23 ` Dave Korn 2009-07-18 19:23 ` Kai Tietz 0 siblings, 2 replies; 20+ messages in thread From: Andrew Pinski @ 2009-07-18 19:19 UTC (permalink / raw) To: Andrew Haley; +Cc: Dave Korn, Kai Tietz, gcc, java On Sat, Jul 18, 2009 at 12:08 PM, Andrew Haley<aph@redhat.com> wrote: > Don't use uintptr_t, use unsigned int __attribute__((mode(PTR)). This > is built-in to gcc, not a dependency on the host libc which might not > be c99..' Except uintptr_t is required to be provided by a non hosted compiler like GCC. -- Pinski ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 19:19 ` Andrew Pinski @ 2009-07-18 19:23 ` Dave Korn 2009-07-18 19:27 ` Kai Tietz 2009-07-18 19:23 ` Kai Tietz 1 sibling, 1 reply; 20+ messages in thread From: Dave Korn @ 2009-07-18 19:23 UTC (permalink / raw) To: Andrew Pinski; +Cc: Andrew Haley, Dave Korn, Kai Tietz, gcc, java Andrew Pinski wrote: > On Sat, Jul 18, 2009 at 12:08 PM, Andrew Haley<aph@redhat.com> wrote: >> Don't use uintptr_t, use unsigned int __attribute__((mode(PTR)). This >> is built-in to gcc, not a dependency on the host libc which might not >> be c99..' > > Except uintptr_t is required to be provided by a non hosted compiler like GCC. Come to think of it, we should use the __UINTPTR_TYPE__ thingy provided by jsm's recent stdint patch, shouldn't we? cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 19:23 ` Dave Korn @ 2009-07-18 19:27 ` Kai Tietz 0 siblings, 0 replies; 20+ messages in thread From: Kai Tietz @ 2009-07-18 19:27 UTC (permalink / raw) To: Dave Korn; +Cc: Andrew Pinski, Andrew Haley, gcc, java 2009/7/18 Dave Korn <dave.korn.cygwin@googlemail.com>: > Andrew Pinski wrote: >> On Sat, Jul 18, 2009 at 12:08 PM, Andrew Haley<aph@redhat.com> wrote: >>> Don't use uintptr_t, use unsigned int __attribute__((mode(PTR)). This >>> is built-in to gcc, not a dependency on the host libc which might not >>> be c99..' >> >> Except uintptr_t is required to be provided by a non hosted compiler like GCC. > > Come to think of it, we should use the __UINTPTR_TYPE__ thingy provided by > jsm's recent stdint patch, shouldn't we? > > cheers, > DaveK > Well, this sounds good. I wasn't aware that __UINTPTR_TYPE__ is present. I just was aware of the SIZE, and PTRDIFF builtin macros, which aren't useful here for all targets. Maybe a __ULONGPTR_TYPE__ should be added as internal macro, too. Because sometimes the widest scalar type between MAX(sizeof(long), sizeof(void *)) is necessary, too. E.g. in libiberty there are such cases I remember. Cheers, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 19:19 ` Andrew Pinski 2009-07-18 19:23 ` Dave Korn @ 2009-07-18 19:23 ` Kai Tietz 2009-07-18 19:29 ` Andrew Pinski 1 sibling, 1 reply; 20+ messages in thread From: Kai Tietz @ 2009-07-18 19:23 UTC (permalink / raw) To: Andrew Pinski; +Cc: Andrew Haley, Dave Korn, gcc, java 2009/7/18 Andrew Pinski <pinskia@gmail.com>: > On Sat, Jul 18, 2009 at 12:08 PM, Andrew Haley<aph@redhat.com> wrote: >> Don't use uintptr_t, use unsigned int __attribute__((mode(PTR)). This >> is built-in to gcc, not a dependency on the host libc which might not >> be c99..' > > Except uintptr_t is required to be provided by a non hosted compiler like GCC. > > -- Pinski > Well, uintptr_t/intptr_t are available to most (but not all hosts). IIRC there is a gcc version of stdint.h (gstdint.h), which could be used here. The mode version is fine too, as long as we can assume that libjava isn't build by any other compiler then gcc. Cheers, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 19:23 ` Kai Tietz @ 2009-07-18 19:29 ` Andrew Pinski 2009-07-19 11:13 ` Kai Tietz 0 siblings, 1 reply; 20+ messages in thread From: Andrew Pinski @ 2009-07-18 19:29 UTC (permalink / raw) To: Kai Tietz; +Cc: Andrew Haley, Dave Korn, gcc, java On Sat, Jul 18, 2009 at 12:22 PM, Kai Tietz<ktietz70@googlemail.com> wrote: > Well, uintptr_t/intptr_t are available to most (but not all hosts). > IIRC there is a gcc version of stdint.h (gstdint.h), which could be > used here. The mode version is fine too, as long as we can assume that > libjava isn't build by any other compiler then gcc. It is provided by GCC even without gstdint.h. See bug 448. Since __UINTPTR_TYPE__ is provided to be able to use stdint.h :). Thanks, Andrew PInski ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 19:29 ` Andrew Pinski @ 2009-07-19 11:13 ` Kai Tietz 2009-07-19 13:16 ` Dave Korn 0 siblings, 1 reply; 20+ messages in thread From: Kai Tietz @ 2009-07-19 11:13 UTC (permalink / raw) To: Andrew Pinski; +Cc: Andrew Haley, Dave Korn, gcc, java [-- Attachment #1: Type: text/plain, Size: 2220 bytes --] 2009/7/18 Andrew Pinski <pinskia@gmail.com>: > On Sat, Jul 18, 2009 at 12:22 PM, Kai Tietz<ktietz70@googlemail.com> wrote: >> Well, uintptr_t/intptr_t are available to most (but not all hosts). >> IIRC there is a gcc version of stdint.h (gstdint.h), which could be >> used here. The mode version is fine too, as long as we can assume that >> libjava isn't build by any other compiler then gcc. > > It is provided by GCC even without gstdint.h. See bug 448. Since > __UINTPTR_TYPE__ is provided to be able to use stdint.h :). > > Thanks, > Andrew PInski > So, I was able to build libjave (using the upstream boehm-gc source as the version in gcc at the moment doesn't support x64) for x64 windows. There are a lot of issues about casting HANDLE values into jint types, which is for x86 valid, but for x64 can lead potential to pointer truncations. Those part need some review by libjava maintainers. My patch simply casts those kind of pointers via __UINTPTR_TYPE__ into integer scalar before casting it into jint. I put comments at those places, where some rework is necessary. Additionally I removed the use of srand()/rand () and just throw an exception in the Win32 specific implementation of gnu::java::security::jce::prng::VMSecureRandom::natGenerateSeed. ChangeLog 2009-07-19 Kai Tietz <kai.tietz@onevision.com> * gnu/java/security/jce/prng/natVMSecureRandomWin32.cc: New Win32 specific implementation. * nclude/win32.h (_Jv_platform_ffi_abi): Use for _WIN64 case FFI_DEFAULT_ABI. * java/lang/natString.cc (UNMASK_PTR, MASK_PTR, PTR_MASKED): Use __UINTPTR_TYPE__ instead of unsigned long for pointer casts. * gnu/java/net/natPlainDatagramSocketImplWin32.cc: Likewise. * gnu/java/net/natPlainSocketImplWin32.cc: Likwise. * gnu/java/nio/channels/natFileChannelWin32.cc: Likewise. * include/jvm.h: Likewise. * java/lang/natClass.cc: Likewise. * java/lang/natClassLoader.cc: Likewise. * java/lang/natWin32Process.cc: Likewise. * link.cc: Likewise. Regression tested for i686-pc-mingw32. Ok, for apply? Cheers, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination [-- Attachment #2: libjave_win.diff --] [-- Type: text/x-c, Size: 11661 bytes --] Index: gcc/libjava/gnu/java/security/jce/prng/natVMSecureRandomWin32.cc =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/libjava/gnu/java/security/jce/prng/natVMSecureRandomWin32.cc 2009-07-19 12:13:45.715476500 +0200 @@ -0,0 +1,32 @@ +// natVMSecureRandomPosix.cc - Native part of VMSecureRandom class for POSIX. + +/* Copyright (C) 2009 Free Software Foundation + + This file is part of libgcj. + +This software is copyrighted work licensed under the terms of the +Libgcj License. Please consult the file "LIBGCJ_LICENSE" for +details. */ + +#include <config.h> + +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> +#include <string.h> +#include <errno.h> + +#include <gcj/cni.h> +#include <java/lang/InternalError.h> +#include <gnu/java/security/jce/prng/VMSecureRandom.h> + +jint +gnu::java::security::jce::prng::VMSecureRandom::natGenerateSeed(jbyteArray byte_array, jint offset, jint length) +{ + if (length != 0) + throw new ::java::lang::InternalError + (JvNewStringLatin1 ("Error function not implemented for Win32 target.")); + return 0; +} + Index: gcc/libjava/gnu/java/net/natPlainDatagramSocketImplWin32.cc =================================================================== --- gcc.orig/libjava/gnu/java/net/natPlainDatagramSocketImplWin32.cc 2009-07-19 12:06:54.197476000 +0200 +++ gcc/libjava/gnu/java/net/natPlainDatagramSocketImplWin32.cc 2009-07-19 12:13:45.721476500 +0200 @@ -75,7 +75,8 @@ // We use native_fd in place of fd here. From leaving fd null we avoid // the double close problem in FileDescriptor.finalize. - native_fd = (jint) hSocket; + // Fixme, it is pretty dangerous to cast here a HANDLE to integer scalar. + native_fd = (jint) (__UINTPTR_TYPE__) hSocket; } void Index: gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc =================================================================== --- gcc.orig/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:06:54.200476000 +0200 +++ gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:13:45.727476500 +0200 @@ -58,7 +58,8 @@ // We use native_fd in place of fd here. From leaving fd null we avoid // the double close problem in FileDescriptor.finalize. - native_fd = (jint) hSocket; + // Fixme, it isn't correct to cast a HANDLE to integer scalar here for x64 + native_fd = (jint) (__UINTPTR_TYPE__) hSocket; } void @@ -304,7 +305,8 @@ // Cast this to a HANDLE so we can make // it non-inheritable via _Jv_platform_close_on_exec. - hSocket = (HANDLE) new_socket; + // Fixme, it isn't correct to cast a integer scalar here to HANDLE for x64 + hSocket = (HANDLE) (__UINTPTR_TYPE__) new_socket; _Jv_platform_close_on_exec (hSocket); jbyteArray raddr; @@ -325,8 +327,8 @@ #endif else throw new ::java::net::SocketException (JvNewStringUTF ("invalid family")); - - s->native_fd = (jint) hSocket; + // Fixme, it isn't correct to cast a HANDLE to integer scalar here for x64 + s->native_fd = (jint) (__UINTPTR_TYPE__) hSocket; s->localport = localport; s->address = ::java::net::InetAddress::getByAddress (raddr); s->port = rport; Index: gcc/libjava/gnu/java/nio/channels/natFileChannelWin32.cc =================================================================== --- gcc.orig/libjava/gnu/java/nio/channels/natFileChannelWin32.cc 2009-07-19 12:06:54.205476000 +0200 +++ gcc/libjava/gnu/java/nio/channels/natFileChannelWin32.cc 2009-07-19 12:13:45.731476500 +0200 @@ -66,11 +66,14 @@ void FileChannelImpl::init(void) { - in = new FileChannelImpl((jint)(GetStdHandle (STD_INPUT_HANDLE)), + // Fixme: it isn't correct to cast HANDLE to lower precission integer scalar for x64 + in = new FileChannelImpl((jint)(__UINTPTR_TYPE__)(GetStdHandle (STD_INPUT_HANDLE)), FileChannelImpl::READ); - out = new FileChannelImpl((jint)(GetStdHandle (STD_OUTPUT_HANDLE)), + // Fixme: it isn't correct to cast HANDLE to lower precission integer scalar for x64 + out = new FileChannelImpl((jint)(__UINTPTR_TYPE__)(GetStdHandle (STD_OUTPUT_HANDLE)), FileChannelImpl::WRITE); - err = new FileChannelImpl((jint)(GetStdHandle (STD_ERROR_HANDLE)), + // Fixme: it isn't correct to cast HANDLE to lower precission integer scalar for x64 + err = new FileChannelImpl((jint)(__UINTPTR_TYPE__)(GetStdHandle (STD_ERROR_HANDLE)), FileChannelImpl::WRITE); } @@ -144,7 +147,8 @@ // closing this file. _Jv_platform_close_on_exec (handle); - return (jint) handle; + // Fixme: it isn't correct to cast HANDLE to lower precission integer scalar for x64 + return (jint) (__UINTPTR_TYPE__) handle; } void @@ -197,8 +201,9 @@ void FileChannelImpl::implCloseChannel (void) { - HANDLE save = (HANDLE)fd; - fd = (jint)INVALID_HANDLE_VALUE; + HANDLE save = (HANDLE) (__UINTPTR_TYPE__) fd; + // Fixme: it isn't correct to cast HANDLE to lower precission integer scalar for x64 + fd = (jint) (__UINTPTR_TYPE__) INVALID_HANDLE_VALUE; if (! CloseHandle (save)) _Jv_ThrowIOException (); } Index: gcc/libjava/include/jvm.h =================================================================== --- gcc.orig/libjava/include/jvm.h 2009-07-19 12:06:54.215476000 +0200 +++ gcc/libjava/include/jvm.h 2009-07-19 12:13:45.737476500 +0200 @@ -481,7 +481,7 @@ // This was chosen to yield relatively well distributed results on // both 32- and 64-bit architectures. Note 0x7fffffff is prime. // FIXME: we assume sizeof(long) == sizeof(void *). - return (jint) ((unsigned long) obj % 0x7fffffff); + return (jint) ((__UINTPTR_TYPE__) obj % 0x7fffffff); } // Return a raw pointer to the elements of an array given the array Index: gcc/libjava/include/win32.h =================================================================== --- gcc.orig/libjava/include/win32.h 2009-07-19 12:06:54.219476000 +0200 +++ gcc/libjava/include/win32.h 2009-07-19 12:13:45.743476500 +0200 @@ -93,7 +93,12 @@ // Type of libffi ABI used by JNICALL methods. NOTE: This must agree // with the JNICALL definition in jni.h +#ifdef _WIN64 +// For x64 Windows there is no stdcall specific call. +#define _Jv_platform_ffi_abi FFI_DEFAULT_ABI +#else #define _Jv_platform_ffi_abi FFI_STDCALL +#endif /* Useful helper classes and methods. */ Index: gcc/libjava/java/lang/natClass.cc =================================================================== --- gcc.orig/libjava/java/lang/natClass.cc 2009-07-19 12:06:54.228476000 +0200 +++ gcc/libjava/java/lang/natClass.cc 2009-07-19 12:13:45.748476500 +0200 @@ -732,7 +732,7 @@ // Step 2. java::lang::Thread *self = java::lang::Thread::currentThread(); - self = (java::lang::Thread *) ((long) self | 1); + self = (java::lang::Thread *) ((__UINTPTR_TYPE__) self | 1); while (state == JV_STATE_IN_PROGRESS && thread && thread != self) wait (); Index: gcc/libjava/java/lang/natClassLoader.cc =================================================================== --- gcc.orig/libjava/java/lang/natClassLoader.cc 2009-07-19 12:06:54.232476000 +0200 +++ gcc/libjava/java/lang/natClassLoader.cc 2009-07-19 12:13:45.754476500 +0200 @@ -290,7 +290,7 @@ { jclass klass = *classes; - _Jv_CheckABIVersion ((unsigned long) klass->next_or_version); + _Jv_CheckABIVersion ((__UINTPTR_TYPE__) klass->next_or_version); (*_Jv_RegisterClassHook) (klass); } } @@ -307,7 +307,7 @@ { jclass klass = classes[i]; - _Jv_CheckABIVersion ((unsigned long) klass->next_or_version); + _Jv_CheckABIVersion ((__UINTPTR_TYPE__) klass->next_or_version); (*_Jv_RegisterClassHook) (klass); } } @@ -317,7 +317,7 @@ _Jv_NewClassFromInitializer (const char *class_initializer) { const unsigned long version - = ((unsigned long) + = ((__UINTPTR_TYPE__) ((::java::lang::Class *)class_initializer)->next_or_version); _Jv_CheckABIVersion (version); @@ -391,7 +391,7 @@ if (system_class_list != SYSTEM_LOADER_INITIALIZED) { - unsigned long abi = (unsigned long) klass->next_or_version; + unsigned long abi = (__UINTPTR_TYPE__) klass->next_or_version; if (! _Jv_ClassForBootstrapLoader (abi)) { klass->next_or_version = system_class_list; Index: gcc/libjava/java/lang/natString.cc =================================================================== --- gcc.orig/libjava/java/lang/natString.cc 2009-07-19 12:06:54.236476000 +0200 +++ gcc/libjava/java/lang/natString.cc 2009-07-19 12:13:45.758476500 +0200 @@ -50,9 +50,9 @@ #define DELETED_STRING ((jstring)(~0)) #define SET_STRING_IS_INTERNED(STR) /* nothing */ -#define UNMASK_PTR(Ptr) (((unsigned long) (Ptr)) & ~0x01) -#define MASK_PTR(Ptr) (((unsigned long) (Ptr)) | 0x01) -#define PTR_MASKED(Ptr) (((unsigned long) (Ptr)) & 0x01) +#define UNMASK_PTR(Ptr) (((__UINTPTR_TYPE__) (Ptr)) & ~0x01) +#define MASK_PTR(Ptr) (((__UINTPTR_TYPE__) (Ptr)) | 0x01) +#define PTR_MASKED(Ptr) (((__UINTPTR_TYPE__) (Ptr)) & 0x01) /* Find a slot where the string with elements DATA, length LEN, and hash HASH should go in the strhash table of interned strings. */ Index: gcc/libjava/java/lang/natWin32Process.cc =================================================================== --- gcc.orig/libjava/java/lang/natWin32Process.cc 2009-07-19 12:06:54.240476000 +0200 +++ gcc/libjava/java/lang/natWin32Process.cc 2009-07-19 12:13:45.762476500 +0200 @@ -59,7 +59,7 @@ if (procHandle) { CloseHandle((HANDLE) procHandle); - procHandle = (jint) INVALID_HANDLE_VALUE; + procHandle = (jint) (__UINTPTR_TYPE__) INVALID_HANDLE_VALUE; } } @@ -221,7 +221,7 @@ { using namespace java::io; - procHandle = (jint) INVALID_HANDLE_VALUE; + procHandle = (jint) (__UINTPTR_TYPE__) INVALID_HANDLE_VALUE; // Reconstruct the command line. jstring *elts = elements (progarray); @@ -293,16 +293,16 @@ : ChildProcessPipe::OUTPUT); outputStream = new FileOutputStream (new FileChannelImpl ( - (jint) aChildStdIn.getParentHandle (), + (jint) (__UINTPTR_TYPE__) aChildStdIn.getParentHandle (), FileChannelImpl::WRITE)); inputStream = new FileInputStream (new FileChannelImpl ( - (jint) aChildStdOut.getParentHandle (), + (jint) (__UINTPTR_TYPE__) aChildStdOut.getParentHandle (), FileChannelImpl::READ)); if (redirect) errorStream = Win32Process$EOFInputStream::instance; else errorStream = new FileInputStream (new FileChannelImpl ( - (jint) aChildStdErr.getParentHandle (), + (jint) (__UINTPTR_TYPE__) aChildStdErr.getParentHandle (), FileChannelImpl::READ)); // Now create the child process. @@ -343,7 +343,7 @@ _Jv_WinStrError (_T("Error creating child process"), dwErrorCode)); } - procHandle = (jint ) pi.hProcess; + procHandle = (jint ) (__UINTPTR_TYPE__) pi.hProcess; _Jv_Free (cmdLine); if (env != NULL) Index: gcc/libjava/link.cc =================================================================== --- gcc.orig/libjava/link.cc 2009-07-19 12:06:54.244476000 +0200 +++ gcc/libjava/link.cc 2009-07-19 12:13:45.767476500 +0200 @@ -1408,7 +1408,7 @@ (const char*)signature->chars()); fprintf (stderr, " [%d] = offset %d\n", index + 1, - (int)(unsigned long)klass->itable->addresses[index * 2 + 1]); + (int)(__UINTPTR_TYPE__)klass->itable->addresses[index * 2 + 1]); } } ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-19 11:13 ` Kai Tietz @ 2009-07-19 13:16 ` Dave Korn 2009-07-19 13:31 ` Kai Tietz 2009-07-19 15:23 ` Andrew Haley 0 siblings, 2 replies; 20+ messages in thread From: Dave Korn @ 2009-07-19 13:16 UTC (permalink / raw) To: Kai Tietz; +Cc: Andrew Pinski, Andrew Haley, Dave Korn, gcc, java Kai Tietz wrote: > There are a lot of issues about casting HANDLE values into jint types, > which is for x86 valid, but for x64 can lead potential to pointer > truncations. Those part need some review by libjava maintainers. My > patch simply casts those kind of pointers via __UINTPTR_TYPE__ into > integer scalar before casting it into jint. I put comments at those > places, where some rework is necessary. Argh. You're replacing a bunch of warnings that draw attention to a real problem by a bunch of silent fixmes in the code. That's a bit scary to me. > Index: gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc > =================================================================== > --- gcc.orig/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:06:54.200476000 +0200 > +++ gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:13:45.727476500 +0200 > @@ -58,7 +58,8 @@ > > // We use native_fd in place of fd here. From leaving fd null we avoid > // the double close problem in FileDescriptor.finalize. > - native_fd = (jint) hSocket; > + // Fixme, it isn't correct to cast a HANDLE to integer scalar here for x64 > + native_fd = (jint) (__UINTPTR_TYPE__) hSocket; > } Question is, can we change the sizes of the members of class objects, such as gnu::java::net::PlainSocketImpl::native_fd, or do these objects and their layout form part of an ABI, and/or do they ever get serialised? The Java guys will be able to tell us. cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-19 13:16 ` Dave Korn @ 2009-07-19 13:31 ` Kai Tietz 2009-07-19 13:45 ` Dave Korn 2009-07-19 15:23 ` Andrew Haley 1 sibling, 1 reply; 20+ messages in thread From: Kai Tietz @ 2009-07-19 13:31 UTC (permalink / raw) To: Dave Korn; +Cc: Andrew Pinski, Andrew Haley, gcc, java 2009/7/19 Dave Korn <dave.korn.cygwin@googlemail.com>: > Kai Tietz wrote: > >> There are a lot of issues about casting HANDLE values into jint types, >> which is for x86 valid, but for x64 can lead potential to pointer >> truncations. Those part need some review by libjava maintainers. My >> patch simply casts those kind of pointers via __UINTPTR_TYPE__ into >> integer scalar before casting it into jint. I put comments at those >> places, where some rework is necessary. > > Argh. You're replacing a bunch of warnings that draw attention to a real > problem by a bunch of silent fixmes in the code. That's a bit scary to me. Right, therefore those comments are for. But otherwise I couldn't get it build, as those kind of failures are treated as errors (what is in fact a good thing). >> Index: gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc >> =================================================================== >> --- gcc.orig/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:06:54.200476000 +0200 >> +++ gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:13:45.727476500 +0200 >> @@ -58,7 +58,8 @@ >> >> // We use native_fd in place of fd here. From leaving fd null we avoid >> // the double close problem in FileDescriptor.finalize. >> - native_fd = (jint) hSocket; >> + // Fixme, it isn't correct to cast a HANDLE to integer scalar here for x64 >> + native_fd = (jint) (__UINTPTR_TYPE__) hSocket; >> } > > > Question is, can we change the sizes of the members of class objects, such > as gnu::java::net::PlainSocketImpl::native_fd, or do these objects and their > layout form part of an ABI, and/or do they ever get serialised? The Java guys > will be able to tell us. > > cheers, > DaveK This was the reason, why I didn't changed api here. The final patch I see here done by the java team, as I have no real idea, if those types and members are part of abi, here. If it is there are ways to solve this (e.g. making abstract handle values for OS handles as example). So it is for sure necessary that a java maintainer takes action here. Cheers, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-19 13:31 ` Kai Tietz @ 2009-07-19 13:45 ` Dave Korn 2009-07-19 13:55 ` Kai Tietz 0 siblings, 1 reply; 20+ messages in thread From: Dave Korn @ 2009-07-19 13:45 UTC (permalink / raw) To: Kai Tietz; +Cc: Dave Korn, Andrew Pinski, Andrew Haley, gcc, java Kai Tietz wrote: > 2009/7/19 Dave Korn <dave.korn.cygwin@googlemail.com>: >> Kai Tietz wrote: >> >>> There are a lot of issues about casting HANDLE values into jint types, >>> which is for x86 valid, but for x64 can lead potential to pointer >>> truncations. Those part need some review by libjava maintainers. My >>> patch simply casts those kind of pointers via __UINTPTR_TYPE__ into >>> integer scalar before casting it into jint. I put comments at those >>> places, where some rework is necessary. >> Argh. You're replacing a bunch of warnings that draw attention to a real >> problem by a bunch of silent fixmes in the code. That's a bit scary to me. > > Right, therefore those comments are for. But otherwise I couldn't get > it build, as those kind of failures are treated as errors (what is in > fact a good thing). Yes, so since they are a good thing, you should *not* get rid of them. It is better for it not to build than for it to silently build bad code. If you want it to work, you should make it *actually* work, otherwise just add it to noconfigdirs until such time as you can make it work. There is not only no point successfully building a broken library, there is _less_ than no point. Whenever adding fixmes, you must plan on there being a very great likelihood of them getting forgotten and never fixed. Rule #1 of maintenance-friendly coding. >> Question is, can we change the sizes of the members of class objects, such >> as gnu::java::net::PlainSocketImpl::native_fd, or do these objects and their >> layout form part of an ABI, and/or do they ever get serialised? The Java guys >> will be able to tell us. > This was the reason, why I didn't changed api here. The final patch I > see here done by the java team, as I have no real idea, if those types > and members are part of abi, here. If it is there are ways to solve > this (e.g. making abstract handle values for OS handles as example). > So it is for sure necessary that a java maintainer takes action here. I fail to see the value of building a broken libjava for w64. It's not a step you need to get past on the roadmap to making a working java for w64. Just don't build it at all. --disable-libjava or $noconfigdirs. Alternatively, wait a few hours until the java guys have a chance to respond. Maybe we can just change the datatypes, after all. But I really can't see any use and only harm in adding a silently broken implementation. cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-19 13:45 ` Dave Korn @ 2009-07-19 13:55 ` Kai Tietz 2009-07-19 14:39 ` Dave Korn 0 siblings, 1 reply; 20+ messages in thread From: Kai Tietz @ 2009-07-19 13:55 UTC (permalink / raw) To: Dave Korn; +Cc: Andrew Pinski, Andrew Haley, gcc, java 2009/7/19 Dave Korn <dave.korn.cygwin@googlemail.com>: > Kai Tietz wrote: >> 2009/7/19 Dave Korn <dave.korn.cygwin@googlemail.com>: >>> Kai Tietz wrote: >>> >>>> There are a lot of issues about casting HANDLE values into jint types, >>>> which is for x86 valid, but for x64 can lead potential to pointer >>>> truncations. Those part need some review by libjava maintainers. My >>>> patch simply casts those kind of pointers via __UINTPTR_TYPE__ into >>>> integer scalar before casting it into jint. I put comments at those >>>> places, where some rework is necessary. >>> Argh. You're replacing a bunch of warnings that draw attention to a real >>> problem by a bunch of silent fixmes in the code. That's a bit scary to me. >> >> Right, therefore those comments are for. But otherwise I couldn't get >> it build, as those kind of failures are treated as errors (what is in >> fact a good thing). > > Yes, so since they are a good thing, you should *not* get rid of them. It > is better for it not to build than for it to silently build bad code. If you > want it to work, you should make it *actually* work, otherwise just add it to > noconfigdirs until such time as you can make it work. There is not only no > point successfully building a broken library, there is _less_ than no point. > > Whenever adding fixmes, you must plan on there being a very great likelihood > of them getting forgotten and never fixed. Rule #1 of maintenance-friendly > coding. > >>> Question is, can we change the sizes of the members of class objects, such >>> as gnu::java::net::PlainSocketImpl::native_fd, or do these objects and their >>> layout form part of an ABI, and/or do they ever get serialised? The Java guys >>> will be able to tell us. > >> This was the reason, why I didn't changed api here. The final patch I >> see here done by the java team, as I have no real idea, if those types >> and members are part of abi, here. If it is there are ways to solve >> this (e.g. making abstract handle values for OS handles as example). >> So it is for sure necessary that a java maintainer takes action here. > > I fail to see the value of building a broken libjava for w64. It's not a > step you need to get past on the roadmap to making a working java for w64. > Just don't build it at all. --disable-libjava or $noconfigdirs. > > Alternatively, wait a few hours until the java guys have a chance to > respond. Maybe we can just change the datatypes, after all. But I really > can't see any use and only harm in adding a silently broken implementation. Well, in standard I agree. But in this special case is the code just broken for apps using more then 32-bit address range for heap (and handles). Btw a run of testsuite works here in pretty much cases. The patch I sent here is more a head-up (and it fixes build for 32-bit windows builds, too). If I keep stuff in my back-chamber, things won't improve and nobody knows that there is an issue present. Cheers, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-19 13:55 ` Kai Tietz @ 2009-07-19 14:39 ` Dave Korn 0 siblings, 0 replies; 20+ messages in thread From: Dave Korn @ 2009-07-19 14:39 UTC (permalink / raw) To: Kai Tietz; +Cc: Dave Korn, Andrew Pinski, Andrew Haley, gcc, java Kai Tietz wrote: > The patch I sent here is more a head-up (and it fixes build for 32-bit > windows builds, too). Ok, I see the point in a headsup (but I'm not sure it was worth spending the time to generate the patch). I take it the sole problem with the 32-bit build is the missing natVMSecureRandomWin32.cc? You should break that out into a separate patch. cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-19 13:16 ` Dave Korn 2009-07-19 13:31 ` Kai Tietz @ 2009-07-19 15:23 ` Andrew Haley 2009-07-19 17:07 ` Dave Korn 1 sibling, 1 reply; 20+ messages in thread From: Andrew Haley @ 2009-07-19 15:23 UTC (permalink / raw) To: Dave Korn; +Cc: Kai Tietz, Andrew Pinski, gcc, java On 07/19/2009 02:29 PM, Dave Korn wrote: > Kai Tietz wrote: > >> There are a lot of issues about casting HANDLE values into jint types, >> which is for x86 valid, but for x64 can lead potential to pointer >> truncations. Those part need some review by libjava maintainers. My >> patch simply casts those kind of pointers via __UINTPTR_TYPE__ into >> integer scalar before casting it into jint. I put comments at those >> places, where some rework is necessary. > > Argh. You're replacing a bunch of warnings that draw attention to a real > problem by a bunch of silent fixmes in the code. That's a bit scary to me. Me too. That patch will not be accepted. >> Index: gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc >> =================================================================== >> --- gcc.orig/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:06:54.200476000 +0200 >> +++ gcc/libjava/gnu/java/net/natPlainSocketImplWin32.cc 2009-07-19 12:13:45.727476500 +0200 >> @@ -58,7 +58,8 @@ >> >> // We use native_fd in place of fd here. From leaving fd null we avoid >> // the double close problem in FileDescriptor.finalize. >> - native_fd = (jint) hSocket; >> + // Fixme, it isn't correct to cast a HANDLE to integer scalar here for x64 >> + native_fd = (jint) (__UINTPTR_TYPE__) hSocket; >> } > > > Question is, can we change the sizes of the members of class objects, such > as gnu::java::net::PlainSocketImpl::native_fd, or do these objects and their > layout form part of an ABI, and/or do they ever get serialised? The Java guys > will be able to tell us. Yes, you can change them. Yes, they are part of an ABI. native_fd should be a jlong. Andrew. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-19 15:23 ` Andrew Haley @ 2009-07-19 17:07 ` Dave Korn 0 siblings, 0 replies; 20+ messages in thread From: Dave Korn @ 2009-07-19 17:07 UTC (permalink / raw) To: Andrew Haley; +Cc: Dave Korn, Kai Tietz, Andrew Pinski, gcc, java Andrew Haley wrote: > > Yes, you can change them. Yes, they are part of an ABI. native_fd should be > a jlong. That's the best possible answer to our questions! Kai? I'll leave this with you; I'm busy tracking down libffi FAILs right now. cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: libjava seems to miss some files for win32 2009-07-18 17:27 ` Kai Tietz 2009-07-18 18:15 ` Dave Korn @ 2009-07-18 18:27 ` Dave Korn 1 sibling, 0 replies; 20+ messages in thread From: Dave Korn @ 2009-07-18 18:27 UTC (permalink / raw) To: Kai Tietz; +Cc: Dave Korn, Andrew Haley, gcc, java Kai Tietz wrote: Oh, I forgot to address: > I just suggested this patch, to have at least an implementation here > for win32 for further improvement This is the java security package. Having a vulnerable implementation is worse IMO than having none at all; I think it would be better to just throw an exception than use rand(), so if you want a patch to just get the build working, that would be a fairly simple solution. cheers, DaveK ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2009-07-19 17:07 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <90baa01f0907180506h1a58152du5d45d66628043ad9@mail.gmail.com> 2009-07-18 13:53 ` RFA: libjava seems to miss some files for win32 Dave Korn 2009-07-18 16:01 ` Kai Tietz 2009-07-18 17:09 ` Dave Korn 2009-07-18 17:27 ` Kai Tietz 2009-07-18 18:15 ` Dave Korn 2009-07-18 19:09 ` Andrew Haley 2009-07-18 19:19 ` Andrew Pinski 2009-07-18 19:23 ` Dave Korn 2009-07-18 19:27 ` Kai Tietz 2009-07-18 19:23 ` Kai Tietz 2009-07-18 19:29 ` Andrew Pinski 2009-07-19 11:13 ` Kai Tietz 2009-07-19 13:16 ` Dave Korn 2009-07-19 13:31 ` Kai Tietz 2009-07-19 13:45 ` Dave Korn 2009-07-19 13:55 ` Kai Tietz 2009-07-19 14:39 ` Dave Korn 2009-07-19 15:23 ` Andrew Haley 2009-07-19 17:07 ` Dave Korn 2009-07-18 18:27 ` Dave Korn
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).