Adding support for suppress_packed and leaving out bitfield paddings at the end
of classes when no alignment info is available accidentally used conf instead
of cconf, where conf can be NULL, which causes cconf to have the default
fprintf config.
Fixes: 986a3b58a8 ("fprintf: Only add bitfield forced paddings when alignment info available")
Fixes: 9a4d719304 ("fprintf: Allow suppressing the inferred __attribute__((__packed__))")
Signed-off-by: Timo Paulssen <timonator@perpetuum-immobile.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
When printing const more early we're not considering "const void *", fix
it.
Fixes: ccd67bdb20 ("fprintf: Print "const" for class members more early, in type__fprintf()")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
In cf459ca16f ("fprintf: Pretty print struct members that are pointers
to nameless structs") we end up breaking 'pahole --expand_types/-E.
All we need there is to print nameless structs when they are referenced
by a pointer member in a struct, i.e. it is defined only there, inline.
Check if that struct is unnamed before emitting it, keeping that fix
while fixing the regression it caused for --expand_types.
Fixes: cf459ca16f ("fprintf: Pretty print struct members that are pointers to nameless structs")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
btf_loader.c and the libbpf obtained via the git module are needed to
be able to build from the tarball generated via:
tar cvfj rpm/SOURCES/dwarves-1.13.tar.bz2 `cat MANIFEST`
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
If we have:
inline void process_adjtimex_modes(const struct __kernel_timex * txc, s32 * time_tai)
{
}
And any other struct receiving as a parameter pointers to 'struct
__kerne_timex', then the source file with the above inline, since it
doesn't have any inline expansion, i.e. 'pfunct --compile' generates
just empty function bodies, the types won't be included in the resulting
.o.
Since the original file has the expansions, type types will be there and
thus we will not be able to compare those types, so ask for any 'inline'
to be stripped, so that we keep those types and 'fullcircle' can do its
work.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This was happening here:
$ pfunct --compile /home/acme/git/build/v5.1-rc4+/block/partitions/check.o > a.c ; head -29 a.c
struct block_device;
struct (null);
typedef _Bool bool;
struct parsed_partitions {
struct block_device * bdev; /* 0 8 */
char name[32]; /* 8 32 */
struct {
sector_t from; /* 40 8 */
sector_t size; /* 48 8 */
int flags; /* 56 4 */
bool has_info; /* 60 1 */
struct partition_meta_info info; /* 61 101 */
} * parts; /* 40 8 */
int next; /* 48 4 */
int limit; /* 52 4 */
bool access_beyond_eod; /* 56 1 */
/* XXX 7 bytes hole, try to pack */
/* --- cacheline 1 boundary (64 bytes) --- */
char * pp_buf; /* 64 8 */
/* size: 72, cachelines: 2, members: 7 */
/* sum members: 65, holes: 1, sum holes: 7 */
/* last cacheline: 8 bytes */
};
$
I.e. we saw a pointer to a struct, so all we need is a forward
declaration for that function, right? Not if it is defined inline, so
just don't emit the forward declaration if the struct name is NULL.
Oops, the offsets in a struct defined inline and that the member is a
pointer need to have its offset restarted from zero...
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
So that we don't try to compile something that wasn't built with a
CFLAGS or even gcc, like .o made from ASM files.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
With this and running like:
$ find ~/git/build/v5.1-rc4+/fs/ -name "*.o" | grep -v .mod.o | \
while read obj ; do
echo $obj ;
fullcircle $obj ;
done
The vast majority of the kernel single compilation unit objects get
the source code for its function prototypes and types used rebuilt,
recompiled and then the original DWARF can be compared with the one
generated from the regenerated C code.
More work needed, but should be a good start and has already helped to
fix several issues, a reported in the previous csets.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Covering typedefs, unions.
Fixes: 99750f244c ("pfunct: Generate a valid return type for the --compile bodies")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We need to dig deeper in some emit cases, but for now lets make this
a bit more robust by checking if the types are unnamed.
Things like:
struct b {
struct {
int a;
};
}
This is well handled in the main fprintf routines, but its interaction
with the emit routines need more work, lets at least remove the segfault
and leave fixing this for good for v1.14.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
/home/acme/git/build/v5.1-rc4+/fs/proc/kcore.o
/tmp/fullcircle.Vnd2oz.c:788:29: error: flexible array member in a struct with no named members
char x[]; /* 0 0 */
Original:
include/linux/mmzone.h, line 109:
/*
* zone->lock and the zone lru_lock are two of the hottest locks in the kernel.
* So add a wild amount of padding here to ensure that they fall into separate
* cachelines. There are very few zone structures in the machine, so space
* consumption is not a concern here.
*/
#if defined(CONFIG_SMP)
struct zone_padding {
char x[0];
} ____cacheline_internodealigned_in_smp;
#define ZONE_PADDING(name) struct zone_padding name;
#else
#define ZONE_PADDING(name)
#endif
B0rken:
struct zone_padding {
char x[]; /* 0 0 */
/* size: 0, cachelines: 0, members: 1 */
} __attribute__((__aligned__(64)));
Fixed:
struct zone_padding {
char x[0]; /* 0 0 */
/* size: 0, cachelines: 0, members: 1 */
} __attribute__((__aligned__(64)));
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Consider:
struct ipc64_perm {
__kernel_key_t key;
__kernel_uid32_t uid;
__kernel_gid32_t gid;
__kernel_uid32_t cuid;
__kernel_gid32_t cgid;
__kernel_mode_t mode;
/* pad if mode_t is u16: */
unsigned char __pad1[4 - sizeof(__kernel_mode_t)];
unsigned short seq;
unsigned short __pad2;
__kernel_ulong_t __unused1;
__kernel_ulong_t __unused2;
};
That is a roundabout way of using __attribute__(__aligned__(4)), but
should work nonetheless.
We were not putting the [0] in that zero sized array which ended up
making gcc complain with:
$ gcc -g -c shm.c
shm.c:199:29: error: flexible array member not at end of struct
unsigned char __pad1[]; /* 24 0 */
^~~~~~
$
Now this works, i.e. generates compilable source code out of the
type tags, be it from BTF or from DWARF, i.e. this is all from the
internal representation of such types, agnostic wrt the original type
format.
So, the full circle:
$ pahole -C ipc64_perm /home/acme/git/build/v5.1-rc4+/ipc/shm.o
struct ipc64_perm {
__kernel_key_t key; /* 0 4 */
__kernel_uid32_t uid; /* 4 4 */
__kernel_gid32_t gid; /* 8 4 */
__kernel_uid32_t cuid; /* 12 4 */
__kernel_gid32_t cgid; /* 16 4 */
__kernel_mode_t mode; /* 20 4 */
unsigned char __pad1[0]; /* 24 0 */
short unsigned int seq; /* 24 2 */
short unsigned int __pad2; /* 26 2 */
/* XXX 4 bytes hole, try to pack */
__kernel_ulong_t __unused1; /* 32 8 */
__kernel_ulong_t __unused2; /* 40 8 */
/* size: 48, cachelines: 1, members: 11 */
/* sum members: 44, holes: 1, sum holes: 4 */
/* last cacheline: 48 bytes */
};
$ pfunct --compile /home/acme/git/build/v5.1-rc4+/ipc/shm.o > shm.c
$ gcc -g -c shm.c
$ pahole -C ipc64_perm shm.o
struct ipc64_perm {
__kernel_key_t key; /* 0 4 */
__kernel_uid32_t uid; /* 4 4 */
__kernel_gid32_t gid; /* 8 4 */
__kernel_uid32_t cuid; /* 12 4 */
__kernel_gid32_t cgid; /* 16 4 */
__kernel_mode_t mode; /* 20 4 */
unsigned char __pad1[0]; /* 24 0 */
short unsigned int seq; /* 24 2 */
short unsigned int __pad2; /* 26 2 */
/* XXX 4 bytes hole, try to pack */
__kernel_ulong_t __unused1; /* 32 8 */
__kernel_ulong_t __unused2; /* 40 8 */
/* size: 48, cachelines: 1, members: 11 */
/* sum members: 44, holes: 1, sum holes: 4 */
/* last cacheline: 48 bytes */
};
$
And for a chuckle, the original source code with a bit of history about
struct layout worries:
include/uapi/asm-generic/ipcbuf.h:
/*
* The generic ipc64_perm structure:
* Note extra padding because this structure is passed back and forth
* between kernel and user space.
*
* ipc64_perm was originally meant to be architecture specific, but
* everyone just ended up making identical copies without specific
* optimizations, so we may just as well all use the same one.
*
* Pad space is left for:
* - 32-bit mode_t on architectures that only had 16 bit
* - 32-bit seq
* - 2 miscellaneous 32-bit values
*/
struct ipc64_perm {
__kernel_key_t key;
__kernel_uid32_t uid;
__kernel_gid32_t gid;
__kernel_uid32_t cuid;
__kernel_gid32_t cgid;
__kernel_mode_t mode;
/* pad if mode_t is u16: */
unsigned char __pad1[4 - sizeof(__kernel_mode_t)];
unsigned short seq;
unsigned short __pad2;
__kernel_ulong_t __unused1;
__kernel_ulong_t __unused2;
};
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>
We use things like DW_AT_alignment, so not all of those attributes are
inferred by formats like BTF that lack that info, allow suppressing the
output and make btfdiff ask for both DWARF and BTF output to have this
suppressed.
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>
When the first arg, the old object file has multiple compile units, i.e.
multiple objects that were then linked into one, and the second just one
.o, or equivalent, i.e. a .BTF file, then codiff shouldn't try to
find the types in the single CU in each of the old CUs.
Think about a .BTF file generated from a multi-CU DWARF binary, it will
contain all the types in all of the DWARF CUs, so if we go on trying to
find all the BTF files in each of the CUs, we'll fail.
It only makes sense to go on the DWARF CUs looking for the type on the
.BTF section and then compare them.
Fixes: 6b1e43f2c1 ("codiff: When comparing against a file with just one CU don't bother finding by name")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Fixes: 13e5b9fc00 ("fprintf: Add unnamed bitfield padding at the end to rebuild original type")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Like in:
static int
cifs_setlease(struct file *file, long arg, struct file_lock **lease, void **priv)
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Check if the size is different than sizeof(int), which should be good
enough for now for both 64-bit and 32-bit targets.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
I.e. honour conf_fprintf.suppress_aligned_attribute, noticed with
btfdiff, as BTF doesn't carries the alignment attribute, so can't
regenerate it, we need to suppress it so as to compare the output of
DWARF with that of the equivalent BTF.
Fixes: b42d77b0bb ("fprintf: Print __attribute__((__aligned__(N))) for structs/classes")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
To match the case when we really have just one dimension, so the
--flat-arrays should show for zero sized arrays, [], not [0]:
Noticed with btfdiff.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We want to reach array__fprintf() from here, with the class_member
name, as __tag__name() isn't handling arrays properly.
I.e. to print an array when we have its name we can't use __tag__name().
This also stops printing 0 for zero sized arrays and trows away the
extra DW_TAG_const_type that comes with zero sized arrays, where we
have:
class_member type: DW_TAG_const_type 1
DW_TAG_const_type 1: DW_TAG_array_type 2
DW_TAG_array_type 2: 0 entries, type: DW_TAG_const_type 3
DW_TAG_const_type 3: real type of the zero sized array
For instance, after this patch we get a sane reconstruction of this
type:
$ pahole -C filename /home/acme/git/build/v5.0-rc2+/ipc/mqueue.o
struct filename {
const char * name; /* 0 8 */
const char * uptr; /* 8 8 */
int refcnt; /* 16 4 */
/* XXX 4 bytes hole, try to pack */
struct audit_names * aname; /* 24 8 */
const char iname[]; /* 32 0 */
/* size: 32, cachelines: 1, members: 5 */
/* sum members: 28, holes: 1, sum holes: 4 */
/* last cacheline: 32 bytes */
};
$
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>