Package: libextutils-cbuilder-perl
Version: 0.280202-1 
Severity: serious
Justification: potential for hidden binary incompatibility
Forwarded: http://rt.perl.org/rt3//Public/Bug/Display.html?id=89478
Tags: patch
x-debbugs-cc: p...@packages.debian.org

Since ExtUtils::CBuilder 0.27_04, CFLAGS and LDFLAGS from the environment
have overridden the Config.pm ccflags and ldflags settings. This can
cause binary incompatibilities between the core Perl and extensions
built with EU::CBuilder.

The issue seems to have been introduced with
 https://github.com/dagolden/extutils-cbuilder/commit/e653d24a

This is particularly nasty on Debian, because dpkg-buildpackage sets
CFLAGS to "-g -O2" by default.

Comparing for example these build logs:

 
https://buildd.debian.org/status/fetch.php?pkg=libparams-validate-perl&arch=amd64&ver=0.93-1&stamp=1259695361
 
https://buildd.debian.org/status/fetch.php?pkg=libparams-validate-perl&arch=amd64&ver=0.97-1&stamp=1303329531

we see the old correct behaviour:

 cc -Ic -I/usr/lib/perl/5.10/CORE -DXS_VERSION="0.93" -DVERSION="0.93" -fPIC -c 
-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe 
-fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE 
-D_FILE_OFFSET_BITS=64 -O2 -g -o lib/Params/Validate.o lib/Params/Validate.c

and the new incorrect one:

 cc -Ic -I/usr/lib/perl/5.10/CORE -DXS_VERSION="0.97" -DVERSION="0.97" -fPIC -c 
-g -O2 -O2 -g -o lib/Params/Validate.o lib/Params/Validate.c

Omitting the Config.pm flags ($Config{ccflags}) can cause hidden
binary incompatibilities between the Perl core and extensions built with
EU::CBuilder. I'm not aware of any reports of this in current sid, but
the problem was originally spotted while testing Perl 5.14.0-RC1 on i386,
where omitting -D_FILE_OFFSET_BITS=64 made some extensions fail to load.
See the threads at
 
http://lists.alioth.debian.org/pipermail/perl-maintainers/2011-April/001883.html
 http://www.nntp.perl.org/group/perl.perl5.porters/2011/04/msg171535.html

I'm attaching the patch I've sent upstream in [perl #89478], it applies
cleanly with -p3. It may be good to wait a bit for upstream comments,
but I think this is a real problem for Debian even if upstream doesn't
care that much.

To be on the safe side, I think that after this is fixed we should
binNMU all the XS modules built with buggy libextutils-builder-perl
package versions (starting at 2010-12-11 at 0.2801-1, inclusive.) I
haven't looked at how to identify those builds.
-- 
Niko Tyni   nt...@debian.org
>From bb249b0b26c2e79a6f55355ef94889070f07fd21 Mon Sep 17 00:00:00 2001
From: Niko Tyni <nt...@debian.org>
Date: Thu, 28 Apr 2011 09:18:54 +0300
Subject: [PATCH] Append CFLAGS and LDFLAGS to their Config.pm counterparts in
 EU::CBuilder

Since ExtUtils::CBuilder 0.27_04 (bleadperl commit 06e8058f27e4),
CFLAGS and LDFLAGS from the environment have overridden the Config.pm
ccflags and ldflags settings. This can cause binary incompatibilities
between the core Perl and extensions built with EU::CBuilder.

Append to the Config.pm values rather than overriding them.
---
 .../lib/ExtUtils/CBuilder/Base.pm                  |    6 +++-
 dist/ExtUtils-CBuilder/t/04-base.t                 |   25 +++++++++++++++++++-
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm
index b572312..2255c51 100644
--- a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm
+++ b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm
@@ -40,11 +40,13 @@ sub new {
     $self->{config}{$k} = $v unless exists $self->{config}{$k};
   }
   $self->{config}{cc} = $ENV{CC} if defined $ENV{CC};
-  $self->{config}{ccflags} = $ENV{CFLAGS} if defined $ENV{CFLAGS};
+  $self->{config}{ccflags} = join(" ", $self->{config}{ccflags}, $ENV{CFLAGS})
+     if defined $ENV{CFLAGS};
   $self->{config}{cxx} = $ENV{CXX} if defined $ENV{CXX};
   $self->{config}{cxxflags} = $ENV{CXXFLAGS} if defined $ENV{CXXFLAGS};
   $self->{config}{ld} = $ENV{LD} if defined $ENV{LD};
-  $self->{config}{ldflags} = $ENV{LDFLAGS} if defined $ENV{LDFLAGS};
+  $self->{config}{ldflags} = join(" ", $self->{config}{ldflags}, $ENV{LDFLAGS})
+     if defined $ENV{LDFLAGS};
 
   unless ( exists $self->{config}{cxx} ) {
     my ($ccpath, $ccbase, $ccsfx ) = fileparse($self->{config}{cc}, qr/\.[^.]*/);
diff --git a/dist/ExtUtils-CBuilder/t/04-base.t b/dist/ExtUtils-CBuilder/t/04-base.t
index c3bf6b5..1bb15aa 100644
--- a/dist/ExtUtils-CBuilder/t/04-base.t
+++ b/dist/ExtUtils-CBuilder/t/04-base.t
@@ -1,7 +1,7 @@
 #! perl -w
 
 use strict;
-use Test::More tests => 50;
+use Test::More tests => 64;
 use Config;
 use Cwd;
 use File::Path qw( mkpath );
@@ -326,6 +326,29 @@ is_deeply( $mksymlists_args,
     "_prepare_mksymlists_args(): got expected arguments for Mksymlists",
 );
 
+my %testvars = (
+    CFLAGS  => 'ccflags',
+    LDFLAGS => 'ldflags',
+);
+
+while (my ($VAR, $var) = each %testvars) {
+    local $ENV{$VAR};
+    $base = ExtUtils::CBuilder::Base->new( quiet => 1 );
+    ok( $base, "ExtUtils::CBuilder::Base->new() returned true value" );
+    isa_ok( $base, 'ExtUtils::CBuilder::Base' );
+    like($base->{config}{$var}, qr/\Q$Config{$var}/,
+        "honours $var from Config.pm");
+
+    $ENV{$VAR} = "-foo -bar";
+    $base = ExtUtils::CBuilder::Base->new( quiet => 1 );
+    ok( $base, "ExtUtils::CBuilder::Base->new() returned true value" );
+    isa_ok( $base, 'ExtUtils::CBuilder::Base' );
+    like($base->{config}{$var}, qr/\Q$ENV{$VAR}/,
+        "honours $VAR from the environment");
+    like($base->{config}{$var}, qr/\Q$Config{$var}/,
+        "doesn't override $var from Config.pm with $VAR from the environment");
+}
+
 #####
 
 for ($source_file, $object_file, $lib_file) {
-- 
1.7.4.4

Reply via email to