Re: [PATCH dwarves 2/3] dwarves_fprintf: support skipping modifier

[Alan Maguire <alan.maguire@oracle.com>]

On 13/03/2023 12:29, Arnaldo Carvalho de Melo wrote:
> 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]$
> 

thanks for finding and fixing this!