Re: [PATCH 2/3] bpf_encoder: Translate SHN_XINDEX in symbol's st_shndx values

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andriin@fb.com>,
	dwarves@vger.kernel.org, Networking <netdev@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>, Yonghong Song <yhs@fb.com>,
	Hao Luo <haoluo@google.com>, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Mark Wielaard <mjw@redhat.com>
Subject: Re: [PATCH 2/3] bpf_encoder: Translate SHN_XINDEX in symbol's st_shndx values
Date: Sat, 23 Jan 2021 22:08:10 -0800	[thread overview]
Message-ID: <CAEf4BzaaP8YwCsQpiSkCCgYFTqFJ-yV234u0dtuxOgEQgwPeiA@mail.gmail.com> (raw)
In-Reply-To: <20210123212341.GC117714@krava>

On Sat, Jan 23, 2021 at 1:23 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote:
>
> SNIP
>
> > But the current variant looks broken. Oh, and
> > elf_symtab__for_each_symbol() is similarly broken, can you please fix
> > that as well?
>
> we'll have to change its callers a bit, because of hanging 'else'
> I'll send this separately if that's ok, when I figure out how to
> test ctf code
>

oh, else sucks. Sure, no problem doing it separately.

> jirka
>
>
> ---
> diff --git a/elf_symtab.h b/elf_symtab.h
> index 489e2b1a3505..6823a8c37ecf 100644
> --- a/elf_symtab.h
> +++ b/elf_symtab.h
> @@ -99,10 +99,9 @@ elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table,
>   * @index: uint32_t index
>   * @sym: GElf_Sym iterator
>   */
> -#define elf_symtab__for_each_symbol(symtab, index, sym) \
> -       for (index = 0, gelf_getsym(symtab->syms, index, &sym);\
> -            index < symtab->nr_syms; \
> -            index++, gelf_getsym(symtab->syms, index, &sym))
> +#define elf_symtab__for_each_symbol(symtab, index, sym)                \
> +       for (index = 0; index < symtab->nr_syms; index++)       \
> +               if (gelf_getsym(symtab->syms, index, &sym))
>
>  /**
>   * elf_symtab__for_each_symbol_index - iterate through all the symbols,
> diff --git a/libctf.h b/libctf.h
> index 749be8955c52..ee5412bec77e 100644
> --- a/libctf.h
> +++ b/libctf.h
> @@ -90,11 +90,9 @@ char *ctf__string(struct ctf *ctf, uint32_t ref);
>   */
>  #define ctf__for_each_symtab_function(ctf, index, sym)                       \
>         elf_symtab__for_each_symbol(ctf->symtab, index, sym)                  \
> -               if (ctf__ignore_symtab_function(&sym,                         \
> +               if (!ctf__ignore_symtab_function(&sym,                        \
>                                                 elf_sym__name(&sym,           \
>                                                               ctf->symtab)))  \
> -                       continue;                                             \
> -               else
>
>  /**
>   * ctf__for_each_symtab_object - iterate thru all the symtab objects
> @@ -105,11 +103,9 @@ char *ctf__string(struct ctf *ctf, uint32_t ref);
>   */
>  #define ctf__for_each_symtab_object(ctf, index, sym)                         \
>         elf_symtab__for_each_symbol(ctf->symtab, index, sym)                  \
> -               if (ctf__ignore_symtab_object(&sym,                           \
> +               if (!ctf__ignore_symtab_object(&sym,                          \
>                                               elf_sym__name(&sym,             \
>                                                             ctf->symtab)))    \
> -                       continue;                                             \
> -               else
>
>
>  #endif /* _LIBCTF_H */
>