diff mbox series

[RFC] cgcc: only define __CYGWIN32__ for -m32 builds

Message ID b342ed82-2949-7a44-3cf7-23ae3d266cbf@ramsayjones.plus.com (mailing list archive)
State Mainlined, archived
Headers show
Series [RFC] cgcc: only define __CYGWIN32__ for -m32 builds | expand

Commit Message

Ramsay Jones Nov. 28, 2019, 5:45 p.m. UTC
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Luc,

This is marked RFC because it only improves the situation on 64-bit cygwin.
Without access to a (up-to-date) 32-bit cygwin, I can't experiment to find
a means to determine what platform I am on. I don't recall what the output
of 'uname' is on 32-bit cygwin, but I have a hunch that you can't tell which
is which from it's output. On 64-bit cygwin:

  $ uname -a
  CYGWIN_NT-10.0 satellite 3.0.7(0.338/5/3) 2019-04-30 18:08 x86_64 Cygwin
  $ uname -s
  CYGWIN_NT-10.0
  $ uname -o
  Cygwin
  $ 

[ie. I don't think 'uname -o' returns Cygwin32 or similar. :( ]

So, I don't know.

ATB,
Ramsay Jones

 cgcc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Luc Van Oostenryck Nov. 28, 2019, 8:06 p.m. UTC | #1
On Thu, Nov 28, 2019 at 05:45:06PM +0000, Ramsay Jones wrote:
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> 
> Hi Luc,

Hi,

> This is marked RFC because it only improves the situation on 64-bit cygwin.
> Without access to a (up-to-date) 32-bit cygwin, I can't experiment to find
> a means to determine what platform I am on. I don't recall what the output
> of 'uname' is on 32-bit cygwin, but I have a hunch that you can't tell which
> is which from it's output. On 64-bit cygwin:
> 
>   $ uname -a
>   CYGWIN_NT-10.0 satellite 3.0.7(0.338/5/3) 2019-04-30 18:08 x86_64 Cygwin
>   $ uname -s
>   CYGWIN_NT-10.0
>   $ uname -o
>   Cygwin
>   $ 
> 
> [ie. I don't think 'uname -o' returns Cygwin32 or similar. :( ]

Indeed, I'm guess it doesn't. 

> So, I don't know.

I see several possibilities:
1)  just this patch, wich is OK for 64-bit platform/compiler
    where 32-bit needs to be forced with -m32
2)  simply not define __CYGWIN32__ at all based on the
    conviction that it's only __CYGWIN__ that should be tested
3a) in cgcc add 'm32=1' in the i386 part if $m64 is not set
3b) in cgcc add 'm64=1' in the x86_64 part if $m32 is not set
    and change this patch to test $m64 instead of testing $m32
4a) in sparse itself, add something like:
	if (arch_mach == MACH_X86_64 && arch_os == OS_CYGWIN)
		add_pre_buffer("#undef __CYGWIN32__");
    or:
	if (arch_m64 != LP32 && arch_os == OS_CYGWIN)
		add_pre_buffer("#undef __CYGWIN32__");
4b) do not define __CYGWIN32__ in cgcc and add something like:
	if (arch_mach == MACH_i386 && arch_os == OS_CYGWIN)
		add_pre_buffer("#define __CYG_WIN32__ 1");
    or:
	if (arch_m64 == LP32 && arch_os == OS_CYGWIN)
		add_pre_buffer("#define __CYGWIN32__ 1");

For the long term, I would prefer something like 4a) or 4b)
but currently it would only work for native builds.

So, I think that 3a) should be the best.

Best regards,
-- Luc
Ramsay Jones Nov. 29, 2019, 1:15 a.m. UTC | #2
On 28/11/2019 20:06, Luc Van Oostenryck wrote:
> On Thu, Nov 28, 2019 at 05:45:06PM +0000, Ramsay Jones wrote:
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> ---
>>
>> Hi Luc,
> 
> Hi,
> 
>> This is marked RFC because it only improves the situation on 64-bit cygwin.
>> Without access to a (up-to-date) 32-bit cygwin, I can't experiment to find
>> a means to determine what platform I am on. I don't recall what the output
>> of 'uname' is on 32-bit cygwin, but I have a hunch that you can't tell which
>> is which from it's output. On 64-bit cygwin:
>>
>>   $ uname -a
>>   CYGWIN_NT-10.0 satellite 3.0.7(0.338/5/3) 2019-04-30 18:08 x86_64 Cygwin
>>   $ uname -s
>>   CYGWIN_NT-10.0
>>   $ uname -o
>>   Cygwin
>>   $ 
>>
>> [ie. I don't think 'uname -o' returns Cygwin32 or similar. :( ]
> 
> Indeed, I'm guess it doesn't. 
> 
>> So, I don't know.
> 
> I see several possibilities:
> 1)  just this patch, wich is OK for 64-bit platform/compiler
>     where 32-bit needs to be forced with -m32
> 2)  simply not define __CYGWIN32__ at all based on the
>     conviction that it's only __CYGWIN__ that should be tested
> 3a) in cgcc add 'm32=1' in the i386 part if $m64 is not set
> 3b) in cgcc add 'm64=1' in the x86_64 part if $m32 is not set
>     and change this patch to test $m64 instead of testing $m32
> 4a) in sparse itself, add something like:
> 	if (arch_mach == MACH_X86_64 && arch_os == OS_CYGWIN)
> 		add_pre_buffer("#undef __CYGWIN32__");
>     or:
> 	if (arch_m64 != LP32 && arch_os == OS_CYGWIN)
> 		add_pre_buffer("#undef __CYGWIN32__");
> 4b) do not define __CYGWIN32__ in cgcc and add something like:
> 	if (arch_mach == MACH_i386 && arch_os == OS_CYGWIN)
> 		add_pre_buffer("#define __CYG_WIN32__ 1");
>     or:
> 	if (arch_m64 == LP32 && arch_os == OS_CYGWIN)
> 		add_pre_buffer("#define __CYGWIN32__ 1");
> 
> For the long term, I would prefer something like 4a) or 4b)
> but currently it would only work for native builds.
> 
> So, I think that 3a) should be the best.

I nearly sent this patch instead:

$ git diff
diff --git a/cgcc b/cgcc
index 2223c97d..ddc6de23 100755
--- a/cgcc
+++ b/cgcc
@@ -252,10 +252,16 @@ sub add_specs {
     } elsif ($spec eq 'unix') {
        return ' -Dunix=1 -D__unix=1 -D__unix__=1';
     } elsif ( $spec =~ /^cygwin/) {
+       my $c32 = 0;
+       my $m = `uname -m`;
+
+       chomp $m;
+       $c32 = 1 if ($m =~ /^(i.?86|athlon)$/i);
+
        return &add_specs ('unix') .
            ' -fshort-wchar' .
            ' -D__CYGWIN__=1' .
-           ($m32 ? ' -D__CYGWIN32__=1' : '') .
+           (($m32 || ($c32 && !$m64)) ? ' -D__CYGWIN32__=1' : '') .
            " -D'_cdecl=__attribute__((__cdecl__))'" .
            " -D'__cdecl=__attribute__((__cdecl__))'" .
            " -D'_stdcall=__attribute__((__stdcall__))'" .
$ 

I was going to use 'uname -p', but the man page on Linux says that
this is 'not portable'.

However, other than 'hand testing', I could not be sure it would
work on an actual 32-bit cygwin system. (hand testing seems to
imply that it should work, but well ... :-D ).

ATB,
Ramsay Jones
Luc Van Oostenryck Nov. 30, 2019, 12:48 a.m. UTC | #3
On Fri, Nov 29, 2019 at 01:15:12AM +0000, Ramsay Jones wrote:
> 
> 
> On 28/11/2019 20:06, Luc Van Oostenryck wrote:
> > On Thu, Nov 28, 2019 at 05:45:06PM +0000, Ramsay Jones wrote:
> >>
> >> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> >> ---
> >>
> >> Hi Luc,
> > 
> > Hi,
> > 
> >> This is marked RFC because it only improves the situation on 64-bit cygwin.
> >> Without access to a (up-to-date) 32-bit cygwin, I can't experiment to find
> >> a means to determine what platform I am on. I don't recall what the output
> >> of 'uname' is on 32-bit cygwin, but I have a hunch that you can't tell which
> >> is which from it's output. On 64-bit cygwin:
> >>
> >>   $ uname -a
> >>   CYGWIN_NT-10.0 satellite 3.0.7(0.338/5/3) 2019-04-30 18:08 x86_64 Cygwin
> >>   $ uname -s
> >>   CYGWIN_NT-10.0
> >>   $ uname -o
> >>   Cygwin
> >>   $ 
> >>
> >> [ie. I don't think 'uname -o' returns Cygwin32 or similar. :( ]
> > 
> > Indeed, I'm guess it doesn't. 
> > 
> >> So, I don't know.
> > 
> > I see several possibilities:
> > 1)  just this patch, wich is OK for 64-bit platform/compiler
> >     where 32-bit needs to be forced with -m32
> > 2)  simply not define __CYGWIN32__ at all based on the
> >     conviction that it's only __CYGWIN__ that should be tested
> > 3a) in cgcc add 'm32=1' in the i386 part if $m64 is not set
> > 3b) in cgcc add 'm64=1' in the x86_64 part if $m32 is not set
> >     and change this patch to test $m64 instead of testing $m32
> > 4a) in sparse itself, add something like:
> > 	if (arch_mach == MACH_X86_64 && arch_os == OS_CYGWIN)
> > 		add_pre_buffer("#undef __CYGWIN32__");
> >     or:
> > 	if (arch_m64 != LP32 && arch_os == OS_CYGWIN)
> > 		add_pre_buffer("#undef __CYGWIN32__");
> > 4b) do not define __CYGWIN32__ in cgcc and add something like:
> > 	if (arch_mach == MACH_i386 && arch_os == OS_CYGWIN)
> > 		add_pre_buffer("#define __CYG_WIN32__ 1");
> >     or:
> > 	if (arch_m64 == LP32 && arch_os == OS_CYGWIN)
> > 		add_pre_buffer("#define __CYGWIN32__ 1");
> > 
> > For the long term, I would prefer something like 4a) or 4b)
> > but currently it would only work for native builds.
> > 
> > So, I think that 3a) should be the best.
> 
> I nearly sent this patch instead:
> 
> $ git diff
> diff --git a/cgcc b/cgcc
> index 2223c97d..ddc6de23 100755
> --- a/cgcc
> +++ b/cgcc
> @@ -252,10 +252,16 @@ sub add_specs {
>      } elsif ($spec eq 'unix') {
>         return ' -Dunix=1 -D__unix=1 -D__unix__=1';
>      } elsif ( $spec =~ /^cygwin/) {
> +       my $c32 = 0;
> +       my $m = `uname -m`;

This won't work correctly with:
	cgcc -target=i386 -target=cygwin ...
wich I'm using for some testing :)

> However, other than 'hand testing', I could not be sure it would
> work on an actual 32-bit cygwin system. (hand testing seems to
> imply that it should work, but well ... :-D ).

In this case, hand testing is perfectly fine.

Best regards,
-- Luc
diff mbox series

Patch

diff --git a/cgcc b/cgcc
index 87f4fc3e..2223c97d 100755
--- a/cgcc
+++ b/cgcc
@@ -254,7 +254,8 @@  sub add_specs {
     } elsif ( $spec =~ /^cygwin/) {
 	return &add_specs ('unix') .
 	    ' -fshort-wchar' .
-	    ' -D__CYGWIN__=1 -D__CYGWIN32__=1' .
+	    ' -D__CYGWIN__=1' .
+	    ($m32 ? ' -D__CYGWIN32__=1' : '') .
 	    " -D'_cdecl=__attribute__((__cdecl__))'" .
 	    " -D'__cdecl=__attribute__((__cdecl__))'" .
 	    " -D'_stdcall=__attribute__((__stdcall__))'" .