Build failures: unresolved symbol vfs_getattr

Build failures: unresolved symbol vfs_getattr

[Veronika Kabatova]

Hello,

we tested the bpf-next tree with CKI and ran across build failures. The
important part of the build log is:

00:18:05   GEN     .version
00:18:05   CHK     include/generated/compile.h
00:18:05   LD      vmlinux.o
00:18:27   MODPOST vmlinux.symvers
00:18:27   MODINFO modules.builtin.modinfo
00:18:27   GEN     modules.builtin
00:18:27   LD      .tmp_vmlinux.btf
00:18:42   BTF     .btf.vmlinux.bin.o
00:19:13   LD      .tmp_vmlinux.kallsyms1
00:19:19   KSYM    .tmp_vmlinux.kallsyms1.o
00:19:22   LD      .tmp_vmlinux.kallsyms2
00:19:25   KSYM    .tmp_vmlinux.kallsyms2.o
00:19:28   LD      vmlinux
00:19:40   BTFIDS  vmlinux
00:19:40 FAILED unresolved symbol vfs_getattr
00:19:40 make[2]: *** [Makefile:1167: vmlinux] Error 255
00:19:40 make[1]: *** [scripts/Makefile.package:109: targz-pkg] Error 2
00:19:40 make: *** [Makefile:1528: targz-pkg] Error 2

Going git-bisect style, the issue is introduced by the patch series in [0].
Commit 2532f849b5134c4c62a20e5aaca33d9fb08af528 (last one before the
mentioned series were merged) passes, testing of
cd04b04de119a222c83936f7e9dbd46a650cb688 (last patch of the series)
fails. The failure is still present with the current top of the tree.

The kernel config used is a Fedora 32 config base + olddefconfig +
kselftest-merge, you can grab it directly from [1]. Environment is
Fedora rawhide.


The failure is easily reproduced with our container image that already
has all the needed dependencies installed:
registry.gitlab.com/cki-project/containers/builder-rawhide:latest


Steps to reproduce after starting the image:

git clone https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git --depth 1
curl https://gitlab.com/-/snippets/2014934/raw -o bpf-next/.config
cd bpf-next/
make -j 10 INSTALL_MOD_STRIP=1 targz-pkg



I already notified Jirka (cced) earlier today but am also sending a
bug report here in case someone else runs into the issue.

I can help with testing of potential fixes if it's needed.

Veronika


[0]: https://lore.kernel.org/bpf/20200825192124.710397-1-jolsa@kernel.org/
[1]: https://gitlab.com/-/snippets/2014934/raw

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Mon, Sep 14, 2020 at 10:48:36AM -0400, Veronika Kabatova wrote:
> 
> Hello,
> 
> we tested the bpf-next tree with CKI and ran across build failures. The
> important part of the build log is:
> 
> 00:18:05   GEN     .version
> 00:18:05   CHK     include/generated/compile.h
> 00:18:05   LD      vmlinux.o
> 00:18:27   MODPOST vmlinux.symvers
> 00:18:27   MODINFO modules.builtin.modinfo
> 00:18:27   GEN     modules.builtin
> 00:18:27   LD      .tmp_vmlinux.btf
> 00:18:42   BTF     .btf.vmlinux.bin.o
> 00:19:13   LD      .tmp_vmlinux.kallsyms1
> 00:19:19   KSYM    .tmp_vmlinux.kallsyms1.o
> 00:19:22   LD      .tmp_vmlinux.kallsyms2
> 00:19:25   KSYM    .tmp_vmlinux.kallsyms2.o
> 00:19:28   LD      vmlinux
> 00:19:40   BTFIDS  vmlinux
> 00:19:40 FAILED unresolved symbol vfs_getattr
> 00:19:40 make[2]: *** [Makefile:1167: vmlinux] Error 255
> 00:19:40 make[1]: *** [scripts/Makefile.package:109: targz-pkg] Error 2
> 00:19:40 make: *** [Makefile:1528: targz-pkg] Error 2

hi,
it looks like broken BTF data to me, I checked that build
and found we have multiple records for functions, like
for filp_close:

	[23381] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
		'(anon)' type_id=464
		'id' type_id=960
	[23382] FUNC 'filp_close' type_id=23381 linkage=static


	[33073] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
		'filp' type_id=464
		'id' type_id=960
	[33074] FUNC 'filp_close' type_id=33073 linkage=static


or vfs_getattr:

	[33513] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
		'path' type_id=741
		'stat' type_id=1095
		'request_mask' type_id=29
		'query_flags' type_id=8

	[33514] FUNC 'vfs_getattr' type_id=33513 linkage=static

	[1094] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
		'(anon)' type_id=741
		'(anon)' type_id=1095
		'(anon)' type_id=29
		'(anon)' type_id=8

	[35099] FUNC 'vfs_getattr' type_id=1094 linkage=static


and because we go through all BTF data until we resolve all we have,
the doubled funcs will screw our internal counter and we skip a function

the change below will workaround that, but I think we should fail in
this case.. if I'm not missing something 2 FUNC records for one function
in BTF data

$ pahole --version
v1.17

HEAD is 2bab48c5b Merge branch 'improve-bpf-tcp-cc-init'

thoughts? thanks
jirka


---
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index dfa540d8a02d..a33e56553e52 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -525,7 +525,7 @@ static int symbols_resolve(struct object *obj)
 		}
 
 		id = btf_id__find(root, str);
-		if (id) {
+		if (id && !id->id) {
 			id->id = type_id;
 			(*nr)--;
 		}

Re: Build failures: unresolved symbol vfs_getattr

[Andrii Nakryiko]

On Mon, Sep 14, 2020 at 11:25 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Sep 14, 2020 at 10:48:36AM -0400, Veronika Kabatova wrote:
> >
> > Hello,
> >
> > we tested the bpf-next tree with CKI and ran across build failures. The
> > important part of the build log is:
> >
> > 00:18:05   GEN     .version
> > 00:18:05   CHK     include/generated/compile.h
> > 00:18:05   LD      vmlinux.o
> > 00:18:27   MODPOST vmlinux.symvers
> > 00:18:27   MODINFO modules.builtin.modinfo
> > 00:18:27   GEN     modules.builtin
> > 00:18:27   LD      .tmp_vmlinux.btf
> > 00:18:42   BTF     .btf.vmlinux.bin.o
> > 00:19:13   LD      .tmp_vmlinux.kallsyms1
> > 00:19:19   KSYM    .tmp_vmlinux.kallsyms1.o
> > 00:19:22   LD      .tmp_vmlinux.kallsyms2
> > 00:19:25   KSYM    .tmp_vmlinux.kallsyms2.o
> > 00:19:28   LD      vmlinux
> > 00:19:40   BTFIDS  vmlinux
> > 00:19:40 FAILED unresolved symbol vfs_getattr
> > 00:19:40 make[2]: *** [Makefile:1167: vmlinux] Error 255
> > 00:19:40 make[1]: *** [scripts/Makefile.package:109: targz-pkg] Error 2
> > 00:19:40 make: *** [Makefile:1528: targz-pkg] Error 2
>
> hi,
> it looks like broken BTF data to me, I checked that build
> and found we have multiple records for functions, like
> for filp_close:
>
>         [23381] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
>                 '(anon)' type_id=464
>                 'id' type_id=960
>         [23382] FUNC 'filp_close' type_id=23381 linkage=static
>
>
>         [33073] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
>                 'filp' type_id=464
>                 'id' type_id=960
>         [33074] FUNC 'filp_close' type_id=33073 linkage=static
>
>
> or vfs_getattr:
>
>         [33513] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
>                 'path' type_id=741
>                 'stat' type_id=1095
>                 'request_mask' type_id=29
>                 'query_flags' type_id=8
>
>         [33514] FUNC 'vfs_getattr' type_id=33513 linkage=static
>
>         [1094] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
>                 '(anon)' type_id=741
>                 '(anon)' type_id=1095
>                 '(anon)' type_id=29
>                 '(anon)' type_id=8
>
>         [35099] FUNC 'vfs_getattr' type_id=1094 linkage=static
>
>
> and because we go through all BTF data until we resolve all we have,
> the doubled funcs will screw our internal counter and we skip a function
>
> the change below will workaround that, but I think we should fail in
> this case.. if I'm not missing something 2 FUNC records for one function
> in BTF data
>
> $ pahole --version
> v1.17
>
> HEAD is 2bab48c5b Merge branch 'improve-bpf-tcp-cc-init'
>
> thoughts? thanks

Can't repro this locally. It must be some bad compiler +  DWARF +
pahole interaction. Can you try building pahole from latest sources
and try again? Also, what compiler did you use? What Kconfig?

> jirka
>
>
> ---
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index dfa540d8a02d..a33e56553e52 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -525,7 +525,7 @@ static int symbols_resolve(struct object *obj)
>                 }
>
>                 id = btf_id__find(root, str);
> -               if (id) {
> +               if (id && !id->id) {
>                         id->id = type_id;
>                         (*nr)--;
>                 }
>

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Mon, Sep 14, 2020 at 03:26:33PM -0700, Andrii Nakryiko wrote:
> On Mon, Sep 14, 2020 at 11:25 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Mon, Sep 14, 2020 at 10:48:36AM -0400, Veronika Kabatova wrote:
> > >
> > > Hello,
> > >
> > > we tested the bpf-next tree with CKI and ran across build failures. The
> > > important part of the build log is:
> > >
> > > 00:18:05   GEN     .version
> > > 00:18:05   CHK     include/generated/compile.h
> > > 00:18:05   LD      vmlinux.o
> > > 00:18:27   MODPOST vmlinux.symvers
> > > 00:18:27   MODINFO modules.builtin.modinfo
> > > 00:18:27   GEN     modules.builtin
> > > 00:18:27   LD      .tmp_vmlinux.btf
> > > 00:18:42   BTF     .btf.vmlinux.bin.o
> > > 00:19:13   LD      .tmp_vmlinux.kallsyms1
> > > 00:19:19   KSYM    .tmp_vmlinux.kallsyms1.o
> > > 00:19:22   LD      .tmp_vmlinux.kallsyms2
> > > 00:19:25   KSYM    .tmp_vmlinux.kallsyms2.o
> > > 00:19:28   LD      vmlinux
> > > 00:19:40   BTFIDS  vmlinux
> > > 00:19:40 FAILED unresolved symbol vfs_getattr
> > > 00:19:40 make[2]: *** [Makefile:1167: vmlinux] Error 255
> > > 00:19:40 make[1]: *** [scripts/Makefile.package:109: targz-pkg] Error 2
> > > 00:19:40 make: *** [Makefile:1528: targz-pkg] Error 2
> >
> > hi,
> > it looks like broken BTF data to me, I checked that build
> > and found we have multiple records for functions, like
> > for filp_close:
> >
> >         [23381] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
> >                 '(anon)' type_id=464
> >                 'id' type_id=960
> >         [23382] FUNC 'filp_close' type_id=23381 linkage=static
> >
> >
> >         [33073] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
> >                 'filp' type_id=464
> >                 'id' type_id=960
> >         [33074] FUNC 'filp_close' type_id=33073 linkage=static
> >
> >
> > or vfs_getattr:
> >
> >         [33513] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
> >                 'path' type_id=741
> >                 'stat' type_id=1095
> >                 'request_mask' type_id=29
> >                 'query_flags' type_id=8
> >
> >         [33514] FUNC 'vfs_getattr' type_id=33513 linkage=static
> >
> >         [1094] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
> >                 '(anon)' type_id=741
> >                 '(anon)' type_id=1095
> >                 '(anon)' type_id=29
> >                 '(anon)' type_id=8
> >
> >         [35099] FUNC 'vfs_getattr' type_id=1094 linkage=static
> >
> >
> > and because we go through all BTF data until we resolve all we have,
> > the doubled funcs will screw our internal counter and we skip a function
> >
> > the change below will workaround that, but I think we should fail in
> > this case.. if I'm not missing something 2 FUNC records for one function
> > in BTF data
> >
> > $ pahole --version
> > v1.17
> >
> > HEAD is 2bab48c5b Merge branch 'improve-bpf-tcp-cc-init'
> >
> > thoughts? thanks
> 
> Can't repro this locally. It must be some bad compiler +  DWARF +
> pahole interaction. Can you try building pahole from latest sources
> and try again? Also, what compiler did you use? What Kconfig?

sorry I cut the original message, there's following container that
reproduces the issue:

	The failure is easily reproduced with our container image that already
	has all the needed dependencies installed:
	registry.gitlab.com/cki-project/containers/builder-rawhide:latest

	Steps to reproduce after starting the image:

	git clone https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git --depth 1
	curl https://gitlab.com/-/snippets/2014934/raw -o bpf-next/.config
	cd bpf-next/
	make -j 10 INSTALL_MOD_STRIP=1 targz-pkg


[root@30a9be783e4e /]# gcc --version
gcc (GCC) 10.2.1 20200826 (Red Hat 10.2.1-3)

I built the latest pahole in that container and still see the issue,
I also tried with v1.16 version and it's still there

I don't see the issue when I build kernel with another .config
so I'll try to check on that now

jirka

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Tue, Sep 15, 2020 at 09:30:33AM +0200, Jiri Olsa wrote:
> On Mon, Sep 14, 2020 at 03:26:33PM -0700, Andrii Nakryiko wrote:
> > On Mon, Sep 14, 2020 at 11:25 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Mon, Sep 14, 2020 at 10:48:36AM -0400, Veronika Kabatova wrote:
> > > >
> > > > Hello,
> > > >
> > > > we tested the bpf-next tree with CKI and ran across build failures. The
> > > > important part of the build log is:
> > > >
> > > > 00:18:05   GEN     .version
> > > > 00:18:05   CHK     include/generated/compile.h
> > > > 00:18:05   LD      vmlinux.o
> > > > 00:18:27   MODPOST vmlinux.symvers
> > > > 00:18:27   MODINFO modules.builtin.modinfo
> > > > 00:18:27   GEN     modules.builtin
> > > > 00:18:27   LD      .tmp_vmlinux.btf
> > > > 00:18:42   BTF     .btf.vmlinux.bin.o
> > > > 00:19:13   LD      .tmp_vmlinux.kallsyms1
> > > > 00:19:19   KSYM    .tmp_vmlinux.kallsyms1.o
> > > > 00:19:22   LD      .tmp_vmlinux.kallsyms2
> > > > 00:19:25   KSYM    .tmp_vmlinux.kallsyms2.o
> > > > 00:19:28   LD      vmlinux
> > > > 00:19:40   BTFIDS  vmlinux
> > > > 00:19:40 FAILED unresolved symbol vfs_getattr
> > > > 00:19:40 make[2]: *** [Makefile:1167: vmlinux] Error 255
> > > > 00:19:40 make[1]: *** [scripts/Makefile.package:109: targz-pkg] Error 2
> > > > 00:19:40 make: *** [Makefile:1528: targz-pkg] Error 2
> > >
> > > hi,
> > > it looks like broken BTF data to me, I checked that build
> > > and found we have multiple records for functions, like
> > > for filp_close:
> > >
> > >         [23381] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
> > >                 '(anon)' type_id=464
> > >                 'id' type_id=960
> > >         [23382] FUNC 'filp_close' type_id=23381 linkage=static
> > >
> > >
> > >         [33073] FUNC_PROTO '(anon)' ret_type_id=19 vlen=2
> > >                 'filp' type_id=464
> > >                 'id' type_id=960
> > >         [33074] FUNC 'filp_close' type_id=33073 linkage=static
> > >
> > >
> > > or vfs_getattr:
> > >
> > >         [33513] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
> > >                 'path' type_id=741
> > >                 'stat' type_id=1095
> > >                 'request_mask' type_id=29
> > >                 'query_flags' type_id=8
> > >
> > >         [33514] FUNC 'vfs_getattr' type_id=33513 linkage=static
> > >
> > >         [1094] FUNC_PROTO '(anon)' ret_type_id=19 vlen=4
> > >                 '(anon)' type_id=741
> > >                 '(anon)' type_id=1095
> > >                 '(anon)' type_id=29
> > >                 '(anon)' type_id=8
> > >
> > >         [35099] FUNC 'vfs_getattr' type_id=1094 linkage=static
> > >
> > >
> > > and because we go through all BTF data until we resolve all we have,
> > > the doubled funcs will screw our internal counter and we skip a function
> > >
> > > the change below will workaround that, but I think we should fail in
> > > this case.. if I'm not missing something 2 FUNC records for one function
> > > in BTF data
> > >
> > > $ pahole --version
> > > v1.17
> > >
> > > HEAD is 2bab48c5b Merge branch 'improve-bpf-tcp-cc-init'
> > >
> > > thoughts? thanks
> > 
> > Can't repro this locally. It must be some bad compiler +  DWARF +
> > pahole interaction. Can you try building pahole from latest sources
> > and try again? Also, what compiler did you use? What Kconfig?
> 
> sorry I cut the original message, there's following container that
> reproduces the issue:
> 
> 	The failure is easily reproduced with our container image that already
> 	has all the needed dependencies installed:
> 	registry.gitlab.com/cki-project/containers/builder-rawhide:latest
> 
> 	Steps to reproduce after starting the image:
> 
> 	git clone https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git --depth 1
> 	curl https://gitlab.com/-/snippets/2014934/raw -o bpf-next/.config
> 	cd bpf-next/
> 	make -j 10 INSTALL_MOD_STRIP=1 targz-pkg
> 
> 
> [root@30a9be783e4e /]# gcc --version
> gcc (GCC) 10.2.1 20200826 (Red Hat 10.2.1-3)
> 
> I built the latest pahole in that container and still see the issue,
> I also tried with v1.16 version and it's still there
> 
> I don't see the issue when I build kernel with another .config
> so I'll try to check on that now

readelf --debug has 2 records for vfs_getattr
fs/stat.c and include/linux/fs.h

fs/stat.c:
	 <1><11f9847>: Abbrev Number: 119 (DW_TAG_subprogram)
	    <11f9848>   DW_AT_external    : 1
	    <11f9848>   DW_AT_name        : (indirect string, offset: 0x9eaeb): vfs_getattr
	    <11f984c>   DW_AT_decl_file   : 1
	    <11f984d>   DW_AT_decl_line   : 116
	    <11f984e>   DW_AT_decl_column : 5
	    <11f984f>   DW_AT_prototyped  : 1
	    <11f984f>   DW_AT_type        : <0x11ebe32>
	    <11f9853>   DW_AT_inline      : 1   (inlined)
	    <11f9854>   DW_AT_sibling     : <0x11f9895>
	 <2><11f9858>: Abbrev Number: 35 (DW_TAG_formal_parameter)
	    <11f9859>   DW_AT_name        : (indirect string, offset: 0x70041): path
	    <11f985d>   DW_AT_decl_file   : 1
	    <11f985e>   DW_AT_decl_line   : 116
	    <11f985f>   DW_AT_decl_column : 36
	    <11f9860>   DW_AT_type        : <0x11f17ed>
	 <2><11f9864>: Abbrev Number: 35 (DW_TAG_formal_parameter)
	    <11f9865>   DW_AT_name        : (indirect string, offset: 0x5fa1): stat
	    <11f9869>   DW_AT_decl_file   : 1
	    <11f986a>   DW_AT_decl_line   : 116
	    <11f986b>   DW_AT_decl_column : 56
	    <11f986c>   DW_AT_type        : <0x11f41e0>
	 <2><11f9870>: Abbrev Number: 35 (DW_TAG_formal_parameter)
	    <11f9871>   DW_AT_name        : (indirect string, offset: 0x8e714): request_mask
	    <11f9875>   DW_AT_decl_file   : 1
	    <11f9876>   DW_AT_decl_line   : 117
	    <11f9877>   DW_AT_decl_column : 7
	    <11f9878>   DW_AT_type        : <0x11ebe93>
	 <2><11f987c>: Abbrev Number: 35 (DW_TAG_formal_parameter)
	    <11f987d>   DW_AT_name        : (indirect string, offset: 0xd789): query_flags
	    <11f9881>   DW_AT_decl_file   : 1
	    <11f9882>   DW_AT_decl_line   : 117
	    <11f9883>   DW_AT_decl_column : 34
	    <11f9884>   DW_AT_type        : <0x11ebde1>

include/linux/fs.h:
	 <1><140d794>: Abbrev Number: 43 (DW_TAG_subprogram)
	    <140d795>   DW_AT_external    : 1
	    <140d795>   DW_AT_name        : (indirect string, offset: 0x9eaeb): vfs_getattr
	    <140d799>   DW_AT_decl_file   : 7
	    <140d79a>   DW_AT_decl_line   : 3148
	    <140d79c>   DW_AT_decl_column : 12
	    <140d79d>   DW_AT_prototyped  : 1
	    <140d79d>   DW_AT_type        : <0x140611a>
	    <140d7a1>   DW_AT_sibling     : <0x140d7ba>
	 <2><140d7a5>: Abbrev Number: 3 (DW_TAG_formal_parameter)
	    <140d7a6>   DW_AT_type        : <0x14087aa>
	 <2><140d7aa>: Abbrev Number: 3 (DW_TAG_formal_parameter)
	    <140d7ab>   DW_AT_type        : <0x140cfb6>
	 <2><140d7af>: Abbrev Number: 3 (DW_TAG_formal_parameter)
	    <140d7b0>   DW_AT_type        : <0x1406176>
	 <2><140d7b4>: Abbrev Number: 3 (DW_TAG_formal_parameter)
	    <140d7b5>   DW_AT_type        : <0x14060c9>
	 <2><140d7b9>: Abbrev Number: 0

the latter is just declaration.. but it's missing the
    <365d69d>   DW_AT_declaration : 1

so it goes through pahole's function processing:

	cu__encode_btf:
	...
        cu__for_each_function(cu, core_id, fn) {
                int btf_fnproto_id, btf_fn_id;

                if (fn->declaration || !fn->external)
                        continue;
	...


CC-ing Frank.. any idea why is the DW_AT_declaration : 1 missing?

thanks,
jirka

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Tue, Sep 15, 2020 at 02:17:46PM +0200, Jiri Olsa wrote:

SNIP

> 	 <2><140d7aa>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> 	    <140d7ab>   DW_AT_type        : <0x140cfb6>
> 	 <2><140d7af>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> 	    <140d7b0>   DW_AT_type        : <0x1406176>
> 	 <2><140d7b4>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> 	    <140d7b5>   DW_AT_type        : <0x14060c9>
> 	 <2><140d7b9>: Abbrev Number: 0
> 
> the latter is just declaration.. but it's missing the
>     <365d69d>   DW_AT_declaration : 1
> 
> so it goes through pahole's function processing:
> 
> 	cu__encode_btf:
> 	...
>         cu__for_each_function(cu, core_id, fn) {
>                 int btf_fnproto_id, btf_fn_id;
> 
>                 if (fn->declaration || !fn->external)
>                         continue;
> 	...
> 
> 
> CC-ing Frank.. any idea why is the DW_AT_declaration : 1 missing?

looks like gcc issue:
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060

let's see ;-)

jirka

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Wed, Sep 16, 2020 at 11:06:27AM +0200, Jiri Olsa wrote:
> On Tue, Sep 15, 2020 at 02:17:46PM +0200, Jiri Olsa wrote:
> 
> SNIP
> 
> > 	 <2><140d7aa>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > 	    <140d7ab>   DW_AT_type        : <0x140cfb6>
> > 	 <2><140d7af>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > 	    <140d7b0>   DW_AT_type        : <0x1406176>
> > 	 <2><140d7b4>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > 	    <140d7b5>   DW_AT_type        : <0x14060c9>
> > 	 <2><140d7b9>: Abbrev Number: 0
> > 
> > the latter is just declaration.. but it's missing the
> >     <365d69d>   DW_AT_declaration : 1
> > 
> > so it goes through pahole's function processing:
> > 
> > 	cu__encode_btf:
> > 	...
> >         cu__for_each_function(cu, core_id, fn) {
> >                 int btf_fnproto_id, btf_fn_id;
> > 
> >                 if (fn->declaration || !fn->external)
> >                         continue;
> > 	...
> > 
> > 
> > CC-ing Frank.. any idea why is the DW_AT_declaration : 1 missing?
> 
> looks like gcc issue:
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060
> 
> let's see ;-)

so this gcc bug did not disappear and the fix might be delayed,
as I was told it's real complex and difficult to fix

and it's no longer just rawhide issue, because I just started to
see it in Fedora 32 after updating to gcc (GCC) 10.2.1 20201005
(Red Hat 10.2.1-5)

I'm checking on pahole's workaround, but so far I can't see dwarf
based solution for that.. any thoughts/ideas? ;-)

thanks,
jirka

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Fri, Oct 16, 2020 at 11:38:37PM +0200, Jiri Olsa wrote:
> On Wed, Sep 16, 2020 at 11:06:27AM +0200, Jiri Olsa wrote:
> > On Tue, Sep 15, 2020 at 02:17:46PM +0200, Jiri Olsa wrote:
> > 
> > SNIP
> > 
> > > 	 <2><140d7aa>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > > 	    <140d7ab>   DW_AT_type        : <0x140cfb6>
> > > 	 <2><140d7af>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > > 	    <140d7b0>   DW_AT_type        : <0x1406176>
> > > 	 <2><140d7b4>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > > 	    <140d7b5>   DW_AT_type        : <0x14060c9>
> > > 	 <2><140d7b9>: Abbrev Number: 0
> > > 
> > > the latter is just declaration.. but it's missing the
> > >     <365d69d>   DW_AT_declaration : 1
> > > 
> > > so it goes through pahole's function processing:
> > > 
> > > 	cu__encode_btf:
> > > 	...
> > >         cu__for_each_function(cu, core_id, fn) {
> > >                 int btf_fnproto_id, btf_fn_id;
> > > 
> > >                 if (fn->declaration || !fn->external)
> > >                         continue;
> > > 	...
> > > 
> > > 
> > > CC-ing Frank.. any idea why is the DW_AT_declaration : 1 missing?
> > 
> > looks like gcc issue:
> >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060
> > 
> > let's see ;-)
> 
> so this gcc bug did not disappear and the fix might be delayed,
> as I was told it's real complex and difficult to fix
> 
> and it's no longer just rawhide issue, because I just started to
> see it in Fedora 32 after updating to gcc (GCC) 10.2.1 20201005
> (Red Hat 10.2.1-5)
> 
> I'm checking on pahole's workaround, but so far I can't see dwarf
> based solution for that.. any thoughts/ideas? ;-)

hi,
FYI there's still no solution yet, so far the progress is:

the proposed workaround was to use the negation -> we don't have
DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
tags have attached code and skip them if they don't have any:
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10

the attached patch is doing that, but the resulting BTF is missing
several functions due to another bug in dwarf:
  https://bugzilla.redhat.com/show_bug.cgi?id=1890107


the only other workaround I can think of is to check each DW_TAG_subprogram
tag name with vmlinux symbol to ensure it's actually present,
I'll check on it, because as Mark suggested it might be good
for future not to completely rely on dwarf being correct, even
if that gcc bug gets eventually fixed

jirka


---
diff --git a/btf_encoder.c b/btf_encoder.c
index e90150784282..51a370d580b7 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -302,8 +302,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 
 	cu__for_each_function(cu, core_id, fn) {
 		int btf_fnproto_id, btf_fn_id;
+		bool has_pc = !!function__addr(fn) || fn->ranges;
 
-		if (fn->declaration || !fn->external)
+		if (!has_pc || !fn->external)
 			continue;
 
 		btf_fnproto_id = btf_elf__add_func_proto(btfe, &fn->proto, type_id_off);
diff --git a/dwarf_loader.c b/dwarf_loader.c
index d3586aa5b0dd..4763b9118475 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -953,6 +953,7 @@ static struct function *function__new(Dwarf_Die *die, struct cu *cu)
 		func->declaration     = dwarf_hasattr(die, DW_AT_declaration);
 		func->external	      = dwarf_hasattr(die, DW_AT_external);
 		func->abstract_origin = dwarf_hasattr(die, DW_AT_abstract_origin);
+		func->ranges          = dwarf_hasattr(die, DW_AT_ranges);
 		dwarf_tag__set_spec(func->proto.tag.priv,
 				    attr_type(die, DW_AT_specification));
 		func->accessibility   = attr_numeric(die, DW_AT_accessibility);
@@ -2023,8 +2024,10 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
 			dtype = dwarf_cu__find_tag_by_ref(cu->priv, &dtag->abstract_origin);
 			if (dtype == NULL)
 				dtype = dwarf_cu__find_tag_by_ref(cu->priv, &specification);
-			if (dtype != NULL)
+			if (dtype != NULL) {
 				fn->name = tag__function(dtype->tag)->name;
+				fn->external = tag__function(dtype->tag)->external;
+			}
 			else {
 				fprintf(stderr,
 					"%s: couldn't find name for "
diff --git a/dwarves.h b/dwarves.h
index 7c4254eded1f..3204f69abfe5 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -813,6 +813,7 @@ struct function {
 	uint8_t		 virtuality:2; /* DW_VIRTUALITY_{none,virtual,pure_virtual} */
 	uint8_t		 declaration:1;
 	uint8_t		 btf:1;
+	uint8_t		 ranges:1;
 	int32_t		 vtable_entry;
 	struct list_head vtable_node;
 	/* fields used by tools */

Re: Build failures: unresolved symbol vfs_getattr

[Andrii Nakryiko]

On Wed, Oct 21, 2020 at 12:42 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Oct 16, 2020 at 11:38:37PM +0200, Jiri Olsa wrote:
> > On Wed, Sep 16, 2020 at 11:06:27AM +0200, Jiri Olsa wrote:
> > > On Tue, Sep 15, 2020 at 02:17:46PM +0200, Jiri Olsa wrote:
> > >
> > > SNIP
> > >
> > > >    <2><140d7aa>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > > >       <140d7ab>   DW_AT_type        : <0x140cfb6>
> > > >    <2><140d7af>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > > >       <140d7b0>   DW_AT_type        : <0x1406176>
> > > >    <2><140d7b4>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > > >       <140d7b5>   DW_AT_type        : <0x14060c9>
> > > >    <2><140d7b9>: Abbrev Number: 0
> > > >
> > > > the latter is just declaration.. but it's missing the
> > > >     <365d69d>   DW_AT_declaration : 1
> > > >
> > > > so it goes through pahole's function processing:
> > > >
> > > >   cu__encode_btf:
> > > >   ...
> > > >         cu__for_each_function(cu, core_id, fn) {
> > > >                 int btf_fnproto_id, btf_fn_id;
> > > >
> > > >                 if (fn->declaration || !fn->external)
> > > >                         continue;
> > > >   ...
> > > >
> > > >
> > > > CC-ing Frank.. any idea why is the DW_AT_declaration : 1 missing?
> > >
> > > looks like gcc issue:
> > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060
> > >
> > > let's see ;-)
> >
> > so this gcc bug did not disappear and the fix might be delayed,
> > as I was told it's real complex and difficult to fix
> >
> > and it's no longer just rawhide issue, because I just started to
> > see it in Fedora 32 after updating to gcc (GCC) 10.2.1 20201005
> > (Red Hat 10.2.1-5)
> >
> > I'm checking on pahole's workaround, but so far I can't see dwarf
> > based solution for that.. any thoughts/ideas? ;-)
>
> hi,
> FYI there's still no solution yet, so far the progress is:
>
> the proposed workaround was to use the negation -> we don't have
> DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> tags have attached code and skip them if they don't have any:
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
>
> the attached patch is doing that, but the resulting BTF is missing
> several functions due to another bug in dwarf:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1890107

It seems fine if there are only few functions (especially if those are
unlikely to be traced). Do you have an estimate of how many functions
have this second DWARF bug?

>
>
> the only other workaround I can think of is to check each DW_TAG_subprogram
> tag name with vmlinux symbol to ensure it's actually present,
> I'll check on it, because as Mark suggested it might be good
> for future not to completely rely on dwarf being correct, even
> if that gcc bug gets eventually fixed

This might be a good thing to do anyways. Currently BTF contains only
global functions, but a lot of static functions that didn't end up
inlined are available for attachment, but because BTF info is not
there, we can't use fentry/fexit on them. Checking against ELF symbols
would match kallsyms, right? So we would be able to drop fn->external
requirement and have all the attachable functions available.

Have you tried this? I'm curious how good the data is and how much
bigger BTF size is with all the functions included?

>
> jirka
>
>
> ---
> diff --git a/btf_encoder.c b/btf_encoder.c
> index e90150784282..51a370d580b7 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -302,8 +302,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>
>         cu__for_each_function(cu, core_id, fn) {
>                 int btf_fnproto_id, btf_fn_id;
> +               bool has_pc = !!function__addr(fn) || fn->ranges;
>
> -               if (fn->declaration || !fn->external)
> +               if (!has_pc || !fn->external)
>                         continue;
>
>                 btf_fnproto_id = btf_elf__add_func_proto(btfe, &fn->proto, type_id_off);
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index d3586aa5b0dd..4763b9118475 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -953,6 +953,7 @@ static struct function *function__new(Dwarf_Die *die, struct cu *cu)
>                 func->declaration     = dwarf_hasattr(die, DW_AT_declaration);
>                 func->external        = dwarf_hasattr(die, DW_AT_external);
>                 func->abstract_origin = dwarf_hasattr(die, DW_AT_abstract_origin);
> +               func->ranges          = dwarf_hasattr(die, DW_AT_ranges);
>                 dwarf_tag__set_spec(func->proto.tag.priv,
>                                     attr_type(die, DW_AT_specification));
>                 func->accessibility   = attr_numeric(die, DW_AT_accessibility);
> @@ -2023,8 +2024,10 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
>                         dtype = dwarf_cu__find_tag_by_ref(cu->priv, &dtag->abstract_origin);
>                         if (dtype == NULL)
>                                 dtype = dwarf_cu__find_tag_by_ref(cu->priv, &specification);
> -                       if (dtype != NULL)
> +                       if (dtype != NULL) {
>                                 fn->name = tag__function(dtype->tag)->name;
> +                               fn->external = tag__function(dtype->tag)->external;
> +                       }
>                         else {
>                                 fprintf(stderr,
>                                         "%s: couldn't find name for "
> diff --git a/dwarves.h b/dwarves.h
> index 7c4254eded1f..3204f69abfe5 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -813,6 +813,7 @@ struct function {
>         uint8_t          virtuality:2; /* DW_VIRTUALITY_{none,virtual,pure_virtual} */
>         uint8_t          declaration:1;
>         uint8_t          btf:1;
> +       uint8_t          ranges:1;
>         int32_t          vtable_entry;
>         struct list_head vtable_node;
>         /* fields used by tools */
>

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote:

SNIP

> >
> > hi,
> > FYI there's still no solution yet, so far the progress is:
> >
> > the proposed workaround was to use the negation -> we don't have
> > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> > tags have attached code and skip them if they don't have any:
> >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
> >
> > the attached patch is doing that, but the resulting BTF is missing
> > several functions due to another bug in dwarf:
> >   https://bugzilla.redhat.com/show_bug.cgi?id=1890107
> 
> It seems fine if there are only few functions (especially if those are
> unlikely to be traced). Do you have an estimate of how many functions
> have this second DWARF bug?

it wasn't that many, I'll recheck

> 
> >
> >
> > the only other workaround I can think of is to check each DW_TAG_subprogram
> > tag name with vmlinux symbol to ensure it's actually present,
> > I'll check on it, because as Mark suggested it might be good
> > for future not to completely rely on dwarf being correct, even
> > if that gcc bug gets eventually fixed
> 
> This might be a good thing to do anyways. Currently BTF contains only
> global functions, but a lot of static functions that didn't end up
> inlined are available for attachment, but because BTF info is not
> there, we can't use fentry/fexit on them. Checking against ELF symbols
> would match kallsyms, right? So we would be able to drop fn->external
> requirement and have all the attachable functions available.

right, both static and global

> 
> Have you tried this? I'm curious how good the data is and how much
> bigger BTF size is with all the functions included?

I did not realize that with droping fn->external this would bring
static functions as well, now I want to try ;-)

jirka

> 
> >
> > jirka
> >
> >
> > ---
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index e90150784282..51a370d580b7 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -302,8 +302,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
> >
> >         cu__for_each_function(cu, core_id, fn) {
> >                 int btf_fnproto_id, btf_fn_id;
> > +               bool has_pc = !!function__addr(fn) || fn->ranges;
> >
> > -               if (fn->declaration || !fn->external)
> > +               if (!has_pc || !fn->external)
> >                         continue;
> >
> >                 btf_fnproto_id = btf_elf__add_func_proto(btfe, &fn->proto, type_id_off);
> > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > index d3586aa5b0dd..4763b9118475 100644
> > --- a/dwarf_loader.c
> > +++ b/dwarf_loader.c
> > @@ -953,6 +953,7 @@ static struct function *function__new(Dwarf_Die *die, struct cu *cu)
> >                 func->declaration     = dwarf_hasattr(die, DW_AT_declaration);
> >                 func->external        = dwarf_hasattr(die, DW_AT_external);
> >                 func->abstract_origin = dwarf_hasattr(die, DW_AT_abstract_origin);
> > +               func->ranges          = dwarf_hasattr(die, DW_AT_ranges);
> >                 dwarf_tag__set_spec(func->proto.tag.priv,
> >                                     attr_type(die, DW_AT_specification));
> >                 func->accessibility   = attr_numeric(die, DW_AT_accessibility);
> > @@ -2023,8 +2024,10 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
> >                         dtype = dwarf_cu__find_tag_by_ref(cu->priv, &dtag->abstract_origin);
> >                         if (dtype == NULL)
> >                                 dtype = dwarf_cu__find_tag_by_ref(cu->priv, &specification);
> > -                       if (dtype != NULL)
> > +                       if (dtype != NULL) {
> >                                 fn->name = tag__function(dtype->tag)->name;
> > +                               fn->external = tag__function(dtype->tag)->external;
> > +                       }
> >                         else {
> >                                 fprintf(stderr,
> >                                         "%s: couldn't find name for "
> > diff --git a/dwarves.h b/dwarves.h
> > index 7c4254eded1f..3204f69abfe5 100644
> > --- a/dwarves.h
> > +++ b/dwarves.h
> > @@ -813,6 +813,7 @@ struct function {
> >         uint8_t          virtuality:2; /* DW_VIRTUALITY_{none,virtual,pure_virtual} */
> >         uint8_t          declaration:1;
> >         uint8_t          btf:1;
> > +       uint8_t          ranges:1;
> >         int32_t          vtable_entry;
> >         struct list_head vtable_node;
> >         /* fields used by tools */
> >
> 

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Fri, Oct 23, 2020 at 07:36:57AM +0200, Jiri Olsa wrote:
> On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote:
> 
> SNIP
> 
> > >
> > > hi,
> > > FYI there's still no solution yet, so far the progress is:
> > >
> > > the proposed workaround was to use the negation -> we don't have
> > > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> > > tags have attached code and skip them if they don't have any:
> > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
> > >
> > > the attached patch is doing that, but the resulting BTF is missing
> > > several functions due to another bug in dwarf:
> > >   https://bugzilla.redhat.com/show_bug.cgi?id=1890107
> > 
> > It seems fine if there are only few functions (especially if those are
> > unlikely to be traced). Do you have an estimate of how many functions
> > have this second DWARF bug?
> 
> it wasn't that many, I'll recheck

127 functions missing if the workaround is applied, list attached

jirka


---
'acpi_ev_default_region_setup'
'acpi_ev_pci_bar_region_setup'
'acpi_ex_pci_bar_space_handler'
'acpi_ex_stop_trace_opcode'
'acpi_os_notify_command_complete'
'arch_cpu_idle_exit'
'arch_unregister_hw_breakpoint'
'bfqg_stats_set_start_empty_time'
'bfqg_stats_set_start_idle_time'
'bfqg_stats_update_avg_queue_size'
'bfqg_stats_update_idle_time'
'bfqg_stats_update_io_merged'
'blk_freeze_queue'
'cap_task_setnice'
'cryptd_aead_queued'
'devm_regmap_field_bulk_free'
'disable_nmi_nosync'
'ext4_ext_release'
'fs_param_is_path'
'HUF_isError'
'__ia32_compat_sys_ftruncate'
'__ia32_compat_sys_getsockopt'
'__ia32_compat_sys_io_pgetevents_time32'
'__ia32_compat_sys_preadv64'
'__ia32_compat_sys_process_vm_readv'
'__ia32_compat_sys_process_vm_writev'
'__ia32_compat_sys_pwritev64'
'__ia32_compat_sys_s390_ipc'
'__ia32_compat_sys_setsockopt'
'__ia32_sys_fadvise64'
'__ia32_sys_getegid16'
'__ia32_sys_geteuid16'
'__ia32_sys_getgid16'
'__ia32_sys_getuid16'
'__ia32_sys_inotify_init'
'__ia32_sys_io_pgetevents_time32'
'__ia32_sys_ipc'
'__ia32_sys_munlockall'
'__ia32_sys_old_msgctl'
'__ia32_sys_old_semctl'
'__ia32_sys_old_shmctl'
'__ia32_sys_pciconfig_iobase'
'__ia32_sys_pciconfig_read'
'__ia32_sys_pciconfig_write'
'__ia32_sys_ppoll_time32'
'__ia32_sys_pselect6_time32'
'__ia32_sys_rtas'
'__ia32_sys_s390_ipc'
'__ia32_sys_s390_pci_mmio_read'
'__ia32_sys_s390_pci_mmio_write'
'__ia32_sys_sgetmask'
'__ia32_sys_spu_create'
'__ia32_sys_spu_run'
'__ia32_sys_subpage_prot'
'__ia32_sys_uselib'
'__ia32_sys_vm86'
'__ia32_sys_vm86old'
'ima_post_path_mknod'
'ima_show_template_buf'
'__kfifo_skip_r'
'kstrtol_from_user'
'mdiobus_read_nested'
'mdiobus_write_nested'
'memcpy'
'memmove'
'memset'
'module_arch_freeing_init'
'native_restore_fl'
'native_save_fl'
'netdev_walk_all_lower_dev_rcu'
'notifier_hangup_irq'
'nsecs_to_jiffies'
'of_set_phy_eee_broken'
'param_set_hexint'
'param_set_ullong'
'pcibios_bus_add_device'
'phy_start_machine'
'pmdp_collapse_flush'
'_raw_write_unlock'
'_raw_write_unlock_bh'
'_raw_write_unlock_irq'
'_raw_write_unlock_irqrestore'
'rcu_barrier_tasks_trace'
'rcu_test_sync_prims'
'regmap_field_bulk_free'
'regset_xregset_fpregs_active'
'seq_hlist_next_rcu'
'set_anon_super_fc'
'simple_strtoll'
'sock_no_getname'
'sock_no_sendmsg_locked'
'sock_no_shutdown'
'synchronize_rcu_tasks'
'synchronize_rcu_tasks_rude'
'text_poke_kgdb'
'tty_driver_kref_put'
'watchdog_nmi_start'
'__x64_sys_fadvise64'
'__x64_sys_getegid16'
'__x64_sys_geteuid16'
'__x64_sys_getgid16'
'__x64_sys_getuid16'
'__x64_sys_inotify_init'
'__x64_sys_io_pgetevents_time32'
'__x64_sys_ipc'
'__x64_sys_munlockall'
'__x64_sys_old_msgctl'
'__x64_sys_old_semctl'
'__x64_sys_old_shmctl'
'__x64_sys_pciconfig_iobase'
'__x64_sys_pciconfig_read'
'__x64_sys_pciconfig_write'
'__x64_sys_ppoll_time32'
'__x64_sys_pselect6_time32'
'__x64_sys_rtas'
'__x64_sys_s390_ipc'
'__x64_sys_s390_pci_mmio_read'
'__x64_sys_s390_pci_mmio_write'
'__x64_sys_sgetmask'
'__x64_sys_spu_create'
'__x64_sys_spu_run'
'__x64_sys_subpage_prot'
'__x64_sys_uselib'
'__x64_sys_vm86'
'__x64_sys_vm86old'
'xen_has_pv_nic_devices'

Re: Build failures: unresolved symbol vfs_getattr

[Andrii Nakryiko]

On Thu, Oct 22, 2020 at 11:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Oct 23, 2020 at 07:36:57AM +0200, Jiri Olsa wrote:
> > On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote:
> >
> > SNIP
> >
> > > >
> > > > hi,
> > > > FYI there's still no solution yet, so far the progress is:
> > > >
> > > > the proposed workaround was to use the negation -> we don't have
> > > > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> > > > tags have attached code and skip them if they don't have any:
> > > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
> > > >
> > > > the attached patch is doing that, but the resulting BTF is missing
> > > > several functions due to another bug in dwarf:
> > > >   https://bugzilla.redhat.com/show_bug.cgi?id=1890107
> > >
> > > It seems fine if there are only few functions (especially if those are
> > > unlikely to be traced). Do you have an estimate of how many functions
> > > have this second DWARF bug?
> >
> > it wasn't that many, I'll recheck
>
> 127 functions missing if the workaround is applied, list attached
>

some of those seem pretty useful... I guess the quick workaround in
pahole would be to just remember function names that were emitted
already. The problem with that is that we can pick a version without
parameter names, which is not the end of the world, but certainly
annoying.

But otherwise, I don't really have a good feeling what's the perfect
solution here...

> jirka
>
>
> ---
> 'acpi_ev_default_region_setup'
> 'acpi_ev_pci_bar_region_setup'
> 'acpi_ex_pci_bar_space_handler'
> 'acpi_ex_stop_trace_opcode'
> 'acpi_os_notify_command_complete'
> 'arch_cpu_idle_exit'
> 'arch_unregister_hw_breakpoint'
> 'bfqg_stats_set_start_empty_time'
> 'bfqg_stats_set_start_idle_time'
> 'bfqg_stats_update_avg_queue_size'
> 'bfqg_stats_update_idle_time'
> 'bfqg_stats_update_io_merged'
> 'blk_freeze_queue'
> 'cap_task_setnice'
> 'cryptd_aead_queued'
> 'devm_regmap_field_bulk_free'
> 'disable_nmi_nosync'
> 'ext4_ext_release'
> 'fs_param_is_path'
> 'HUF_isError'
> '__ia32_compat_sys_ftruncate'
> '__ia32_compat_sys_getsockopt'
> '__ia32_compat_sys_io_pgetevents_time32'
> '__ia32_compat_sys_preadv64'
> '__ia32_compat_sys_process_vm_readv'
> '__ia32_compat_sys_process_vm_writev'
> '__ia32_compat_sys_pwritev64'
> '__ia32_compat_sys_s390_ipc'
> '__ia32_compat_sys_setsockopt'
> '__ia32_sys_fadvise64'
> '__ia32_sys_getegid16'
> '__ia32_sys_geteuid16'
> '__ia32_sys_getgid16'
> '__ia32_sys_getuid16'
> '__ia32_sys_inotify_init'
> '__ia32_sys_io_pgetevents_time32'
> '__ia32_sys_ipc'
> '__ia32_sys_munlockall'
> '__ia32_sys_old_msgctl'
> '__ia32_sys_old_semctl'
> '__ia32_sys_old_shmctl'
> '__ia32_sys_pciconfig_iobase'
> '__ia32_sys_pciconfig_read'
> '__ia32_sys_pciconfig_write'
> '__ia32_sys_ppoll_time32'
> '__ia32_sys_pselect6_time32'
> '__ia32_sys_rtas'
> '__ia32_sys_s390_ipc'
> '__ia32_sys_s390_pci_mmio_read'
> '__ia32_sys_s390_pci_mmio_write'
> '__ia32_sys_sgetmask'
> '__ia32_sys_spu_create'
> '__ia32_sys_spu_run'
> '__ia32_sys_subpage_prot'
> '__ia32_sys_uselib'
> '__ia32_sys_vm86'
> '__ia32_sys_vm86old'
> 'ima_post_path_mknod'
> 'ima_show_template_buf'
> '__kfifo_skip_r'
> 'kstrtol_from_user'
> 'mdiobus_read_nested'
> 'mdiobus_write_nested'
> 'memcpy'
> 'memmove'
> 'memset'
> 'module_arch_freeing_init'
> 'native_restore_fl'
> 'native_save_fl'
> 'netdev_walk_all_lower_dev_rcu'
> 'notifier_hangup_irq'
> 'nsecs_to_jiffies'
> 'of_set_phy_eee_broken'
> 'param_set_hexint'
> 'param_set_ullong'
> 'pcibios_bus_add_device'
> 'phy_start_machine'
> 'pmdp_collapse_flush'
> '_raw_write_unlock'
> '_raw_write_unlock_bh'
> '_raw_write_unlock_irq'
> '_raw_write_unlock_irqrestore'
> 'rcu_barrier_tasks_trace'
> 'rcu_test_sync_prims'
> 'regmap_field_bulk_free'
> 'regset_xregset_fpregs_active'
> 'seq_hlist_next_rcu'
> 'set_anon_super_fc'
> 'simple_strtoll'
> 'sock_no_getname'
> 'sock_no_sendmsg_locked'
> 'sock_no_shutdown'
> 'synchronize_rcu_tasks'
> 'synchronize_rcu_tasks_rude'
> 'text_poke_kgdb'
> 'tty_driver_kref_put'
> 'watchdog_nmi_start'
> '__x64_sys_fadvise64'
> '__x64_sys_getegid16'
> '__x64_sys_geteuid16'
> '__x64_sys_getgid16'
> '__x64_sys_getuid16'
> '__x64_sys_inotify_init'
> '__x64_sys_io_pgetevents_time32'
> '__x64_sys_ipc'
> '__x64_sys_munlockall'
> '__x64_sys_old_msgctl'
> '__x64_sys_old_semctl'
> '__x64_sys_old_shmctl'
> '__x64_sys_pciconfig_iobase'
> '__x64_sys_pciconfig_read'
> '__x64_sys_pciconfig_write'
> '__x64_sys_ppoll_time32'
> '__x64_sys_pselect6_time32'
> '__x64_sys_rtas'
> '__x64_sys_s390_ipc'
> '__x64_sys_s390_pci_mmio_read'
> '__x64_sys_s390_pci_mmio_write'
> '__x64_sys_sgetmask'
> '__x64_sys_spu_create'
> '__x64_sys_spu_run'
> '__x64_sys_subpage_prot'
> '__x64_sys_uselib'
> '__x64_sys_vm86'
> '__x64_sys_vm86old'
> 'xen_has_pv_nic_devices'
>

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Fri, Oct 23, 2020 at 11:22:05AM -0700, Andrii Nakryiko wrote:
> On Thu, Oct 22, 2020 at 11:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, Oct 23, 2020 at 07:36:57AM +0200, Jiri Olsa wrote:
> > > On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote:
> > >
> > > SNIP
> > >
> > > > >
> > > > > hi,
> > > > > FYI there's still no solution yet, so far the progress is:
> > > > >
> > > > > the proposed workaround was to use the negation -> we don't have
> > > > > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> > > > > tags have attached code and skip them if they don't have any:
> > > > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
> > > > >
> > > > > the attached patch is doing that, but the resulting BTF is missing
> > > > > several functions due to another bug in dwarf:
> > > > >   https://bugzilla.redhat.com/show_bug.cgi?id=1890107
> > > >
> > > > It seems fine if there are only few functions (especially if those are
> > > > unlikely to be traced). Do you have an estimate of how many functions
> > > > have this second DWARF bug?
> > >
> > > it wasn't that many, I'll recheck
> >
> > 127 functions missing if the workaround is applied, list attached
> >
> 
> some of those seem pretty useful... I guess the quick workaround in
> pahole would be to just remember function names that were emitted
> already. The problem with that is that we can pick a version without
> parameter names, which is not the end of the world, but certainly
> annoying.

right, we can generate them in bpftrace, but it's a shame


> 
> But otherwise, I don't really have a good feeling what's the perfect
> solution here...

I tried the check of dwarf record against function symbols
with adresses mentioned earlier (attached)

getting more functions of course ;-)

$ bpftool btf dump file ./vmlinux | grep 'FUNC '  | wc -l
46606

compared to 22869 on the same .config with working gcc
and current pahole

and resolve_btfids is happy, because there are no duplications

jirka


---
diff --git a/btf_encoder.c b/btf_encoder.c
index 2a6455be4c52..468781204cd9 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -326,6 +326,18 @@ static int find_all_percpu_vars(struct btf_elf *btfe)
 	return 0;
 }
 
+static bool btfe__generate_func(const struct btf_elf *btfe, const char *name)
+{
+	struct symbol *sym;
+
+	sym = btfe__find_symbol(btfe, name);
+	if (!sym || sym->generated || !sym->addr)
+		return false;
+
+	sym->generated = true;
+	return true;
+}
+
 int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		   bool skip_encoding_vars)
 {
@@ -354,6 +366,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		if (!skip_encoding_vars && find_all_percpu_vars(btfe))
 			goto out;
 
+		if (btfe__load_symbols(btfe, cu))
+			goto out;
+
 		has_index_type = false;
 		need_index_type = false;
 		array_index_id = 0;
@@ -401,7 +416,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		int btf_fnproto_id, btf_fn_id;
 		const char *name;
 
-		if (fn->declaration || !fn->external)
+		if (!btfe__generate_func(btfe, function__name(fn, cu)))
 			continue;
 
 		btf_fnproto_id = btf_elf__add_func_proto(btfe, cu, &fn->proto, type_id_off);
diff --git a/elf_symtab.h b/elf_symtab.h
index 359add69c8ab..094ec4683d01 100644
--- a/elf_symtab.h
+++ b/elf_symtab.h
@@ -63,6 +63,14 @@ static inline uint64_t elf_sym__value(const GElf_Sym *sym)
 	return sym->st_value;
 }
 
+static inline int elf_sym__is_function(const GElf_Sym *sym)
+{
+	return (elf_sym__type(sym) == STT_FUNC ||
+		elf_sym__type(sym) == STT_GNU_IFUNC) &&
+		sym->st_name != 0 &&
+		sym->st_shndx != SHN_UNDEF;
+}
+
 static inline bool elf_sym__is_local_function(const GElf_Sym *sym)
 {
 	return elf_sym__type(sym) == STT_FUNC &&
diff --git a/libbtf.c b/libbtf.c
index babf4fe8cd9e..6c4b8a069b85 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -30,6 +30,91 @@
 uint8_t btf_elf__verbose;
 uint8_t btf_elf__force;
 
+static int btfe__insert_symbol(struct btf_elf *btfe, const char *name,
+			     uint64_t addr)
+{
+	struct rb_node **p = &btfe->symbols.rb_node;
+	struct rb_node *parent = NULL;
+	struct symbol *sym;
+	int err;
+
+	while (*p != NULL) {
+		parent = *p;
+		sym = rb_entry(parent, struct symbol, rb_node);
+
+
+		err = strcmp(sym->name, name);
+		if (err < 0)
+			p = &(*p)->rb_left;
+		else if (err > 0)
+			p = &(*p)->rb_right;
+		else
+			return 0;
+	}
+
+	sym = malloc(sizeof(*sym));
+	if (!sym)
+		return -1;
+	sym->name = name;
+	sym->addr = addr;
+	sym->generated = false;
+
+	rb_link_node(&sym->rb_node, parent, p);
+	rb_insert_color(&sym->rb_node, &btfe->symbols);
+	return 0;
+}
+
+struct symbol *btfe__find_symbol(const struct btf_elf *btfe, const char *name)
+{
+	struct symbol *sym;
+	struct rb_node *n;
+	int err;
+
+	n = btfe->symbols.rb_node;
+
+	while (n) {
+		sym = rb_entry(n, struct symbol, rb_node);
+		err = strcmp(sym->name, name);
+		if (err < 0)
+			n = n->rb_left;
+		else if (err > 0)
+			n = n->rb_right;
+		else
+			return sym;
+	}
+
+	return NULL;
+}
+
+int btfe__load_symbols(struct btf_elf *btfe, struct cu *cu)
+{
+	const char *name;
+	GElf_Sym sym;
+	uint32_t id;
+
+	cu__cache_symtab(cu);
+
+	cu__for_each_cached_symtab_entry(cu, id, sym, name) {
+		if (!elf_sym__is_function(&sym))
+			continue;
+		if (btfe__insert_symbol(btfe, name, elf_sym__value(&sym)))
+			return -1;
+	}
+	return 0;
+}
+
+static void btfe__delete_symbols(struct btf_elf *btfe)
+{
+	struct rb_node *n = rb_first(&btfe->symbols);
+	struct symbol *sym;
+
+	while (n) {
+		sym = rb_entry(n, struct symbol, rb_node);
+		n = rb_next(&sym->rb_node);
+		free(sym);
+	}
+}
+
 static int btf_var_secinfo_cmp(const void *a, const void *b)
 {
 	const struct btf_var_secinfo *av = a;
@@ -72,6 +157,7 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
 	if (!btfe)
 		return NULL;
 
+	btfe->symbols = RB_ROOT;
 	btfe->in_fd = -1;
 	btfe->filename = strdup(filename);
 	if (btfe->filename == NULL)
@@ -177,6 +263,7 @@ void btf_elf__delete(struct btf_elf *btfe)
 			elf_end(btfe->elf);
 	}
 
+	btfe__delete_symbols(btfe);
 	elf_symtab__delete(btfe->symtab);
 	__gobuffer__delete(&btfe->percpu_secinfo);
 	btf__free(btfe->btf);
diff --git a/libbtf.h b/libbtf.h
index 887b5bc55c8e..f4a58834c390 100644
--- a/libbtf.h
+++ b/libbtf.h
@@ -12,6 +12,14 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include "lib/bpf/src/btf.h"
+#include "rbtree.h"
+
+struct symbol {
+	const char	*name;
+	uint64_t	 addr;
+	bool		 generated;
+	struct rb_node	 rb_node;
+};
 
 struct btf_elf {
 	void		  *priv;
@@ -27,6 +35,7 @@ struct btf_elf {
 	uint32_t	  percpu_shndx;
 	uint64_t	  percpu_base_addr;
 	struct btf	  *btf;
+	struct rb_root	  symbols;
 };
 
 extern uint8_t btf_elf__verbose;
@@ -66,4 +75,7 @@ int  btf_elf__encode(struct btf_elf *btf, uint8_t flags);
 const char *btf_elf__string(struct btf_elf *btf, uint32_t ref);
 int btf_elf__load(struct btf_elf *btf);
 
+struct symbol *btfe__find_symbol(const struct btf_elf *btfe, const char *name);
+int btfe__load_symbols(struct btf_elf *btfe, struct cu *cu);
+
 #endif /* _LIBBTF_H */

Re: Build failures: unresolved symbol vfs_getattr

[Andrii Nakryiko]

On Fri, Oct 23, 2020 at 1:17 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Oct 23, 2020 at 11:22:05AM -0700, Andrii Nakryiko wrote:
> > On Thu, Oct 22, 2020 at 11:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Fri, Oct 23, 2020 at 07:36:57AM +0200, Jiri Olsa wrote:
> > > > On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote:
> > > >
> > > > SNIP
> > > >
> > > > > >
> > > > > > hi,
> > > > > > FYI there's still no solution yet, so far the progress is:
> > > > > >
> > > > > > the proposed workaround was to use the negation -> we don't have
> > > > > > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> > > > > > tags have attached code and skip them if they don't have any:
> > > > > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
> > > > > >
> > > > > > the attached patch is doing that, but the resulting BTF is missing
> > > > > > several functions due to another bug in dwarf:
> > > > > >   https://bugzilla.redhat.com/show_bug.cgi?id=1890107
> > > > >
> > > > > It seems fine if there are only few functions (especially if those are
> > > > > unlikely to be traced). Do you have an estimate of how many functions
> > > > > have this second DWARF bug?
> > > >
> > > > it wasn't that many, I'll recheck
> > >
> > > 127 functions missing if the workaround is applied, list attached
> > >
> >
> > some of those seem pretty useful... I guess the quick workaround in
> > pahole would be to just remember function names that were emitted
> > already. The problem with that is that we can pick a version without
> > parameter names, which is not the end of the world, but certainly
> > annoying.
>
> right, we can generate them in bpftrace, but it's a shame
>
>
> >
> > But otherwise, I don't really have a good feeling what's the perfect
> > solution here...
>
> I tried the check of dwarf record against function symbols
> with adresses mentioned earlier (attached)
>
> getting more functions of course ;-)
>
> $ bpftool btf dump file ./vmlinux | grep 'FUNC '  | wc -l
> 46606
>
> compared to 22869 on the same .config with working gcc
> and current pahole

Just curious, what's the change in BTF size due to this?

>
> and resolve_btfids is happy, because there are no duplications
>
> jirka
>
>
> ---

[...]

>  static int btf_var_secinfo_cmp(const void *a, const void *b)
>  {
>         const struct btf_var_secinfo *av = a;
> @@ -72,6 +157,7 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
>         if (!btfe)
>                 return NULL;
>
> +       btfe->symbols = RB_ROOT;

Can you please check what we do for per-cpu variables with ELF
symbols? Perhaps we can unify approaches. I'd also favor using a sort
+ bsearch approach instead of rb_tree, given we don't really need to
dynamically add/delete elements, it's a one-time operation to iterate
and initialize everything. Also binary search of linear arrays would
be more memory-efficient and cache-efficient, most probably.

>         btfe->in_fd = -1;
>         btfe->filename = strdup(filename);
>         if (btfe->filename == NULL)
> @@ -177,6 +263,7 @@ void btf_elf__delete(struct btf_elf *btfe)
>                         elf_end(btfe->elf);
>         }
>
> +       btfe__delete_symbols(btfe);
>         elf_symtab__delete(btfe->symtab);
>         __gobuffer__delete(&btfe->percpu_secinfo);
>         btf__free(btfe->btf);

[...]

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Fri, Oct 23, 2020 at 01:32:44PM -0700, Andrii Nakryiko wrote:

SNIP

> > right, we can generate them in bpftrace, but it's a shame
> >
> >
> > >
> > > But otherwise, I don't really have a good feeling what's the perfect
> > > solution here...
> >
> > I tried the check of dwarf record against function symbols
> > with adresses mentioned earlier (attached)
> >
> > getting more functions of course ;-)
> >
> > $ bpftool btf dump file ./vmlinux | grep 'FUNC '  | wc -l
> > 46606
> >
> > compared to 22869 on the same .config with working gcc
> > and current pahole
> 
> Just curious, what's the change in BTF size due to this?

current: 3342279
new:     4361045

so about 1MB

> 
> >
> > and resolve_btfids is happy, because there are no duplications
> >
> > jirka
> >
> >
> > ---
> 
> [...]
> 
> >  static int btf_var_secinfo_cmp(const void *a, const void *b)
> >  {
> >         const struct btf_var_secinfo *av = a;
> > @@ -72,6 +157,7 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
> >         if (!btfe)
> >                 return NULL;
> >
> > +       btfe->symbols = RB_ROOT;
> 
> Can you please check what we do for per-cpu variables with ELF
> symbols? Perhaps we can unify approaches. I'd also favor using a sort
> + bsearch approach instead of rb_tree, given we don't really need to
> dynamically add/delete elements, it's a one-time operation to iterate
> and initialize everything. Also binary search of linear arrays would
> be more memory-efficient and cache-efficient, most probably.

ok, will check

jirka

> 
> >         btfe->in_fd = -1;
> >         btfe->filename = strdup(filename);
> >         if (btfe->filename == NULL)
> > @@ -177,6 +263,7 @@ void btf_elf__delete(struct btf_elf *btfe)
> >                         elf_end(btfe->elf);
> >         }
> >
> > +       btfe__delete_symbols(btfe);
> >         elf_symtab__delete(btfe->symtab);
> >         __gobuffer__delete(&btfe->percpu_secinfo);
> >         btf__free(btfe->btf);
> 
> [...]
> 

Re: Build failures: unresolved symbol vfs_getattr

[Andrii Nakryiko]

On Fri, Oct 23, 2020 at 1:45 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Oct 23, 2020 at 01:32:44PM -0700, Andrii Nakryiko wrote:
>
> SNIP
>
> > > right, we can generate them in bpftrace, but it's a shame
> > >
> > >
> > > >
> > > > But otherwise, I don't really have a good feeling what's the perfect
> > > > solution here...
> > >
> > > I tried the check of dwarf record against function symbols
> > > with adresses mentioned earlier (attached)
> > >
> > > getting more functions of course ;-)
> > >
> > > $ bpftool btf dump file ./vmlinux | grep 'FUNC '  | wc -l
> > > 46606
> > >
> > > compared to 22869 on the same .config with working gcc
> > > and current pahole
> >
> > Just curious, what's the change in BTF size due to this?
>
> current: 3342279
> new:     4361045
>
> so about 1MB

ok, not too bad for almost 24k functions and bringing fentry/fexit on
par with kprobe/kretprobe in terms of what to attach to

>
> >
> > >
> > > and resolve_btfids is happy, because there are no duplications
> > >
> > > jirka
> > >
> > >
> > > ---
> >
> > [...]
> >
> > >  static int btf_var_secinfo_cmp(const void *a, const void *b)
> > >  {
> > >         const struct btf_var_secinfo *av = a;
> > > @@ -72,6 +157,7 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
> > >         if (!btfe)
> > >                 return NULL;
> > >
> > > +       btfe->symbols = RB_ROOT;
> >
> > Can you please check what we do for per-cpu variables with ELF
> > symbols? Perhaps we can unify approaches. I'd also favor using a sort
> > + bsearch approach instead of rb_tree, given we don't really need to
> > dynamically add/delete elements, it's a one-time operation to iterate
> > and initialize everything. Also binary search of linear arrays would
> > be more memory-efficient and cache-efficient, most probably.
>
> ok, will check
>

thanks!

> jirka
>
> >
> > >         btfe->in_fd = -1;
> > >         btfe->filename = strdup(filename);
> > >         if (btfe->filename == NULL)
> > > @@ -177,6 +263,7 @@ void btf_elf__delete(struct btf_elf *btfe)
> > >                         elf_end(btfe->elf);
> > >         }
> > >
> > > +       btfe__delete_symbols(btfe);
> > >         elf_symtab__delete(btfe->symtab);
> > >         __gobuffer__delete(&btfe->percpu_secinfo);
> > >         btf__free(btfe->btf);
> >
> > [...]
> >
>

Re: Build failures: unresolved symbol vfs_getattr

[Jiri Olsa]

On Fri, Oct 23, 2020 at 11:22:05AM -0700, Andrii Nakryiko wrote:
> On Thu, Oct 22, 2020 at 11:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, Oct 23, 2020 at 07:36:57AM +0200, Jiri Olsa wrote:
> > > On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote:
> > >
> > > SNIP
> > >
> > > > >
> > > > > hi,
> > > > > FYI there's still no solution yet, so far the progress is:
> > > > >
> > > > > the proposed workaround was to use the negation -> we don't have
> > > > > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> > > > > tags have attached code and skip them if they don't have any:
> > > > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
> > > > >
> > > > > the attached patch is doing that, but the resulting BTF is missing
> > > > > several functions due to another bug in dwarf:
> > > > >   https://bugzilla.redhat.com/show_bug.cgi?id=1890107
> > > >
> > > > It seems fine if there are only few functions (especially if those are
> > > > unlikely to be traced). Do you have an estimate of how many functions
> > > > have this second DWARF bug?
> > >
> > > it wasn't that many, I'll recheck
> >
> > 127 functions missing if the workaround is applied, list attached
> >
> 
> some of those seem pretty useful... I guess the quick workaround in
> pahole would be to just remember function names that were emitted
> already. The problem with that is that we can pick a version without
> parameter names, which is not the end of the world, but certainly
> annoying.

with the change below I seem to get all argument names

the change assumes that we can skip dwarf functions with
no argument names, because if the function is defined in
the object, it needs to have argument names

so there will be eventualy dwarf definition of that function
with full argument names

I wonder we could use this code as a check that the function
is present in the object ;-) but I think we need to keep
the symbols check as well

I'll send that out together with the rest of the changes

jirka


---
diff --git a/btf_encoder.c b/btf_encoder.c
index 7e370eb48174..0c14ac210425 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -397,6 +397,19 @@ static int config(struct btf_elf *btfe, bool do_percpu_vars)
 	return 0;
 }
 
+static bool has_arg_names(struct cu *cu, struct ftype *ftype)
+{
+	struct parameter *param;
+	const char *name;
+
+	ftype__for_each_parameter(ftype, param) {
+		name = dwarves__active_loader->strings__ptr(cu, param->name);
+		if (name == NULL)
+			return false;
+	}
+	return true;
+}
+
 int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		   bool skip_encoding_vars)
 {
@@ -472,6 +485,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		int btf_fnproto_id, btf_fn_id;
 		const char *name;
 
+		if (!has_arg_names(cu, &fn->proto))
+			continue;
+
 		if (!generate_func(btfe, function__name(fn, cu)))
 			continue;
 

Re: Build failures: unresolved symbol vfs_getattr

[Andrii Nakryiko]

On Mon, Oct 26, 2020 at 3:14 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Oct 23, 2020 at 11:22:05AM -0700, Andrii Nakryiko wrote:
> > On Thu, Oct 22, 2020 at 11:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Fri, Oct 23, 2020 at 07:36:57AM +0200, Jiri Olsa wrote:
> > > > On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote:
> > > >
> > > > SNIP
> > > >
> > > > > >
> > > > > > hi,
> > > > > > FYI there's still no solution yet, so far the progress is:
> > > > > >
> > > > > > the proposed workaround was to use the negation -> we don't have
> > > > > > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram
> > > > > > tags have attached code and skip them if they don't have any:
> > > > > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10
> > > > > >
> > > > > > the attached patch is doing that, but the resulting BTF is missing
> > > > > > several functions due to another bug in dwarf:
> > > > > >   https://bugzilla.redhat.com/show_bug.cgi?id=1890107
> > > > >
> > > > > It seems fine if there are only few functions (especially if those are
> > > > > unlikely to be traced). Do you have an estimate of how many functions
> > > > > have this second DWARF bug?
> > > >
> > > > it wasn't that many, I'll recheck
> > >
> > > 127 functions missing if the workaround is applied, list attached
> > >
> >
> > some of those seem pretty useful... I guess the quick workaround in
> > pahole would be to just remember function names that were emitted
> > already. The problem with that is that we can pick a version without
> > parameter names, which is not the end of the world, but certainly
> > annoying.
>
> with the change below I seem to get all argument names
>
> the change assumes that we can skip dwarf functions with
> no argument names, because if the function is defined in
> the object, it needs to have argument names

I'd love it if it was that simple, but I learn time and time again
that DWARF and compilers tend to always have some bugs and gotchas
with type information. But let's try and see, of course.

>
> so there will be eventualy dwarf definition of that function
> with full argument names
>
> I wonder we could use this code as a check that the function
> is present in the object ;-) but I think we need to keep
> the symbols check as well
>
> I'll send that out together with the rest of the changes
>
> jirka
>
>
> ---
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 7e370eb48174..0c14ac210425 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -397,6 +397,19 @@ static int config(struct btf_elf *btfe, bool do_percpu_vars)
>         return 0;
>  }
>
> +static bool has_arg_names(struct cu *cu, struct ftype *ftype)
> +{
> +       struct parameter *param;
> +       const char *name;
> +
> +       ftype__for_each_parameter(ftype, param) {
> +               name = dwarves__active_loader->strings__ptr(cu, param->name);
> +               if (name == NULL)
> +                       return false;
> +       }
> +       return true;
> +}
> +
>  int cu__encode_btf(struct cu *cu, int verbose, bool force,
>                    bool skip_encoding_vars)
>  {
> @@ -472,6 +485,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>                 int btf_fnproto_id, btf_fn_id;
>                 const char *name;
>
> +               if (!has_arg_names(cu, &fn->proto))
> +                       continue;
> +
>                 if (!generate_func(btfe, function__name(fn, cu)))
>                         continue;
>
>