On 13/03/2023 13:50, Eduard Zingerman wrote:
> On Fri, 2023-03-10 at 14:50 +0000, Alan Maguire wrote:
>> 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.)
>
> Hi Alan,
>
> Could you please share which command/flags do you use to generate the
> 'memchr_inv' with 'const const'?
sure; try adding "--skip_encoding_btf_inconsistent_proto --btf_gen_optimized".
I was testing with gcc 11.2.1.
> I tried the ones used in 'btfdiff':
> - pahole -F dwarf --flat_arrays --sort --jobs --suppress_aligned_attribute \
> --suppress_force_paddings --suppress_packed --lang_exclude rust \
> --show_private_classes ./vmlinux
> - pahole -F btf --sort --suppress_aligned_attribute --suppress_packed ./vmlinux
>
> But don't see any function prototypes generated with 'const const'.
>
> On the other hand, I see it in a few structure definitions, e.g. here
> is original C code (include/linux/sysrq.h:32):
>
> struct sysrq_key_op {
> void (* const handler)(int);
> const char * const help_msg;
> const char * const action_msg;
> const int enable_mask;
> };
>
> And here is how it is reconstructed from DWARF (same happens when
> reconstructed from BTF):
>
> struct sysrq_key_op {
> const void (*handler)(int); /* 0 8 */
> const const char * help_msg; /* 8 8 */
> const const char * action_msg; /* 16 8 */
> const int enable_mask; /* 24 4 */
>
> /* size: 32, cachelines: 1, members: 4 */
> /* padding: 4 */
> /* last cacheline: 32 bytes */
> };
>
> So it seems to be a general issue with modifiers printing.
>
So it seems like the modifier ordering isn't preserved, even though
the final BTF representation looks right? Thanks!
Alan
> Thanks,
> Eduard
>>
>> 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.
>>
>> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
>> ---
>> 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) {
>
