[RFC PATCH] lib: move memcat_p.o to obj-y

[RFC PATCH] lib: move memcat_p.o to obj-y

[Ilie Halip]

The semantics of `--undefined` differ between ld.bfd and ld.lld:
    $ echo | gcc -x c -c - -o a.o
    $ echo "EXTERN(test)" > a.lds
    $ ld.bfd a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
    0000000000000000         *UND*      0000000000000000 test
    $ ld.lld a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
    $

When building with CONFIG_STM=m, there are no in-kernel users of memcat_p()
and ld.lld discards this symbol because it's linked in via --no-whole-archive.
It's marked as undefined by a linker script but ld.lld discards it anyway (as
seen in the above snippet).

Work around the ld.lld behavior by moving the object file to obj-y.

Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/515
---
 lib/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Makefile b/lib/Makefile
index 93217d44237f..eac343b81f77 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -30,7 +30,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 flex_proportions.o ratelimit.o show_mem.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
 	 earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
-	 nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o
+	 nmi_backtrace.o nodemask.o win_minmax.o
 
 lib-$(CONFIG_PRINTK) += dump_stack.o
 lib-$(CONFIG_MMU) += ioremap.o
@@ -45,7 +45,7 @@ obj-y += bcd.o sort.o parser.o debug_locks.o random32.o \
 	 bsearch.o find_bit.o llist.o memweight.o kfifo.o \
 	 percpu-refcount.o rhashtable.o \
 	 once.o refcount.o usercopy.o errseq.o bucket_locks.o \
-	 generic-radix-tree.o
+	 generic-radix-tree.o memcat_p.o
 obj-$(CONFIG_STRING_SELFTEST) += test_string.o
 obj-y += string_helpers.o
 obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o

base-commit: ee02f9544ebd8d38900dfe955710897b99483686
-- 
2.17.1

Re: [RFC PATCH] lib: move memcat_p.o to obj-y

[Nick Desaulniers]

On Wed, Jan 15, 2020 at 3:53 AM Ilie Halip <ilie.halip@gmail.com> wrote:
>
> The semantics of `--undefined` differ between ld.bfd and ld.lld:
>     $ echo | gcc -x c -c - -o a.o
>     $ echo "EXTERN(test)" > a.lds
>     $ ld.bfd a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
>     0000000000000000         *UND*      0000000000000000 test
>     $ ld.lld a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
>     $
>
> When building with CONFIG_STM=m, there are no in-kernel users of memcat_p()
> and ld.lld discards this symbol because it's linked in via --no-whole-archive.
> It's marked as undefined by a linker script but ld.lld discards it anyway (as
> seen in the above snippet).
>
> Work around the ld.lld behavior by moving the object file to obj-y.
>
> Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/515

Thanks for the patch.  This solves the observed linkage failure.
Maybe Masahiro has more input based on the discussion in the above
Link?
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  lib/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/Makefile b/lib/Makefile
> index 93217d44237f..eac343b81f77 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -30,7 +30,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
>          flex_proportions.o ratelimit.o show_mem.o \
>          is_single_threaded.o plist.o decompress.o kobject_uevent.o \
>          earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
> -        nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o
> +        nmi_backtrace.o nodemask.o win_minmax.o
>
>  lib-$(CONFIG_PRINTK) += dump_stack.o
>  lib-$(CONFIG_MMU) += ioremap.o
> @@ -45,7 +45,7 @@ obj-y += bcd.o sort.o parser.o debug_locks.o random32.o \
>          bsearch.o find_bit.o llist.o memweight.o kfifo.o \
>          percpu-refcount.o rhashtable.o \
>          once.o refcount.o usercopy.o errseq.o bucket_locks.o \
> -        generic-radix-tree.o
> +        generic-radix-tree.o memcat_p.o
>  obj-$(CONFIG_STRING_SELFTEST) += test_string.o
>  obj-y += string_helpers.o
>  obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
>
> base-commit: ee02f9544ebd8d38900dfe955710897b99483686
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200115115249.27774-1-ilie.halip%40gmail.com.



-- 
Thanks,
~Nick Desaulniers

Re: [RFC PATCH] lib: move memcat_p.o to obj-y

[Masahiro Yamada]

On Fri, Jan 17, 2020 at 7:30 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Jan 15, 2020 at 3:53 AM Ilie Halip <ilie.halip@gmail.com> wrote:
> >
> > The semantics of `--undefined` differ between ld.bfd and ld.lld:
> >     $ echo | gcc -x c -c - -o a.o
> >     $ echo "EXTERN(test)" > a.lds
> >     $ ld.bfd a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
> >     0000000000000000         *UND*      0000000000000000 test
> >     $ ld.lld a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
> >     $
> >
> > When building with CONFIG_STM=m, there are no in-kernel users of memcat_p()
> > and ld.lld discards this symbol because it's linked in via --no-whole-archive.
> > It's marked as undefined by a linker script but ld.lld discards it anyway (as
> > seen in the above snippet).
> >
> > Work around the ld.lld behavior by moving the object file to obj-y.
> >
> > Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> > Link: https://github.com/ClangBuiltLinux/linux/issues/515
>
> Thanks for the patch.  This solves the observed linkage failure.
> Maybe Masahiro has more input based on the discussion in the above
> Link?

Sorry, I did not notice that thread until now.
I will take a look at it when I have time.


This patch does not seem to be a proper fix.

If this fix-up were acceptable, theoretically,
all objects that contain EXPORTED_SYMBOL
would need to be turned into obj-y from lib-y ?







> Tested-by: Nick Desaulniers <ndesaulniers@google.com>







-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH] lib: move memcat_p.o to obj-y

[Fāng-ruì Sòng]

On Fri, Jan 17, 2020 at 10:01 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Jan 17, 2020 at 7:30 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Wed, Jan 15, 2020 at 3:53 AM Ilie Halip <ilie.halip@gmail.com> wrote:
> > >
> > > The semantics of `--undefined` differ between ld.bfd and ld.lld:
> > >     $ echo | gcc -x c -c - -o a.o
> > >     $ echo "EXTERN(test)" > a.lds
> > >     $ ld.bfd a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
> > >     0000000000000000         *UND*      0000000000000000 test
> > >     $ ld.lld a.o -t a.lds -o - 2>/dev/null && objdump -t | grep test
> > >     $
> > >
> > > When building with CONFIG_STM=m, there are no in-kernel users of memcat_p()
> > > and ld.lld discards this symbol because it's linked in via --no-whole-archive.
> > > It's marked as undefined by a linker script but ld.lld discards it anyway (as
> > > seen in the above snippet).
> > >
> > > Work around the ld.lld behavior by moving the object file to obj-y.
> > >
> > > Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/515
> >
> > Thanks for the patch.  This solves the observed linkage failure.
> > Maybe Masahiro has more input based on the discussion in the above
> > Link?
>
> Sorry, I did not notice that thread until now.
> I will take a look at it when I have time.
>
>
> This patch does not seem to be a proper fix.
>
> If this fix-up were acceptable, theoretically,
> all objects that contain EXPORTED_SYMBOL
> would need to be turned into obj-y from lib-y ?
>
> > Tested-by: Nick Desaulniers <ndesaulniers@google.com>

Hi Masahiro,

lld ELF maintainer here. This seems to the only blocker of `LD=ld.lld
allmodconfig` (https://github.com/ClangBuiltLinux/linux/issues/839).
Hope you can help if you think there is a more complete fix.

Below is some (probably obscure) explanation why I think the GNU ld -u
behavior is not consistent and lld should not match:

If a.o has an undefined symbol foo, `{ld.bfd,ld.lld} -shared a.o` will
place foo in .dynsym .

GNU ld's EXTERN(foo) and -u foo behavior is consistent with its
documentation with regard to .symtab, but it is not consistent with
regard to .dynsym.
`ld.bfd -shared -u test a.o` does not place test in .dynsym. So an
undefined symbol added by -u is different from a regular undefined
symbol.

The different behavior for .dynsym is understandable: if GNU ld places
foo in .dynsym, the shared object will likely have an unresolved
reference at run time and will thus be rejected by ld.so.
So for .dynsym, GNU ld's behavior is actually similar to lld's: -u foo
triggers linking of an archive (if the archive defines foo).

Additionally, lld has an extension --undefined-glob=<pattern>, which
is similar to multiple -u. The only reasonable behavior for a pattern
argument is to trigger linking of archive definitions. This is
consistent with lld's current interpretation of -u.


Reviewed-by: Fangrui Song <maskray@google.com>