In function ‘enumeration__calc_prefix’,
inlined from ‘enumeration__calc_prefix’ at /home/acme/git/pahole/dwarves.c:1661:6:
/home/acme/git/pahole/dwarves.c:1683:38: warning: ‘strndup’ specified bound 2147483647 exceeds source size 1 [-Wstringop-overread]
1683 | enumeration->member_prefix = strndup(curr_name, common_part);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$ gcc --version | head -1
gcc (GCC) 11.0.0 20210123 (Red Hat 11.0.0-0)
$
So check if we actually found the common part, even with that meaning
the enumeration has no entries.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
In Makefiles we want to do purely numeric comparisions, such as in the
Linux kernel Makefiles and scripts, that have things like this at the
moment:
$ grep PAHOLE */*.sh
scripts/link-vmlinux.sh: if ! [ -x "$(command -v ${PAHOLE})" ]; then
scripts/link-vmlinux.sh: echo >&2 "BTF: ${1}: pahole (${PAHOLE}) is not available"
scripts/link-vmlinux.sh: pahole_ver=$(${PAHOLE} --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
scripts/link-vmlinux.sh: echo >&2 "BTF: ${1}: pahole version $(${PAHOLE} --version) is too old, need at least v1.16"
scripts/link-vmlinux.sh: LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
$
So just provide:
$ pahole --numeric_version
118
$
While keeping the --version output for older Makefiles and scripts.
Cc: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Nothing changes now, this continues to work just the same:
$ pahole --version
v1.18
$ pfunct --version
v1.18
$
This just paves the way for us to have a '--numeric-version' that will
do away with the dot and the leading 'v' and that can be used in
Makefiles to check if the required minimum version is available, to
avoid what we have now in the Linux kernel:
config PAHOLE_HAS_SPLIT_BTF
def_bool $(success, test `$(PAHOLE) --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "119")
With the next cset we'll be able to do just:
test `$(PAHOLE) --numeric-version` -ge "119"
Cc: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This was detected with:
In file included from /home/acme/git/pahole/strings.h:9,
from /usr/include/string.h:432,
from /home/acme/git/pahole/lib/bpf/src/libbpf_common.h:12,
from /home/acme/git/pahole/lib/bpf/src/libbpf.h:20,
from /home/acme/git/pahole/lib/bpf/src/ringbuf.c:20:
/home/acme/git/pahole/lib/bpf/src/btf.h:33:11: error: expected ‘;’ before ‘void’
33 | LIBBPF_API void btf__free(struct btf *btf);
| ^~~~~
| ;
libbpf_common.h has:
#include <string.h>
#ifndef LIBBPF_API
#define LIBBPF_API __attribute__((visibility("default")))
#endif
So before defining LIBBPF_API it includes libc's string.h that in turn
includes pahole's strings.h and now it includes:
#include "lib/bpf/src/btf.h"
That will need the LIBBPF_API, b00m.
So lets just rename pahole's strings.h to pahole_strings.h to avoid this
pitfall.
This patch was moved to before this problem takes place so that we keep
everything bisectable.
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Maintain a pointer to debug_fmt_ops corresponding to currently used debug info
format loader (DWARF, BTF, or CTF), to allow various parts of libdwarves to do
things like resolve string offset to actual string pointer in
a format-agnostic format. This allows to, say, load DWARF debug info, and use
it for BTF generation, without either of them making assumptions about how
strings are actually stored internally.
This is going to be used in the next patch to allow BTF loader and encoder to
use a very different way of storing strings (not a global shared gobuffer).
Committer notes:
Since it is available in multiple object files, add a dwarves__ prefix
namespace and add an extern for it in dwarves.h.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This was found in a ADA object, part of gdb's test suite, for now just
make the code a bit more robust when not finding a type for some struct
member, etc, which avoids segfaults and produces output from ADA
objects, but there are other problems to solve as this is a _type tag, I
need to provide some better support so that type resolution works.
[foo.debug.gz](https://github.com/acmel/dwarves/files/5257332/foo.debug.gz)
Foo.debug, an ada exec:
```
$ ~/dwarves/build/pahole foo.debug
die__process_unit: DW_TAG_subrange_type (0x21) @ <0x10b> not handled!
die__process_unit: DW_TAG_subrange_type (0x21) @ <0x134> not handled!
die__process_unit: DW_TAG_subrange_type (0x21) @ <0x148> not handled!
die__process_class: DW_TAG_subrange_type (0x21) @ <0x201> not handled!
Segmentation fault (core dumped)
$
These are fixed, the warnings continue to be produced.
Reported-by: Tom de Vries
Bugtracker: https://github.com/acmel/dwarves/issues/9#issuecomment-696282005
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We don't really reconstruct source code for FORTRAN, we just print it as
if it was C:
$ pahole examples/fortran95/derived-type.debug
struct bar {
integer(kind=4) c; /* 0 4 */
real(kind=4) d; /* 4 4 */
/* size: 8, cachelines: 1, members: 2 */
/* last cacheline: 8 bytes */
};
struct foo {
real(kind=4) a; /* 0 4 */
struct bar x; /* 4 8 */
string b[7]; /* 12 7 */
/* size: 20, cachelines: 1, members: 3 */
/* padding: 1 */
/* last cacheline: 20 bytes */
};
$
This comes from GCC build tests:
$ readelf -wi examples/fortran95/derived-type.debug | grep Fortran -A2
<9c> DW_AT_producer : (indirect string, offset: 0x1fb): GNU Fortran2008 10.2.1 20200728 [revision c0438ced53bcf57e4ebb1c38c226e41571aca892] -mtune=generic -march=x86-64 -g -fno-stack-protector -J /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.fortran/derived-type -fintrinsic-modules-path /usr/lib64/gcc/x86_64-suse-linux/10/finclude -fpre-include=/usr/include/finclude/math-vector-fortran.h
<a0> DW_AT_language : 14 (Fortran 95)
<a1> DW_AT_identifier_case: 2 (down_case)
<a2> DW_AT_name : (indirect string, offset: 0x365): /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.fortran/derived-type.f90
[acme@five pahole]$ readelf -wi examples/fortran95/derived-type.debug | grep DW_TAG_string_type -A2
<1><122>: Abbrev Number: 6 (DW_TAG_string_type)
<123> DW_AT_byte_size : 7
$
Now lets see whats more that is there segfaulting pahole, but for now I
think I don't have any segfaults, so just wait a bit for Hao to submit
the patch to selectively encode the per-cpu variables in BTF and then
cut v1.18.
Reported-by: Tom de Vries
Bugtracker: https://github.com/acmel/dwarves/issues/9
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This dodges a SEGFAULT at type__check_structs_at_unnatural_alignments()
so that we can finish processing, give the warnings and produce as much
as we can:
$ pahole examples/fortran95/derived-type.debug
die__process_unit: DW_TAG_string_type (0x12) @ <0x122> not handled!
namespace__recode_dwarf_types: couldn't find 0x122 type for 0x116 (member)!
struct bar {
integer(kind=4) c; /* 0 4 */
real(kind=4) d; /* 4 4 */
/* size: 8, cachelines: 1, members: 2 */
/* last cacheline: 8 bytes */
};
struct foo {
real(kind=4) a; /* 0 4 */
struct bar x; /* 4 8 */
<ERROR(__class__fprintf:1519): 0 not found!>
/* size: 20, cachelines: 1, members: 3 */
/* padding: 8 */
/* last cacheline: 20 bytes */
};
$
Reported-by: Tom de Vries
Bugtracker: https://github.com/acmel/dwarves/issues/9
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
As sometimes we have multiple enums to represent some struct type, like
with perf_event_attr->type, that has 'enum perf_event_type' in Linux's
UAPI and 'enum perf_user_event_type' for purely userspace types, like
the ones synthesized for Intel PT, like PERF_RECORD_AUXTRACE, etc.
This patch just transforms type->type_enum into a list, the support for
multiple types comes next.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We'll use it in an upcoming feature, to pretty print fields that albeit
not declared as an enum, have values coming from one.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
To cope with this warning on s390x:
/usr/bin/cc -DDWARVES_VERSION=\"v1.16\" -D_GNU_SOURCE -Ddwarves_EXPORTS -I/builddir/build/BUILD/dwarves-1.16 -I/builddir/build/BUILD/dwarves-1.16/lib/bpf/include/uapi -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=zEC12 -mtune=z13 -fasynchronous-unwind-tables -fstack-clash-protection -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Werror -ggdb -O2 -fPIC -o CMakeFiles/dwarves.dir/libctf.c.o -c /builddir/build/BUILD/dwarves-1.16/libctf.c
/builddir/build/BUILD/dwarves-1.16/dwarves.h: In function 'cus__load_dir':
/builddir/build/BUILD/dwarves-1.16/dwarves.c:1663:44: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size between 0 and 4095 [-Werror=format-truncation=]
1663 | snprintf(pathname, sizeof(pathname), "%s/%s",
| ^~
In file included from /usr/include/stdio.h:867,
from /builddir/build/BUILD/dwarves-1.16/dwarves.c:18:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 2 and 4352 bytes into a destination of size 4096
67 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
68 | __bos (__s), __fmt, __va_arg_pack ());
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/dwarves-1.16/dwarves.c:1663:44: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size between 0 and 4095 [-Werror=format-truncation=]
1663 | snprintf(pathname, sizeof(pathname), "%s/%s",
| ^~
In file included from /usr/include/stdio.h:867,
from /builddir/build/BUILD/dwarves-1.16/dwarves.c:18:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 2 and 4352 bytes into a destination of size 4096
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Don't use it if NULL, fixing a segfault with dtagnames as reported in:
https://bugzilla.redhat.com/show_bug.cgi?id=1795379
Reported-by: Jan Pokorný <jpokorny@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
When we call the tools without specifying a file format, all the
available format loaders are called, in sequence, till we find the
relevant information, now that we support raw BTF we better move it in
front of the CTF one, as it is way more common in the Linux community.
With this we will not see this warning anymore:
$ pahole -C list_head /sys/kernel/btf/vmlinux
ctf__new: cannot get elf header.
struct list_head {
struct list_head * next; /* 0 8 */
struct list_head * prev; /* 8 8 */
/* size: 16, cachelines: 1, members: 2 */
/* last cacheline: 16 bytes */
};
$
That warning has to go, so that other formats, if present after the CTF
one, can be tried.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Be it automatically when no -F option is passed and
/sys/kernel/btf/vmlinux is available, or when /sys/kernel/btf/vmlinux is
passed as the filename to the tool, i.e.:
$ pahole -C list_head
struct list_head {
struct list_head * next; /* 0 8 */
struct list_head * prev; /* 8 8 */
/* size: 16, cachelines: 1, members: 2 */
/* last cacheline: 16 bytes */
};
$ strace -e openat pahole -C list_head |& grep /sys/kernel/btf/
openat(AT_FDCWD, "/sys/kernel/btf/vmlinux", O_RDONLY) = 3
$
$ pahole -C list_head /sys/kernel/btf/vmlinux
struct list_head {
struct list_head * next; /* 0 8 */
struct list_head * prev; /* 8 8 */
/* size: 16, cachelines: 1, members: 2 */
/* last cacheline: 16 bytes */
};
$
If one wants to grab the matching vmlinux to use its DWARF info instead,
which is useful to compare the results with what we have from BTF, for
instance, its just a matter of using '-F dwarf'.
This in turn shows something that at first came as a surprise, but then
has a simple explanation:
For very common data structures, that will probably appear in all of the
DWARF CUs (Compilation Units), like 'struct list_head', using '-F dwarf'
is faster:
[acme@quaco pahole]$ perf stat -e cycles pahole -F btf -C list_head > /dev/null
Performance counter stats for 'pahole -F btf -C list_head':
45,722,518 cycles:u
0.023717300 seconds time elapsed
0.016474000 seconds user
0.007212000 seconds sys
[acme@quaco pahole]$ perf stat -e cycles pahole -F dwarf -C list_head > /dev/null
Performance counter stats for 'pahole -F dwarf -C list_head':
14,170,321 cycles:u
0.006668904 seconds time elapsed
0.005562000 seconds user
0.001109000 seconds sys
[acme@quaco pahole]$
But for something that is more specific to a subsystem, the DWARF loader
will have to process way more stuff till it gets to that struct:
$ perf stat -e cycles pahole -F dwarf -C tcp_sock > /dev/null
Performance counter stats for 'pahole -F dwarf -C tcp_sock':
31,579,795,238 cycles:u
8.332272930 seconds time elapsed
8.032124000 seconds user
0.286537000 seconds sys
$
While using the BTF loader the time should be constant, as it loads
everything from /sys/kernel/btf/vmlinux:
$ perf stat -e cycles pahole -F btf -C tcp_sock > /dev/null
Performance counter stats for 'pahole -F btf -C tcp_sock':
48,823,488 cycles:u
0.024102760 seconds time elapsed
0.012035000 seconds user
0.012046000 seconds sys
$
Above I used '-F btf' just to show that it can be used, but its not
really needed, i.e. those are equivalent:
$ strace -e openat pahole -F btf -C list_head |& grep /sys/kernel/btf/vmlinux
openat(AT_FDCWD, "/sys/kernel/btf/vmlinux", O_RDONLY) = 3
$ strace -e openat pahole -C list_head |& grep /sys/kernel/btf/vmlinux
openat(AT_FDCWD, "/sys/kernel/btf/vmlinux", O_RDONLY) = 3
$
The btf_raw__load() function that ends up being grafted into the
preexisting btf_elf routines was based on libbpf's btf_load_raw().
Acked-by: Alexei Starovoitov <ast@fb.com>
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Since we iterate over these ptr_tables entry by entry, we better zero
out the new ranges, i.e. whatever is after the previous allocated space
up to the new one, returned by realloc.
Fixes a bug found using pahole -F btf on a vmlinux file, now btfdiff on
this same file comes clean, i.e. the output from its BTF tags is the
same as with its DWARF ones, for the features present in both type
information formats.
Tested-by: Alexei Starovoitov <ast@fb.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
It was possible to have an infinite loop trying to determine the
alignment of a type that had a static data member of its own type.
Example code illustrating the problem.
struct X { static X thing; };
Committer testing:
Before:
$ pwd
/home/acme/git/pahole/examples/gareth
$ cat static_member.cpp
struct X { static X thing; };
static struct X f;
$ g++ -g -c static_member.cpp -o static_member.o
$ pahole static_member.o
Segmentation fault (core dumped)
$
After:
$ pahole static_member.o
struct X {
static struct X thing; /* 0 0 */
/* XXX last struct has 1 byte of padding */
/* size: 1, cachelines: 0, members: 0, static members: 1 */
/* padding: 1 */
/* paddings: 1, sum paddings: 1 */
/* last cacheline: 1 bytes */
};
$
Signed-off-by: Gareth Lloyd <gareth.lloyd@uk.ibm.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
To agree with what ptr_table__add() does, i.e. the number of entries
(pt->nr_entries) is in fact the max id number in the table + 1, we can't
allow to add an id that is not contiguous and simply increment
nr_entries, as cu__for_each_variable(cu, id, pos) will iterate based on
nr_entries, skipping NULL buckets.
Detected when adding support for BTF variables, that share the id space
with types, but end up in different ptr_tables, so we end up with:
cu__table_add_tag(BTF_KIND_VAR)
ptr_table__add_with_id(cu->tags_table)
but:
cu__table_add_tag()
ptr_table__add_with_id(cu->types_table)
for all the other BTF_KIND_s, fix it so that ptr->nr_entries copes with
not receiving strictly monotonically incremented by one ids.
We'll have to fix this more properly by defining which tags are
supported by some debugging format and this what should go to a
ptr_table so that we have all its entries monononically incremented by
one and avoid tag__check_id_drift() returning true in the per debugging
format encoders (e.g.: btf_encoder.c).
This patch allows us to move forward and apply the BTF_KIND_VAR patch
where we'll at least produce, for global variables and a simple BPF
program with both DWARF and BTF tags the same results:
[root@quaco tracebuffer]# pglobal -F dwarf -v bristot.o
struct ____btf_map_tracebuffer__bristot ____btf_map_tracebuffer__bristot;; /* 0 */
char _license[4];; /* 0 */
int _version;; /* 0 */
struct bpf_map tracebuffer__bristot;; /* 0 */
[root@quaco tracebuffer]# pglobal -F btf -v bristot.o
BTF: idx: 17, off: 352, Unknown kind 15
BTF: idx: 18, off: 364, Unknown kind 0
BTF: idx: 19, off: 376, Unknown kind 15
BTF: idx: 20, off: 388, Unknown kind 0
BTF: idx: 21, off: 400, Unknown kind 15
BTF: idx: 22, off: 412, Unknown kind 0
BTF: idx: 23, off: 424, Unknown kind 15
BTF: idx: 24, off: 436, Unknown kind 0
struct ____btf_map_tracebuffer__bristot ____btf_map_tracebuffer__bristot;; /* 0 */
char _license[4];; /* 0 */
int _version;; /* 0 */
struct bpf_map tracebuffer__bristot;; /* 0 */
[root@quaco tracebuffer]#
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Not used at all, brought when adding vmlinux searching, but ended up not
being used, ditch it.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Addresses this coverity report entry:
Error: FORWARD_NULL (CWE-476): [#def12]
dwarves-1.13/dwarves.c:1708: var_compare_op: Comparing "conf" to null implies that "conf" might be null.
dwarves-1.13/dwarves.c:1743: var_deref_op: Dereferencing null pointer "conf".
# 1741|
# 1742| while (debug_fmt_table[i] != NULL) {
# 1743|-> if (conf->conf_fprintf)
# 1744| conf->conf_fprintf->has_alignment_info = debug_fmt_table[i]->has_alignment_info;
# 1745| if (debug_fmt_table[i]->load_file(cus, conf, filename) == 0)
The first check setting added by 49c27bdd66 ("core: Allow the loaders
to advertise features they have") was inside a block where conf was
already checked, the second wasn't, fix it.
Reported-by: William Cohen <wcohen@redhat.com>
Fixes: 49c27bdd66 ("core: Allow the loaders to advertise features they have")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We stopped scrubbing type when looking for type->type by assigning the
result of cu__type(cu, type->type) to a temporary 'tag' variable but
continued to check the result of cu__type() looking at type... Check
'tag' instead.
Addresses these coverity report entries:
Error: NULL_RETURNS (CWE-476): [#def9]
dwarves-1.13/dwarves.c:333: returned_null: "cu__type" returns "NULL" (checked 54 out of 62 times).
dwarves-1.13/dwarves.c:333: var_assigned: Assigning: "tag" = "NULL" return value from "cu__type".
dwarves-1.13/dwarves.c:338: alias: Assigning: "type" = "tag". Both pointers are now "NULL".
dwarves-1.13/dwarves.c:312: dereference: Dereferencing "type", which is known to be "NULL".
dwarves-1.13/codiff.c:137: example_assign: Example 1: Assigning: "old_type" = return value from "cu__type(old_cu, old->tag.type)".
dwarves-1.13/codiff.c:141: example_checked: Example 1 (cont.): "old_type" has its value checked in "old_type == NULL".
dwarves-1.13/ctracer.c:356: example_assign: Example 2: Assigning: "type" = return value from "cu__type(cu, tag->type)".
dwarves-1.13/ctracer.c:358: example_checked: Example 2 (cont.): "type" has its value checked in "type == NULL".
dwarves-1.13/dwarves.c:914: example_assign: Example 3: Assigning: "type" = return value from "cu__type(cu, tag->type)".
dwarves-1.13/dwarves.c:916: example_checked: Example 3 (cont.): "type" has its value checked in "type == NULL".
dwarves-1.13/dwarves.c:941: example_assign: Example 4: Assigning: "tag" = return value from "cu__type(cu, var->ip.tag.type)".
dwarves-1.13/dwarves.c:942: example_checked: Example 4 (cont.): "tag" has its value checked in "tag != NULL".
dwarves-1.13/dwarves_emit.c:139: example_assign: Example 5: Assigning: "ptr_type" = return value from "cu__type(cu, type->type)".
dwarves-1.13/dwarves_emit.c:141: example_checked: Example 5 (cont.): "ptr_type" has its value checked in "ptr_type == NULL".
# 310| }
# 311| reevaluate:
# 312|-> switch (type->tag) {
# 313| case DW_TAG_base_type:
# 314|
Error: REVERSE_INULL (CWE-476): [#def10]
dwarves-1.13/dwarves.c:312: deref_ptr: Directly dereferencing pointer "type".
dwarves-1.13/dwarves.c:334: check_after_deref: Null-checking "type" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
# 332| case DW_TAG_volatile_type: {
# 333| struct tag *tag = cu__type(cu, type->type);
# 334|-> if (type == NULL) {
# 335| tag__id_not_found_fprintf(stderr, type->type);
# 336| continue;
Looking at this last one shows the problem in detail, check for NULL to
then deref it, phew.
This is just used in 'pahole --show_first_biggest_size_base_type_member'
and when fixing up alignment in the --reorganize code tho.
Reported-by: William Cohen <wcohen@redhat.com>
Fixes: 7fc7148be7 ("core: Fix thinko in type__find_first_biggest_size_base_type_member")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Since we don't have something like DW_AT_alignment for
__attribute__((__packed__)), we need to use whatever hints that are
there in the alignments to figure out if a naturally packed struct has
the __attribute__((packed)) in the original sources, because that is
needed to waiver its natural alignment requisites.
For instance,
/* Used at: btrfs.c */
/* <1e7b> /home/acme/git/pahole/btrfs.c:199 */
struct btrfs_block_group_cache {
struct btrfs_key key; /* 0 17 */
struct btrfs_block_group_item item; /* 17 24 */
/* XXX 7 bytes hole, try to pack */
struct btrfs_fs_info * fs_info; /* 48 8 */
struct inode * inode; /* 56 8 */
In the original source code, btrfs_block_group_item is marked
__packed__, and being so, even seemingly unnecessarily, makes it, when
embedded in another struct, like the above, forfeit its natural
alingment, that would be 8 bytes, and instead appear right at the 17th
byte offset...
struct btrfs_block_group_item {
__le64 used; /* 0 8 */
__le64 chunk_objectid; /* 8 8 */
__le64 flags; /* 16 8 */
/* size: 24, cachelines: 1, members: 3 */
/* last cacheline: 24 bytes */
} __attribute__((__packed__));
So we need to, seeing its use at a unnatural offset, go backwards to the
btrfs_block_group_item pahole internal data structure, 'struct type' and
mark is_packed field as 'true', despite it not looking like a packed
struct.
Same thing with:
struct ieee80211_mcs_info {
u8 rx_mask[10]; /* 0 10 */
__le16 rx_highest; /* 10 2 */
u8 tx_params; /* 12 1 */
u8 reserved[3]; /* 13 3 */
/* size: 16, cachelines: 1, members: 4 */
/* last cacheline: 16 bytes */
};
That is naturally aligned and as 16 bytes, a power of two, then when it appears at the end of:
$ pahole -IC ieee80211_sta_ht_cap vht.o
/* Used at: vht.c */
/* <31ea> /home/acme/git/pahole/vht.c:1769 */
struct ieee80211_sta_ht_cap {
u16 cap; /* 0 2 */
bool ht_supported; /* 2 1 */
u8 ampdu_factor; /* 3 1 */
u8 ampdu_density; /* 4 1 */
/* XXX 1 byte hole, try to pack */
struct ieee80211_mcs_info mcs; /* 6 16 */
/* size: 22, cachelines: 1, members: 5 */
/* sum members: 21, holes: 1, sum holes: 1 */
/* last cacheline: 22 bytes */
};
$
We get that one byte hole if ieee80211_mcs_info isn't marked __packed__, as soon as we mark it:
$ pahole -IC ieee80211_sta_ht_cap vht.o
/* Used at: vht.c */
/* <31ea> /home/acme/git/pahole/vht.c:1769 */
struct ieee80211_sta_ht_cap {
u16 cap; /* 0 2 */
bool ht_supported; /* 2 1 */
u8 ampdu_factor; /* 3 1 */
u8 ampdu_density; /* 4 1 */
struct ieee80211_mcs_info mcs; /* 5 16 */
/* size: 22, cachelines: 1, members: 5 */
/* padding: 1 */
/* last cacheline: 22 bytes */
};
[acme@quaco pahole]$
It works, so __packed__ in this case just says: trow away the natural
alignment, make it 1 in whatever container structs.
So, before emitting the types for some struct, we go back looking at
each of its members and checking for such unnatural offsets, marking the
types as __packed__. Now:
$ pfunct --compile /home/acme/git/build/v5.1-rc4+/net/mac80211/vht.o | grep "^struct ieee80211_mcs_info" -A8
struct ieee80211_mcs_info {
u8 rx_mask[10]; /* 0 10 */
__le16 rx_highest; /* 10 2 */
u8 tx_params; /* 12 1 */
u8 reserved[3]; /* 13 3 */
/* size: 16, cachelines: 1, members: 4 */
/* last cacheline: 16 bytes */
} __attribute__((__packed__));
$
$ pfunct --compile /home/acme/git/build/v5.1-rc4+/fs/btrfs/free-space-tree.o | grep "^struct btrfs_block_group_item" -A7
struct btrfs_block_group_item {
__le64 used; /* 0 8 */
__le64 chunk_objectid; /* 8 8 */
__le64 flags; /* 16 8 */
/* size: 24, cachelines: 1, members: 3 */
/* last cacheline: 24 bytes */
} __attribute__((__packed__));
$
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
For instance, DWARF has DW_AT_alignment, and some output features
require that, so let loaders advertise such things, next patch will use
this info.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Will be used when considering comparing multiple CU entries in a struct
cus to the sole compile unit in a second file, like when comparing the
types in a multi-CU DWARF file like vmlinux against the combined,
deduplicated entries in that vmlinux .BTF section.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
For the usual idiom to ask if a tag is a pointer, removing a bit of
DWARFism and shortening the operation.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Instead of relying on error-prone adjustment of bit/byte holes, use
class__find_holes() to re-calculate them after members are moved around.
As part of that change, fix bug with not adjusting bit_offset, when
changing byte_offset.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: dwarves@vger.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
pahole --reorganize heavily depends on member's bit_hole and hole fields
to denote bit/byte holes *after* member. Previous commit "dwarves: use
bit sizes and bit/byte hole info in __class__fprintf" changed its
meaning to bit/byte hole *before* member to accomodate possible bit/byte
holes at the beginning of a struct. This change broke reorganization
algorithm, though, which is quite involved and isn't trivially
modifiable to accomodate new semantics.
This patch reverts the meaning of bit_hole and hole, but also introduces
per class pre_bit_hole/pre_hole to record initial bit/byte hole of a
struct. This allows to fix reorg code more easily and still handle
initial holes cases, if at the expense of being not as elegant.
Committer testing:
$ time pahole -F btf --packable vmlinux | sort -nr -k4 | head
bts_ctx 12288 8192 4096
swsusp_info 4096 432 3664
vc_data 792 496 296
pci_dev 2488 2320 168
rcu_state 3392 3240 152
ptr_ring 192 40 152
xdp_sock 960 840 120
zone 1664 1552 112
rcu_data 576 472 104
rcu_node 576 480 96
real 0m0.038s
user 0m0.029s
sys 0m0.017s
$
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: dwarves@vger.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This patch changes __class__fprintf to rely on bit_size/bitfield_size,
calculated in corresponding loaders, instead of trying to guess correct
sizes on its own.
Bit and byte hole information is now stored in current field member and
stores information about holes *before* field. Previously hole
information was stored in previous field and was meant to represent
"holes after a field". Such approach makes it hard to report bit holes
at the very beginning of the struct:
struct s {
int:4; /* this is bit hole, not a field in DWARF/BTF */
int x:8;
};
With this change and previous bitfield calculation/fixup logic fixes,
there are no more discrepancies between DWARF/BTF for allyesconfig
kernel. There are also no more BRAIN FART ALERTS!
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Mark Wielaard <mark@klomp.org>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: dwarves@vger.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This patch rewrites hole detection logic. Now many crazy combinations of
bitfields and normal fields are handled correctly.
This was tested on allyesconfig kernel and differences before/after were
always in favor of new algorithm.
With subsequent change in next patch, there are no more BRAIN FART
ALERTs for allyesconfig and DWARF and BTF outputs have no discrepanies.
Example:
$ cat test.c
struct s {
short : 4; /* this one is not emitted in DWARF/BTF */
short a : 4;
int x;
int : 10;
int y : 4;
short zz;
short zzz : 4;
long z;
short : 4;
};
int main() {
struct s s;
return 0;
}
$ gcc -g test.c -o test
$ ~/pahole/build/pahole -J test
$ ~/pahole/build/pahole -F dwarf test
struct s {
/* XXX 4 bits hole, try to pack */
short int a:4; /* 0: 8 2 */
/* XXX 8 bits hole, try to pack */
/* XXX 2 bytes hole, try to pack */
int x; /* 4 4 */
/* XXX 10 bits hole, try to pack */
int y:4; /* 8:18 4 */
/* Bitfield combined with next fields */
/* XXX 2 bits hole, try to pack */
short int zz; /* 10 2 */
short int zzz:4; /* 12:12 2 */
/* XXX 12 bits hole, try to pack */
/* XXX 2 bytes hole, try to pack */
long int z; /* 16 8 */
/* size: 32, cachelines: 1, members: 6 */
/* sum members (bits): 124, holes: 2, sum holes: 4 */
/* bit holes: 5, sum bit holes: 36 bits */
/* padding: 8 */
/* last cacheline: 32 bytes */
};
No discrepanies between BTF/DWARF:
$ ../btfdiff test
$
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Mark Wielaard <mark@klomp.org>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: dwarves@vger.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Arnaldo Carvalho de Melo
Andrii Nakryiko
Gareth Lloyd