From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Eduard Zingerman <eddyz87@gmail.com>
Cc: Alan Maguire <alan.maguire@oracle.com>,
ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
haoluo@google.com, jolsa@kernel.org, john.fastabend@gmail.com,
kpsingh@chromium.org, sinquersw@gmail.com, martin.lau@kernel.org,
songliubraving@fb.com, sdf@google.com, timo@incline.eu,
yhs@fb.com, bpf@vger.kernel.org
Subject: Re: [PATCH dwarves 2/3] dwarves_fprintf: support skipping modifier
Date: Mon, 13 Mar 2023 15:28:48 -0300 [thread overview]
Message-ID: <ZA9rYJzz6mCuQ6gh@kernel.org> (raw)
In-Reply-To: <6dfa7235106db98698fe013cde74666f7d485669.camel@gmail.com>
Em Mon, Mar 13, 2023 at 07:12:43PM +0200, Eduard Zingerman escreveu:
> On Mon, 2023-03-13 at 16:37 +0000, Alan Maguire wrote:
> [...]
> > sure; try adding "--skip_encoding_btf_inconsistent_proto --btf_gen_optimized".
> > I was testing with gcc 11.2.1.
>
> pahole -F dwarf \
> --flat_arrays \
> --sort --jobs \
> --suppress_aligned_attribute \
> --suppress_force_paddings \
> --suppress_packed \
> --lang_exclude rust \
> --show_private_classes \
> --skip_encoding_btf_inconsistent_proto \
> --btf_gen_optimized \
> ./vmlinux
>
> Like this, right?
> gcc 11.3, pahole master, still don't see this in function prototypes,
> maybe I have a simpler kernel config...
>
> [...]
>
> > > On the other hand, I see it in a few structure definitions, e.g. here
> > > is original C code (include/linux/sysrq.h:32):
> > >
> > > struct sysrq_key_op {
> > > void (* const handler)(int);
> > > const char * const help_msg;
> > > const char * const action_msg;
> > > const int enable_mask;
> > > };
> > >
> > > And here is how it is reconstructed from DWARF (same happens when
> > > reconstructed from BTF):
> > >
> > > struct sysrq_key_op {
> > > const void (*handler)(int); /* 0 8 */
> > > const const char * help_msg; /* 8 8 */
> > > const const char * action_msg; /* 16 8 */
> > > const int enable_mask; /* 24 4 */
> > >
> > > /* size: 32, cachelines: 1, members: 4 */
> > > /* padding: 4 */
> > > /* last cacheline: 32 bytes */
> > > };
> > >
> > > So it seems to be a general issue with modifiers printing.
> > >
> >
> > So it seems like the modifier ordering isn't preserved, even though
> > the final BTF representation looks right? Thanks!
>
> Yes, BTF looks right, bpftool prints the structure correctly.
Yes, the problem is in pahole's fprintf.c code
⬢[acme@toolbox pahole]$ cat const-pointer-const.c
#include <stdio.h>
struct foo {
const char * const s;
};
int main(int argc, const char *argv[])
{
struct foo bar = { .s = argv[1], };
return printf("%s: %s\n", argv[0], bar.s);
}
⬢[acme@toolbox pahole]$ gcc -g const-pointer-const.c -o const-pointer-const
⬢[acme@toolbox pahole]$ pahole const-pointer-const
struct foo {
const constchar * s; /* 0 8 */
/* size: 8, cachelines: 1, members: 1 */
/* last cacheline: 8 bytes */
};
⬢[acme@toolbox pahole]$
Seems a long standing bug, so if you fix the whitespace issue we can
progress and not let this problem prevent the release of 1.25, agreed?
- Arnaldo
next prev parent reply other threads:[~2023-03-13 18:30 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-10 14:50 [PATCH dwarves 0/3] dwarves: improve BTF encoder comparison method Alan Maguire
2023-03-10 14:50 ` [PATCH dwarves 1/3] dwarves_fprintf: generalize function prototype print to support passing conf Alan Maguire
2023-03-10 14:50 ` [PATCH dwarves 2/3] dwarves_fprintf: support skipping modifier Alan Maguire
2023-03-13 12:20 ` Arnaldo Carvalho de Melo
2023-03-13 12:29 ` Arnaldo Carvalho de Melo
2023-03-13 13:16 ` Alan Maguire
2023-03-13 13:50 ` Eduard Zingerman
2023-03-13 16:37 ` Alan Maguire
2023-03-13 17:12 ` Eduard Zingerman
2023-03-13 18:28 ` Arnaldo Carvalho de Melo [this message]
2023-03-13 14:45 ` Eduard Zingerman
2023-03-13 17:18 ` Alan Maguire
2023-03-13 18:26 ` Arnaldo Carvalho de Melo
2023-03-10 14:50 ` [PATCH dwarves 3/3] btf_encoder: compare functions via prototypes not parameter names Alan Maguire
2023-03-10 15:18 ` [PATCH dwarves 0/3] dwarves: improve BTF encoder comparison method Arnaldo Carvalho de Melo
2023-03-13 9:40 ` Jiri Olsa
2023-03-13 12:33 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZA9rYJzz6mCuQ6gh@kernel.org \
--to=acme@kernel.org \
--cc=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@chromium.org \
--cc=martin.lau@kernel.org \
--cc=sdf@google.com \
--cc=sinquersw@gmail.com \
--cc=songliubraving@fb.com \
--cc=timo@incline.eu \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.