Re: [PATCHv3 0/2] btf_encoder: Fix functions BTF data generation

From: Jiri Olsa <jolsa@redhat.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	dwarves@vger.kernel.org, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andriin@fb.com>, Yonghong Song <yhs@fb.com>,
	Hao Luo <haoluo@google.com>
Subject: Re: [PATCHv3 0/2] btf_encoder: Fix functions BTF data generation
Date: Sun, 22 Nov 2020 23:56:17 +0100	[thread overview]
Message-ID: <20201122225617.GA1902740@krava> (raw)
In-Reply-To: <CAEf4BzZ-0exZK7skcB_UjyatAx_R=hNqAXKVZ8EXgmSsHmthFg@mail.gmail.com>

On Fri, Nov 20, 2020 at 05:13:24PM -0800, Andrii Nakryiko wrote:
> On Sat, Nov 14, 2020 at 2:39 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > recent btf encoder's changes brakes BTF data for some gcc
> > versions. The problem is that some functions can appear
> > in dwarf data in some instances without arguments, while
> > they are defined with some.
> 
> Hey Jiri,
> 
> So this approach with __start_mcount_loc/__stop_mcount_loc works for
> vmlinux only, but it doesn't work for kernel modules. For kernel
> modules there is a dedicated "__mcount_loc" section, but no
> __start/__stop symbols. I'm working around for now by making sure
> functions that I need are global, but it would be nice to have this
> working for modules out of the box as well.

hi,
I checked and it's bit more tricky than with vmlinux,
addresses are in __mcount_loc, but it's all zeros and
it gets filled after via relocation from .rela__mcount_loc

I think we could do relocation of __mcount_loc section
with zero base and get all base addresses.. and then
continue from there with current code checks

I'll check on it tomorrow

> 
> If you get a chance to fix this soon, that would be great. If not,
> I'll try to get to this ASAP as well, because it would be nice to have
> this in the same version of pahole that got static function BTFs for
> vmlinux (if Arnaldo doesn't mind, of course).

we're eagerly expecting the new pahole with the DWARF bug
workaround, so we asked Arnaldo to release soon, how big
problem is it for you if the modules fix is in the next one?

thanks,
jirka

> 
> >
> > v3 changes:
> >   - move 'generated' flag set out of should_generate_function
> >   - rename should_generate_function to find_function
> >   - added ack
> >
> > v2 changes:
> >   - drop patch 3 logic and just change conditions
> >     based on Andrii's suggestion
> >   - drop patch 2
> >   - add ack for patch 1
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > Jiri Olsa (2):
> >       btf_encoder: Generate also .init functions
> >       btf_encoder: Fix function generation
> >
> >  btf_encoder.c | 86 +++++++++++++++++++++-----------------------------------------------------------------
> >  1 file changed, 21 insertions(+), 65 deletions(-)
> >
> 