public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
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:20:33 -0300	[thread overview]
Message-ID: <ZA8VEfKWuQYH/Jnx@kernel.org> (raw)
In-Reply-To: <1678459850-16140-3-git-send-email-alan.maguire@oracle.com>

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.

- 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

  reply	other threads:[~2023-03-13 12:20 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 [this message]
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
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=ZA8VEfKWuQYH/Jnx@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