We need to fix some bugs introduced recently, till then, disable steps
that try to demote the base type of bitfields and those that
move/combine bitfields to save space.
We'll revisit those later, bringing them back to the reorg codebase.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Mark Wielaard <mark@klomp.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>#
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
So that we can use it in things like btfdiff.
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Mark Wielaard <mark@klomp.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
DWARF got a DW_AT_alignment as described in:
http://dwarfstd.org/ShowIssue.php?issue=140528.1
This appeared first in DWARF5:
http://dwarfstd.org/doc/DWARF5.pdf
In:
----------------------------------------------------------------------
Chapter 2.
General Description
2.24 Alignment
A debugging information entry may have a DW_AT_alignment attribute whose
value of class constant is a positive, non-zero, integer describing the
alignment of the entity.
For example, an alignment attribute whose value is 8 indicates that the
entity to which it applies occurs at an address that is a multiple of
eight (not a multiple of 8 or 256)
----------------------------------------------------------------------
Use it on a struct present in the running kernel, i.e. not specifying
which ELF file to look for the DWARF info to use:
$ pahole -C inet_timewait_death_row
struct inet_timewait_death_row {
atomic_t tw_count; /* 0 4 */
/* XXX 60 bytes hole, try to pack */
/* --- cacheline 1 boundary (64 bytes) --- */
struct inet_hashinfo * hashinfo __attribute__((__aligned__(64)); /* 64 8 */
int sysctl_max_tw_buckets; /* 72 4 */
/* size: 128, cachelines: 2, members: 3 */
/* sum members: 16, holes: 1, sum holes: 60 */
/* padding: 52 */
};
$
Now to do some tweaking to get that "__attribute..." part nicely, hum,
aligned in the pahole output :-)
BTW: the original struct is in the kernel sources:
include/net/netns/ipv4.h
struct inet_timewait_death_row {
atomic_t tw_count;
struct inet_hashinfo *hashinfo ____cacheline_aligned_in_smp;
int sysctl_max_tw_buckets;
};
Reported-by: Mark Wielaard <mark@klomp.org>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
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>
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 records for each CU whether it's in little-endian or
big-endian data format. This flag will be used in subsequent commits to
adjust bit offsets where necessary, to make them uniform across
endianness. This patch doesn't have any effect on pahole's output.
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 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>
This patch uses similar approach to btf_loader's one and
adjusts/calculates bitfield parameters in such a way, that their byte
offset is always naturally aligned according to underlying base type
alignment requirement. This is consistent with btf_loader behavior and
helps to get closer to zero discrepancies between DWARF/BTF.
We also make sure that bitfield_offset never stays negative, which can
surprise some other parts of pahole logic.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: dwarves@vger.kernel.org
Cc: Mark Wielaard <mark@klomp.org>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Bitfield offsets can be negative, if field "borrows" few bits from
following aligned field. This causes a bunch of surprises down the line
in pahole's logic (e.g., for hole calculation logic), so instead of
waiting till printf routines adjust this for display, adjust them early
and keep less surprising semantics.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This patch disables bitfield recoding logic in DWARF loader.
This logic is already disabled during DWARF->BTF conversion and without
it pahole reliably produces correct BTF bitfield offsets.
If this functionality is enabled, we are losing correct enum size
information. So let's disable and probably eventually remove it altogether.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
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>
Get latest changes and fixes for libbpf. Most importantly it pulls in
enum fwd resolution in btf_dedup().
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>
After we made -C apply for unions we had to stop calling
class__find_holes() on them, but forgot to also filter them out when
using --packable, which lead us to, in print_packable_info to access
class->priv for unions, as they were not filtered out, which made
it think that class->priv had the reordered cloned class, b00m.
Fix it by filtering out unions when doing --packable.
Fixes: 3ffe5ba93b ("pahole: Do not apply 'struct class' filters to 'struct type'")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
With BTF dedup we end up with a CU with all the types, and with
something like an allmodconfig vmlinux image it ends up with more than
65535 types, so use uint32_t for the type IDs to accomodate that.
Reported-by: Andrii Nakryiko <andriin@fb.com>
Tested-by:Andrii Nakryiko <andrii.nakryiko@gmail.com>
Link: https://lore.kernel.org/bpf/CAEf4Bzb0SpvXdDKMMnUof==kp4Y0AP54bKFjeCzX_AsmDm7k7g@mail.gmail.com/
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
the CTF and BTF loaders come already with the id to use, while the DWARF
loader comes with a Dwarf_Off that needs to be converted into the
ptr_table index.
So keep the cu__add_tag(cu, tag, &id) method to ask ask for the index to
be allocated in the ptr_table and the result to come back via the 'id'
parameter, now a uint32_t and introduce a cu__add_tag_with_id(cu, tag, id)
method to indicate that the 'uint32_t id' is the one to use.
With this we can use a uint32_t for the id both on 32-bit and 64-bit
arches.
Reported-by: Andrii Nakryiko <andriin@fb.com>
Tested-by:Andrii Nakryiko <andrii.nakryiko@gmail.com>
Link: https://lore.kernel.org/bpf/CAEf4Bzb0SpvXdDKMMnUof==kp4Y0AP54bKFjeCzX_AsmDm7k7g@mail.gmail.com/
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We were using 'long', so that we could return -ENOMEM, but since we need
struct ptr_table members are already uint32_t, meaning we can use the
entire range, make the return be just an int and be just for error
reporting and pass a uint32_t pointer to return the index used for the
new entry.
Reported-by: Andrii Nakryiko <andriin@fb.com>
Tested-by:Andrii Nakryiko <andrii.nakryiko@gmail.com>
Link: https://lore.kernel.org/bpf/CAEf4Bzb0SpvXdDKMMnUof==kp4Y0AP54bKFjeCzX_AsmDm7k7g@mail.gmail.com/
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>
Latest version of libbpf has btf_dedup() fixes preventing it from stucking
in a loop on allyesconfig kernels.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: andrii.nakryiko@gmail.com
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
[ Fixed up to get the right upstream commit, the originally submitted wasn't at the github repo ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
BTF data can represent packed enums correctly without any special
handling from pahole side. Previously pahole's own `enum vscope` would
be omitted causing problems.
Original commit tried to generate correct struct bitfield member type if
the member is an enum. This was dated before kind_flag implementation.
Later, kind_flag support was added and now pahole always generates BTF
with kind_flag = 1 for structures with bitfield, where bitfield size is
encoded in btf_member, so this workaround is not needed any more.
Removing this "hack" makes handling it easier to handle packed enums
correctly.
Repro:
$ cat test/packed_enum.c
enum packed_enum {
VALUE1,
VALUE2,
VALUE3
} __attribute__((packed));
struct s {
int x;
enum packed_enum e;
int y;
};
int main()
{
struct s s;
return 0;
}
$ gcc -g -c test/packed_enum.c -o test/packed_enum.o
$ ~/local/pahole/build/pahole -JV test/packed_enum.o
File test/packed_enum.o:
[1] INT (anon) size=1 bit_offset=0 nr_bits=8 encoding=SIGNED
[2] STRUCT s kind_flag=0 size=12 vlen=3
x type_id=3 bits_offset=0
e type_id=1 bits_offset=32
y type_id=3 bits_offset=64
[3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
$ ~/local/pahole/build/pahole -F dwarf test/packed_enum.o
struct s {
int x; /* 0 4 */
enum packed_enum e; /* 4 1 */
/* XXX 3 bytes hole, try to pack */
int y; /* 8 4 */
/* size: 12, cachelines: 1, members: 3 */
/* sum members: 9, holes: 1, sum holes: 3 */
/* last cacheline: 12 bytes */
};
$ ~/local/pahole/build/pahole -F btf test/packed_enum.o
struct s {
int x; /* 0 4 */
nameless base type! e; /* 4 1 */
/* XXX 3 bytes hole, try to pack */
int y; /* 8 4 */
/* size: 12, cachelines: 1, members: 3 */
/* sum members: 9, holes: 1, sum holes: 3 */
/* last cacheline: 12 bytes */
};
Notice how pahole's log doesn't have a mention of encoding 'packed_enum'
(anonymous integer is generated instead), which causes 'nameless base
type!' output above.
Fix this change:
$ ~/local/pahole/build/pahole -JV test/packed_enum.o
File test/packed_enum.o:
[1] ENUM packed_enum size=1 vlen=3
VALUE1 val=0
VALUE2 val=1
VALUE3 val=2
[2] STRUCT s kind_flag=0 size=12 vlen=3
x type_id=3 bits_offset=0
e type_id=1 bits_offset=32
y type_id=3 bits_offset=64
[3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
$ ~/local/pahole/build/pahole -F btf test/packed_enum.o
struct s {
int x; /* 0 4 */
enum packed_enum e; /* 4 1 */
/* XXX 3 bytes hole, try to pack */
int y; /* 8 4 */
/* size: 12, cachelines: 1, members: 3 */
/* sum members: 9, holes: 1, sum holes: 3 */
/* last cacheline: 12 bytes */
};
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff test/packed_enum.o
Also verified on pahole, kernel and glibc:
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/pahole.debug
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/vmlinux4
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
Fixes: b18354f64c ("btf: Generate correct struct bitfield member types")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Without it we end up with these messages and a zero sized _Float128
type.
$ btfdiff libc-2.28.so.debug
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
--- /tmp/btfdiff.dwarf.M7kavg 2019-02-26 10:13:06.633184595 -0300
+++ /tmp/btfdiff.btf.vlWt5u 2019-02-26 10:13:06.640184669 -0300
@@ -2142,7 +2149,7 @@ struct ucontext_t {
/* last cacheline: 8 bytes */
};
union ieee854_float128 {
- _Float128 d; /* 0 16 */
+ _Float128 d; /* 0 0 */
struct {
unsigned int mantissa3:32; /* 0: 0 4 */
unsigned int mantissa2:32; /* 4: 0 4 */
After this patch these messages are gone and for pahole's needs we have
enough information, no need to wait for BTF to have explicit support for
floating point base types.
Reported-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Link: https://lore.kernel.org/bpf/20190226131156.GA26786@kernel.org/
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
For instance:
$ cat examples/andrii/hsw_tsx_tuning.c
// https://lore.kernel.org/bpf/CAEf4BzYoJgxYWX9iS7pda62cnGr5VOC70NGgqPurMNWP_w1daA@mail.gmail.com/
typedef unsigned long long u64;
typedef unsigned int u32;
union hsw_tsx_tuning {
struct {
u32 cycles_last_block : 32,
hle_abort : 1,
rtm_abort : 1,
instruction_abort : 1,
non_instruction_abort : 1,
retry : 1,
data_conflict : 1,
capacity_writes : 1,
capacity_reads : 1;
};
u64 value;
};
int main() {
union hsw_tsx_tuning t;
return 0;
}
$
When recoding the DWARF bitfield sizes we look for a base type that has
32 bits and that has the same name, in this case:
$ pahole --expand_types examples/andrii/hsw_tsx_tuning-gcc | grep cycles_last_block
/* typedef u32 */ unsigned int cycles_last_block:32; /* 0: 0 4 */
$
We look for a 'unsigned int' with 32 bits, the same way as we do for all
the other bitfield entries. That worked well when the bit_size wasn't
the same as existing base type like 'unsigned int', which ended up
making that field get recoded to point to 'unsigned int', effectively
removing the original name, 'u32' in the case above, with the base type
it ultimately points to, 'unsigned int'.
Fix it making it use the same logic as for bit sizes that are different
from the base_type it points to, i.e. go ahead and create a new typedef
pointing to to a new base_type that has the right bit_size.
So, to sum up, before this fix:
$ btfdiff hsw_tsx_tuning-gcc
--- /tmp/btfdiff.dwarf.ErXffk 2019-02-25 10:26:56.863625697 -0300
+++ /tmp/btfdiff.btf.Y5EDdM 2019-02-25 10:26:56.864625706 -0300
@@ -1,6 +1,6 @@
union hsw_tsx_tuning {
struct {
- unsigned int cycles_last_block:32; /* 0: 0 4 */
+ u32 cycles_last_block:32; /* 0: 0 4 */
u32 hle_abort:1; /* 4:31 4 */
u32 rtm_abort:1; /* 4:30 4 */
u32 instruction_abort:1; /* 4:29 4 */
$
Now btfdiff shows there are no differences, DWARF and BTF output produce
the same output, that matches the original struct.
The same tests were performed with vmlinux files produced by me and
Andrii.
Reported-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Andrii Nakryiko <andriin@fb.com>
Tested-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Existing code assumes alignment of any integer type, which breaks for
packed structs.
This patch fixes all the current discrepanies between dwarf and btf
loader, when compared using btfdiff.
It preserves bit_offset of non-bitfield members, while for bitfield ones
it re-calculates initial byte/bit offset using natural alignment of the
underlying integer type, which seems to be always the case for
bitfields.
I've tested this on toy examples for both x86-64 and arm targets, there
were no differences reported by btfdiff.
Testing on vmlinux on x86-64 shows only these discrepancies, which are
unrelated to bit offsets:
$ ./btfdiff /tmp/vmlinux4
--- /tmp/btfdiff.dwarf.GIVfpr 2019-02-20 12:18:29.138788970 -0800
+++ /tmp/btfdiff.btf.c3x2KY 2019-02-20 12:18:29.351786365 -0800
@@ -16884,7 +16884,7 @@ struct pebs_record_nhm {
};
union hsw_tsx_tuning {
struct {
- unsigned int cycles_last_block:32; /* 0: 0 4 */
+ u32 cycles_last_block:32; /* 0: 0 4 */
u32 hle_abort:1; /* 4:31 4 */
u32 rtm_abort:1; /* 4:30 4 */
u32 instruction_abort:1; /* 4:29 4 */
@@ -26154,7 +26154,7 @@ struct acpi_device_power {
/* last cacheline: 40 bytes */
};
struct acpi_device_perf_flags {
- unsigned char reserved:8; /* 0: 0 1 */
+ u8 reserved:8; /* 0: 0 1 */
/* size: 1, cachelines: 1, members: 1 */
/* last cacheline: 1 bytes */
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Tested-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
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
clang seems to generate base type with name "short", instead of "short
in", but it also isn't inconceivable to imagine other compilers
generating just "long" and/or "long long". This patch adds all those
short forms to a list of base type names.
$ cat type_test.c
struct s {
short x1;
long x2;
long long x3;
};
int main() {
struct s s;
return 0;
}
$ clang -g type_test.c -o type_test && ~/local/pahole/build/pahole -JV type_test
File type_test:
[1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[2] STRUCT s kind_flag=0 size=24 vlen=3
x1 type_id=3 bits_offset=0
x2 type_id=4 bits_offset=64
x3 type_id=5 bits_offset=128
[3] INT short size=2 bit_offset=0 nr_bits=16 encoding=SIGNED
[4] INT long int size=8 bit_offset=0 nr_bits=64 encoding=SIGNED
[5] INT long long int size=8 bit_offset=0 nr_bits=64 encoding=SIGNED
Before:
$ ~/local/pahole/build/pahole -F btf type_test
base_type__name_to_size: base_type short
class__fixup_btf_bitfields: unknown base type name "short"!
struct s {
short x1; /* 0 0 */
/* XXX 8 bytes hole, try to pack */
long int x2; /* 8 8 */
long long int x3; /* 16 8 */
/* size: 24, cachelines: 1, members: 3 */
/* sum members: 16, holes: 1, sum holes: 8 */
/* last cacheline: 24 bytes */
};
After:
$ ~/local/pahole/build/pahole -F btf type_test
struct s {
short x1; /* 0 2 */
/* XXX 6 bytes hole, try to pack */
long int x2; /* 8 8 */
long long int x3; /* 16 8 */
/* size: 24, cachelines: 1, members: 3 */
/* sum members: 18, holes: 1, sum holes: 6 */
/* last cacheline: 24 bytes */
};
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Tested-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
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
During development it's convenient to be able to specify custom location
of pahole binary, built locally.
E.g.:
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff /tmp/vmlinux4
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Tested-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
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
In DWARF we have the information if a struct/class is defined only
inside another struct/class or in a function, and then we consider those
to be 'private classes', requiring the use of --show_private_classes to
see those when asking for all structs.
That is not the case with BTF, that doesn't have that info and thus
shows all structs, private or not.
So, to compare the outputs in btfdiff we need to ask for
--show_private_classes when printing from DWARF.
With the Linux kernel vmlinux file the only private structure noticed
when not using this option, i.e. the only private class, as 'struct
sun_disklabel', defined in the block/partitions/sun.c file, inside the
'sun_partition' function.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
As libbpf is not yet widely available, it's safer to statically link it
into libdwarves for now. Easiest way to define that in cmake is through
OBJECT library with PIC.
Committer testing:
$ file build/pahole
build/pahole: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=666c97e5763ac0f4c5eff229be1532f1e60195e6, with debug_info, not stripped
$ ldd build/pahole
linux-vdso.so.1 (0x00007ffe5fdf8000)
libdwarves_reorganize.so.1 => /home/acme/git/pahole/build/libdwarves_reorganize.so.1 (0x00007f1c84fa4000)
libdwarves.so.1 => /home/acme/git/pahole/build/libdwarves.so.1 (0x00007f1c84f5e000)
libdw.so.1 => /lib64/libdw.so.1 (0x00007f1c84eee000)
libelf.so.1 => /lib64/libelf.so.1 (0x00007f1c84ed4000)
libz.so.1 => /lib64/libz.so.1 (0x00007f1c84eba000)
libc.so.6 => /lib64/libc.so.6 (0x00007f1c84cf4000)
libdl.so.2 => /lib64/libdl.so.2 (0x00007f1c84cec000)
liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f1c84cc3000)
libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f1c84cb0000)
/lib64/ld-linux-x86-64.so.2 (0x00007f1c84fad000)
libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f1c84c8e000)
$
$ nm build/libdwarves.so.1.0.0 | grep b[pt]f__
0000000000028aae T btf__dedup
0000000000027d44 T btf__fd
0000000000027a37 T btf__find_by_name
0000000000027ad9 T btf__free
0000000000027da8 T btf__get_from_id
0000000000027f6c T btf__get_map_kv_tids
0000000000027739 T btf__get_nr_types
0000000000027d55 T btf__get_raw_data
0000000000027c2e T btf__load
0000000000027d77 T btf__name_by_offset
0000000000027b36 T btf__new
00000000000277eb T btf__resolve_size
0000000000027960 T btf__resolve_type
000000000002774a T btf__type_by_id
$
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
Cc: kernel-team@fb.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We need to link with libbpf, that is not yet generally available, so we
need to link it into libdwarves for now, to do that we need to use the
OBJECT library with PIC, and that requires we use at least cmake version
2.8.8, so bump the minimum required cmake version.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
Cc: kernel-team@fb.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Bring in latest changes from libbpf which allow to use btf__dedup() for
big binaries (e.g., linux kernel image).
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: bpf@vger.kernel.org
Cc: dwarves@vger.kernel.org
Cc: kernel-team@fb.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
That is the idiom for free its members and then free itself, 'free' is
just to free its members.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
So that we don't clash with libbpf's 'struct btf', in time more internal
state now in 'struct btf_elf' will refer to the equivalent internal
state in libbpf's 'struct btf', as they have lots in common.
Requested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Acked-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Martin Lau <kafai@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Now that libbpf is a submodule, we don't need to copy/paste btf.h header
with BTF type definitions.
This is a first step in migrating parts of libbtf, btf_encoder and
btf_loader to use libbpf and starting to use btf__dedup().
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
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 change allows to use libbpf definitions and APIs from pahole.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
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>