Re: [PATCH] Cygwin: Build cygwin-console-helper with correct compiler

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Jun 26 01:48, Mark Geisert wrote:
> Corinna Vinschen wrote:
> > On Jun 25 00:54, Mark Geisert wrote:
> > > ---
> > >   winsup/utils/Makefile.in | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/winsup/utils/Makefile.in b/winsup/utils/Makefile.in
> > > index b64f457e7..cebf39572 100644
> > > --- a/winsup/utils/Makefile.in
> > > +++ b/winsup/utils/Makefile.in
> > > @@ -64,7 +64,7 @@ MINGW_BINS := ${addsuffix .exe,cygcheck cygwin-console-helper ldh strace}
> > >   # List all objects to be compiled in MinGW mode.  Any object not on this
> > >   # list will will be compiled in Cygwin mode implicitly, so there is no
> > >   # need for a CYGWIN_OBJS.
> > > -MINGW_OBJS := bloda.o cygcheck.o dump_setup.o ldh.o path.o strace.o
> > > +MINGW_OBJS := bloda.o cygcheck.o cygwin-console-helper.o dump_setup.o ldh.o path.o strace.o
> > >   MINGW_LDFLAGS:=-static
> > >   CYGCHECK_OBJS:=cygcheck.o bloda.o path.o dump_setup.o
> > > -- 
> > > 2.21.0
> > 
> > Careful!  This leads to a warning when building on 64 bit:
> > 
> >    cygwin-console-helper.cc: In function 'int main(int, char**)':
> >    cygwin-console-helper.cc:8:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >     HANDLE h = (HANDLE) strtoul (argv[1], &end, 0);
> >                                                  ^
> >    cygwin-console-helper.cc:10:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >     h = (HANDLE) strtoul (argv[2], &end, 0);
> >                                           ^
> > 
> > Note that strtoul returns an unsigned long.  Mingw compiles
> > for native Windows, which is LLP64 rather than LP64:
> > 
> >    mingw:sizeof(long) == 4
> >    cygwin:sizeof(long) == 8
> > 
> > This needs fixing as well (use strtoull).
> 
> I appreciate the comments.  These warnings have "always" been present.

I don't see any warning in terms of building cygwin-console-helper in 
the current state.  However, it suddenly occured to me that, even without
a warning, this was always wrong.  The compiler generates code for 8 byte
long and the linker links against libs expecting 4 byte longs.

The fact that this works is ... not exactly magic, but first, the
arguments and return values are in registers anyway, and second, HANDLEs
are only using the lower 4 bytes even though they are 8 bytes in size.

> I didn't make clear the reason for this one-line patch to Makefile.in: A
> 'make -j 6' over the Cygwin source tree would sometimes fail because the
> link step for cygwin-console-helper uses a different gcc than the compile
> step did in parallel builds.

Huh, really?  I'm building with make -j42 on a 32 core machine, but I
never saw this problem.

>   Can you accept this patch as-is for what it
> does for builds?

Yes, we can do that, but it might be a good idea to order the patches
so that the *first* patch fixes the actual problem in cygwin-console-helper
and the *second* patch fixes the Makefile.  Consider this patch approved,
I just wait for the other one, ok?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Attachment: signature.asc
Description: PGP signature