As there are cases where we jump to the type not found label without
having done the full copy of *conf to tconf, so at least the
type_spacing needs to have been set.
This addresses this coverity report entry:
Error: UNINIT (CWE-457): [#def23]
dwarves-1.13/dwarves_fprintf.c:600: var_decl: Declaring variable "tconf" without initializer.
dwarves-1.13/dwarves_fprintf.c:779: uninit_use_in_call: Using uninitialized value "tconf.type_spacing" when calling "fprintf".
# 777| return printed;
# 778| out_type_not_found:
# 779|-> printed = fprintf(fp, "%-*s %s", tconf.type_spacing, "<ERROR>", name);
# 780| goto out;
# 781| }
Reported-by: William Cohen <wcohen@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
In some exit paths we were accessing tconf before we had copied it from
conf, and we also were losing track of the original type that could have
been expanded and where we bumped the recursivity level member, so just
store that original type and if it is set, decrement its recursivity
level.
This addresses these coverity report entries:
Error: UNINIT (CWE-457): [#def21]
dwarves-1.13/dwarves_fprintf.c:600: var_decl: Declaring variable "tconf" without initializer.
dwarves-1.13/dwarves_fprintf.c:774: uninit_use: Using uninitialized value "tconf.expand_types".
# 772| }
# 773| out:
# 774|-> if (tconf.expand_types)
# 775| --type->recursivity_level;
# 776|
Error: FORWARD_NULL (CWE-476): [#def22]
dwarves-1.13/dwarves_fprintf.c:605: var_compare_op: Comparing "type" to null implies that "type" might be null.
dwarves-1.13/dwarves_fprintf.c:775: var_deref_op: Dereferencing null pointer "type".
# 773| out:
# 774| if (tconf.expand_types)
# 775|-> --type->recursivity_level;
# 776|
# 777| return printed;
Reported-by: William Cohen <wcohen@redhat.com>
Fixes: f84bf73d54 ("dwarves: Move the fprintf code to a new source file.")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
[ 22%] Building C object CMakeFiles/dwarves.dir/dwarves_fprintf.c.o
/home/acme/git/pahole/dwarves_fprintf.c: In function ‘union__fprintf’:
/home/acme/git/pahole/dwarves_fprintf.c:963:34: error: pointer targets in assignment from ‘int *’ to ‘uint32_t *’ {aka ‘unsigned int *’} differ in signedness [-Werror=pointer-sign]
uconf.cachelinep = &cacheline;
^
cc1: all warnings being treated as errors
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
In cf459ca16f ("fprintf: Pretty print struct members that are pointers
to nameless structs") I added some recursive logic that theoretically
may end up doing an overlapping copy as reported by coverity:
Error: OVERLAPPING_COPY: [#def19]
dwarves-1.13/dwarves_fprintf.c:707: assign: Assigning: "name" = "namebfptr".
dwarves-1.13/dwarves_fprintf.c:705: equal: "name" is equal to the address of "namebfptr".
dwarves-1.13/dwarves_fprintf.c:705: overlapping_copy: In the call to function "snprintf", the arguments "name" and "namebfptr" may point to the same object.
# 703| if (tag__is_struct(ptype) || tag__is_union(ptype) ||
# 704| tag__is_enumeration(ptype)) {
# 705|-> snprintf(namebfptr, sizeof(namebfptr), "* %s", name);
# 706| tconf.rel_offset = 1;
# 707|
Look at cf459ca16f to see what this is about, but for now I'm just
checking if this is the case and adding a guard, at some point I'll
address this properly to allow for pointers to pointers to nameless
struct/union/enums.
Reported-by: William Cohen <wcohen@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
As we end up goto'ing to a place where we need that original type and
were having just a NULL pointer, oops, fix it.
Related to:
Error: NULL_RETURNS (CWE-476): [#def18]
dwarves-1.13/dwarves_fprintf.c:727: returned_null: "cu__type" returns "NULL" (checked 54 out of 62 times).
dwarves-1.13/dwarves_fprintf.c:727: var_assigned: Assigning: "type" = "NULL" return value from "cu__type".
dwarves-1.13/dwarves_fprintf.c:686: 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".
# 684|
# 685| next_type:
# 686|-> switch (type->tag) {
# 687| case DW_TAG_pointer_type:
# 688|
Reported-by: William Cohen <wcohen@redhat.com>
Fixes: 568dae4bd4 ("printf: Fixup printing "const" early with "const void"")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
A cut'n'paste error, noticed by coverity:
Error: PRINTF_ARGS: [#def16]
dwarves-1.13/dwarves_fprintf.c:369: extra_argument: This argument was not used by the format string: "conf->suffix". [Note: The source code implementation of the function has been overridden by a builtin model.]
# 367| */
# 368| if (type->size / 8 != sizeof(int))
# 369|-> printed += fprintf(fp, " __attribute__((__packed__))", conf->suffix);
# 370|
# 371| if (conf->suffix)
Reported-by: William Cohen <wcohen@redhat.com>
Fixes: 28a3bc7add ("fprintf: Support packed enums")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Will Cohen reported this NULL pointer dereference while processing some
object linking with cuda:
#0 0x00007ffff7f91453 in __class__fprintf (class=0x522560, cu=0x40ff80, conf=0x7fffffffa930, fp=0x7ffff7ece780 <_IO_2_1_stdout_>)
at /home/acme/git/pahole/dwarves_fprintf.c:1624
#1 0x00007ffff7f92195 in tag__fprintf (tag=0x522560, cu=0x40ff80, conf=0x7fffffffa930, fp=0x7ffff7ece780 <_IO_2_1_stdout_>)
at /home/acme/git/pahole/dwarves_fprintf.c:1835
#2 0x00007ffff7f90b57 in __class__fprintf (class=0x5224c0, cu=0x40ff80, conf=0x7fffffffaaa0, fp=0x7ffff7ece780 <_IO_2_1_stdout_>)
at /home/acme/git/pahole/dwarves_fprintf.c:1406
#3 0x00007ffff7f92195 in tag__fprintf (tag=0x5224c0, cu=0x40ff80, conf=0x40a200 <conf>, fp=0x7ffff7ece780 <_IO_2_1_stdout_>)
at /home/acme/git/pahole/dwarves_fprintf.c:1835
#4 0x0000000000402d03 in class_formatter (class=0x5224c0, cu=0x40ff80, id=1257) at /home/acme/git/pahole/pahole.c:224
#5 0x0000000000403074 in print_classes (cu=0x40ff80) at /home/acme/git/pahole/pahole.c:319
#6 0x0000000000404bb2 in pahole_stealer (cu=0x40ff80, conf_load=0x40a240 <conf_load>) at /home/acme/git/pahole/pahole.c:1174
#7 0x00007ffff7f9ff73 in finalize_cu (cus=0x40b2b0, cu=0x40ff80, dcu=0x7fffffffacf0, conf=0x40a240 <conf_load>)
at /home/acme/git/pahole/dwarf_loader.c:2227
#8 0x00007ffff7f9ffac in finalize_cu_immediately (cus=0x40b2b0, cu=0x40ff80, dcu=0x7fffffffacf0, conf=0x40a240 <conf_load>)
at /home/acme/git/pahole/dwarf_loader.c:2236
#9 0x00007ffff7fa064c in cus__load_module (cus=0x40b2b0, conf=0x40a240 <conf_load>, mod=0x40d760, dw=0x40e980, elf=0x40b360,
filename=0x7fffffffd5e3 "examples/wcohen/02_Exercise.cuda") at /home/acme/git/pahole/dwarf_loader.c:2389
#10 0x00007ffff7fa0760 in cus__process_dwflmod (dwflmod=0x40d760, userdata=0x40d770, name=0x40d910 "examples/wcohen/02_Exercise.cuda",
base=4194304, arg=0x7fffffffcf10) at /home/acme/git/pahole/dwarf_loader.c:2434
#11 0x00007ffff7f32be1 in dwfl_getmodules () from /lib64/libdw.so.1
#12 0x00007ffff7fa0820 in cus__process_file (cus=0x40b2b0, conf=0x40a240 <conf_load>, fd=3,
filename=0x7fffffffd5e3 "examples/wcohen/02_Exercise.cuda") at /home/acme/git/pahole/dwarf_loader.c:2487
#13 0x00007ffff7fa089c in dwarf__load_file (cus=0x40b2b0, conf=0x40a240 <conf_load>, filename=0x7fffffffd5e3 "examples/wcohen/02_Exercise.cuda")
at /home/acme/git/pahole/dwarf_loader.c:2504
#14 0x00007ffff7f8b0dd in cus__load_file (cus=0x40b2b0, conf=0x40a240 <conf_load>, filename=0x7fffffffd5e3 "examples/wcohen/02_Exercise.cuda")
at /home/acme/git/pahole/dwarves.c:1745
#15 0x00007ffff7f8bc2a in cus__load_files (cus=0x40b2b0, conf=0x40a240 <conf_load>, filenames=0x7fffffffd150)
at /home/acme/git/pahole/dwarves.c:2109
#16 0x0000000000404ff0 in main (argc=2, argv=0x7fffffffd148) at /home/acme/git/pahole/pahole.c:1294
(gdb)
(gdb) p class__name(class, cu)
$6 = 0x5cbb85 "__nv_hdl_helper_trait<__nv_dl_tag<int (*)(int, char**), main, 1u>, void (main(int, char**)::__lambda0::*)(int, double&)const>"
(gdb) p class->type.nr_members
$7 = 0
(gdb) p last
$8 = (struct class_member *) 0x0
(gdb)
So, before checking for bitfield details, first check if there were
members.
Now, if we show all structs/classes in that object file and look for the
above data structure, we find it inside another:
$ pahole examples/wcohen/02_Exercise.cuda
<SNIP>
struct __nv_hdl_helper_trait_outer<false, false, int, Kokkos::View<double**>, Kokkos::View<double*>, Kokkos::View<double*> > {
struct __nv_hdl_helper_trait<__nv_dl_tag<int (*)(int, char**), main, 1u>, void (main(int, char**)::__lambda0::*)(int, double&)const> {
class __nv_hdl_wrapper_t<false, false, __nv_dl_tag<int (*)(int, char**), main, 1u>, void(int, double&), int, Kokkos::View<doubl get<main(int, char**)::__lambda0>(class __lambda0, int, class View<double**>, class View<double*>, class View<double*>);
/* size: 1, cachelines: 0, members: 0 */
/* padding: 1 */
/* last cacheline: 1 bytes */
};
/* size: 1, cachelines: 0, members: 0 */
/* padding: 1 */
/* last cacheline: 1 bytes */
};
<SNIP>
$
Reported-by: William Cohen <wcohen@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>
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>
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>
/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>
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>
Fixes: 13e5b9fc00 ("fprintf: Add unnamed bitfield padding at the end to rebuild original type")
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>
I.e. when we find that the last member has a bit_hole, i.e. it is part
of a bitfield, and the current field has a bitfield_size, i.e. it _also_
is part of a bitfield, the only explanation is that they were
artificially put in different base types, i.e. like in these fields
in the linux kernel 'struct task_struct', here reconstructed by pahole:
$ pahole -C task_struct ~/git/build/v5.1-rc2+/kernel/sched/core.o | grep :0 -B9 -A12
unsigned int personality; /* 1128 4 */
unsigned int sched_reset_on_fork:1; /* 1132: 0 4 */
unsigned int sched_contributes_to_load:1; /* 1132: 1 4 */
unsigned int sched_migrated:1; /* 1132: 2 4 */
unsigned int sched_remote_wakeup:1; /* 1132: 3 4 */
/* XXX 28 bits hole, try to pack */
/* Force alignment to the next boundary: */
unsigned int :0;
unsigned int in_execve:1; /* 1136: 0 4 */
unsigned int in_iowait:1; /* 1136: 1 4 */
unsigned int restore_sigmask:1; /* 1136: 2 4 */
unsigned int in_user_fault:1; /* 1136: 3 4 */
unsigned int no_cgroup_migration:1; /* 1136: 4 4 */
unsigned int use_memdelay:1; /* 1136: 5 4 */
/* XXX 26 bits hole, try to pack */
/* XXX 4 bytes hole, try to pack */
long unsigned int atomic_flags; /* 1144 8 */
$
This matches the original definition in the original kernel sources, and
further more, the following sequence proves that with this and DW_AT_alignment,
we can go full circle, i.e.:
1. from an object file reconstruct the source code for all the types that
appears in function signatures, if pointers, them they will be fully defined,
not just forward declared:
$ pfunct --compile=sched_change_group ~/git/build/v5.1-rc2+/kernel/sched/core.o | egrep -w 'sched_change_group|task_struct {' -B10 -A5
/* --- cacheline 3 boundary (192 bytes) --- */
struct fpu fpu __attribute__((__aligned__(64))); /* 192 4160 */
/* size: 4352, cachelines: 68, members: 21 */
/* sum members: 4316, holes: 2, sum holes: 32 */
/* sum bitfield members: 2 bits, bit holes: 1, sum bit holes: 30 bits */
/* forced alignments: 1, forced holes: 1, sum forced holes: 28 */
};
struct task_struct {
struct thread_info thread_info; /* 0 16 */
/* XXX last struct has 4 bytes of padding */
volatile long int state; /* 16 8 */
--
/* --- cacheline 104 boundary (6656 bytes) --- */
struct thread_struct thread __attribute__((__aligned__(64))); /* 6656 4352 */
/* size: 11008, cachelines: 172, members: 207 */
/* sum members: 10902, holes: 16, sum holes: 98 */
/* sum bitfield members: 10 bits, bit holes: 2, sum bit holes: 54 bits */
/* paddings: 3, sum paddings: 14 */
/* forced alignments: 6, forced holes: 1, sum forced holes: 40 */
};
void sched_change_group(struct task_struct * tsk, int type)
{
}
$
2. Build the regenerated skeleton function + its types:
$ pfunct --compile=sched_change_group ~/git/build/v5.1-rc2+/kernel/sched/core.o > sched_change_group.c
$ gcc -g -c sched_change_group.c
$ file sched_change_group.o
sched_change_group.o: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), with debug_info, not stripped
$
3. Now lets see if the original 'struct task_struct' printed by pahole, matches
the the output printed by pahole for the DWARF info generated for the regenerated
'struct task_struct' source code in sched_change_group.c:
$ pahole -C task_struct sched_change_group.o | tail
/* --- cacheline 104 boundary (6656 bytes) --- */
struct thread_struct thread __attribute__((__aligned__(64))); /* 6656 4352 */
/* size: 11008, cachelines: 172, members: 207 */
/* sum members: 10902, holes: 16, sum holes: 98 */
/* sum bitfield members: 10 bits, bit holes: 2, sum bit holes: 54 bits */
/* paddings: 3, sum paddings: 14 */
/* forced alignments: 6, forced holes: 1, sum forced holes: 40 */
};
$ pahole -C task_struct ~/git/build/v5.1-rc2+/kernel/sched/core.o | tail
/* --- cacheline 104 boundary (6656 bytes) --- */
struct thread_struct thread __attribute__((__aligned__(64))); /* 6656 4352 */
/* size: 11008, cachelines: 172, members: 207 */
/* sum members: 10902, holes: 16, sum holes: 98 */
/* sum bitfield members: 10 bits, bit holes: 2, sum bit holes: 54 bits */
/* paddings: 3, sum paddings: 14 */
/* forced alignments: 6, forced holes: 1, sum forced holes: 40 */
};
$
Furthermore:
$ pahole -C task_struct ~/git/build/v5.1-rc2+/kernel/sched/core.o > /tmp/original
$ pahole -C task_struct sched_change_group.o > /tmp/regenerated
$ diff -u /tmp/original /tmp/regenerated
$
So one of the most complex data structures in the Linux kernel seems to be under control,
and it uses zero sized unnamed bitfields and __attribute__((aligned(N))), a DWARF5 goodie,
time to go tag v1.13!
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
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>
The last problem with 'pfunct --compile' at least for tcp.o:
Before:
$ pfunct --compile examples/tcp.o > tcp.pahole.c
$ gcc -c tcp.pahole.c -g
tcp.pahole.c:1808:48: error: unknown type name ‘u8const’; did you mean ‘const’?
inline void tcp_set_ca_state(struct sock * sk, u8const ca_state)
^~~~~~~
const
tcp.pahole.c:5346:56: error: unknown type name ‘intconst’; did you mean ‘const’?
inline void skb_set_tail_pointer(struct sk_buff * skb, intconst offset)
^~~~~~~~
const
tcp.pahole.c:5914:37: error: unknown type name ‘gfp_tconst’; did you mean ‘gfp_t’?
inline bool gfpflags_allow_blocking(gfp_tconst gfp_flags)
^~~~~~~~~~
gfp_t
tcp.pahole.c:5926:24: error: unknown type name ‘ktime_tconst’; did you mean ‘ktime_t’?
inline s64 ktime_to_ns(ktime_tconst kt)
^~~~~~~~~~~~
ktime_t
tcp.pahole.c:5939:54: warning: ‘struct timespec64const’ declared inside parameter list will not be visible outside of this definition or declaration
inline struct timespec timespec64_to_timespec(struct timespec64const ts64)
^~~~~~~~~~~~~~~
tcp.pahole.c:5939:70: error: parameter 1 (‘ts64’) has incomplete type
inline struct timespec timespec64_to_timespec(struct timespec64const ts64)
~~~~~~~~~~~~~~~~~~~~~~~^~~~
$
After:
$ pfunct --compile examples/tcp.o > tcp.pahole.c
$ gcc -c tcp.pahole.c -g
Because:
$ grep -A2 tcp_set_ca_state tcp.pahole.c
inline void tcp_set_ca_state(struct sock * sk, const u8 ca_state)
{
}
$
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>
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>
Arnaldo Carvalho de Melo
Timo Paulssen
Andrii Nakryiko
Domenico Andreoli