Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 565358

Summary: >=sys-devel/llvm-3.5.0: llvm-config gives bogus results
Product: Gentoo Linux Reporter: Steven Newbury <steve>
Component: [OLD] Core systemAssignee: Bernard Cafarelli <voyageur>
Status: RESOLVED FIXED    
Severity: normal CC: mgorny, trupanka
Priority: Normal Keywords: PATCH
Version: unspecified   
Hardware: All   
OS: Linux   
See Also: https://bugs.gentoo.org/show_bug.cgi?id=501684
Whiteboard:
Package list:
Runtime testing required: ---
Attachments: Fix up llvm-config
Updated patch

Description Steven Newbury 2015-11-10 12:15:21 UTC
llvm-config has a number of bugs which can cause projects utilising sys-devel/llvm build problems:

Since sys-devel/llvm switched over to the cmake build system static libraries are not built for LLVM.  However, the llvm-config is hard-coded to handle '.a' library archives.

This results in projects which call llvm-config --libfiles, or --libnames etc to give bogus results and build failure.

This doesn't directly affect llvm-config --libs, which is probably why it hasn't been noticed since that's the most commonly used option for getting the required linkages, as is used by media-libs/mesa, for example.  This isn't by design, though, since the link flags are derived internally by the non-existent static archive names.

Furthermore, the llvm-config --cflags/--cxxflags provide the _optimization_ flags used to build sys-devel/llvm, which means any project which uses a different compiler _and_ llvm-config --cflags/--cxxflags will fail to build if any GCC compiler flags are unsupported.  This affects dev-libs/libclc (which builds with clang) for example.  Unfortunately, at the least, the C++ standard flag, for example '-std=c++11' is necessary since C++11 headers are included when building against LLVM.

I think we're okay hard coding '-std=c++11', since we know our build dependencies are going to be met.  I don't believe any of the other flags produced by cmake/modules/HandleLLVMOptions.cmake are essential to build against LLVM for our supported architectures.

Reproducible: Always

Steps to Reproduce:
Emerge sys-devel/llvm, run llvm-config with various flags.
Actual Results:  
llvm-config produces bogus/invalid results

Expected Results:  
llvm-config should produce correct information for packages to build against sys-devel/llvm.


I'm going to attach a patch which addresses the reported problems, it's specifically a downstream "Gentoo" patch.  It does not attempt to fix the upstream deficiencies from which the bugs manifest.
Comment 1 Steven Newbury 2015-11-10 13:38:35 UTC
Created attachment 416600 [details, diff]
Fix up llvm-config

Attached patch hard-codes dynamic '.so' libraries instead of '.a' static archives since the sys-devel/llvm ebuild only supports shared builds anyway.

It also changes the cflags/cxxflags not to print LLVM build-time optimization flags while ensuring '-std=c++11' is used for '--cxxflags'.
Comment 2 Steven Newbury 2015-11-10 13:41:44 UTC
Above patch is tested against media-libs/mesa[llvm], dev-libs/libclc and a custom net-libs/webkit-gtk build with support for ftl-jit*.


* This patch is all that's required to get WebKit to build with the LLVM based ftl-jit engine.
Comment 3 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-11-26 14:22:33 UTC
Thanks a lot for the patch. I'm leaning towards applying it. However, would you be able to try getting the library suffix from somewhere for improved portability? I don't want to break Prefix by relying on .so (assuming it worked at all).
Comment 4 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-12-02 14:25:21 UTC
I'm going to take it from here. I think LTDL_SHLIB_EXT is the macro we can use to get correct suffix.
Comment 5 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-12-02 16:28:09 UTC
Created attachment 418386 [details, diff]
Updated patch

So far built llvm-config looks good but I'll wait for LLVM to finish building, and then test some revdeps. Since that's going to take some time, feel free to test the patch yourself as well.
Comment 6 Steven Newbury 2015-12-02 18:02:05 UTC
Thanks Michał,
I considered making an proper upstream-able patch but figured it was sufficient until upstream sorted themselves out.
Comment 7 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-12-02 19:57:19 UTC
commit 8eff1b80294e92ebc2f0273980ccabdc615f85f3
Author: Michał Górny <mgorny@gentoo.org>
Date:   Wed Dec 2 20:27:50 2015 +0100

    sys-devel/llvm: Fix bogus flags and paths in llvm-config, #565358
    
    Fix llvm-config to avoid bogus results. In particular:
    
    1. Limit --cflags and --cxxflags to package-specific flags. Do not
    output the whole flag-string used during the build. This fixes libclc
    build issues when LLVM build flags were not tolerated by clang.
    
    2. Fix library names and paths to use shared library suffix rather than
    static library suffix, especially that we do not install static
    libraries.
    
    3. Wipe out --system-libs since they should not be required for dynamic
    linking.
    
    4. Ban --obj-root and --src-root when running outside source tree, since
    we are not installing any sources and therefore their results would
    always be bogus.
    
    Based on patch provided by Steven Newbury.
    
    Fixes: https://bugs.gentoo.org/565358


I've confirmed that mesa still builds fine, and I was finally able to upgrade libclc. Bad news is that the patch doesn't apply to -9999 but that version is currently broken anyway, so I've added the failing epatch anyway not to forget we have to update it.
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-12-02 20:01:35 UTC
*** Bug 561662 has been marked as a duplicate of this bug. ***
Comment 9 Steven Newbury 2015-12-16 11:18:33 UTC
Michał, wiping system-libs has broken FindLLVM.cmake from the Beignet project.  I don't know if wiping it fixes anything else but Beignet will no longer build.  I could send them a patch to change their FindLLVM.cmake if that's preferred, I don't know whether they accept it just for Gentoo..?

What was the problem with --system-libs?  It simply returns "-lrt -ldl -lcurses -lpthread -lz -lm" without being wiped which is ostensibly correct with respect to the LLVM build.
Comment 10 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-12-16 12:33:07 UTC
I would appreciate if you were more specific on what is broken and how. The idea is that --system-libs should not inject random libraries that are not necessary to link against llvm. Most of the libraries you listed belong to that category.

Unless I'm missing some implicit API parts, the only thing potentially meaningful is pthread support -- assuming you can't link against LLVM without pthread being enabled in your program. Is that the case here? Is there a real case when you'd link to LLVM, you need to link using -pthread and your program does not use threads?

If that's not the case, then it's simply another case of missing linkage in Beignet which was only uncovered by the fix in llvm-config.
Comment 11 Steven Newbury 2015-12-16 13:38:58 UTC
It's simply be a bug in how Beignet's FindLLVM.cmake detects LLVM.  In principle, I definitely agree with you about not listing unnecessary libraries.

I wonder if other versions of FindLLVM.cmake out there exhibit the same issue.

if (LLVM_VERSION_NODOT VERSION_GREATER 34)
execute_process(
  COMMAND ${LLVM_CONFIG_EXECUTABLE} --system-libs
  OUTPUT_VARIABLE LLVM_SYSTEM_LIBS_ORIG
  OUTPUT_STRIP_TRAILING_WHITESPACE
)
string(REGEX REPLACE " *\n" "" LLVM_SYSTEM_LIBS ${LLVM_SYSTEM_LIBS_ORIG})
endif (LLVM_VERSION_NODOT VERSION_GREATER 34)

It seems if --system-libs is empty the REGEX fails.
Comment 12 Steven Newbury 2015-12-17 10:21:45 UTC
I've sent a bugfix to the beignet mailing list.