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>
Counting field sizes only in bits causes confusion and lots of differing
output, when compared to previous logic. This commit changes logic so
that it counts bit size of bitfield fields separately from byte size of
non-bitfield fields. In the end, if there were bit holes, this bit size
is emitted explicitly. This makes output for struct/unions not using
bitfields identical, while also preserving correctness (and data
completeness) for cases with bitfields and bit holes.
Example (-before/+after):
struct cfg80211_pmsr_request_peer {
u8 addr[6]; /* 0 6 */
/* XXX 2 bytes hole, try to pack */
struct cfg80211_chan_def chandef; /* 8 24 */
/* XXX last struct has 4 bytes of padding */
u8 report_ap_tsf:1; /* 32: 0 1 */
/* XXX 7 bits hole, try to pack */
/* XXX 3 bytes hole, try to pack */
struct cfg80211_pmsr_ftm_request_peer ftm; /* 36 12 */
/* XXX last struct has 1 byte of padding */
/* size: 48, cachelines: 1, members: 4 */
- /* sum members: 43, holes: 2, sum holes: 5 */
- /* bit holes: 1, sum bit holes: 7 bits */
+ /* sum members: 42, holes: 2, sum holes: 5 */
+ /* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 7 bits */
/* paddings: 2, sum paddings: 5 */
/* last cacheline: 48 bytes */
};
For cases where there is only byte or bit hole, we still emit total byte and
bit sizes of all members as to not mislead user:
struct sched_dl_entity {
... <snip ...
unsigned int dl_non_contending:1; /* 84: 3 4 */
unsigned int dl_overrun:1; /* 84: 4 4 */
/* XXX 27 bits hole, try to pack */
struct hrtimer dl_timer; /* 88 64 */
/* XXX last struct has 5 bytes of padding */
/* --- cacheline 2 boundary (128 bytes) was 24 bytes ago --- */
struct hrtimer inactive_timer; /* 152 64 */
/* XXX last struct has 5 bytes of padding */
/* size: 216, cachelines: 4, members: 16 */
- /* bit holes: 1, sum bit holes: 27 bits */
+ /* sum members: 212 */
+ /* sum bitfield members: 5 bits, bit holes: 1, sum bit holes: 27 bits */
/* paddings: 2, sum paddings: 10 */
/* last cacheline: 24 bytes */
};
For structs with tightly packed bitfield, we emit total number of bits and also
convert them to bytes. E.g., for struct sock output :
struct sock {
... <snip ...
/* size: 720, cachelines: 12, members: 84 */
- /* sum members: 712, holes: 4, sum holes: 8 */
+ /* sum members: 707, holes: 4, sum holes: 8 */
+ /* sum bitfield members: 40 bits (5 bytes) */
/* paddings: 1, sum paddings: 4 */
/* last cacheline: 16 bytes */
};
Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
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>
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>
Existing code base assumes that single CU doesn't have more than 65535
types per each CU, which might be a reasonable assumption for DWARF
data. With BTF, though, all we get is single, potentially huge, CU which
can easily have more than 65k types. For example, this is the case for
allyesconfig version of Linux kernel, which has >200k types.
Due to this assumption, libdwarves and other parts of pahole are using
16-bit counters to iterate over entities within CU. This can cause
infinite loops when iterating BTF data, if there are more than 65535
types. This patch changes non-public variables to use 32-bit integers,
where appropriate.
This still leads to invalid reported data when using BTF loader (due to using
(X & 0xFFFF) type ID, instead of X, when X > 65535) and loading huge files,
but at least it's not stuck in an infinite loop anymore.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
[ Removed non type ID conversions, for instance for the kind of tag, like in type->namespace.tag.tag, that can remain a uint16_t ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
union__fprintf() unconditionally uses conf.cachelinep, assuming there is
where the current cacheline is being kept, but if we call
union__fprintf() from something other than __class__fprintf(), then that
pointer is NULL, fix that.
Reported-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1633348
Fixes: e975ff247a ("dwarves_fprintf: Print cacheline boundaries in multiple union members")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We'll use location in the DWARF sense, i.e. location lists, etc, i.e.
where is this variable? In a register? The stack? etc.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
To help with using just that object file, avoiding processing big files
such as vmlinux, e.g.:
$ pahole -I vmlinux
<SNIP>
/* Used at: /home/acme/git/perf/init/main.c */
/* <1f4a5> /home/acme/git/perf/arch/x86/include/asm/orc_types.h:85 */
struct orc_entry {
s16 sp_offset; /* 0 2 */
s16 bp_offset; /* 2 2 */
unsigned int sp_reg:4; /* 4:28 4 */
unsigned int bp_reg:4; /* 4:24 4 */
unsigned int type:2; /* 4:22 4 */
/* size: 6, cachelines: 1, members: 5 */
/* padding: 65534 */
/* bit_padding: 22 bits */
/* last cacheline: 6 bytes */
/* BRAIN FART ALERT! 6 != 8 + 0(holes), diff = -2 */
};
<SNIP>
So I noticed that BFA, need to work on it, to make the testing process
faster, better not process vmlinux.o, instead, do:
$ pahole -C orc_entry ${kernel_build_dir}/init/main.o
Much faster, as main.o is much smaller than the vmlinux file.
Now to fix the processing of 'struct orc_entry'.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Take 'struct task_struct' in the Linux kernel, these fields:
/* --- cacheline 2 boundary (128 bytes) --- */
struct sched_entity se; /* 128 448 */
/* XXX last struct has 24 bytes of padding */
/* --- cacheline 9 boundary (576 bytes) --- */
struct sched_rt_entity rt; /* 576 48 */
The sched_entity struct has 24 bytes of padding, and that info would
only appear when printing 'struct task_struct' if class__find_holes()
had previously been run on 'struct sched_entity' which wasn't always the
case, make sure that happens.
This results in this extra stat being printed for 'struct task_struct':
/* paddings: 4, sum paddings: 38 */
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
A diff for 'pahole -EC task_struct vmlinux' should clarify what this fixes:
[acme@jouet linux]$ diff -u /tmp/before.c /tmp/after.c | head -30
--- /tmp/before.c 2016-06-29 17:00:38.082647281 -0300
+++ /tmp/a.c 2016-06-29 17:03:36.913124779 -0300
@@ -43,8 +43,8 @@
struct list_head * prev; /* 176 8 */
} group_node; /* 168 16 */
unsigned int on_rq; /* 184 4 */
+ /* --- cacheline 3 boundary (192 bytes) --- */
/* typedef u64 */ long long unsigned int exec_start; /* 192 8 */
- /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
/* typedef u64 */ long long unsigned int sum_exec_runtime; /* 200 8 */
/* typedef u64 */ long long unsigned int vruntime; /* 208 8 */
/* typedef u64 */ long long unsigned int prev_sum_exec_runtime; /* 216 8 */
@@ -53,40 +53,40 @@
/* typedef u64 */ long long unsigned int wait_start; /* 232 8 */
/* typedef u64 */ long long unsigned int wait_max; /* 240 8 */
/* typedef u64 */ long long unsigned int wait_count; /* 248 8 */
+ /* --- cacheline 4 boundary (256 bytes) --- */
/* typedef u64 */ long long unsigned int wait_sum; /* 256 8 */
/* typedef u64 */ long long unsigned int iowait_count; /* 264 8 */
/* typedef u64 */ long long unsigned int iowait_sum; /* 272 8 */
/* typedef u64 */ long long unsigned int sleep_start; /* 280 8 */
/* typedef u64 */ long long unsigned int sleep_max; /* 288 8 */
- /* --- cacheline 1 boundary (64 bytes) --- */
/* typedef s64 */ long long int sum_sleep_runtime; /* 296 8 */
/* typedef u64 */ long long unsigned int block_start; /* 304 8 */
/* typedef u64 */ long long unsigned int block_max; /* 312 8 */
+ /* --- cacheline 5 boundary (320 bytes) --- */
/* typedef u64 */ long long unsigned int exec_max; /* 320 8 */
/* typedef u64 */ long long unsigned int slice_max; /* 328 8 */
/* typedef u64 */ long long unsigned int nr_migrations_cold; /* 336 8 */
[acme@jouet linux]$
I.e. the boundary detection was being reset at each expanded struct, do the math globally,
using the member offset, that was already done globally and correctly.
Reported-and-Tested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
That conf_fprintf can be elided as it is always NULL for the root call,
i.e. only when expanding types is that it will be called recursively.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Still need to check what to fprintf for this, but at least have it in
the type lists so that we can find it.
Reported-by: Christophe Fergeau <cfergeau@redhat.com>
Cc: Dodji Seketeli <dodji@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
DW_TAG_mutable_type was a mistake in an early DWARFv3 draft and was
removed in the final version.
http://dwarfstd.org/ShowIssue.php?issue=050223.1
Signed-off-by: Mark Wielaard <mjw@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
When computing the size of a class, leave, caused problems in
some cases, links to the reports are in the comments.
Reported-by: Nicolas <nikos42@gmail.com>
Suggested-by: Mark Wielaard <mjw@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
As Thomas Gleixner wisely pointed out, using 'self' is stupid, it
doesn't convey useful information, so use sensible names.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
RHEL5 and Fedora 11 were not building due to the GNU attributes stuff,
cope with that using a define we know is not present in both RHEL5 and
Fedora 11 to #ifdef those parts. Ugly, but _ELFUTILS_PREREQ, i.e.
elfutils/version.h is not present in RHEL5 either.
Reported-by: Jon Stanley <jstanley@fedoraproject.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
[acme@doppio pahole]$ pahole -F ctf /media/tb/debuginfo/usr/lib/debug/usr/bin/greycstoration4integration.debug > /tmp/bla
<ERROR(tag__size:837): detected type loop: type=572, tag=const_type>
<ERROR(tag__size:837): detected type loop: type=572, tag=const_type>
[acme@doppio pahole]$
These type loops are problems in the CTF encoding, that should be fixed, but
should not cause the core code to segfault on an infinite recursion.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
I.e.:
[acme@doppio pahole]$ cat tests/jengelh@medozas.de/const_const.c
struct x {
const char *const s;
} y;
int main(void)
{
return !y.s;
}
[acme@doppio pahole]$ pahole tests/jengelh@medozas.de/const_const
struct x {
char const * const s; /* 0 8 */
/* size: 8, cachelines: 1, members: 1 */
/* last cacheline: 8 bytes */
};
[acme@doppio pahole]$
One more reason to devote some time to RTT, i.e. Round Trip Testing, where
pahole will be used to regenerate the source code, then feed the result to
gcc -g, run again, use codiff, that should produce no diff.
Reported-by: Jan Engelhardt <jengelh@medozas.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
That asks dwarf_fprintf to always use "struct" in places where it would
use "class", because CTF doesn't have the "class" concept, so for
'regtest diffctf' sake, we use this.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
If it is C++ add DW_TAG_member entries to cu->tags_table and at
imported_declaration__fprintf fallback to cu__tag() if cu__function()
fails.
The right thing tho, long term, is to have a class for
"DW_TAG_imported_declaration" to register to what kind of tag this
points, if for DW_TAG_subprogram or to DW_TAG_member, the info is in the
DWARF DW_AT_import attribute, but so far we're not decoding it.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Next we'll add a new kind of tag, DW_TAG_perf_counter, that will come
from perf.data generated by 'perf report'.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
JAVA support needs to be checked, but from a very quick skim it
looks ok'ish.
First detected with /usr/bin/fstack from frysk.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Sharing the same space with abstract_origin, so that we can remove the last
Dwarf_Off in dwarf_fprintf.c.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>