From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Andrii Nakryiko <andriin@fb.com>
Cc: arnaldo.melo@gmail.com, andrii.nakryiko@gmail.com, ast@fb.com,
yhs@fb.com, dwarves@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH pahole] pahole: use 32-bit integers for iterations within CU
Date: Thu, 7 Mar 2019 11:09:17 -0300 [thread overview]
Message-ID: <20190307140917.GW13100@kernel.org> (raw)
In-Reply-To: <20190307140247.GV13100@kernel.org>
Em Thu, Mar 07, 2019 at 11:02:47AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 06, 2019 at 04:23:21PM -0800, Andrii Nakryiko escreveu:
> > Due to this assumption, libdwarves and other parts of pahole are using 16-bit
> > counters to iterate over entities within CU. This can cause infinite loop when
> > iterating BTF data, if there are more than 65535 types. This patch changes
> > non-public variables to use 32-bit integers, where appropriate.
> Ok, there are some bits that are unrelated, I'm comment on it and remove
> before applying.
Ooops, see below, I'm removing the non-type related parts, and will look
at changing the types till btfdiff with that allyesconfig aarch64
vmlinux image shows no diffs.
- Arnaldo
> - Arnaldo
>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > btf_loader.c | 4 ++--
> > dwarves.c | 16 ++++++++--------
> > dwarves_fprintf.c | 8 ++++----
> > pahole.c | 36 ++++++++++++++++++------------------
> > 4 files changed, 32 insertions(+), 32 deletions(-)
> >
> > diff --git a/btf_loader.c b/btf_loader.c
> > index 867c608..3de774a 100644
> > --- a/btf_loader.c
> > +++ b/btf_loader.c
> > @@ -45,7 +45,7 @@ static void *tag__alloc(const size_t size)
> > return tag;
> > }
> >
> > -static int btf_elf__load_ftype(struct btf_elf *btfe, struct ftype *proto, uint16_t tag,
> > +static int btf_elf__load_ftype(struct btf_elf *btfe, struct ftype *proto, uint32_t tag,
> > uint32_t type, uint16_t vlen, struct btf_param *args, long id)
> > {
> > int i;
> > @@ -100,7 +100,7 @@ static struct base_type *base_type__new(strings_t name, uint32_t attrs,
> > return bt;
> > }
> >
> > -static void type__init(struct type *type, uint16_t tag,
> > +static void type__init(struct type *type, uint32_t tag,
> > strings_t name, size_t size)
> > {
> > INIT_LIST_HEAD(&type->node);
> > diff --git a/dwarves.c b/dwarves.c
> > index 111797b..3594ae3 100644
> > --- a/dwarves.c
> > +++ b/dwarves.c
> > @@ -349,7 +349,7 @@ reevaluate:
> >
> > static void cu__find_class_holes(struct cu *cu)
> > {
> > - uint16_t id;
> > + uint32_t id;
> > struct class *pos;
> >
> > cu__for_each_struct(cu, id, pos)
> > @@ -566,7 +566,7 @@ struct tag *cu__type(const struct cu *cu, const uint16_t id)
> > struct tag *cu__find_first_typedef_of_type(const struct cu *cu,
> > const uint16_t type)
> > {
> > - uint16_t id;
> > + uint32_t id;
> > struct tag *pos;
> >
> > if (cu == NULL || type == 0)
> > @@ -582,7 +582,7 @@ struct tag *cu__find_first_typedef_of_type(const struct cu *cu,
> > struct tag *cu__find_base_type_by_name(const struct cu *cu,
> > const char *name, uint16_t *idp)
> > {
> > - uint16_t id;
> > + uint32_t id;
> > struct tag *pos;
> >
> > if (cu == NULL || name == NULL)
> > @@ -611,7 +611,7 @@ struct tag *cu__find_base_type_by_sname_and_size(const struct cu *cu,
> > uint16_t bit_size,
> > uint16_t *idp)
> > {
> > - uint16_t id;
> > + uint32_t id;
> > struct tag *pos;
> >
> > if (sname == 0)
> > @@ -638,7 +638,7 @@ struct tag *cu__find_enumeration_by_sname_and_size(const struct cu *cu,
> > uint16_t bit_size,
> > uint16_t *idp)
> > {
> > - uint16_t id;
> > + uint32_t id;
> > struct tag *pos;
> >
> > if (sname == 0)
> > @@ -663,7 +663,7 @@ struct tag *cu__find_enumeration_by_sname_and_size(const struct cu *cu,
> > struct tag *cu__find_struct_by_sname(const struct cu *cu, strings_t sname,
> > const int include_decls, uint16_t *idp)
> > {
> > - uint16_t id;
> > + uint32_t id;
> > struct tag *pos;
> >
> > if (sname == 0)
> > @@ -699,7 +699,7 @@ static struct tag *__cu__find_struct_by_name(const struct cu *cu, const char *na
> > if (cu == NULL || name == NULL)
> > return NULL;
> >
> > - uint16_t id;
> > + uint32_t id;
> > struct tag *pos;
> > cu__for_each_type(cu, id, pos) {
> > struct type *type;
> > @@ -1128,7 +1128,7 @@ void lexblock__add_label(struct lexblock *block, struct label *label)
> > }
> >
> > const struct class_member *class__find_bit_hole(const struct class *class,
> > - const struct class_member *trailer,
> > + const struct class_member *trailer,
This one is unrelated, so I'll remove from this patch.
> > const uint16_t bit_hole_size)
> > {
> > struct class_member *pos;
> > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> > index 490dc30..530c27b 100644
> > --- a/dwarves_fprintf.c
> > +++ b/dwarves_fprintf.c
> > @@ -163,7 +163,7 @@ static const char *tag__accessibility(const struct tag *tag)
> > return NULL;
> > }
> >
> > -static size_t __tag__id_not_found_snprintf(char *bf, size_t len, uint16_t id,
> > +static size_t __tag__id_not_found_snprintf(char *bf, size_t len, uint32_t id,
> > const char *fn, int line)
> > {
> > return snprintf(bf, len, "<ERROR(%s:%d): %#llx not found!>", fn, line,
> > @@ -425,7 +425,7 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
> > return tag__ptr_name(tag, cu, bf, len, "&");
> > case DW_TAG_ptr_to_member_type: {
> > char suffix[512];
> > - uint16_t id = tag__ptr_to_member_type(tag)->containing_type;
> > + uint32_t id = tag__ptr_to_member_type(tag)->containing_type;
Not strictly need as we haven't changed ptr_to_member_type's 'containing_type'
to uint32_t, it remains uint16_t as it is part of the ABI, right?
> > type = cu__type(cu, id);
> > if (type != NULL)
> > @@ -1213,7 +1213,7 @@ static size_t __class__fprintf(struct class *class, const struct cu *cu,
> > struct type *type = &class->type;
> > size_t last_size = 0, size;
> > uint8_t newline = 0;
> > - uint16_t nr_paddings = 0;
> > + uint32_t nr_paddings = 0;
No need to convert this one as this is not a type id?
> > uint32_t sum = 0;
> > uint32_t sum_holes = 0;
> > uint32_t sum_paddings = 0;
> > @@ -1225,7 +1225,7 @@ static size_t __class__fprintf(struct class *class, const struct cu *cu,
> > struct tag *tag_pos;
> > const char *current_accessibility = NULL;
> > struct conf_fprintf cconf = conf ? *conf : conf_fprintf__defaults;
> > - const uint16_t t = type->namespace.tag.tag;
> > + const uint32_t t = type->namespace.tag.tag;
Ditto
> > size_t printed = fprintf(fp, "%s%s%s%s%s",
> > cconf.prefix ?: "", cconf.prefix ? " " : "",
> > ((cconf.classes_as_structs ||
> > diff --git a/pahole.c b/pahole.c
> > index 2230e37..bec279c 100644
> > --- a/pahole.c
> > +++ b/pahole.c
> > @@ -41,9 +41,9 @@ static size_t cu__exclude_prefix_len;
> > static char *decl_exclude_prefix;
> > static size_t decl_exclude_prefix_len;
> >
> > -static uint16_t nr_holes;
> > -static uint16_t nr_bit_holes;
> > -static uint16_t hole_size_ge;
> > +static uint32_t nr_holes;
> > +static uint32_t nr_bit_holes;
> > +static uint32_t hole_size_ge;
Ditto for these 3
> > static uint8_t show_packable;
> > static uint8_t global_verbose;
> > static uint8_t recursive;
> > @@ -156,7 +156,7 @@ static void nr_definitions_formatter(struct structure *st)
> > }
> >
> > static void nr_members_formatter(struct class *class,
> > - struct cu *cu, uint16_t id __unused)
> > + struct cu *cu, uint32_t id __unused)
> > {
> > printf("%s%c%u\n", class__name(class, cu), separator,
> > class__nr_members(class));
> > @@ -168,26 +168,26 @@ static void nr_methods_formatter(struct structure *st)
> > }
> >
> > static void size_formatter(struct class *class,
> > - struct cu *cu, uint16_t id __unused)
> > + struct cu *cu, uint32_t id __unused)
> > {
> > printf("%s%c%d%c%u\n", class__name(class, cu), separator,
> > class__size(class), separator, class->nr_holes);
> > }
> >
> > static void class_name_len_formatter(struct class *class, struct cu *cu,
> > - uint16_t id __unused)
> > + uint32_t id __unused)
> > {
> > const char *name = class__name(class, cu);
> > printf("%s%c%zd\n", name, separator, strlen(name));
> > }
> >
> > static void class_name_formatter(struct class *class,
> > - struct cu *cu, uint16_t id __unused)
> > + struct cu *cu, uint32_t id __unused)
> > {
> > puts(class__name(class, cu));
> > }
> >
> > -static void class_formatter(struct class *class, struct cu *cu, uint16_t id)
> > +static void class_formatter(struct class *class, struct cu *cu, uint32_t id)
> > {
> > struct tag *typedef_alias = NULL;
> > struct tag *tag = class__tag(class);
> > @@ -224,7 +224,7 @@ static void class_formatter(struct class *class, struct cu *cu, uint16_t id)
> > putchar('\n');
> > }
> >
> > -static void print_packable_info(struct class *c, struct cu *cu, uint16_t id)
> > +static void print_packable_info(struct class *c, struct cu *cu, uint32_t id)
> > {
> > const struct tag *t = class__tag(c);
> > const size_t orig_size = class__size(c);
> > @@ -267,14 +267,14 @@ static void print_stats(void)
> > }
> >
> > static struct class *class__filter(struct class *class, struct cu *cu,
> > - uint16_t tag_id);
> > + uint32_t tag_id);
> >
> > static void (*formatter)(struct class *class,
> > - struct cu *cu, uint16_t id) = class_formatter;
> > + struct cu *cu, uint32_t id) = class_formatter;
> >
> > static void print_classes(struct cu *cu)
> > {
> > - uint16_t id;
> > + uint32_t id;
> > struct class *pos;
> >
> > cu__for_each_struct_or_union(cu, id, pos) {
> > @@ -352,7 +352,7 @@ static int class__packable(struct class *class, struct cu *cu)
> > }
> >
> > static struct class *class__filter(struct class *class, struct cu *cu,
> > - uint16_t tag_id)
> > + uint32_t tag_id)
> > {
> > struct tag *tag = class__tag(class);
> > const char *name;
> > @@ -608,7 +608,7 @@ static void cu_fixup_word_size_iterator(struct cu *cu)
> > original_word_size = cu->addr_size;
> > cu->addr_size = word_size;
> >
> > - uint16_t id;
> > + uint32_t id;
> > struct tag *pos;
> > cu__for_each_type(cu, id, pos)
> > tag__fixup_word_size(pos, cu);
> > @@ -659,11 +659,11 @@ static void cu__account_nr_methods(struct cu *cu)
> >
> > static char tab[128];
> >
> > -static void print_structs_with_pointer_to(const struct cu *cu, uint16_t type)
> > +static void print_structs_with_pointer_to(const struct cu *cu, uint32_t type)
> > {
> > struct class *pos;
> > struct class_member *pos_member;
> > - uint16_t id;
> > + uint32_t id;
> >
> > cu__for_each_struct(cu, id, pos) {
> > bool looked = false;
> > @@ -702,10 +702,10 @@ static void print_structs_with_pointer_to(const struct cu *cu, uint16_t type)
> > }
> > }
> >
> > -static void print_containers(const struct cu *cu, uint16_t type, int ident)
> > +static void print_containers(const struct cu *cu, uint32_t type, int ident)
> > {
> > struct class *pos;
> > - uint16_t id;
> > + uint32_t id;
> >
> > cu__for_each_struct(cu, id, pos) {
> > if (pos->type.namespace.name == 0)
> > --
> > 2.17.1
>
> --
>
> - Arnaldo
--
- Arnaldo
next prev parent reply other threads:[~2019-03-07 14:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-07 0:23 [PATCH pahole] pahole: use 32-bit integers for iterations within CU Andrii Nakryiko
2019-03-07 14:02 ` Arnaldo Carvalho de Melo
2019-03-07 14:09 ` Arnaldo Carvalho de Melo [this message]
2019-03-07 14:11 ` Arnaldo Carvalho de Melo
2019-03-08 19:39 ` Andrii Nakryiko
2019-03-08 0:26 ` Arnaldo Carvalho de Melo
2019-03-08 18:45 ` Arnaldo Carvalho de Melo
2019-03-08 19:42 ` Andrii Nakryiko
2019-03-11 4:31 ` Andrii Nakryiko
2019-03-11 14:53 ` Arnaldo Carvalho de Melo
2019-03-11 16:39 ` Andrii Nakryiko
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=20190307140917.GW13100@kernel.org \
--to=arnaldo.melo@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=dwarves@vger.kernel.org \
--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