Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 687198 - flag-o-matic.eclass - strip-unsupported-flags should not strip '-B' flag with clang
Summary: flag-o-matic.eclass - strip-unsupported-flags should not strip '-B' flag with...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All All
: Normal normal (vote)
Assignee: Gentoo Toolchain Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-02 13:30 UTC by Julian Cléaud
Modified: 2019-11-20 19:51 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments
sys-libs/compiler-rt-8.0.0::gentoo build failure with clang's '-B' flag (build-compiler-rt.log,13.38 KB, text/plain)
2019-06-02 13:30 UTC, Julian Cléaud
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Cléaud 2019-06-02 13:30:39 UTC
Created attachment 578400 [details]
sys-libs/compiler-rt-8.0.0::gentoo build failure with clang's '-B' flag

When using the '-B' flag with clang, 'strip-unsupported-flags' strips this flag. This cause build failures with all ebuilds using 'strip-unsupported-flags' function.

Clang's '-B' flag work with a second argument which is necessary for clang to run properly (for example : '-B /usr/lib/llvm/8/binutils-bin').
It adds an implicit path to find build-time binaries (like as, nm, objcopy, ...) which is useful to force usage of llvm binaries instead of the GNU ones for example (just create symlinks to llvm-objcopy as objcopy into this directoy).

This flag is always stripped because 'strip-unsupported-flags' pass the flags to 'test-flags-PROG' which test the flags individually. The '-B' flag is therefore tested by 'test-flag-PROG' individually while it should be used with the next argument (the path).

As a result, '-B' flag is stripped but the path to the directory is not, causing build failures with a message like 'ld.lld: error: cannot open /usr/lib/llvm/8/binutils-bin: Is a directory'.
This behaviour affects clang/clang++ and probably any clang-based compiler.

This behaviour can be fixed by changing the 'test-flags-PROG' function : 

case "$1" in
  --param)
    [...]
  *)
    [...]
esac

to :

case "$1" in
  --param|-B)
    [...]
  *)
    [...]
esac

The attachments contains a log of an example build failure with 'sys-libs/compiler-rt-8.0.0::gentoo'.
Comment 1 Sergei Trofimovich (RETIRED) gentoo-dev 2019-06-22 17:18:25 UTC
strip-unsupported-flags is not an ideal options parser. I would generally advise against passing complicated options via CFLAGS at least to ebuilds that use it.

Why do you need to pass -B explicitly? It messes up toolchains' defaults.

If you really need to you can also try passing it as part of CC as: CC="clang -B/path/to/tools". That arguably is closer to what you want to achieve.
Comment 2 Julian Cléaud 2019-06-22 17:53:10 UTC
I use the '-B' flag to ensure package builds are done through the entire LLVM stack.

This means I replace the following binaries:
- ar      --> llvm-ar
- as      --> llvm-as
- dwp     --> llvm-dwp
- g++     --> clang++
- gcc     --> clang
- ld      --> ld.lld
- nm      --> llvm-nm
- objcopy --> llvm-objcopy
- objdump --> llvm-objdump
- ranlib  --> llvm-ranlib
- readelf --> llvm-readelf
- size    --> llvm-size
- strings --> llvm-strings

This makes the whole process of building anything with clang as smooth as butter because there are no incompatibility between llvm tools (as I already encountered some errors using gnu tools with clang).
Moreover, this fixes a lot lot of Makefiles which are poorly written (I'm looking at you Perl) and which uses GNU tools by defaults (gcc and such) instead of compilers and flags in CC/CXX like environment variables.

This also allows to easily change part of a toolchain (use GNU tools with clang, or entire GNU toolchain when necessary) for packages which doesn't work with LLVM (GNU specific flags passed or non-standard C/C++ code for example) by simply using packages.env.

I've already tried for some times to pass the '-B' flag through CC environment variable, but this isn't really portable as a lot of automake/Makefile packages tends to assume that CC is a binary path without flags, and tries executing the whole content as a binary path (including spaces) which results in errors.

This solution is the simplest I've found to keep a Clang-built system (which you can't do half when you start using llvm libc++) without hassles. The only thing which keeps me from using this setup without struggling is the 'strip-unsupported-flags' function of emerge.
Comment 3 Sergei Trofimovich (RETIRED) gentoo-dev 2019-06-22 20:38:01 UTC
(In reply to Julian Cléaud from comment #2)
> I use the '-B' flag to ensure package builds are done through the entire
> LLVM stack.
> 
> This means I replace the following binaries:
> - ar      --> llvm-ar
> - as      --> llvm-as
> - dwp     --> llvm-dwp
> - g++     --> clang++
> - gcc     --> clang
> - ld      --> ld.lld
> - nm      --> llvm-nm
> - objcopy --> llvm-objcopy
> - objdump --> llvm-objdump
> - ranlib  --> llvm-ranlib
> - readelf --> llvm-readelf
> - size    --> llvm-size
> - strings --> llvm-strings

Most of these tools are not called by compiler driver (the one that accepts -B).  Of the above list clang probably uses only 'ld' from -B path. Rest of tools are discovered by ./configure script amd called directly by makefile. And thus should be orthogonal to handling of -B option.

> I've already tried for some times to pass the '-B' flag through CC
> environment variable, but this isn't really portable as a lot of
> automake/Makefile packages tends to assume that CC is a binary path without
> flags, and tries executing the whole content as a binary path (including
> spaces) which results in errors.

CFLAGS are frequently mangled the same way in packages and in libtool and get lost on their way.

> This solution is the simplest I've found to keep a Clang-built system (which
> you can't do half when you start using llvm libc++) without hassles. The
> only thing which keeps me from using this setup without struggling is the
> 'strip-unsupported-flags' function of emerge.

It's still not clear for me why you need -B option as opposed to just calling 'clang'.
Comment 4 Julian Cléaud 2019-06-22 22:46:11 UTC
(In reply to Sergei Trofimovich from comment #3)
> Most of these tools are not called by compiler driver (the one that accepts
> -B).  Of the above list clang probably uses only 'ld' from -B path. Rest of
> tools are discovered by ./configure script amd called directly by makefile.
> And thus should be orthogonal to handling of -B option.

Indeed I don't really know which ones are really called by the driver itself. I have had examples of packages which required this setup but this was a long time ago and I can't remember one of them as of today.

I symlinked all of those binaries because I also add this folder right within the $PATH variable but this has nothing to do with the compiler driver.

> CFLAGS are frequently mangled the same way in packages and in libtool and
> get lost on their way.

I can't remember building a package which strips the -B flag. 

Moreover, when a package (using libtool or whatever) strips a flag I needed, I can easily patch it with /etc/portage/patches.
I can't do such a thing with eclass. I can maintain a localrepo in which I fixes calls to strip-unsupported-flags inside each package ebuild, or patch the strip-unsupported-flags itself into the eclass, but this is a heavy task (do this for each package update).

> It's still not clear for me why you need -B option as opposed to just
> calling 'clang'.

The simplest example I can remember is with the '-fuse-ld=lld' flag which sometimes get either stripped, or blocks compilation for some reason in some packages (their are some cases where this flag is misinterpreted).
Using the '-B' flag allows to default to something else than the bfd linker (in this case: lld) which is essential to build LTO binaries, without any patch modification.

Of course, this is only a convenience flag to fix some package builds, and in an ideal world, it shouldn't be necessary as all the flags exist to achieve the same purpose, but in some case, this is the only fix which unblocked the situation (and which avoid making compromises such as disabling LTO or falling back to gcc when clang is entirely capable of building it).

Sadly, this has been a long time since I switched to the '-B' setup by default for all packages and I can't give you any example of such a configuration that I remember and which fails to build without the '-B' flag (and without any other alternatives).
Comment 5 Sergei Trofimovich (RETIRED) gentoo-dev 2019-06-23 08:37:19 UTC
(In reply to Julian Cléaud from comment #4)
> (In reply to Sergei Trofimovich from comment #3)
> > Most of these tools are not called by compiler driver (the one that accepts
> > -B).  Of the above list clang probably uses only 'ld' from -B path. Rest of
> > tools are discovered by ./configure script amd called directly by makefile.
> > And thus should be orthogonal to handling of -B option.
> 
> Indeed I don't really know which ones are really called by the driver
> itself. I have had examples of packages which required this setup but this
> was a long time ago and I can't remember one of them as of today.
> 
> I symlinked all of those binaries because I also add this folder right
> within the $PATH variable but this has nothing to do with the compiler
> driver.
> 
> > CFLAGS are frequently mangled the same way in packages and in libtool and
> > get lost on their way.
> 
> I can't remember building a package which strips the -B flag. 
> 
> Moreover, when a package (using libtool or whatever) strips a flag I needed,
> I can easily patch it with /etc/portage/patches.
> I can't do such a thing with eclass.

You can override an eclass with 'eclass-overrides' in repos-conf.

> I can maintain a localrepo in which I
> fixes calls to strip-unsupported-flags inside each package ebuild, or patch
> the strip-unsupported-flags itself into the eclass, but this is a heavy task
> (do this for each package update).

Or you can ask ebuild maintainers for individual ebuilds to expose a hatch to allow unmangled flags, similar to https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=9ecb3b089082f39a15c4c1a315ed91b388a80ade

> > It's still not clear for me why you need -B option as opposed to just
> > calling 'clang'.
> 
> The simplest example I can remember is with the '-fuse-ld=lld' flag which
> sometimes get either stripped, or blocks compilation for some reason in some
> packages (their are some cases where this flag is misinterpreted).
> Using the '-B' flag allows to default to something else than the bfd linker
> (in this case: lld) which is essential to build LTO binaries, without any
> patch modification.

-fuse-ld=* was added to whitelist in bug #662718.

> Sadly, this has been a long time since I switched to the '-B' setup by
> default for all packages and I can't give you any example of such a
> configuration that I remember and which fails to build without the '-B' flag
> (and without any other alternatives).

On another note if you want '-B /foo' to be filtered safely you can use it as a single parameter as '-B/foo'. But we can also can add a 2-parameter version as you suggested in #comment1.

My general feeling is that strip-unsupported-flags should not whitelist -B as a supported flag.
Comment 6 Larry the Git Cow gentoo-dev 2019-06-23 08:53:49 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=82b182fb344a841fccb2b599fc4ea0d5c4ab254f

commit 82b182fb344a841fccb2b599fc4ea0d5c4ab254f
Author:     Sergei Trofimovich <slyfox@gentoo.org>
AuthorDate: 2019-06-23 08:45:11 +0000
Commit:     Sergei Trofimovich <slyfox@gentoo.org>
CommitDate: 2019-06-23 08:53:42 +0000

    flag-o-matic.eclass: filter out '-B/foo' and '-B /foo' equally
    
    In bug #687198 Julian noticed that strip-unsupported-flags()
    filters out '-B' but not '/foo' in CFLAGS='-B /foo' and causes
    breakage.
    
    This change still does not allow -B flag but at least filters
    out both '-B' and it's parameter.
    
    Reported-by: Julian Cléaud
    Bug: https://bugs.gentoo.org/687198
    Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>

 eclass/flag-o-matic.eclass   |  3 ++-
 eclass/tests/flag-o-matic.sh | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
Comment 7 Julian Cléaud 2019-06-23 09:14:03 UTC
(In reply to Sergei Trofimovich from comment #5)

> You can override an eclass with 'eclass-overrides' in repos-conf.

> -fuse-ld=* was added to whitelist in bug #662718.

That's good to know, thanks !

> On another note if you want '-B /foo' to be filtered safely you can use it
> as a single parameter as '-B/foo'. But we can also can add a 2-parameter
> version as you suggested in #comment1.
> 
> My general feeling is that strip-unsupported-flags should not whitelist -B
> as a supported flag.

I didn't even think of that, thanks. At least, this is patched know.

You may probably close this issue as of know, I'll come back here if I really find an unsolvable situation which absolutely requires this flag.
Comment 8 Sergei Trofimovich (RETIRED) gentoo-dev 2019-06-23 10:23:46 UTC
(In reply to Julian Cléaud from comment #7)
> (In reply to Sergei Trofimovich from comment #5)
> 
> > You can override an eclass with 'eclass-overrides' in repos-conf.
> 
> > -fuse-ld=* was added to whitelist in bug #662718.
> 
> That's good to know, thanks !
> 
> > On another note if you want '-B /foo' to be filtered safely you can use it
> > as a single parameter as '-B/foo'. But we can also can add a 2-parameter
> > version as you suggested in #comment1.
> > 
> > My general feeling is that strip-unsupported-flags should not whitelist -B
> > as a supported flag.
> 
> I didn't even think of that, thanks. At least, this is patched know.
> 
> You may probably close this issue as of know, I'll come back here if I
> really find an unsolvable situation which absolutely requires this flag.

Sounds good!
Comment 9 Larry the Git Cow gentoo-dev 2019-11-20 19:51:45 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=3b75b027410fa2f5aca262794173086ec6b19d90

commit 3b75b027410fa2f5aca262794173086ec6b19d90
Author:     Sergei Trofimovich <slyfox@gentoo.org>
AuthorDate: 2019-11-20 19:49:35 +0000
Commit:     Sergei Trofimovich <slyfox@gentoo.org>
CommitDate: 2019-11-20 19:51:36 +0000

    eclass/tests/flag-o-matic.sh: fix strip-unsupported-flags -B* set of tests
    
    Don't know how I tested previous state, it certainly could not work.
    The fix itself is fine though. The change updates expected output.
    
    Reported-by: Michał Górny
    Bug: https://bugs.gentoo.org/687198
    Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>

 eclass/tests/flag-o-matic.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)