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

Mark Geisert mark@maxrnd.com
Thu Jun 27 05:48:00 GMT 2019


Corinna Vinschen wrote:
> 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 have to amend my statement.  Warnings during whole-tree builds in general have 
"always" been present but I cannot be 100% sure this particular warning was 
present.  I looked through my most recent make log and there are only compiler 
warnings in newlib, not in winsup.  Sorry for my confusion.

> 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.

Well that's a very good counterexample.  I attributed it to parallel make steps 
but it was rare even with that condition.  When the build failed it was because 
linking with cygwin-console-helper.o was provoking an error something like "file 
format not recognized".  Which now appears to me to possibly be a 32/64 issue of 
mismatched tools.

Let's drop this report for now and I'll be sure to report more accurately if it 
happens again.

>>    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?

I've now submitted a patch for the actual problem separately.  Thanks for 
patiently helping me help where I can.

..mark



More information about the Cygwin-patches mailing list