PING: Re: patch: libiberty adjustments for _WIN64

PING: Re: patch: libiberty adjustments for _WIN64

[Kai Tietz]

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

Hi Jakub,

> xrealloc guarantees returning non-NULL or exit, so you don't have to 
worry
> about allocation failures.

Thank you for this simplification. I added to the attached patch.

Cheers,
 i.A. Kai Tietz



|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger


[-- Attachment #2: iberty_win64.txt --]
[-- Type: text/plain, Size: 1663 bytes --]

Index: libiberty/pex-win32.c
===================================================================
--- libiberty.orig/pex-win32.c
+++ libiberty/pex-win32.c
@@ -63,6 +63,12 @@ Boston, MA 02110-1301, USA.  */
 
 extern char *stpcpy (char *dst, const char *src);
 
+/* Some specific maps for handles to "long" sized pid.  */
+#ifdef _WIN64
+static HANDLE *pid_map = NULL;
+static size_t pid_map_max = 0;
+#endif
+
 /* Ensure that the executable pathname uses Win32 backslashes. This
    is not necessary on NT, but on W9x, forward slashes causes
    failure of spawn* and exec* functions (and probably any function
@@ -606,7 +612,33 @@ win32_spawn (const char *executable,
   if (env_block)
     free (env_block);
 
+#ifndef _WIN64
   return (long) pi->hProcess;
+#else
+  {
+    long ret = 0;
+
+    if (pid_map)
+      while (ret < pid_map_max && pid_map[ret] != -1)
+        ++ret;
+
+    if (ret == pid_map_max)
+      {
+        pid_map = (HANDLE *) xrealloc (pid_map, sizeof (HANDLE) * (pid_map_max + 8));
+        memset (&pid_map[pid_map_max], -1, sizeof (HANDLE) * 8);
+        pid_map_max += 8;
+        pid_map[ret] = ps->hProcess;
+        return ret;
+      }
+  }
+#endif
 
  error:
   if (env_block)
@@ -818,8 +850,12 @@ pex_win32_wait (struct pex_obj *obj ATTR
   if (time != NULL)
     memset (time, 0, sizeof *time);
 
+#ifndef _WIN64
   h = (HANDLE) pid;
-
+#else
+  h = (pid_map ? NULL : pid_map[pid]);
+  pid_map[pid] = (HANDLE) -1;
+#endif
   /* FIXME: If done is non-zero, we should probably try to kill the
      process.  */
   if (WaitForSingleObject (h, INFINITE) != WAIT_OBJECT_0)
=

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Ian Lance Taylor]

Kai Tietz <Kai.Tietz@onevision.com> writes:

> +/* Some specific maps for handles to "long" sized pid.  */
> +#ifdef _WIN64
> +static HANDLE *pid_map = NULL;
> +static size_t pid_map_max = 0;
> +#endif

This code all seems like a complex workaround for the fact that
pex-common.h uses "long" as the return type for the exec_child hook.
But pex-common.c really doesn't care what the value is.  It just takes
the value from the exec_child hook and passes it to the wait hook.  So
rather than introducing a new array in the heap, let's just change the
interface.

One approach would be to have libiberty/configure.ac define a header
file to be included for each supported target, and to have that header
file create a typedef to be used in pex-common.h and pex-common.c.
For pex-win32, this type would be HANDLE.

Ian

Re: PING: Re: patch: libiberty adjustments for _WIN64

[DJ Delorie]

> This code all seems like a complex workaround for the fact that
> pex-common.h uses "long" as the return type for the exec_child hook.
> But pex-common.c really doesn't care what the value is.  It just
> takes the value from the exec_child hook and passes it to the wait
> hook.  So rather than introducing a new array in the heap, let's
> just change the interface.

We've had this discussion before; the problem is that some platforms
have sizeof(long) > sizeof(void*) and others have sizeof(long) <
sizeof(void*).  So you can't just pass "anything" through that API
unless you have configure pick a suitable type and build libiberty
with it as well as your application.

It would be nice to have a way for libiberty's configure to provide a
.h for other applications to use; this would solve a couple of other
problems also.  I'm not sure how easy that would be to implement.
Alternative is to provide a *.m4 file for clients to include, so that
at least the tests are consistent.  Otherwise we risk having a client
use a different ABI than libiberty.

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Ian Lance Taylor]

DJ Delorie <dj@redhat.com> writes:

> > This code all seems like a complex workaround for the fact that
> > pex-common.h uses "long" as the return type for the exec_child hook.
> > But pex-common.c really doesn't care what the value is.  It just
> > takes the value from the exec_child hook and passes it to the wait
> > hook.  So rather than introducing a new array in the heap, let's
> > just change the interface.
> 
> We've had this discussion before; the problem is that some platforms
> have sizeof(long) > sizeof(void*) and others have sizeof(long) <
> sizeof(void*).  So you can't just pass "anything" through that API
> unless you have configure pick a suitable type and build libiberty
> with it as well as your application.

As far as I can see, the type used here is never exposed outside of
the pex_run interface.

Ian

Re: PING: Re: patch: libiberty adjustments for _WIN64

[DJ Delorie]

> As far as I can see, the type used here is never exposed outside of
> the pex_run interface.

Hmmm.. maybe I was thinking of some other libiberty interface, then.
Perhaps hash tables?

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Daniel Jacobowitz]

On Wed, Jul 25, 2007 at 04:51:02PM -0400, DJ Delorie wrote:
> 
> > As far as I can see, the type used here is never exposed outside of
> > the pex_run interface.
> 
> Hmmm.. maybe I was thinking of some other libiberty interface, then.
> Perhaps hash tables?

I think that was the splay tree keys.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Kai Tietz]

Daniel Jacobowitz wrote on 25.07.2007 23:48:07:

> On Wed, Jul 25, 2007 at 04:51:02PM -0400, DJ Delorie wrote:
> > 
> > > As far as I can see, the type used here is never exposed outside of
> > > the pex_run interface.
> > 
> > Hmmm.. maybe I was thinking of some other libiberty interface, then.
> > Perhaps hash tables?
> 
> I think that was the splay tree keys.

Yes, the splay tree keys are this issue.

But why can't we use the 'pid_t' type for pex ?

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Ian Lance Taylor]

Kai Tietz <Kai.Tietz@onevision.com> writes:

> Daniel Jacobowitz wrote on 25.07.2007 23:48:07:
> 
> > On Wed, Jul 25, 2007 at 04:51:02PM -0400, DJ Delorie wrote:
> > > 
> > > > As far as I can see, the type used here is never exposed outside of
> > > > the pex_run interface.
> > > 
> > > Hmmm.. maybe I was thinking of some other libiberty interface, then.
> > > Perhaps hash tables?
> > 
> > I think that was the splay tree keys.
> 
> Yes, the splay tree keys are this issue.
> 
> But why can't we use the 'pid_t' type for pex ?

Just because the pex code is intended to be highly portable, and not
every system defines a pid_t type.  You can use pid_t if you make sure
that it is defined reasonably.

Ian

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Kai Tietz]

Hi Ian

> > Daniel Jacobowitz wrote on 25.07.2007 23:48:07:
> > 
> > > On Wed, Jul 25, 2007 at 04:51:02PM -0400, DJ Delorie wrote:
> > > > 
> > > > > As far as I can see, the type used here is never exposed outside 
of
> > > > > the pex_run interface.
> > > > 
> > > > Hmmm.. maybe I was thinking of some other libiberty interface, 
then.
> > > > Perhaps hash tables?
> > > 
> > > I think that was the splay tree keys.
> > 
> > Yes, the splay tree keys are this issue.
> > 
> > But why can't we use the 'pid_t' type for pex ?
> 
> Just because the pex code is intended to be highly portable, and not
> every system defines a pid_t type.  You can use pid_t if you make sure
> that it is defined reasonably.
It is defined by configure to an 'int' if not present. Alternative we 
could use the same kind of types defined in splay-tree.h (libi_uhostptr_t, 
...) as pex_uhostpid_t ?

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

patch: libiberty pex for _WIN64

[Kai Tietz]

Ian,

I prepared a patch for pex using pid_t instead of the type 'long' for 
system handles. This is necessary for _WIN64, because the long type isn't 
wide enougth to hold a pointer for this ABI The type pid_t is always 
defined by the configure in config.h if not defined by the header-set. 
Therefore it seems to be the best way to make this run.

ChangeLog:

2007-08-03  Kai Tietz   <kai.tietz@onevision.com>

        * pex-common.h: (pex_funcs): Retyped wait and exec_child to pid_t.
        * pex-djgpp.c: Likewise.
        * pex-msdos.c: Likewise.
        * pex-unix.c: Likewise.
        * pex-win32.c: Likewise.

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Ian Lance Taylor]

Kai Tietz <Kai.Tietz@onevision.com> writes:

> > > But why can't we use the 'pid_t' type for pex ?
> > 
> > Just because the pex code is intended to be highly portable, and not
> > every system defines a pid_t type.  You can use pid_t if you make sure
> > that it is defined reasonably.
> It is defined by configure to an 'int' if not present.

OK, do that, then.

> Alternative we 
> could use the same kind of types defined in splay-tree.h (libi_uhostptr_t, 
> ...) as pex_uhostpid_t ?

Please don't, I really dislike the #ifdef _WIN64 test and I think we
need to figure out how to get rid of it.

Ian

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Kai Tietz]

Ian,

> > Alternative we 
> > could use the same kind of types defined in splay-tree.h 
(libi_uhostptr_t, 
> > ...) as pex_uhostpid_t ?
> 
> Please don't, I really dislike the #ifdef _WIN64 test and I think we
> need to figure out how to get rid of it.

Maybe we could use a type test in configure for 'void *' and 'long' type 
and decide by there sizes ?

Regards,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Ian Lance Taylor]

Kai Tietz <Kai.Tietz@onevision.com> writes:

> > > Alternative we 
> > > could use the same kind of types defined in splay-tree.h 
> (libi_uhostptr_t, 
> > > ...) as pex_uhostpid_t ?
> > 
> > Please don't, I really dislike the #ifdef _WIN64 test and I think we
> > need to figure out how to get rid of it.
> 
> Maybe we could use a type test in configure for 'void *' and 'long' type 
> and decide by there sizes ?

Is the issue just void* and long?  In that case, perhaps we can use
intptr_t.  I thought we had to handle long long as well.

Ian

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Kai Tietz]

Ian,

> > Maybe we could use a type test in configure for 'void *' and 'long' 
type 
> > and decide by there sizes ?
> 
> Is the issue just void* and long?  In that case, perhaps we can use
> intptr_t.  I thought we had to handle long long as well.

For most targets the intptr_t would be perfect. But there are targets (or 
just one I am uncertain) that has in pointer less than a long. This would 
break things here.

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Ian Lance Taylor]

Kai Tietz <Kai.Tietz@onevision.com> writes:

> > Is the issue just void* and long?  In that case, perhaps we can use
> > intptr_t.  I thought we had to handle long long as well.
> 
> For most targets the intptr_t would be perfect. But there are targets (or 
> just one I am uncertain) that has in pointer less than a long. This would 
> break things here.

Arrggh, right.

Ian

Re: PING: Re: patch: libiberty adjustments for _WIN64

[Kai Tietz]

Ian,

> > For most targets the intptr_t would be perfect. But there are targets 
(or 
> > just one I am uncertain) that has in pointer less than a long. This 
would 
> > break things here.
> 
> Arrggh, right.

What we would need here is a 'longptr_t' definition for ISO, but there 
isn't one :(

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

PING: Re: patch: libiberty adjustments for _WIN64

[Kai Tietz]

Kai Tietz/Onevision wrote on 06.07.2007 14:24:28:

> Hello all,
> 
> This is a change for the x86_64-pc-mingw32 target in libiberty. For 
> _WIN64 HANDLE is bigger in size than the "long" type used in pex for
> the pid. Therefore on back-casting from a "long" to the HANDLE value
> (in pex_win32_wait) the system handle is getting corrupted.
> 
> ChangeLog:
> 2007-07-06 Kai Tietz <Kai.Tietz@onevision.com>
>  * libiberty/pex-win32.c: (pid_map, pid_map_max): New.
>  (win32_spawn): Use for _WIN64 pid_map for "long" typed
>  pids.
>  (pex_win32_wait): Likewise.

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger