Hi! On Fri, 2018-12-07 at 15:05:39 +0000, Iain Lane wrote: > Package: libdpkg-perl > Version: 1.19.1 > Severity: normal > Tags: patch
> In Ubuntu we recently got a version of dpkg including > d5374bc618310917557daa9c9ac2f4930515a0b2 for the first time, and I > noticed that our ppc64el -O3 build flag wasn't being used - -O2 was > being passed instead. > > See the attached commit for my proposed fix - please let me know what > you think. There are other valid approaches such as having Debian.pm > check the value doesn't contain `-g -O\d' before appending it. On Fri, 2018-12-07 at 11:02:49 -0700, Adam Conrad wrote: > In Ubuntu, the attached patch was applied to achieve the following: > > * scripts/Dpkg/Vendor/Ubuntu.pm: Instead of running the Debian hooks after > the Ubuntu buildflags are set up, run them first, and then strip/prepend > the bits we need to change. This fixes compiler optimisation on ppc64el. > > This is vaguely similar to Laney's patch, but produces more pleasant output, > and is also what I uploaded to Ubuntu just now to solve the problem. Right, sorry for missing this! And thanks for the patches. As I mentioned to Adam on IRC, I went with a solution I think should be more future proof, and in addition also added a unit test to detect similar failures in the future, see the attached patch, which is queued for merging. Iain: Should your mail address really be @debian or perhaps it should have been either @canonical or @ubuntu instead? For better © tracking. Thanks, Guillem
From eb27f9167602a4ca18d409164f0500f3977e1b8d Mon Sep 17 00:00:00 2001 From: Guillem Jover <guil...@debian.org> Date: Sat, 8 Dec 2018 00:28:57 +0100 Subject: [PATCH] Dpkg::Vendor::Ubuntu: Fix buildflags override after default setting move The default buildflags got moved from the Dpkg::BuildFlags module to the Dpkg::Vendor::Debian, but this module was not addapted to match. Instead of running the Debian hooks after the Ubuntu buildflags are set up, run them first, and then modify/prepend the bits we need to change. This fixes compiler optimization on ppc64el, and makes setting it more future proof. Co-Author: Iain Lane <la...@debian.org> Co-Author: Adam Conrad <adcon...@ubuntu.com> Fixes: commit d5374bc618310917557daa9c9ac2f4930515a0b2 Closes: #915881 --- scripts/Dpkg/Vendor/Ubuntu.pm | 12 ++++--- scripts/Makefile.am | 1 + scripts/t/Dpkg_BuildFlags_Ubuntu.t | 57 ++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 scripts/t/Dpkg_BuildFlags_Ubuntu.t diff --git a/scripts/Dpkg/Vendor/Ubuntu.pm b/scripts/Dpkg/Vendor/Ubuntu.pm index eb2dffefe..e6335c204 100644 --- a/scripts/Dpkg/Vendor/Ubuntu.pm +++ b/scripts/Dpkg/Vendor/Ubuntu.pm @@ -98,6 +98,9 @@ sub run_hook { } elsif ($hook eq 'update-buildflags') { my $flags = shift @params; + # Run the Debian hook to add hardening flags + $self->SUPER::run_hook($hook, $flags); + require Dpkg::BuildOptions; my $build_opts = Dpkg::BuildOptions->new(); @@ -109,15 +112,14 @@ sub run_hook { if (Dpkg::Arch::debarch_eq($arch, 'ppc64el')) { for my $flag (qw(CFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS FCFLAGS)) { - $flags->set($flag, '-g -O3', 'vendor'); + my $value = $flags->get($flag); + $value =~ s/-O[0-9]/-O3/; + $flags->set($flag, $value); } } } # Per https://wiki.ubuntu.com/DistCompilerFlags - $flags->set('LDFLAGS', '-Wl,-Bsymbolic-functions', 'vendor'); - - # Run the Debian hook to add hardening flags - $self->SUPER::run_hook($hook, $flags); + $flags->prepend('LDFLAGS', '-Wl,-Bsymbolic-functions'); } else { return $self->SUPER::run_hook($hook, @params); } diff --git a/scripts/Makefile.am b/scripts/Makefile.am index d7bd8d2f7..31df4d8df 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -219,6 +219,7 @@ test_scripts = \ t/Dpkg_Shlibs_Cppfilt.t \ t/Dpkg_Shlibs.t \ t/Dpkg_BuildFlags.t \ + t/Dpkg_BuildFlags_Ubuntu.t \ t/Dpkg_BuildOptions.t \ t/Dpkg_BuildProfiles.t \ t/Dpkg_Build_Env.t \ diff --git a/scripts/t/Dpkg_BuildFlags_Ubuntu.t b/scripts/t/Dpkg_BuildFlags_Ubuntu.t new file mode 100644 index 000000000..3cfdb268f --- /dev/null +++ b/scripts/t/Dpkg_BuildFlags_Ubuntu.t @@ -0,0 +1,57 @@ +#!/usr/bin/perl +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <https://www.gnu.org/licenses/>. + +use strict; +use warnings; + +use Test::More tests => 16; + +BEGIN { + use_ok('Dpkg::BuildFlags'); +} + +sub test_optflag +{ + my ($bf, $optflag) = @_; + + foreach my $flag (qw(CFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS + FFLAGS FCFLAGS)) { + my $value = $bf->get($flag); + ok($value =~ m/$optflag/, "$flag contains $optflag: $value"); + } +} + +my $bf; + +# Force loading the Dpkg::Vendor::Ubuntu module. +$ENV{DEB_VENDOR} = 'Ubuntu'; + +# Test the optimization flag inherited from the Dpkg::Vendor::Debian module. +$ENV{DEB_HOST_ARCH} = 'amd64'; +$bf = Dpkg::BuildFlags->new(); + +test_optflag($bf, '-O2'); + +# Test the overlaid Ubuntu-specific linker flag. +ok($bf->get('LDFLAGS') =~ m/-Wl,-Bsymbolic-functions/, + 'LDFLAGS contains -Bsymbolic-functions'); + +# Test the optimization flag override only for ppc64el. +$ENV{DEB_HOST_ARCH} = 'ppc64el'; +$bf = Dpkg::BuildFlags->new(); + +test_optflag($bf, '-O3'); + +1; -- 2.20.0.rc2.403.gdbc3b29805