Re: RFA: libjava seems to miss some files for win32

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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

Re: RFA: libjava seems to miss some files for win32

[Kai Tietz]

[-- 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;
+}
+

Re: RFA: libjava seems to miss some files for win32

[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

Re: RFA: libjava seems to miss some files for win32

[Kai Tietz]

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

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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

Re: RFA: libjava seems to miss some files for win32

[Andrew Haley]

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.

Re: RFA: libjava seems to miss some files for win32

[Andrew Pinski]

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

Re: RFA: libjava seems to miss some files for win32

[Kai Tietz]

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

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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

Re: RFA: libjava seems to miss some files for win32

[Kai Tietz]

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

Re: RFA: libjava seems to miss some files for win32

[Andrew Pinski]

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

Re: RFA: libjava seems to miss some files for win32

[Kai Tietz]

[-- 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]);
 	    }
 
 	}

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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

Re: RFA: libjava seems to miss some files for win32

[Kai Tietz]

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

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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

Re: RFA: libjava seems to miss some files for win32

[Kai Tietz]

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

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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

Re: RFA: libjava seems to miss some files for win32

[Andrew Haley]

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.

Re: RFA: libjava seems to miss some files for win32

[Dave Korn]

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