From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
eddyz87@gmail.com, 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 09:29:59 -0300 [thread overview]
Message-ID: <ZA8XRweCvk6CZIly@kernel.org> (raw)
In-Reply-To: <ZA8VEfKWuQYH/Jnx@kernel.org>
Em Mon, Mar 13, 2023 at 09:20:33AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Mar 10, 2023 at 02:50:49PM +0000, Alan Maguire escreveu:
> > When doing BTF comparisons between functions defined in multiple
> > CUs, it was noticed a few critical functions failed prototype
> > comparisons due to multiple "const" modifiers; for example:
> >
> > function mismatch for 'memchr_inv'('memchr_inv'): 'void * ()(const const void * , int, size_t)' != 'void * ()(const void *, int, size_t)'
> >
> > function mismatch for 'strnlen'('strnlen'): '__kernel_size_t ()(const const char * , __kernel_size_t)' != '__kernel_size_t ()(const char *, size_t)'
> >
> > (note the "const const" in the first parameter.)
> >
> > As such it would be useful to omit modifiers for comparison
> > purposes. Also noted was the fact that for the "no_parm_names"
> > case, an extra space was being emitted in some cases, also
> > throwing off string comparisons of prototypes.
>
> Running 'btfdiff vmlinux' after this change ends up in a segfault:
>
> ⬢[acme@toolbox pahole]$ btfdiff vmlinux
> /var/home/acme/bin/btfdiff: line 34: 8183 Segmentation fault (core dumped) ${pahole_bin} -F dwarf --flat_arrays --sort --jobs --suppress_aligned_attribute --suppress_force_paddings --suppress_packed --lang_exclude rust --show_private_classes $dwarf_input > $dwarf_output
> /var/home/acme/bin/btfdiff: line 39: 8237 Segmentation fault (core dumped) ${pahole_bin} -F btf --sort --suppress_aligned_attribute --suppress_packed $btf_input > $btf_output
> ⬢[acme@toolbox pahole]$
>
> Investigating.
(gdb) run -F dwarf --flat_arrays --sort --jobs --suppress_aligned_attribute --suppress_force_paddings --suppress_packed --lang_exclude rust --show_private_classes vmlinux
Starting program: /var/home/acme/bin/pahole -F dwarf --flat_arrays --sort --jobs --suppress_aligned_attribute --suppress_force_paddings --suppress_packed --lang_exclude rust --show_private_classes vmlinux
Thread 1 "pahole" received signal SIGSEGV, Segmentation fault.
0x00007ffff7f26cff in __tag__name (tag=0x7fff88016a20, cu=0x7fff88001e30, bf=0x7fffffffce90 "void ()(void)", len=1024, conf=0x0) at /var/home/acme/git/pahole/dwarves_fprintf.c:584
584 if (!conf->skip_emitting_modifier) {
(gdb) bt
#0 0x00007ffff7f26cff in __tag__name (tag=0x7fff88016a20, cu=0x7fff88001e30, bf=0x7fffffffce90 "void ()(void)", len=1024, conf=0x0) at /var/home/acme/git/pahole/dwarves_fprintf.c:584
#1 0x00007ffff7f26873 in tag__ptr_name (tag=0x7fff88016990, cu=0x7fff88001e30, bf=0x7fffffffd9d0 "long unsigned int", len=1024, ptr_suffix=0x7ffff7f88fb0 "*", conf=0x0) at /var/home/acme/git/pahole/dwarves_fprintf.c:515
#2 0x00007ffff7f26acd in __tag__name (tag=0x7fff88016990, cu=0x7fff88001e30, bf=0x7fffffffd9d0 "long unsigned int", len=1024, conf=0x0) at /var/home/acme/git/pahole/dwarves_fprintf.c:551
#3 0x00007ffff7f270d5 in tag__name (tag=0x7fff88016990, cu=0x7fff88001e30, bf=0x7fffffffd9d0 "long unsigned int", len=1024, conf=0x0) at /var/home/acme/git/pahole/dwarves_fprintf.c:639
#4 0x0000000000404042 in type__compare_members_types (a=0x7fff9401bc30, cu_a=0x7fff94001e30, b=0x7fff8801bba0, cu_b=0x7fff88001e30) at /var/home/acme/git/pahole/pahole.c:258
#5 0x0000000000404cd0 in resort_add (resorted=0x7fffffffded8, str=0x7fff8801d120) at /var/home/acme/git/pahole/pahole.c:649
#6 0x0000000000404d7e in resort_classes (resorted=0x7fffffffded8, head=0x411420 <structures.list>) at /var/home/acme/git/pahole/pahole.c:668
#7 0x0000000000404dda in print_ordered_classes () at /var/home/acme/git/pahole/pahole.c:678
#8 0x000000000040a93c in main (argc=13, argv=0x7fffffffe068) at /var/home/acme/git/pahole/pahole.c:3528
(gdb)
I'm adding this:
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index b20a473125c3aa41..c2fdcdad078a5335 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -506,7 +506,7 @@ static const char *tag__ptr_name(const struct tag *tag, const struct cu *cu,
struct tag *next_type = cu__type(cu, type->type);
if (next_type && tag__is_pointer(next_type)) {
- if (!conf->skip_emitting_modifier)
+ if (!(conf && conf->skip_emitting_modifier))
const_pointer = "const ";
type = next_type;
}
@@ -581,7 +581,7 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
*type_str = __tag__name(type, cu, tmpbf,
sizeof(tmpbf),
pconf);
- if (!conf->skip_emitting_modifier) {
+ if (!pconf->skip_emitting_modifier) {
switch (tag->tag) {
case DW_TAG_volatile_type: prefix = "volatile "; break;
case DW_TAG_const_type: prefix = "const"; break;
@@ -590,7 +590,7 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
}
}
snprintf(bf, len, "%s%s%s%s", prefix, type_str, suffix,
- conf->no_parm_names ? "" : " ");
+ pconf->no_parm_names ? "" : " ");
}
break;
case DW_TAG_array_type:
With it:
⬢[acme@toolbox pahole]$ btfdiff vmlinux
⬢[acme@toolbox pahole]$
> - Arnaldo
>
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> > dwarves.h | 1 +
> > dwarves_fprintf.c | 26 ++++++++++++++++----------
> > 2 files changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/dwarves.h b/dwarves.h
> > index d04a36d..7a319d1 100644
> > --- a/dwarves.h
> > +++ b/dwarves.h
> > @@ -134,6 +134,7 @@ struct conf_fprintf {
> > uint8_t strip_inline:1;
> > uint8_t skip_emitting_atomic_typedefs:1;
> > uint8_t skip_emitting_errors:1;
> > + uint8_t skip_emitting_modifier:1;
> > };
> >
> > struct cus;
> > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> > index 5c6bf9c..b20a473 100644
> > --- a/dwarves_fprintf.c
> > +++ b/dwarves_fprintf.c
> > @@ -506,7 +506,8 @@ static const char *tag__ptr_name(const struct tag *tag, const struct cu *cu,
> > struct tag *next_type = cu__type(cu, type->type);
> >
> > if (next_type && tag__is_pointer(next_type)) {
> > - const_pointer = "const ";
> > + if (!conf->skip_emitting_modifier)
> > + const_pointer = "const ";
> > type = next_type;
> > }
> > }
> > @@ -580,13 +581,16 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
> > *type_str = __tag__name(type, cu, tmpbf,
> > sizeof(tmpbf),
> > pconf);
> > - switch (tag->tag) {
> > - case DW_TAG_volatile_type: prefix = "volatile "; break;
> > - case DW_TAG_const_type: prefix = "const "; break;
> > - case DW_TAG_restrict_type: suffix = " restrict"; break;
> > - case DW_TAG_atomic_type: prefix = "_Atomic "; break;
> > + if (!conf->skip_emitting_modifier) {
> > + switch (tag->tag) {
> > + case DW_TAG_volatile_type: prefix = "volatile "; break;
> > + case DW_TAG_const_type: prefix = "const"; break;
> > + case DW_TAG_restrict_type: suffix = " restrict"; break;
> > + case DW_TAG_atomic_type: prefix = "_Atomic "; break;
> > + }
> > }
> > - snprintf(bf, len, "%s%s%s ", prefix, type_str, suffix);
> > + snprintf(bf, len, "%s%s%s%s", prefix, type_str, suffix,
> > + conf->no_parm_names ? "" : " ");
> > }
> > break;
> > case DW_TAG_array_type:
> > @@ -818,9 +822,11 @@ print_default:
> > case DW_TAG_const_type:
> > modifier = "const";
> > print_modifier: {
> > - size_t modifier_printed = fprintf(fp, "%s ", modifier);
> > - tconf.type_spacing -= modifier_printed;
> > - printed += modifier_printed;
> > + if (!conf->skip_emitting_modifier) {
> > + size_t modifier_printed = fprintf(fp, "%s ", modifier);
> > + tconf.type_spacing -= modifier_printed;
> > + printed += modifier_printed;
> > + }
> >
> > struct tag *ttype = cu__type(cu, type->type);
> > if (ttype) {
> > --
> > 1.8.3.1
> >
>
> --
>
> - Arnaldo
--
- Arnaldo
next prev parent reply other threads:[~2023-03-13 12: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 [this message]
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
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=ZA8XRweCvk6CZIly@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox