lkml.org 
[lkml]   [2019]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3] Makefile: lld: tell clang to use lld
    On Fri, Apr 5, 2019 at 9:11 AM Kees Cook <keescook@chromium.org> wrote:
    >
    > On Fri, Apr 5, 2019 at 3:17 AM Masahiro Yamada
    > <yamada.masahiro@socionext.com> wrote:
    > > I want to propose alternative solution.
    > > Please check the attached patches.

    ```
    My plan is to rewrite all VDSO Makefiles to use $(LD), then delete cc-ldoption.
    ```

    I agree with that as a possible solution as well; I think it's best to
    just invoke the linker directly rather than use the compiler as the
    driver. Just small nits with the 2 attached patches, but I think they
    will also work for us.

    Looks like there's 15 call sites across 12 files, plus Documentation/
    and scripts/Kbuild.include; removing it seems feasible and I'd be
    happy to help if you'd welcome the patches?

    Let's start with that series of changes, but I suspect we will
    ultimately need to revisit -fuse-ld=. I was just thinking that even
    my proposed patch doesn't do anything for KBUILD_HOSTCFLAGS, for
    example. I worry that bfd may still be invoked by Clang at some point
    in the build. I will try to test in an environment with no GNU
    binutils.

    > In the 0001 patch of arch/arm/vdso/Makefile:
    > > ...
    > > -VDSO_LDFLAGS += $(call cc-ldoption, -fuse-ld=bfd)
    > > +ldflags-y = -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \
    > > + -z max-page-size=4096 -z common-page-size=4096 \
    > > + -nostdlib -shared \
    > > + $(call ld-option, --hash-style=sysv) \

    I think the vdso's should all REQUIRE --hash-style=sysv; the kselftest
    for vdso's will fail otherwise. I could always follow up with patches
    for that if we didn't want to conflate changes.

    > > + $(call ld-option, --build-id) \
    > > + -T

    Was it intentional to add -T standalone? The man page for `ld` says
    it needs an additional filename to follow -T. Or rather is optional
    if a -L flag is specified?

    >
    > I see that "-fuse-ld=bfd" is explicitly dropped here, which could
    > reintroduce the problem fixed in d2b30cd4b722 ("ARM: 8384/1: VDSO:
    > force use of BFD linker") (and 3473f26592c1c). Does ld.gold still
    > exhibit this problem? If so, we'll need to detect gold and force bfd
    > still maybe?

    The arm32 vdso Makefile is an oddity in this regard. I think what
    Kees described is best. Rather than always set the linker to bfd
    (which harms linkers like lld), detect if gold is being used and if so
    set the linker back to bfd.

    For arm64, lld has this issue with -n,
    https://github.com/ClangBuiltLinux/linux/issues/340, but I think we
    can fix that in a follow up patch. Same thoughts on --hash-style and
    -T.

    --
    Thanks,
    ~Nick Desaulniers

    \
     
     \ /
      Last update: 2019-04-05 18:53    [W:2.391 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site