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 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.