New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 640358 link

Starred by 1 user

Issue metadata

Status: Verified
Owner: ----
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 638456
issue 640385
issue 537368



Sign in to add a comment

We need to unbreak packages to enable clang-style FORTIFY

Project Member Reported by g...@chromium.org, Aug 23 2016

Issue description

We need to figure out how to unbreak the following list of packages on clang-FORTIFY (see  bug 638456 ), all of which are in portage-stable:

app-misc/screen
sys-devel/make
dev-libs/libtasn1
net-dns/c-ares
net-misc/curl
sys-apps/coreutils
sys-apps/i2ctools
app-shells/bash
sys-apps/findutils
sys-apps/sandbox
net-libs/gnutls

Note that this list may not be entirely complete (e.g. if foo is broken, foo blocks bar from building, and bar is broken...), but it should contain the vast majority of things we need to fix.

A few other breakages exist, but they're all in chromium-overlay, so I'm assuming we can fix those without too much hassle. :)

The ideal path is "commit patch upstream, wait for changes to hit Gentoo, pull it back down." For cases where this takes too long (e.g. sed 4.2.1 was released in 2009, 4.2.2 was released late 2012), we need to figure out a way to sneak a -D option into the packages' cflags+cxxflags.

FWIW, the patches I've made for them (...which, to be clear, I don't plan to commit directly to portage-stable :) ) are:

https://chromium-review.googlesource.com/#/c/372264/
https://chromium-review.googlesource.com/#/c/372478/
https://chromium-review.googlesource.com/#/c/372322/
https://chromium-review.googlesource.com/#/c/372401/
https://chromium-review.googlesource.com/#/c/372460/
https://chromium-review.googlesource.com/#/c/372479/
https://chromium-review.googlesource.com/#/c/372265/
https://chromium-review.googlesource.com/#/c/373370/
https://chromium-review.googlesource.com/#/c/373738/
https://chromium-review.googlesource.com/#/c/373604/
https://chromium-review.googlesource.com/#/c/373624/
 

Comment 1 by g...@chromium.org, Aug 23 2016

Blocking: 640385
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 25 2016

Labels: merge-merged-chromeos-20150119
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/ltp/+/7df860dd2389c5c9a5b2ed9ac8a7f1d48d6fed03

commit 7df860dd2389c5c9a5b2ed9ac8a7f1d48d6fed03
Author: George Burgess IV <gbiv@google.com>
Date: Tue Aug 23 22:32:54 2016

ltp: remove redeclarations of getcwd().

commit f3ce2d59c97a49a15520f4551d74f9a76e7431d3
Author: Mike Frysinger <vapier@chromium.org>
Date:   Tue Aug 23 04:44:38 2016 +0000

    symlink01: drop bogus getcwd prototype

    Signed-off-by: Mike Frysinger <vapier@chromium.org>

BUG= chromium:640358 
TEST=The errors for symlink01.c go away with clang-FORTIFY enabled.

Change-Id: I11a76bbb5be49c6cd8674c920e6b6f26b0583f15
Reviewed-on: https://chromium-review.googlesource.com/374503
Commit-Ready: George Burgess <gbiv@google.com>
Tested-by: George Burgess <gbiv@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/7df860dd2389c5c9a5b2ed9ac8a7f1d48d6fed03/testcases/kernel/syscalls/symlink/symlink01.c

Blocking: 537368
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/5e7f9a24b3277ae7106d647132716625dc471181

commit 5e7f9a24b3277ae7106d647132716625dc471181
Author: George Burgess IV <gbiv@google.com>
Date: Tue Aug 23 04:17:33 2016

autotest-deps-cellular: Fix clang-style FORTIFY.

All redefinitions/redeclarations of FORTIFY'ed functions need the
overloadable attribute applied to them when clang-style FORTIFY is
enabled.

BUG= chromium:640358 
TEST=emerge-x86-generic builds with clang-style FORTIFY enabled.

Change-Id: I9da17d8d35a49c16a61ab001ab9cb83af6fc80fe
Reviewed-on: https://chromium-review.googlesource.com/374165
Commit-Ready: George Burgess <gbiv@google.com>
Tested-by: George Burgess <gbiv@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/5e7f9a24b3277ae7106d647132716625dc471181/client/deps/fakegudev/src/fakesyscalls.c

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/db0bf2da43af64c9e794329bf7ca0fb5baa5a54d

commit db0bf2da43af64c9e794329bf7ca0fb5baa5a54d
Author: George Burgess IV <gbiv@google.com>
Date: Mon Aug 22 03:55:28 2016

Make strongswan compile under clang-style FORTIFY.

Clang-style FORTIFY turns some warnings into errors. This patch fixes
said errors.

BUG= chromium:640358 
TEST=emerge-x86-generic with clang FORTIFY enabled builds.

Change-Id: I3f03b08712a1725b89f258cd1cfe6b492a9b7521
Reviewed-on: https://chromium-review.googlesource.com/373726
Commit-Ready: George Burgess <gbiv@google.com>
Tested-by: George Burgess <gbiv@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/db0bf2da43af64c9e794329bf7ca0fb5baa5a54d/net-misc/strongswan/files/strongswan-clang-fortify-fix.patch
[modify] https://crrev.com/db0bf2da43af64c9e794329bf7ca0fb5baa5a54d/net-misc/strongswan/strongswan-5.0.2.ebuild
[rename] https://crrev.com/db0bf2da43af64c9e794329bf7ca0fb5baa5a54d/net-misc/strongswan/strongswan-5.0.2-r19.ebuild

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/f257778c501ac43cbb4b69fb937ea866064f1f9e

commit f257778c501ac43cbb4b69fb937ea866064f1f9e
Author: George Burgess IV <gbiv@chromium.org>
Date: Sat Sep 03 20:28:54 2016

clang: Cherrypick fixes for the overloadable attr.

This patch cherrypicks two commits to clang. The goal of these commits
is to relax the overloadable attribute in C. As a result, clang FORTIFY
requires a lot fewer changes in non-library code to work properly.

This change is local to the `overloadable` attribute in clang. Given
that `overloadable` is apparently *very* rarely used outside of FORTIFY
(at the moment), this patch shouldn't cause any regressions.

commit 567da71b9bac96d6e87854f330139710fa2653a7
Author: George Burgess IV <george.burgess.iv@gmail.com>
Date:   Fri Sep 2 22:59:57 2016 +0000

    [Sema] Relax overloading restrictions in C.

    This patch allows us to perform incompatible pointer conversions when
    resolving overloads in C. So, the following code will no longer fail to
    compile (though it will still emit warnings, assuming the user hasn't
    opted out of them):

    ```
    void foo(char *) __attribute__((overloadable));
    void foo(int) __attribute__((overloadable));

    void callFoo() {
      unsigned char bar[128];
      foo(bar); // selects the char* overload.
    }
    ```

    These conversions are ranked below all others, so:

      A. Any other viable conversion will win out
      B. If we had another incompatible pointer conversion in the example
         above (e.g. `void foo(int *)`), we would complain about
         an ambiguity.

    Differential Revision: https://reviews.llvm.org/D24113

    git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@280553 91177308-0d34-0410-b5e6-96231b3b80d8

commit 2cef254afa0c2c82d87d37e7e5d57788061d44a2
Author: George Burgess IV <george.burgess.iv@gmail.com>
Date:   Wed Sep 7 20:03:19 2016 +0000

    [Sema] Compare bad conversions in overload resolution.

    r280553 introduced an issue where we'd emit ambiguity errors for code
    like:

    ```
    void foo(int *, int);
    void foo(unsigned int *, unsigned int);

    void callFoo() {
      unsigned int i;
      foo(&i, 0); // ambiguous: int->unsigned int is worse than int->int,
                  // but unsigned int*->unsigned int* is better than
                  // int*->int*.
    }
    ```

    This patch fixes this issue by changing how we handle ill-formed (but
    valid) implicit conversions. Candidates with said conversions now always
    rank worse than candidates without them, and two candidates are
    considered to be equally bad if they both have these conversions for
    the same argument.

    Additionally, this fixes a case in C++11 where we'd complain about an
    ambiguity in a case like:

    ```
    void f(char *, int);
    void f(const char *, unsigned);
    void g() { f("abc", 0); }
    ```

    ...Since conversion to char* from a string literal is considered
    ill-formed in C++11 (and deprecated in C++03), but we accept it as an
    extension.

    git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@280847 91177308-0d34-0410-b5e6-96231b3b80d8

BUG= chromium:640358 
TEST=Building oak locally from the ground up works. I'll also throw this
on some buildbots, as a part of FORTIFY testing, later this weekend.

Change-Id: I7527161aca758098bebf96dcc07a9c3d75ab155e
Reviewed-on: https://chromium-review.googlesource.com/381013
Commit-Ready: Luis Lozano <llozano@chromium.org>
Tested-by: Luis Lozano <llozano@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[add] https://crrev.com/f257778c501ac43cbb4b69fb937ea866064f1f9e/sys-devel/llvm/files/cherry/2cef254afa0c2c82d87d37e7e5d57788061d44a2.patch
[add] https://crrev.com/f257778c501ac43cbb4b69fb937ea866064f1f9e/sys-devel/llvm/files/cherry/567da71b9bac96d6e87854f330139710fa2653a7.patch
[rename] https://crrev.com/f257778c501ac43cbb4b69fb937ea866064f1f9e/sys-devel/llvm/llvm-3.9_pre265926-r11.ebuild

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/portage-stable/+/8560f4c0b3eaf89a28de4132c999dc5143a51d30

commit 8560f4c0b3eaf89a28de4132c999dc5143a51d30
Author: George Burgess IV <gbiv@chromium.org>
Date: Sat Sep 10 01:35:05 2016

eselect-awk: upgraded package to upstream

Upgraded app-eselect/eselect-awk to version 0.2 on amd64, arm, x86

This is a new dependency of sys-apps/mawk=1.3.4_p20150503.

BUG= chromium:640358 
TEST=Emerges on oak, daisy, amd64-generic. Will run on oak/daisy/peppy
buildbots to make sure they're happy.

Change-Id: I91103f3bd2469b4fe5734631865916cf2fbe6c92
Reviewed-on: https://chromium-review.googlesource.com/384173
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/8560f4c0b3eaf89a28de4132c999dc5143a51d30/metadata/md5-cache/app-eselect/eselect-awk-0.2
[add] https://crrev.com/8560f4c0b3eaf89a28de4132c999dc5143a51d30/app-eselect/eselect-awk/Manifest
[add] https://crrev.com/8560f4c0b3eaf89a28de4132c999dc5143a51d30/app-eselect/eselect-awk/metadata.xml
[add] https://crrev.com/8560f4c0b3eaf89a28de4132c999dc5143a51d30/app-eselect/eselect-awk/eselect-awk-0.2.ebuild

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/portage-stable/+/990837163709ad47572aa0e350e3b1bff2b7d71b

commit 990837163709ad47572aa0e350e3b1bff2b7d71b
Author: George Burgess IV <gbiv@google.com>
Date: Sat Sep 10 04:48:19 2016

mawk: upgraded package to upstream

Upgraded sys-apps/mawk to version 1.3.4_p20150503 on amd64, x86, arm.

Note that this package isn't officially available upstream for ARM.

BUG= chromium:640358 
TEST=Emerges locally on oak, daisy, amd64-generic. Will run
oak/daisy/peppy buildbots prior to committing.

Change-Id: I4641a2ad53eeba5c33dceb1b23fe22e434184873
Reviewed-on: https://chromium-review.googlesource.com/384214
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[delete] https://crrev.com/8560f4c0b3eaf89a28de4132c999dc5143a51d30/sys-apps/mawk/mawk-1.3.4_p20100625.ebuild
[delete] https://crrev.com/8560f4c0b3eaf89a28de4132c999dc5143a51d30/metadata/md5-cache/sys-apps/mawk-1.3.4_p20100625
[modify] https://crrev.com/990837163709ad47572aa0e350e3b1bff2b7d71b/sys-apps/mawk/Manifest
[modify] https://crrev.com/990837163709ad47572aa0e350e3b1bff2b7d71b/sys-apps/mawk/metadata.xml
[add] https://crrev.com/990837163709ad47572aa0e350e3b1bff2b7d71b/sys-apps/mawk/mawk-1.3.4_p20150503.ebuild
[add] https://crrev.com/990837163709ad47572aa0e350e3b1bff2b7d71b/metadata/md5-cache/sys-apps/mawk-1.3.4_p20150503
[delete] https://crrev.com/8560f4c0b3eaf89a28de4132c999dc5143a51d30/sys-apps/mawk/files/mawk-1.3.4-cross-compile.patch

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/a81b53cd1b65d1554d4330087f021fdbb01effb9

commit a81b53cd1b65d1554d4330087f021fdbb01effb9
Author: George Burgess IV <gbiv@chromium.org>
Date: Fri Sep 09 21:17:44 2016

env: Add workarounds to get clang FORTIFY to work.

This adds workarounds (in the form of cflags) for packages that break
with clang-style FORTIFY enabled. "Break", in this case, means
compile-time failures that we've upstreamed fixes for (or will upstream
fixes for shortly).

BUG= chromium:640358 
TEST=All packages with flags added build with clang FORTIFY enabled.

Change-Id: I850cb86939ccc1f0a93ea2762b7c1a238cd71616
Reviewed-on: https://chromium-review.googlesource.com/383952
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/a81b53cd1b65d1554d4330087f021fdbb01effb9/chromeos/config/env/sys-apps/sandbox
[add] https://crrev.com/a81b53cd1b65d1554d4330087f021fdbb01effb9/chromeos/config/env/sys-apps/coreutils

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/8b2365a9a710c0b2cd3094c8cb4249b7d4b52870

commit 8b2365a9a710c0b2cd3094c8cb4249b7d4b52870
Author: George Burgess IV <gbiv@chromium.org>
Date: Fri Sep 09 23:20:30 2016

net-dialup/ppp: remove ttyname redecl.

This removes a useless ttyname() redecl in ppp. This redeclaration was
causing a build breakage on clang with FORTIFY enabled, since it didn't
have the overloadable attribute.

BUG= chromium:640358 
TEST=Emerges on amd64-generic with clang FORTIFY enabled.

Change-Id: I31fd79a69fa7d791df9a8c19e88bbd0495674d95
Reviewed-on: https://chromium-review.googlesource.com/383883
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/8b2365a9a710c0b2cd3094c8cb4249b7d4b52870/net-dialup/ppp/files/ppp-remove-ttyname.patch
[rename] https://crrev.com/8b2365a9a710c0b2cd3094c8cb4249b7d4b52870/net-dialup/ppp/ppp-2.4.6-r7.ebuild

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/39990c990eb8bc264c2ef9981af0623e9bfd162c

commit 39990c990eb8bc264c2ef9981af0623e9bfd162c
Author: George Burgess IV <gbiv@google.com>
Date: Wed Sep 14 21:11:47 2016

env: Add more FORTIFY-related fixes.

Upstreaming some changes to sys-devel/crossdev is taking a bit longer
than we had planned. Disabling clang-specific FORTIFY for these packages
allows us to work around the lack of a patches to crossdev. These can be
removed once we pull said crossdev changes back in.

BUG= chromium:640358 
TEST=All 4 packages emerge on amd64-generic with clang-FORTIFY applied.
Will run on daisy/oak/peppy to be sure nothing's broken.

Change-Id: I49a99977dd3026bdda085d04045514daebe8c111
Reviewed-on: https://chromium-review.googlesource.com/385122
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/39990c990eb8bc264c2ef9981af0623e9bfd162c/chromeos/config/env/sys-apps/findutils
[add] https://crrev.com/39990c990eb8bc264c2ef9981af0623e9bfd162c/chromeos/config/env/net-misc/curl
[add] https://crrev.com/39990c990eb8bc264c2ef9981af0623e9bfd162c/chromeos/config/env/app-shells/bash
[add] https://crrev.com/39990c990eb8bc264c2ef9981af0623e9bfd162c/chromeos/config/env/net-dns/c-ares

Comment 13 by g...@chromium.org, Sep 17 2016

Status: Fixed (was: Available)
FORTIFY is in, so presumably all necessary fixes are in, too.
Labels: VerifyIn-55
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/da3f4d50a75936cff6d0e2aeff0d287581f784de

commit da3f4d50a75936cff6d0e2aeff0d287581f784de
Author: Mattias Nissler <mnissler@chromium.org>
Date: Mon Oct 24 18:53:23 2016

chromeos/config: -D_CLANG_FORTIFY_DISABLE belongs in CPPFLAGS

Some incoming ebuild uprevs (I noticed with c-ares) are complaining
about the define incorrectly being present in CFLAGS.

BUG= chromium:640358 
TEST=Packages continue building.

Change-Id: Ib289abb89d7ce562d3be8c4a07d9b325fd31098b
Reviewed-on: https://chromium-review.googlesource.com/402328
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/da3f4d50a75936cff6d0e2aeff0d287581f784de/chromeos/config/env/sys-apps/sandbox
[modify] https://crrev.com/da3f4d50a75936cff6d0e2aeff0d287581f784de/chromeos/config/env/net-misc/curl
[modify] https://crrev.com/da3f4d50a75936cff6d0e2aeff0d287581f784de/chromeos/config/env/sys-apps/findutils
[modify] https://crrev.com/da3f4d50a75936cff6d0e2aeff0d287581f784de/chromeos/config/env/app-shells/bash
[modify] https://crrev.com/da3f4d50a75936cff6d0e2aeff0d287581f784de/chromeos/config/env/net-dns/c-ares
[modify] https://crrev.com/da3f4d50a75936cff6d0e2aeff0d287581f784de/chromeos/config/env/sci-visualization/gnuplot
[modify] https://crrev.com/da3f4d50a75936cff6d0e2aeff0d287581f784de/chromeos/config/env/sys-apps/coreutils

Comment 16 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56

Comment 17 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 18 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 19 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 20 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61
Status: Verified (was: Fixed)
Closing. Please reopen it if its not fixed. Thanks!

Sign in to add a comment