Re: [PATCH pahole] pahole: use 32-bit integers for iterations within CU

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

Em Wed, Mar 06, 2019 at 04:23:21PM -0800, Andrii Nakryiko escreveu:
> Existing code base assumes that single CU doesn't have more than 65535 types
> per each CU, which might be a reasonable assumption for DWARF data. With BTF,
> though, all we get is single, potentially huge, CU which can easily have more
> than 65k types. For example, this is the case for allyesconfig version of
> Linux kernel, which has >200k types.

And your patch seems to be a step in removing this limitation, thanks
for working on it.
 
> 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.
 
> This still leads to invalid reported data when using BTF loader (due to using
> (X & 0xFFFF) type ID, instead of X, when X > 65535) and loading huge files,
> but at least it's not stuck in an infinite loop anymore.

This can be a followup patch, I'm unaware of any usage of libdwarves,
but if this leads to ABI breakage, then we can come up with a new
SONAME.

Mostly the libdwarves has been a internal thing so that the various
tools don't have to statically link with it.

- 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,
>  						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;
>  
>  		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;
>  	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;
>  	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;
>  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