Fixing this coverity report:
Error: OVERRUN (CWE-119): [#def7]
dwarves-1.13/dwarf_loader.c:54: cond_at_most: Checking "tag < DW_TAG_MIPS_loop" implies that "tag" may be up to 16512 on the true branch.
dwarves-1.13/dwarf_loader.c:55: overrun-local: Overrunning array "dwarf_tags_warned" of 66 bytes at byte offset 16512 using index "tag" (which evaluates to 16512).
# 53|
# 54| if (tag < DW_TAG_MIPS_loop) {
# 55|-> if (dwarf_tags_warned[tag])
# 56| return;
# 57| dwarf_tags_warned[tag] = true;
Reported-by: William Cohen <wcohen@redhat.com>
Fixes: c7fa6a9a40 ("dwarf loader: Catch some more template tags")
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>
Just add it to the current lexblock.
This removes the warnings that started to appear with the fix in the
previous patch, i.e. these:
die__process_inline_expansion: DW_TAG_label (0xa) @ <0x3aefafe> not handled!
die__process_inline_expansion: DW_TAG_label (0xa) @ <0x3aeff8a> not handled!
die__process_inline_expansion: DW_TAG_label (0xa) @ <0x3af02e0> not handled!
Reported-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Yonghong Song <yhs@fb.com>
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>
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 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>
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>
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>
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>
This patch fixed two issues with BTF. One is related to struct/union
bitfield encoding and the other is related to forward type.
Issue #1 and solution:
======================
Current btf encoding of bitfield follows what pahole generates.
For each bitfield, pahole will duplicate the type chain and
put the bitfield size at the final int or enum type.
Since the BTF enum type cannot encode bit size,
commit b18354f64c ("btf: Generate correct struct bitfield
member types") workarounds the issue by generating
an int type whenever the enum bit size is not 32.
The above workaround is not ideal as we lost original type
in BTF. Another undesiable fact is the type duplication
as the pahole duplicates the type chain.
To fix this issue, this patch implemented a compatible
change for BTF struct type encoding:
. the bit 31 of type->info, previously reserved,
now is used to indicate whether bitfield_size is
encoded in btf_member or not.
. if bit 31 of struct_type->info is set,
btf_member->offset will encode like:
bit 0 - 23: bit offset
bit 24 - 31: bitfield size
if bit 31 is not set, the old behavior is preserved:
bit 0 - 31: bit offset
So if the struct contains a bit field, the maximum bit offset
will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum
bitfield size will be 255 which is enough for today as maximum
bitfield in compiler can be 128 where int128 type is supported.
A new global, no_bitfield_type_recode, is introduced and which
will be set to true if BTF encoding is enabled. This global
will prevent pahole duplicating the bitfield types to avoid
type duplication in BTF.
Issue #2 and solution:
======================
Current forward type in BTF does not specify whether the original
type is struct or union. This will not work for type pretty print
and BTF-to-header-file conversion as struct/union must be specified.
To fix this issue, similar to issue #1, type->info bit 31
is used. If the bit is set, it is union type. Otherwise, it is
a struct type.
Examples:
=========
-bash-4.4$ cat t.c
struct s;
union u;
typedef int ___int;
enum A { A1, A2, A3 };
struct t {
int a[5];
___int b:4;
volatile enum A c:4;
struct s *p1;
union u *p2;
} g;
-bash-4.4$ gcc -c -O2 -g t.c
Without this patch:
$ pahole -JV t.o
[1] TYPEDEF ___int type_id=2
[2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[3] ENUM A size=4 vlen=3
A1 val=0
A2 val=1
A3 val=2
[4] STRUCT t size=40 vlen=5
a type_id=5 bits_offset=0
b type_id=13 bits_offset=160
c type_id=15 bits_offset=164
p1 type_id=9 bits_offset=192
p2 type_id=11 bits_offset=256
[5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5
[6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none)
[7] VOLATILE (anon) type_id=3
[8] FWD s type_id=0
[9] PTR (anon) type_id=8
[10] FWD u type_id=0
[11] PTR (anon) type_id=10
[12] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none)
[13] TYPEDEF ___int type_id=12
[14] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED
[15] VOLATILE (anon) type_id=14
With this patch:
$ pahole -JV t.o
File t.o:
[1] TYPEDEF ___int type_id=2
[2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[3] ENUM A size=4 vlen=3
A1 val=0
A2 val=1
A3 val=2
[4] STRUCT t kind_flag=1 size=40 vlen=5
a type_id=5 bitfield_size=0 bits_offset=0
b type_id=1 bitfield_size=4 bits_offset=160
c type_id=7 bitfield_size=4 bits_offset=164
p1 type_id=9 bitfield_size=0 bits_offset=192
p2 type_id=11 bitfield_size=0 bits_offset=256
[5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5
[6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none)
[7] VOLATILE (anon) type_id=3
[8] FWD s struct
[9] PTR (anon) type_id=8
[10] FWD u union
[11] PTR (anon) type_id=10
The fix removed the type duplication, preserved the enum type for the
bitfield, and have correct struct/union information for the forward
type.
Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@fb.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This is DWARF specific, we don't have in CTF, AFAIK, info about where a
variable is put, i.e. in a register? in the stack? etc.
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>
This allows processing DW_TAG_partial_unit sections, which gets us from
no tags processed in files contained such tags to at least showing the
tags present in those sections.
Further work is required to support DW_TAG_compile_unit sections using
DW_TAG_imported_unit to import those DW_TAG_partial_unit sections, which
will be done by basically readding the contents of the
DW_TAG_partial_unit sections to the DW_TAG_compile_unit sections
importing them and then recoding as if all the tags in the partial units
were in the compile units.
This will make sure we have a contiguous series of types used in a
compile unit so that the converting routines to CTF and BTF can work
just as before.
On a fedora 27 system:
Before:
$ pahole /usr/lib/debug/usr/lib64/libgtk-3.so.0.2200.19.debug
die__process: DW_TAG_compile_unit or DW_TAG_type_unit expected got partial_unit!
$
After:
$ pahole /usr/lib/debug/usr/lib64/libgtk-3.so.0.2200.19.debug
struct _GTimeVal {
glong tv_sec; /* 0 8 */
glong tv_usec; /* 8 8 */
/* size: 16, cachelines: 1, members: 2 */
/* last cacheline: 16 bytes */
};
struct _GError {
GQuark domain; /* 0 4 */
gint code; /* 4 4 */
gchar * message; /* 8 8 */
/* size: 16, cachelines: 1, members: 3 */
/* last cacheline: 16 bytes */
};
struct _GCond {
gpointer p; /* 0 8 */
guint i[2]; /* 8 8 */
/* size: 16, cachelines: 1, members: 2 */
/* last cacheline: 16 bytes */
};
<SNIP some more structs found in DW_TAG_partial unit sections...>
struct _GSourceFuncs {
gboolean (*prepare)(GSource *, gint *); /* 0 8 */
gboolean (*check)(GSource *); /* 8 8 */
gboolean (*dispatch)(GSource *, GSourceFunc, gpointer); /* 16 8 */
void (*finalize)(GSource *); /* 24 8 */
GSourceFunc closure_callback; /* 32 8 */
GSourceDummyMarshal closure_marshal; /* 40 8 */
/* size: 48, cachelines: 1, members: 6 */
/* last cacheline: 48 bytes */
};
struct _GThreadFunctions {
GMutex * (*mutex_new)(void); /* 0 8 */
void (*mutex_lock)(GMutex *); /* 8 8 */
gboolean (*mutex_trylock)(GMutex *)tag__recode_dwarf_type: couldn't find 0x74 type for 0x7fc (typedef)!
tag__recode_dwarf_type: couldn't find 0x7e type for 0x829 (pointer_type)!
tag__recode_dwarf_type: couldn't find 0x829 type for 0x844 (variable)!
tag__recode_dwarf_type: couldn't find 0x7e type for 0x850 (variable)!
tag__recode_dwarf_type: couldn't find 0x22 type for 0x85b (variable)!
tag__recode_dwarf_type: couldn't find 0x22 type for 0x866 (variable)!
tag__recode_dwarf_type: couldn't find 0x22 type for 0x871 (variable)!
namespace__recode_dwarf_types: couldn't find 0x7fc type for 0x8a4 (member)!
tag__recode_dwarf_type: couldn't find 0xfa type for 0x8e8 (volatile_type)!
namespace__recode_dwarf_types: couldn't find 0x8b2 type for 0x90d (member)!
tag__recode_dwarf_type: couldn't find 0x1b type for 0x941 (typedef)!
<SNIP>
$
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
We need to get it under some ifdefs to avoid breaking the build in
distros where these tags are not defined.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This causes a segfault:
at /home/teuf/freesoftware/pahole/dwarf_loader.c:141
at /home/teuf/freesoftware/pahole/dwarf_loader.c:170
cu=0x66bc70) at /home/teuf/freesoftware/pahole/dwarf_loader.c:1535
at /home/teuf/freesoftware/pahole/dwarf_loader.c:1552
fn=0x7ffff79cd160 <__FUNCTION__.8133> "die__process_class")
at /home/teuf/freesoftware/pahole/dwarf_loader.c:1593
at /home/teuf/freesoftware/pahole/dwarf_loader.c:1288
We're not supposed to hash those unsupported tags, so just check for it and
avoid hashing.
Reported-by: Christophe Fergeau <cfergeau@redhat.com>
Cc: Dodji Seketeli <dodji@redhat.com>
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>
No tool in this suite has any use for that, so just leave the comments
and pointers to the documentation for these tags and stop bothering the
user.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Need to read more on http://www.artima.com/cppsource/rvalue.html, but
handling it mostly like DW_TAG_typedef so that at least references to it
are resolved, we can get its byte size, etc.
FIXME: look at the vtable parameters, some are resolving to "(null)".
Reported-by: Benjamin Kosnik <bkoz@redhat.com>
Reported-by: Mark Wieelard <mjw@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=962571
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
In class_member__dwarf_recode_bitfield, making it more robust, as
now we're in the process of supporting DW_TAG_partial_unit, where
some types seem to be in some ther unit and thus not being found
at the moment.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
It just about having a location expression that is referenced by other
tags to save space, since we don't have use for loc exprs so far, just
ignore it instead of spamming the user with "tag not supported"
warnings.
First seen on binaries generated by gcc 4.9
Reported-by: Diego Elio Pettenò <flameeyes@flameeyes.eu>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
obstack_alloc was used in __tag__alloc to allocate tag object. As the
result some fields of the tag object are not initialized.
Signed-off-by: Masatake YAMATO <yamato@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>
After emitting a warning that a tag is not supported __die__process_tag
was returning NULL, making die__process_unit think that the problem
was insufficient memory.
Introduce a global variable 'unsupported_tag' and return it instead,
that way die__process_unit can distinguish ENOMEM from unsupported tags.
Reported-by: Thiago Macieira <thiago.macieira@intel.com>
Tested-by: Thiago Macieira <thiago.macieira@intel.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
The 2938a70 changeset incorectly changed some searches that needed to be
done on the all tags hashtable to the type only one.
This lead to problems such as:
[acme@sandy pahole]$ build/pahole /usr/lib/debug/usr/libexec/pt_chown.debug > /dev/null
tag__recode_dwarf_type: couldn't find 0xf7a type for 0x1133 (inlined_subroutine)!
tag__recode_dwarf_type: couldn't find 0x1047 type for 0x1166 (inlined_subroutine)!
tag__recode_dwarf_type: couldn't find 0xf7a type for 0x11a8 (inlined_subroutine)!
lexblock__recode_dwarf_types: couldn't find 0xf7a type for 0x1133 (inlined_subroutine)!
lexblock__recode_dwarf_types: couldn't find 0x1047 type for 0x1166 (inlined_subroutine)!
lexblock__recode_dwarf_types: couldn't find 0xf7a type for 0x11a8 (inlined_subroutine)!
[acme@sandy pahole]$
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
.debug_types is a new DWARF 4 feature that adds some simple
compression to DWARF. The basic idea is that identical type
definitions are merged by the linker and put into a new .debug_types
section.
This introduces 'dwarf_off_ref', which is like Dwarf_Off, but carries
a flag indicating if the DIE came from .debug_types. This allows
future uses to find the DIE properly.
Type units are read a little differently from ordinary CUs. All type
units are read at once, then types are recoded for them. (I think
something like this is needed more generally, to support inter-CU
references, but I have not tried that.)
The type unit is also kept around longer, because any other CU may
refer to it. This necessitated a change to load_steal_kind to replace
the notion of a "stolen" CU with a request by the "stealer" for the
caller to delete the CU when appropriate.
I need elfutils patch 581c3f60e2b1fc7ddaf4260bb5a9cb59f8e3f0d0
to make this work properly; without it I see crashes.
You can make test cases by compiling with '-gdwarf-4 -fdebug-types-section'.
Before this patch, running pahole on an executable compiled
with -gdwarf-4 gave:
DW_AT_<0x3c>=0x19
0x19 is DW_FORM_flag_present.
Also, DW_FORM_flag takes an argument.
Changing attr_numeric to call dwarf_formflag fixes both problems.
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>
Andrii Nakryiko
Arnaldo Carvalho de Melo
Domenico Andreoli
Yonghong Song
Masatake YAMATO
Tom Tromey