Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GIMME_V broken with 5.10.0/GCC and XS? #9215

Closed
p5pRT opened this issue Jan 29, 2008 · 17 comments
Closed

GIMME_V broken with 5.10.0/GCC and XS? #9215

p5pRT opened this issue Jan 29, 2008 · 17 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 29, 2008

Migrated from rt.perl.org#50386 (status was 'resolved')

Searchable as RT50386$

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2008

From rob@themayfamily.me.uk

Created by robertmay@cpan.org

This is a bug report for perl from robertmay@​cpan.org,
generated with the help of perlbug 1.36 running under perl 5.10.0.

-----------------------------------------------------------------
I'm building the following XS extension with
gcc 3.4.5 (mingw special) against ActiveState Perl 5.10.0
(builds 1001 and 1002)

GIMME_V appears to always return G_VOID (128), regardless of the
context the XSUB is called in.

The same code and compiler works fine when built against ActiveState
Perl 5.8.8 (build 822). It is also fine when compiled with VC++ 6
for all 3 perl builds.

#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"

MODULE = GimmeVTest PACKAGE = GimmeVTest

int
context()
  CODE​:
  RETVAL = GIMME_V;
  OUTPUT​:
  RETVAL

Testing with the following code​:

my $scalar = GimmeVTest​::context();
is($scalar , G_SCALAR, 'in scalar context');
my ($x) = GimmeVTest​::context();
is($x , G_ARRAY, 'in list context');
my ($y) = GimmeVTest​::context();
is($y , G_ARRAY, 'in list context');
my $z = (GimmeVTest​::context())[0];
is($z , G_ARRAY, 'in list context');

results in the following output​:

not ok 1 - in scalar context
# Failed test 'in scalar context'
# at t\01-gimme_v-test.t line 14.
# got​: '128'
# expected​: '0'

not ok 2 - in list context
# Failed test 'in list context'
# at t\01-gimme_v-test.t line 17.
# got​: '128'
# expected​: '1'

not ok 3 - in list context
# Failed test 'in list context'
# at t\01-gimme_v-test.t line 20.
# got​: '128'
# expected​: '1'

not ok 4 - in list context
# Failed test 'in list context'
# at t\01-gimme_v-test.t line 23.
# got​: '128'
# expected​: '1'
# Looks like you failed 4 tests of 4.

I have reached the end of my knowledge in the internals trying
to track down the problem.

Module distribution for the above code and tests attached to save
you having to type anything.

Regards,
Rob.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.10.0:

Configured by SYSTEM at Thu Jan 10 11:00:30 2008.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
  Platform:
    osname=MSWin32, osvers=5.00, archname=MSWin32-x86-multi-thread
    uname=''
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=undef
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-DNDEBUG -DWIN32 -D_CONSOLE -DNO_STRICT
-DHAVE_DES_FCRYPT -DUSE_SITECUSTOMIZE -DPRIVLIB_LAST_IN_INC
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO
-DPERL_MSVCRT_READFIX -DHASATTRIBUTE -fno-strict-aliasing',
    optimize='-O2',
    cppflags='-DWIN32'
    ccversion='', gccversion='3.4.5 (mingw special)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
    ivtype='long', ivsize=4, nvtype='double', nvsize=8,
Off_t='__int64', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='g++', ldflags ='-L"C:\Perl\AS1002\lib\CORE"'
    libpth=\lib
    libs=-lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32
-lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm
-lversion -lodbc32 -lodbccp32 -lmsvcrt
    perllibs=-lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32
-lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lmsvcrt
    libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl510.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-mdll -L"C:\Perl\AS1002\lib\CORE"'

Locally applied patches:
    ACTIVEPERL_LOCAL_PATCHES_ENTRY
    32809 Load 'loadable object' with non-default file extension
    32728 64-bit fix for Time::Local


@INC for perl 5.10.0:
    C:/Perl/AS1002/site/lib
    C:/Perl/AS1002/lib
    .


Environment for perl 5.10.0:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\Perl\AS1002\site\bin;C:\Perl\AS1002\bin;C:\pgmfiles\unxutil;C:\MinGW\bin;C:\Windows\system32
    PERL_BADLANG (unset)
    SHELL (unset)

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2008

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

From @jandubois

On Tue, 29 Jan 2008, Robert May (via RT) wrote​:

-----------------------------------------------------------------
I'm building the following XS extension with
gcc 3.4.5 (mingw special) against ActiveState Perl 5.10.0
(builds 1001 and 1002)

GIMME_V appears to always return G_VOID (128), regardless of the
context the XSUB is called in.

The same code and compiler works fine when built against ActiveState
Perl 5.8.8 (build 822). It is also fine when compiled with VC++ 6
for all 3 perl builds.

This is not really a core Perl bug; the problem is that VC++ and GCC
are incompatible as far as bit-fields are concerned. In Perl 5.10
the BASEOP structure contains bitfields​:

  unsigned op_type​:9; \
  unsigned op_opt​:1; \
  unsigned op_latefree​:1; \
  unsigned op_latefreed​:1; \
  unsigned op_attached​:1; \
  unsigned op_spare​:3; \

In VC++ the combined size of the fields above is 4 bytes, whereas
in GCC it is only 2. This also pushes the size of the complete
structure from 20 to 24 bytes for VC++.

The GIMME_V() function retrieves the context information from the
op_flags byte. Under GCC the op_flags member fetches one of the
2 filler bytes VC++ inserts into the structure and is therefore
wrong.

VC++ uses the base type (unsigned int, in this case) to determine
the alignment requirements and total size of the combined bit-field.
You can use the non-standard extension of using smaller integer sizes
to force the fields into a smaller size and different alignment.
In this case using "unsigned short" for the type of the bit-fields
should work.

I guess for Perl 5.12 we need a way to let us use the smaller base
size for VC++ to keep the size of the OP structure down. I'll create
a patch for it eventually, when I get a bit more time. I also need
to check if there are other places where Perl is now using bit fields
in its structure definitions.

I would suggest to keep this bug report open until the size of
the BASEOP structure in bleadperl has been reduced for VC++.

For ActivePerl 5.10.x we will probably modify op.h to provide an
alternate definition of BASEOP for GCC that contains 2 filler bytes to
make everything align the same way as it does for VC++ (unless we decide
to break binary compatibility between 5.10 and 5.10.1, which I would not
be too fond of).

I have not yet verified that otherwise the bit mapping between
VC++ and GCC would be compatible though; many details of C bit-field
implementation is really up for the compiler to decide.

The ability to compile extensions with GCC for a Perl compiled
with VC++ is really an ActivePerl feature and not guaranteed
by core Perl. You have also opened a corresponding ActivePerl bug
that should be used to track that part of the issue​:

  http​://bugs.activestate.com/show_bug.cgi?id=74532

Cheers,
-Jan

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

From rob@themayfamily.me.uk

Jan,

Many thanks for the very quick analysis, and clear explanation.

Further research turns up the '-mms-bitfields' option for (mingw
only?) gcc, which causes it to use the same alignment as Microsoft's
compilers. I can confirm that using this option at least fixes my
immediate problem[1]. It might be a better option for 5.10.x to patch
ActivePerl​::Config to add this flag? I'll generate a patch and attach
it to the ActiveState Bug report for this item.

Regards,
Rob.

[1] Although I have in the process discovered that
ExtUtils​::CBuilder​::Windows doesn't honour the extra_compiler_flags
option - patch forthcoming.

On 30/01/2008, Jan Dubois <jand@​activestate.com> wrote​:

On Tue, 29 Jan 2008, Robert May (via RT) wrote​:

-----------------------------------------------------------------
I'm building the following XS extension with
gcc 3.4.5 (mingw special) against ActiveState Perl 5.10.0
(builds 1001 and 1002)

GIMME_V appears to always return G_VOID (128), regardless of the
context the XSUB is called in.

The same code and compiler works fine when built against ActiveState
Perl 5.8.8 (build 822). It is also fine when compiled with VC++ 6
for all 3 perl builds.

This is not really a core Perl bug; the problem is that VC++ and GCC
are incompatible as far as bit-fields are concerned. In Perl 5.10
the BASEOP structure contains bitfields​:

unsigned    op\_type&#8203;:9;              \\
unsigned    op\_opt&#8203;:1;               \\
unsigned    op\_latefree&#8203;:1;          \\
unsigned    op\_latefreed&#8203;:1;         \\
unsigned    op\_attached&#8203;:1;          \\
unsigned    op\_spare&#8203;:3;             \\

In VC++ the combined size of the fields above is 4 bytes, whereas
in GCC it is only 2. This also pushes the size of the complete
structure from 20 to 24 bytes for VC++.

The GIMME_V() function retrieves the context information from the
op_flags byte. Under GCC the op_flags member fetches one of the
2 filler bytes VC++ inserts into the structure and is therefore
wrong.

VC++ uses the base type (unsigned int, in this case) to determine
the alignment requirements and total size of the combined bit-field.
You can use the non-standard extension of using smaller integer sizes
to force the fields into a smaller size and different alignment.
In this case using "unsigned short" for the type of the bit-fields
should work.

I guess for Perl 5.12 we need a way to let us use the smaller base
size for VC++ to keep the size of the OP structure down. I'll create
a patch for it eventually, when I get a bit more time. I also need
to check if there are other places where Perl is now using bit fields
in its structure definitions.

I would suggest to keep this bug report open until the size of
the BASEOP structure in bleadperl has been reduced for VC++.

For ActivePerl 5.10.x we will probably modify op.h to provide an
alternate definition of BASEOP for GCC that contains 2 filler bytes to
make everything align the same way as it does for VC++ (unless we decide
to break binary compatibility between 5.10 and 5.10.1, which I would not
be too fond of).

I have not yet verified that otherwise the bit mapping between
VC++ and GCC would be compatible though; many details of C bit-field
implementation is really up for the compiler to decide.

The ability to compile extensions with GCC for a Perl compiled
with VC++ is really an ActivePerl feature and not guaranteed
by core Perl. You have also opened a corresponding ActivePerl bug
that should be used to track that part of the issue​:

http&#8203;://bugs\.activestate\.com/show\_bug\.cgi?id=74532

Cheers,
-Jan

--
Please update your address book with my new email address​:
rob@​themayfamily.me.uk

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

From @jandubois

On Wed, 30 Jan 2008, Robert May wrote​:

Further research turns up the '-mms-bitfields' option for (mingw
only?) gcc, which causes it to use the same alignment as Microsoft's
compilers. I can confirm that using this option at least fixes my
immediate problem[1]. It might be a better option for 5.10.x to patch
ActivePerl​::Config to add this flag? I'll generate a patch and attach
it to the ActiveState Bug report for this item.

Awesome. Yes, this is much better than having to hack op.h to
conditionally add filler bytes.

Cheers,
-Jan

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

From rob@themayfamily.me.uk

On 30/01/2008, Jan Dubois <jand@​activestate.com> wrote​:

On Wed, 30 Jan 2008, Robert May wrote​:

Further research turns up the '-mms-bitfields' option for (mingw
only?) gcc, which causes it to use the same alignment as Microsoft's
compilers. I can confirm that using this option at least fixes my
immediate problem[1]. It might be a better option for 5.10.x to patch
ActivePerl​::Config to add this flag? I'll generate a patch and attach
it to the ActiveState Bug report for this item.

Awesome. Yes, this is much better than having to hack op.h to
conditionally add filler bytes.

So, in the meantime, can anyone suggest a mechanism for testing
whether the perl that is running my Makefile.PL was compiled with a
Microsoft compiler, given that if gcc is being used the Config.pm
values will already have been re-set (with ExtUtils​::FakeConfig,
ActivePerl​::Config, or similar).

I can't just blindly add -mms-bitfields' if the compiler is gcc, as
that would break compatibility with gcc-built perls (Strawberry perl
...), and I don't only want to find ActiveState Perl builds - and I
need it to work back to 5.6.1

Or am I stuck with putting a big warning in the README (which won't
stop the bug reports).

Thanks for any insight.
Rob.

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

From @jandubois

On Wed, 30 Jan 2008, Robert May wrote​:

So, in the meantime, can anyone suggest a mechanism for testing
whether the perl that is running my Makefile.PL was compiled with a
Microsoft compiler, given that if gcc is being used the Config.pm
values will already have been re-set (with ExtUtils​::FakeConfig,
ActivePerl​::Config, or similar).

I can't just blindly add -mms-bitfields' if the compiler is gcc, as
that would break compatibility with gcc-built perls (Strawberry perl
...), and I don't only want to find ActiveState Perl builds - and I
need it to work back to 5.6.1

Or am I stuck with putting a big warning in the README (which won't
stop the bug reports).

I don't understand why you need to do this at all; you should just
rely on the $Config{ccflags} value.

Or are you just creating a workaround for the already released
ActivePerl 1001/1002? In that case you can just explicitly
test for it​:

  if (defined &Win32​::BuildNumber) {
  }

I doubt that there are any other configurations out there where
someone builds their own Perl with VC++ and then tries to install
modules with GCC using ExtUtils​::FakeConfig. The combination
of ActivePerl and ExtUtils​::FakeConfig should be covered by the
test above too.

If you want make sure you also catch any instance where somebody
uses the ActivePerl sources and builds them with GCC (thereby
adding Win32​::BuildNumber() as well), then you can use something
like this​:

  use Config qw(%Config);
  my $cc = $Config{cc};
  if (defined &ActivePerl​::Config​::_orig_conf) {
  $cc = ActivePerl​::Config​::_orig_conf('cc');
  }

I dislike this one though, as it calls a private function, so
maybe something like

  use Config qw(%Config);
  my $cc = $Config{cc};
  $cc = "cl" if defined %ActivePerl​::Config​::;

Maybe the most general solution would be​:

  my $cc;
  for my $file (qw(Config_heavy.pl Config.pm)) {
  my $fullname = "$Config{privlib}/$file";
  open(my $fh, "<", $fullname) or die "Can't read $fullname​: $!\n";
  while (<$fh>) {
  $cc = $1, last if /^cc='(.*)'/;
  }
  last if defined $cc;
  }

This of course will still fail if someone builds with VC++ and then
manually patches Config_heavy.pl instead of overriding it
programmatically, but those people should be on their own anyways. :)

Cheers,
-Jan

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

From rob@themayfamily.me.uk

On 31/01/2008, Jan Dubois <jand@​activestate.com> wrote​:

On Wed, 30 Jan 2008, Robert May wrote​:

So, in the meantime, can anyone suggest a mechanism for testing
whether the perl that is running my Makefile.PL was compiled with a
Microsoft compiler, given that if gcc is being used the Config.pm
values will already have been re-set (with ExtUtils​::FakeConfig,
ActivePerl​::Config, or similar).

I can't just blindly add -mms-bitfields' if the compiler is gcc, as
that would break compatibility with gcc-built perls (Strawberry perl
...), and I don't only want to find ActiveState Perl builds - and I
need it to work back to 5.6.1

Or am I stuck with putting a big warning in the README (which won't
stop the bug reports).

I don't understand why you need to do this at all; you should just
rely on the $Config{ccflags} value.

Or are you just creating a workaround for the already released
ActivePerl 1001/1002? In that case you can just explicitly
test for it​:

That's it exactly - I'm on the verge of making a 5.10 compatible
release, and don't want it to fail immediately.

if \(defined &Win32&#8203;::BuildNumber\) \{
\}

But won't Strawberry Perl have this too? Are you saying that
Win32​::BuildNumber() is an ActivePerl only thing? If so, then I think
this is sufficient for me.

use Config qw\(%Config\);
my $cc = $Config\{cc\};
$cc = "cl" if defined %ActivePerl&#8203;::Config&#8203;::;

But that won't catch someone who built Perl from the official sources
with an MSVC compiler.

Maybe the most general solution would be​:

my $cc;
for my $file \(qw\(Config\_heavy\.pl Config\.pm\)\) \{
    my $fullname = "$Config\{privlib\}/$file";
    open\(my $fh\, "\<"\, $fullname\) or die "Can't read $fullname&#8203;: $\!\\n";
    while \(\<$fh>\) \{
        $cc = $1\, last if /^cc='\(\.\*\)'/;
    \}
    last if defined $cc;
\}

I actually just wrote this​:
#!perl -w
use strict;
use warnings;

use Config;

print +(is_msvc() ? '' : 'NOT '), "MSVC\n";

sub is_msvc {
  require Config;
  my $file=$INC{'Config.pm'};

  for ('Config.pm', 'config_heavy.pl') {
  $file =~ s/Config.pm$/$_/;
  open (my $fh, '<', $file) or next;
  while (my $l = <$fh>) {
  next unless $l =~ /^cc=/;
  # File closed by $fh going out of scope
  return $l =~ 'cl';
  }
  close($fh);
  }
  return 0;
}
__END__

Yours is much more elegant though.

This of course will still fail if someone builds with VC++ and then
manually patches Config_heavy.pl instead of overriding it
programmatically, but those people should be on their own anyways. :)

Indeed :-)

Many thanks,
Rob.

@p5pRT
Copy link
Author

p5pRT commented Jan 30, 2008

From @jandubois

On Wed, 30 Jan 2008, Robert May wrote​:

On 31/01/2008, Jan Dubois <jand@​activestate.com> wrote​:
That's it exactly - I'm on the verge of making a 5.10 compatible
release, and don't want it to fail immediately.

if \(defined &Win32&#8203;::BuildNumber\) \{
\}

But won't Strawberry Perl have this too? Are you saying that
Win32​::BuildNumber() is an ActivePerl only thing? If so, then I think
this is sufficient for me.

It is an ActivePerl addition, reporting the ActivePerl build number.
The name is historical; if you only care about 5.10, then you could
also check for "defined &ActivePerl​::BUILD", which has been added
sometime in the 8xx timeframe and is also available on non-Win32
platforms.

Strawberry Perl is build from the original P5P sources, so it won't have
these symbols. Only Perl built from ActivePerl sources will have them
(they can be downloaded from http​://downloads.activestate.com/ActivePerl/src/).

I doubt many people do this though.

use Config qw\(%Config\);
my $cc = $Config\{cc\};
$cc = "cl" if defined %ActivePerl&#8203;::Config&#8203;::;

But that won't catch someone who built Perl from the official sources
with an MSVC compiler.

Yes, but this is only an issue if after building Perl they suddenly
switch compilers, which again should be rare. If you already have VC++,
why would you switch to GCC, which doesn't run any faster and doesn't
produce any faster code either (well, at least didn't used to in the
past; haven't checked lately).

Maybe the most general solution would be​:

my $cc;
for my $file \(qw\(Config\_heavy\.pl Config\.pm\)\) \{
    my $fullname = "$Config\{privlib\}/$file";
    open\(my $fh\, "\<"\, $fullname\) or die "Can't read $fullname&#8203;: $\!\\n";

As your example below points out, it shouldn't die, but just skip the
file because 5.6 won't have Config_heavy.pl.

    while \(\<$fh>\) \{
        $cc = $1\, last if /^cc='\(\.\*\)'/;
    \}
    last if defined $cc;
\}

Cheers,
-Jan

@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2008

From @jandubois

On Wed, 30 Jan 2008, Jan Dubois wrote​:

This is not really a core Perl bug; the problem is that VC++ and GCC
are incompatible as far as bit-fields are concerned. In Perl 5.10
the BASEOP structure contains bitfields​:

unsigned    op\_type&#8203;:9;        \\
unsigned    op\_opt&#8203;:1;        \\
unsigned    op\_latefree&#8203;:1;        \\
unsigned    op\_latefreed&#8203;:1;        \\
unsigned    op\_attached&#8203;:1;        \\
unsigned    op\_spare&#8203;:3;        \\

In VC++ the combined size of the fields above is 4 bytes, whereas
in GCC it is only 2. This also pushes the size of the complete
structure from 20 to 24 bytes for VC++.

[...]

VC++ uses the base type (unsigned int, in this case) to determine
the alignment requirements and total size of the combined bit-field.
You can use the non-standard extension of using smaller integer sizes
to force the fields into a smaller size and different alignment.
In this case using "unsigned short" for the type of the bit-fields
should work.

I guess for Perl 5.12 we need a way to let us use the smaller base
size for VC++ to keep the size of the OP structure down. I'll create
a patch for it eventually, when I get a bit more time. I also need
to check if there are other places where Perl is now using bit fields
in its structure definitions.

I've attached a patch that defined PERL_BITFIELD8, PERL_BITFIELD16
and PERL_BITFIELD32 symbols to be used to specify bitfields whose
base size and alignment requirements are 1, 2 or 4 bytes respectively.
The "PERL_" prefix is a bit awkward, but these are names that must
be defined even when PERL_CORE is not.

These symbols are then being redefined for VC (and MinGW GCC) to use the
corresponding smaller sizes as the base types.

I've also added -mms-bitfields to the default options for MinGW in
makefile.mk. That way building additional XS modules that interface
to VC compatible libraries using bit-fields in their external APIs
should be easier.

Cheers,
-Jan

@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2008

From @jandubois

diff

@p5pRT
Copy link
Author

p5pRT commented Feb 11, 2008

From @jandubois

Are there any questions about this patch, or is there some other reason
it has not been applied to 5.12?

Cheers,
-Jan

On Fri, 01 Feb 2008, Jan Dubois wrote​:

On Wed, 30 Jan 2008, Jan Dubois wrote​:

This is not really a core Perl bug; the problem is that VC++ and GCC
are incompatible as far as bit-fields are concerned. In Perl 5.10
the BASEOP structure contains bitfields​:

unsigned    op\_type&#8203;:9;        \\
unsigned    op\_opt&#8203;:1;        \\
unsigned    op\_latefree&#8203;:1;        \\
unsigned    op\_latefreed&#8203;:1;        \\
unsigned    op\_attached&#8203;:1;        \\
unsigned    op\_spare&#8203;:3;        \\

In VC++ the combined size of the fields above is 4 bytes, whereas
in GCC it is only 2. This also pushes the size of the complete
structure from 20 to 24 bytes for VC++.

[...]

VC++ uses the base type (unsigned int, in this case) to determine
the alignment requirements and total size of the combined bit-field.
You can use the non-standard extension of using smaller integer sizes
to force the fields into a smaller size and different alignment.
In this case using "unsigned short" for the type of the bit-fields
should work.

I guess for Perl 5.12 we need a way to let us use the smaller base
size for VC++ to keep the size of the OP structure down. I'll create
a patch for it eventually, when I get a bit more time. I also need
to check if there are other places where Perl is now using bit fields
in its structure definitions.

I've attached a patch that defined PERL_BITFIELD8, PERL_BITFIELD16
and PERL_BITFIELD32 symbols to be used to specify bitfields whose
base size and alignment requirements are 1, 2 or 4 bytes respectively.
The "PERL_" prefix is a bit awkward, but these are names that must
be defined even when PERL_CORE is not.

These symbols are then being redefined for VC (and MinGW GCC) to use the
corresponding smaller sizes as the base types.

I've also added -mms-bitfields to the default options for MinGW in
makefile.mk. That way building additional XS modules that interface
to VC compatible libraries using bit-fields in their external APIs
should be easier.

Cheers,
-Jan

@p5pRT
Copy link
Author

p5pRT commented Feb 13, 2008

From @jandubois

Size of OP structures fixed for VC with change 33292​:

  http​://public.activestate.com/cgi-bin/perlbrowse/p/33292

This change also adds the -mms-bitfields flag to MinGW compiler options.

@p5pRT
Copy link
Author

p5pRT commented Feb 13, 2008

@jandubois - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Feb 13, 2008
@p5pRT
Copy link
Author

p5pRT commented May 27, 2008

From @iabyn

On Fri, Feb 01, 2008 at 01​:40​:41PM -0800, Jan Dubois wrote​:

On Wed, 30 Jan 2008, Jan Dubois wrote​:

This is not really a core Perl bug; the problem is that VC++ and GCC
are incompatible as far as bit-fields are concerned. In Perl 5.10
the BASEOP structure contains bitfields​:

unsigned    op\_type&#8203;:9;        \\
unsigned    op\_opt&#8203;:1;        \\
unsigned    op\_latefree&#8203;:1;        \\
unsigned    op\_latefreed&#8203;:1;        \\
unsigned    op\_attached&#8203;:1;        \\
unsigned    op\_spare&#8203;:3;        \\

In VC++ the combined size of the fields above is 4 bytes, whereas
in GCC it is only 2. This also pushes the size of the complete
structure from 20 to 24 bytes for VC++.

[...]

VC++ uses the base type (unsigned int, in this case) to determine
the alignment requirements and total size of the combined bit-field.
You can use the non-standard extension of using smaller integer sizes
to force the fields into a smaller size and different alignment.
In this case using "unsigned short" for the type of the bit-fields
should work.

I guess for Perl 5.12 we need a way to let us use the smaller base
size for VC++ to keep the size of the OP structure down. I'll create
a patch for it eventually, when I get a bit more time. I also need
to check if there are other places where Perl is now using bit fields
in its structure definitions.

I've attached a patch that defined PERL_BITFIELD8, PERL_BITFIELD16
and PERL_BITFIELD32 symbols to be used to specify bitfields whose
base size and alignment requirements are 1, 2 or 4 bytes respectively.
The "PERL_" prefix is a bit awkward, but these are names that must
be defined even when PERL_CORE is not.

These symbols are then being redefined for VC (and MinGW GCC) to use the
corresponding smaller sizes as the base types.

I've also added -mms-bitfields to the default options for MinGW in
makefile.mk. That way building additional XS modules that interface
to VC compatible libraries using bit-fields in their external APIs
should be easier.

Jan, is this patch safe for 5.10.1 - specifically is it binary compatible?

--
A major Starfleet emergency breaks out near the Enterprise, but
fortunately some other ships in the area are able to deal with it to
everyone's satisfaction.
  -- Things That Never Happen in "Star Trek" #13

@p5pRT
Copy link
Author

p5pRT commented May 27, 2008

From @jandubois

On Tue, 27 May 2008, Dave Mitchell wrote​:

On Fri, Feb 01, 2008 at 01​:40​:41PM -0800, Jan Dubois wrote​:

On Wed, 30 Jan 2008, Jan Dubois wrote​:

This is not really a core Perl bug; the problem is that VC++ and GCC
are incompatible as far as bit-fields are concerned. In Perl 5.10
the BASEOP structure contains bitfields​:

unsigned    op\_type&#8203;:9;        \\
unsigned    op\_opt&#8203;:1;        \\
unsigned    op\_latefree&#8203;:1;        \\
unsigned    op\_latefreed&#8203;:1;        \\
unsigned    op\_attached&#8203;:1;        \\
unsigned    op\_spare&#8203;:3;        \\

In VC++ the combined size of the fields above is 4 bytes, whereas
in GCC it is only 2. This also pushes the size of the complete
structure from 20 to 24 bytes for VC++.

[...]

VC++ uses the base type (unsigned int, in this case) to determine
the alignment requirements and total size of the combined bit-field.
You can use the non-standard extension of using smaller integer sizes
to force the fields into a smaller size and different alignment.
In this case using "unsigned short" for the type of the bit-fields
should work.

I guess for Perl 5.12 we need a way to let us use the smaller base
size for VC++ to keep the size of the OP structure down. I'll create
a patch for it eventually, when I get a bit more time. I also need
to check if there are other places where Perl is now using bit fields
in its structure definitions.

I've attached a patch that defined PERL_BITFIELD8, PERL_BITFIELD16
and PERL_BITFIELD32 symbols to be used to specify bitfields whose
base size and alignment requirements are 1, 2 or 4 bytes respectively.
The "PERL_" prefix is a bit awkward, but these are names that must
be defined even when PERL_CORE is not.

These symbols are then being redefined for VC (and MinGW GCC) to use the
corresponding smaller sizes as the base types.

I've also added -mms-bitfields to the default options for MinGW in
makefile.mk. That way building additional XS modules that interface
to VC compatible libraries using bit-fields in their external APIs
should be easier.

Jan, is this patch safe for 5.10.1 - specifically is it binary compatible?

No, it is not. :( For 5.10 we'll have to live with the slightly bloated
OP structures.

Cheers,
-Jan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant