Re: [PATCH v4 0/3] kbuild: Avoid weak external linkage where possible

[Ard Biesheuvel]

On Sat, 20 Apr 2024 at 15:42, Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> On Sat, Apr 20, 2024 at 9:35 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >
> > On Sat, 20 Apr 2024 at 14:32, Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
> > >
> > > On Fri, Apr 19, 2024 at 4:57 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> > > >
> > > > On Tue, 16 Apr 2024 at 16:40, <patchwork-bot+netdevbpf@xxxxxxxxxx> wrote:
> > > > >
> > > > > Hello:
> > > > >
> > > > > This series was applied to bpf/bpf-next.git (master)
> > > > > by Daniel Borkmann <daniel@xxxxxxxxxxxxx>:
> > > > >
> > > > > On Mon, 15 Apr 2024 18:20:42 +0200 you wrote:
> > > > > > From: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > > > >
> > > > > > Weak external linkage is intended for cases where a symbol reference
> > > > > > can remain unsatisfied in the final link. Taking the address of such a
> > > > > > symbol should yield NULL if the reference was not satisfied.
> > > > > >
> > > > > > Given that ordinary RIP or PC relative references cannot produce NULL,
> > > > > > some kind of indirection is always needed in such cases, and in position
> > > > > > independent code, this results in a GOT entry. In ordinary code, it is
> > > > > > arch specific but amounts to the same thing.
> > > > > >
> > > > > > [...]
> > > > >
> > > > > Here is the summary with links:
> > > > >   - [v4,1/3] kallsyms: Avoid weak references for kallsyms symbols
> > > > >     (no matching commit)
> > > > >   - [v4,2/3] vmlinux: Avoid weak reference to notes section
> > > > >     (no matching commit)
> > > > >   - [v4,3/3] btf: Avoid weak external references
> > > > >     https://git.kernel.org/bpf/bpf-next/c/fc5eb4a84e4c
> > > > >
> > > >
> > > >
> > > > Thanks.
> > > >
> > > > Masahiro, could you pick up patches #1 and #2 please?
> > > >
> > >
> > >
> > > I do not like PROVIDE() because it potentially shifts
> > > a build error (i.e. link error) into
> > > a run-time error, which is usually more difficult to debug
> > > than build error.
> > >
> > > If someone references the kallsyms_* symbols
> > > when CONFIG_KALLSYMS=n, it is likely a mistake.
> > > In general, it should be reported as a link error.
> > >
> >
> > OK, so the PROVIDE() should be conditional on CONFIG_KALLSYM=y. I can fix that.
>
>
> You may need to take care of the dependency
> between CONFIG_KALLSYMS and CONFIG_VMCORE_INFO
> because kernel/vmcore_info.c has references
> to the kallsyms_* symbols.
>
> (I am still not a big fan of PROVIDE() though)
>


OK, how about we use weak definitions (as opposed to weak references)
in kernel/kallsyms.c, which will get superseded by the actual ones in
the second linker pass.

The only difference is that we will use some space in the binary for
the weak definitions that are never used in the final build.