Em Thu, Oct 14, 2021 at 12:48:49PM +0100, Douglas RAILLARD escreveu: > From: Douglas Raillard <douglas.raillard@xxxxxxx> > > This code: > > struct X { > struct { > } __attribute__((foo)) x __attribute__((bar)); > } > > Was wrongly printed as: > > struct X { > struct { > } x __attribute__((foo)) __attribute__((bar)); > } > > This unfortunately matters a lot, since "bar" is suppose to apply to > "x", but "foo" to typeof(x). In the wrong form, both apply to "x", > leading to e.g. incorrect layout for __aligned__ attribute. I couldn't quickly find a change in behaviour with a random vmlinux I have for testing: ⬢[acme@toolbox pahole]$ build/pahole.old -F dwarf vmlinux > before ⬢[acme@toolbox pahole]$ build/pahole -F dwarf vmlinux > after ⬢[acme@toolbox pahole]$ diff -u before after ⬢[acme@toolbox pahole]$ grep "__attribute__.*__attribute__" before | wc -l 40 ⬢[acme@toolbox pahole]$ btdiff is happy tho: ⬢[acme@toolbox pahole]$ btfdiff vmlinux ⬢[acme@toolbox pahole]$ cat btfdiff #!/bin/bash # SPDX-License-Identifier: GPL-2.0-only # Copyright © 2019 Red Hat Inc, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> # Use pahole to produce output from BTF and from DWARF, then do a diff # Use --flat_arrays with DWARF as BTF, like CTF, flattens arrays. # Use --show_private_classes as BTF shows all structs, while pahole knows # if some struct is defined only inside another struct/class or in a function, # this information is not available when loading from BTF. if [ $# -eq 0 ] ; then echo "Usage: btfdiff <filename_with_DWARF_and_maybe_BTF_info> [<filename_with_BTF_info>]" exit 1 fi dwarf_input=$1 btf_input=$dwarf_input if [ $# -eq 2 ] ; then btf_input=$2 fi btf_output=$(mktemp /tmp/btfdiff.btf.XXXXXX) dwarf_output=$(mktemp /tmp/btfdiff.dwarf.XXXXXX) pahole_bin=${PAHOLE-"pahole"} ${pahole_bin} -F dwarf \ --flat_arrays \ --sort \ --jobs \ --suppress_aligned_attribute \ --suppress_force_paddings \ --suppress_packed \ --show_private_classes $dwarf_input > $dwarf_output ${pahole_bin} -F btf \ --sort \ --suppress_packed \ $btf_input > $btf_output diff -up $dwarf_output $btf_output rm -f $btf_output $dwarf_output exit 0 ⬢[acme@toolbox pahole]$ As is fullcircle: ⬢[acme@toolbox pahole]$ fullcircle tcp.o ⬢[acme@toolbox pahole]$ cat fullcircle #!/bin/bash # SPDX-License-Identifier: GPL-2.0-only # Copyright © 2019 Red Hat Inc, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> # Use pfunct to produce compilable output from a object, then do a codiff -s # To see if the type information generated from source code generated # from type information in a file compiled from the original source code matches. if [ $# -eq 0 ] ; then echo "Usage: fullcircle <filename_with_type_info>" exit 1 fi file=$1 nr_cus=$(readelf -wi ${file} | grep DW_TAG_compile_unit | wc -l) if [ $nr_cus -gt 1 ]; then exit 0 fi c_output=$(mktemp /tmp/fullcircle.XXXXXX.c) o_output=$(mktemp /tmp/fullcircle.XXXXXX.o) pfunct_bin=${PFUNCT-"pfunct"} codiff_bin=${CODIFF-"codiff"} # See how your DW_AT_producer looks like and find the # right regexp to get after the GCC version string, this one # seems good enough for Red Hat/Fedora/CentOS that look like: # # DW_AT_producer : (indirect string, offset: 0x3583): GNU C89 8.2.1 20181215 (Red Hat 8.2.1-6) -mno-sse -mno-mmx # # So we need from -mno-sse onwards CFLAGS=$(readelf -wi $file | grep -w DW_AT_producer | sed -r 's/.*\)( -[[:alnum:]]+.*)+/\1/g') # Check if we managed to do the sed or if this is something like GNU AS [ "${CFLAGS/DW_AT_producer/}" != "${CFLAGS}" ] && exit ${pfunct_bin} --compile $file > $c_output gcc $CFLAGS -c -g $c_output -o $o_output ${codiff_bin} -q -s $file $o_output rm -f $c_output $o_output exit 0 ⬢[acme@toolbox pahole]$ - Arnaldo > Signed-off-by: Douglas Raillard <douglas.raillard@xxxxxxx> > --- > dwarves_fprintf.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c > index c35868a..1c1d949 100644 > --- a/dwarves_fprintf.c > +++ b/dwarves_fprintf.c > @@ -787,7 +787,7 @@ print_default: > (type->tag == DW_TAG_class_type && > !tconf.classes_as_structs) ? "class" : "struct", > tconf.type_spacing - 7, > - type__name(ctype), name); > + type__name(ctype), name ?: ""); > } else { > struct class *cclass = tag__class(type); > > @@ -802,7 +802,7 @@ print_default: > ctype = tag__type(type); > > if (type__name(ctype) != NULL && !expand_types) { > - printed += fprintf(fp, "union %-*s %s", tconf.type_spacing - 6, type__name(ctype), name); > + printed += fprintf(fp, "union %-*s %s", tconf.type_spacing - 6, type__name(ctype), name ?: ""); > } else { > tconf.type_spacing -= 8; > printed += union__fprintf(ctype, cu, &tconf, fp); > @@ -812,7 +812,7 @@ print_default: > ctype = tag__type(type); > > if (type__name(ctype) != NULL) > - printed += fprintf(fp, "enum %-*s %s", tconf.type_spacing - 5, type__name(ctype), name); > + printed += fprintf(fp, "enum %-*s %s", tconf.type_spacing - 5, type__name(ctype), name ?: ""); > else > printed += enumeration__fprintf(type, &tconf, fp); > break; > @@ -863,7 +863,21 @@ static size_t class_member__fprintf(struct class_member *member, bool union_memb > if (member->is_static) > printed += fprintf(fp, "static "); > > - printed += type__fprintf(type, cu, name, &sconf, fp); > + /* For struct-like constructs, the name of the member cannot be > + * conflated with the name of its type, otherwise __attribute__ are > + * printed in the wrong order. > + */ > + if (tag__is_union(type) || tag__is_struct(type) || > + tag__is_enumeration(type)) { > + printed += type__fprintf(type, cu, NULL, &sconf, fp); > + if (name) { > + if (!type__name(tag__type(type))) > + printed += fprintf(fp, " "); > + printed += fprintf(fp, "%s", name); > + } > + } else { > + printed += type__fprintf(type, cu, name, &sconf, fp); > + } > > if (member->is_static) { > if (member->const_value != 0) > -- > 2.25.1 -- - Arnaldo