diff mbox series

[1/1] pahole/Rust: Check that we're adding DW_TAG_member sorted by byte offset

Message ID Y+atpJV5rqo08dQJ@kernel.org (mailing list archive)
State Not Applicable
Delegated to: BPF
Headers show
Series [1/1] pahole/Rust: Check that we're adding DW_TAG_member sorted by byte offset | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-PR fail merge-conflict
bpf/vmtest-bpf-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-7 success Logs for llvm-toolchain
bpf/vmtest-bpf-VM_Test-8 success Logs for set-matrix
bpf/vmtest-bpf-VM_Test-9 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-10 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-11 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-VM_Test-12 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-13 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-14 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-15 success Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-16 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-VM_Test-17 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-18 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-19 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-20 success Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-21 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-VM_Test-22 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-23 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-25 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-26 success Logs for test_progs_no_alu32_parallel on s390x with gcc
bpf/vmtest-bpf-VM_Test-27 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-28 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-29 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-30 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-31 success Logs for test_progs_parallel on s390x with gcc
bpf/vmtest-bpf-VM_Test-32 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-33 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-34 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-35 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-36 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-37 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-38 success Logs for test_verifier on x86_64 with llvm-16

Commit Message

Arnaldo Carvalho de Melo Feb. 10, 2023, 8:48 p.m. UTC 
Hi Miguel, after a long winter, I'm trying to get Rust properly
supported on pahole, please check that this specific use case is working
for you as well.

I'll go thru the others to see if they are easy (or at least restricted
to Rust CUs) as this one.

Thanks,

- Arnaldo

---

Rust may reorder struct fields and pahole assumes them to be in order,
as is the case for languages like C and C++, etc. So after having the
class member bit and byte offsets sorted out, sort Rust CU types by
offset.

Using: https://github.com/Rust-for-Linux/pahole-rust-cases/blob/main/inverted.o

Before:

  $ pahole --show_private_classes ../pahole-rust-cases/inverted.o
  struct S {

  	/* XXX 4 bytes hole, try to pack */

  	bool                       a __attribute__((__aligned__(1))); /*     4     1 */

  	/* XXX 65531 bytes hole, try to pack */
  	/* Bitfield combined with previous fields */

  	u32                        b __attribute__((__aligned__(4))); /*     0     4 */

  	/* size: 8, cachelines: 1, members: 2 */
  	/* sum members: 5, holes: 2, sum holes: 65535 */
  	/* padding: 4 */
  	/* forced alignments: 2, forced holes: 2, sum forced holes: 65535 */
  	/* last cacheline: 8 bytes */

  	/* BRAIN FART ALERT! 8 bytes != 5 (member bytes) + 0 (member bits) + 65535 (byte holes) + 0 (bit holes), diff = -524288 bits */
  } __attribute__((__aligned__(4)));
  $

After:

  $ readelf -wi ../pahole-rust-cases/inverted.o | grep DW_TAG_compile_unit -A9
   <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
      <c>   DW_AT_producer    : (indirect string, offset: 0x0): clang LLVM (rustc version 1.60.0 (7737e0b5c 2022-04-04))
      <10>   DW_AT_language    : 28	(Rust)
      <12>   DW_AT_name        : (indirect string, offset: 0x39): inverted.rs/@/inverted.c4dda47b-cgu.0
      <16>   DW_AT_stmt_list   : 0x0
      <1a>   DW_AT_comp_dir    : (indirect string, offset: 0x5f): /root/pahole-rust
      <1e>   DW_AT_GNU_pubnames: 1
      <1e>   DW_AT_low_pc      : 0x0
      <26>   DW_AT_high_pc     : 0x62
   <1><2a>: Abbrev Number: 2 (DW_TAG_namespace)
  $ pahole --show_private_classes ../pahole-rust-cases/inverted.o
  struct S {
  	u32                        b __attribute__((__aligned__(4))); /*     0     4 */
  	bool                       a __attribute__((__aligned__(1))); /*     4     1 */

  	/* size: 8, cachelines: 1, members: 2 */
  	/* padding: 3 */
  	/* forced alignments: 2 */
  	/* last cacheline: 8 bytes */
  } __attribute__((__aligned__(4)));
  $

  $ cp ../pahole-rust-cases/inverted.o .
  $ pahole --btf_encode inverted.o
  $ readelf -SW inverted.o  | grep -i BTF
    [26] .BTF              PROGBITS        0000000000000000 000922 00006c 00      0   0  1
  $
  $ pahole -F btf inverted.o
  struct S {
  	u32                        b;                    /*     0     4 */
  	bool                       a;                    /*     4     1 */

  	/* size: 8, cachelines: 1, members: 2 */
  	/* padding: 3 */
  	/* last cacheline: 8 bytes */
  };
  $

Reported-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Eric Curtin <ecurtin@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Cc: Neal Gompa <neal@gompa.dev>
Cc: Yonghong Song <yhs@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 dwarf_loader.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

Comments

Arnaldo Carvalho de Melo Feb. 13, 2023, 12:09 p.m. UTC | #1 
Em Fri, Feb 10, 2023 at 05:48:36PM -0300, Arnaldo Carvalho de Melo escreveu:
> I'll go thru the others to see if they are easy (or at least restricted
> to Rust CUs) as this one.

The namespace.o seems to be ok:

⬢[acme@toolbox pahole]$ cat ../pahole-rust-cases/namespace.rs
pub struct S {
    pub a: i32,
}

pub static S: (i32, S) = (42, S { a: 42 });
⬢[acme@toolbox pahole]$

⬢[acme@toolbox pahole]$ pahole --show_private_classes ../pahole-rust-cases/namespace.o
struct S {
	i32                        a __attribute__((__aligned__(4))); /*     0     4 */

	/* size: 4, cachelines: 1, members: 1 */
	/* forced alignments: 1 */
	/* last cacheline: 4 bytes */
} __attribute__((__aligned__(4)));
struct (i32, namespace::S) {
	i32                        __0 __attribute__((__aligned__(4))); /*     0     4 */
	struct S                   __1 __attribute__((__aligned__(4))); /*     4     4 */

	/* size: 8, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 8 bytes */
} __attribute__((__aligned__(4)));
⬢[acme@toolbox pahole]$

And encoding/decoding BTF for it:

⬢[acme@toolbox pahole]$ cp ../pahole-rust-cases/namespace.o .
⬢[acme@toolbox pahole]$ pahole --btf_encode namespace.o
⬢[acme@toolbox pahole]$ pahole -F btf namespace.o
struct S {
	i32                        a;                    /*     0     4 */

	/* size: 4, cachelines: 1, members: 1 */
	/* last cacheline: 4 bytes */
};
struct (i32, namespace::S) {
	i32                        __0;                  /*     0     4 */
	struct S                   __1;                  /*     4     4 */

	/* size: 8, cachelines: 1, members: 2 */
	/* last cacheline: 8 bytes */
};
⬢[acme@toolbox pahole]$ readelf -SW namespace.o | grep BTF
  [18] .BTF              PROGBITS        0000000000000000 00065c 000089 00      0   0  1
⬢[acme@toolbox pahole]$


The core one needs work:

⬢[acme@toolbox pahole]$ pahole ../pahole-rust-cases/core.o |& head
die__process_class: tag not supported 0x2f (template_type_parameter)!
die__process_class: tag not supported 0x33 (variant_part)!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x2fd8d> not handled!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x2fdf7> not handled!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x2fe61> not handled!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x2fecb> not handled!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x2ff35> not handled!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x2ff9f> not handled!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x30009> not handled!
die__create_new_enumeration: DW_TAG_subprogram (0x2e) @ <0x30073> not handled!
⬢[acme@toolbox pahole]$

 <1><90>: Abbrev Number: 7 (DW_TAG_namespace)
    <91>   DW_AT_name        : (indirect string, offset: 0x147): core
 <2><95>: Abbrev Number: 7 (DW_TAG_namespace)
    <96>   DW_AT_name        : (indirect string, offset: 0x14c): str
 <3><9a>: Abbrev Number: 7 (DW_TAG_namespace)
    <9b>   DW_AT_name        : (indirect string, offset: 0x150): iter
 <4><9f>: Abbrev Number: 8 (DW_TAG_structure_type)
    <a0>   DW_AT_name        : (indirect string, offset: 0x2e1): Split<core::str::IsWhitespace>
    <a4>   DW_AT_byte_size   : 64
    <a5>   DW_AT_alignment   : 8
 <5><a6>: Abbrev Number: 9 (DW_TAG_template_type_param)
    <a7>   DW_AT_type        : <0x41fc>
    <ab>   DW_AT_name        : (indirect string, offset: 0x162): P
 <5><af>: Abbrev Number: 4 (DW_TAG_member)
    <b0>   DW_AT_name        : (indirect string, offset: 0x164): __0
    <b4>   DW_AT_type        : <0xbb>
    <b8>   DW_AT_alignment   : 8
    <b9>   DW_AT_data_member_location: 0
 <5><ba>: Abbrev Number: 0
 <4><bb>: Abbrev Number: 8 (DW_TAG_structure_type)
    <bc>   DW_AT_name        : (indirect string, offset: 0x2ba): SplitInternal<core::str::IsWhitespace>
    <c0>   DW_AT_byte_size   : 64
    <c1>   DW_AT_alignment   : 8
 <5><c2>: Abbrev Number: 9 (DW_TAG_template_type_param)
    <c3>   DW_AT_type        : <0x41fc>
    <c7>   DW_AT_name        : (indirect string, offset: 0x162): P

- Arnaldo
Miguel Ojeda Feb. 13, 2023, 12:45 p.m. UTC | #2 
Hi Arnaldo,

On Mon, Feb 13, 2023 at 1:09 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> The namespace.o seems to be ok:

I saw the other message too -- this looks great, thanks a ton.

> The core one needs work:

If `core.o` works, then I think it is likely other things will work :)

I can try to extract the cases for those into simpler `.o` files, if
you would find simpler test cases useful (perhaps for the test suite
etc.).

Cheers,
Miguel
Eric Curtin Feb. 13, 2023, 12:53 p.m. UTC | #3 
On Mon, 13 Feb 2023 at 12:45, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Hi Arnaldo,
>
> On Mon, Feb 13, 2023 at 1:09 PM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > The namespace.o seems to be ok:
>
> I saw the other message too -- this looks great, thanks a ton.
>
> > The core one needs work:
>
> If `core.o` works, then I think it is likely other things will work :)

Hi Guys,

I'll leave this to the experts, but if we get this to the point where
we are happy to enable again for Rust CUs, could we request another
version bump? It just makes it easier to integrate with the kernel
scripts when we want to enable again.

>
> I can try to extract the cases for those into simpler `.o` files, if
> you would find simpler test cases useful (perhaps for the test suite
> etc.).
>
> Cheers,
> Miguel
>
Arnaldo Carvalho de Melo Feb. 13, 2023, 9:05 p.m. UTC | #4 
Em Mon, Feb 13, 2023 at 01:45:02PM +0100, Miguel Ojeda escreveu:
> On Mon, Feb 13, 2023 at 1:09 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > The namespace.o seems to be ok:
 
> I saw the other message too -- this looks great, thanks a ton.
 
> > The core one needs work:
 
> If `core.o` works, then I think it is likely other things will work :)
 
> I can try to extract the cases for those into simpler `.o` files, if
> you would find simpler test cases useful (perhaps for the test suite
> etc.).

That would be great!

I tried starting this path with this, spitted out by ChatGPT (minus the
'pub' in from of main):

⬢[acme@toolbox pahole-rust-cases]$ cat template_type.rs
// Provided by ChatGPT

use std::ops::Add;

// Define a generic struct template with a single type parameter `T`.
struct Point<T> {
    x: T,
    y: T,
}

// Implement a method for the `Point` struct that adds two points together.
// The method uses the `Add` trait to ensure that the type `T` supports addition.
impl<T: Add<Output=T>> Point<T> {
    fn add_points(self, other: Point<T>) -> Point<T> {
        Point {
            x: self.x + other.x,
            y: self.y + other.y,
        }
    }
}

pub fn main() {
    // Create two `Point` instances, using `i32` as the type parameter.
    let p1 = Point { x: 1, y: 2 };
    let p2 = Point { x: 3, y: 4 };

    // Add the two points together using the `add_points` method.
    let p3 = p1.add_points(p2);

    // Print the result.
    println!("Point: ({}, {})", p3.x, p3.y);
}
⬢[acme@toolbox pahole-rust-cases]$

And then handle the DW_TAG_template_type_parameter for a
DW_TAG_subroutine:

 <3><12d>: Abbrev Number: 5 (DW_TAG_structure_type)
    <12e>   DW_AT_name        : (indirect string, offset: 0x299): ArgumentV1
    <132>   DW_AT_byte_size   : 16
    <133>   DW_AT_alignment   : 8
 <4><134>: Abbrev Number: 6 (DW_TAG_member)
    <135>   DW_AT_name        : (indirect string, offset: 0xd7): value
    <139>   DW_AT_type        : <0x43a>
    <13d>   DW_AT_alignment   : 8
    <13e>   DW_AT_data_member_location: 0
 <4><13f>: Abbrev Number: 6 (DW_TAG_member)
    <140>   DW_AT_name        : (indirect string, offset: 0x10e): formatter
    <144>   DW_AT_type        : <0x447>
    <148>   DW_AT_alignment   : 8
    <149>   DW_AT_data_member_location: 8
 <4><14a>: Abbrev Number: 11 (DW_TAG_subprogram)
    <14b>   DW_AT_linkage_name: (indirect string, offset: 0x2a8): _ZN4core3fmt10ArgumentV13new17h167b8c43a2ee7614E
    <14f>   DW_AT_name        : (indirect string, offset: 0x2d9): new<i32>
    <153>   DW_AT_decl_file   : 2
    <154>   DW_AT_decl_line   : 333
    <156>   DW_AT_type        : <0x12d>
    <15a>   DW_AT_inline      : 1       (inlined)
 <5><15b>: Abbrev Number: 12 (DW_TAG_template_type_param)
    <15c>   DW_AT_type        : <0x4e3>
    <160>   DW_AT_name        : (indirect string, offset: 0x125): T
 <5><164>: Abbrev Number: 13 (DW_TAG_variable)

I'll have to stop now, but was at the point of changing ftype__fprintf()
somehow to show that template type parameter, with the patch at the end
of this message I get this:

⬢[acme@toolbox pahole]$ pahole --show_private_classes ../pahole-rust-cases/template_type.o
die__process_class: tag not supported 0x33 (variant_part)!
die__process_class: tag not supported 0x2f (template_type_parameter)!
struct Argument {
	usize                      position __attribute__((__aligned__(8))); /*     0     8 */
	struct FormatSpec          format __attribute__((__aligned__(8))); /*     8    48 */

	/* XXX last struct has 7 bytes of padding */

	/* size: 56, cachelines: 1, members: 2 */
	/* paddings: 1, sum paddings: 7 */
	/* forced alignments: 2 */
	/* last cacheline: 56 bytes */
} __attribute__((__aligned__(8)));
struct FormatSpec {
	struct Count               precision __attribute__((__aligned__(8))); /*     0    16 */

	/* XXX last struct has 16 bytes of padding */

	struct Count               width __attribute__((__aligned__(8))); /*    16    16 */

	/* XXX last struct has 16 bytes of padding */

	char                       fill __attribute__((__aligned__(4))); /*    32     4 */
	u32                        flags __attribute__((__aligned__(4))); /*    36     4 */
	enum Alignment             align __attribute__((__aligned__(1))); /*    40     1 */

	/* size: 48, cachelines: 1, members: 5 */
	/* padding: 7 */
	/* paddings: 2, sum paddings: 32 */
	/* forced alignments: 5 */
	/* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));
struct Is {

	/* XXX 8 bytes hole, try to pack */

	usize                      __0 __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 1 */
	/* sum members: 8, holes: 1, sum holes: 8 */
	/* forced alignments: 1, forced holes: 1, sum forced holes: 8 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Param {

	/* XXX 8 bytes hole, try to pack */

	usize                      __0 __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 1 */
	/* sum members: 8, holes: 1, sum holes: 8 */
	/* forced alignments: 1, forced holes: 1, sum forced holes: 8 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Implied {

	/* size: 16, cachelines: 1, members: 0 */
	/* padding: 16 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Count {

	/* size: 16, cachelines: 1, members: 0 */
	/* padding: 16 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct ArgumentV1 {
	struct Opaque *            value __attribute__((__aligned__(8))); /*     0     8 */
	struct Result<(), core::fmt::Error> (*formatter)(struct Opaque *, struct Formatter *) __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Opaque {

	/* size: 0, cachelines: 0, members: 0 */
} __attribute__((__aligned__(1)));
struct Error {

	/* size: 0, cachelines: 0, members: 0 */
} __attribute__((__aligned__(1)));
struct Formatter {
	struct Option<usize>       width __attribute__((__aligned__(8))); /*     0    16 */

	/* XXX last struct has 16 bytes of padding */

	struct Option<usize>       precision __attribute__((__aligned__(8))); /*    16    16 */

	/* XXX last struct has 16 bytes of padding */

	struct &mut dyn core::fmt::Write buf __attribute__((__aligned__(8))); /*    32    16 */
	u32                        flags __attribute__((__aligned__(4))); /*    48     4 */
	char                       fill __attribute__((__aligned__(4))); /*    52     4 */
	enum Alignment             align __attribute__((__aligned__(1))); /*    56     1 */

	/* size: 64, cachelines: 1, members: 6 */
	/* padding: 7 */
	/* paddings: 2, sum paddings: 32 */
	/* forced alignments: 6 */
} __attribute__((__aligned__(8)));
struct Arguments {
	struct &[&str]             pieces __attribute__((__aligned__(8))); /*     0    16 */
	struct Option<&[core::fmt::rt::v1::Argument]> fmt __attribute__((__aligned__(8))); /*    16    16 */

	/* XXX last struct has 16 bytes of padding */

	struct &[core::fmt::ArgumentV1] args __attribute__((__aligned__(8))); /*    32    16 */

	/* size: 48, cachelines: 1, members: 3 */
	/* paddings: 1, sum paddings: 16 */
	/* forced alignments: 3 */
	/* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));
struct Ok {

	/* XXX 1 byte hole, try to pack */

	()                         __0 __attribute__((__aligned__(1))); /*     1     0 */

	/* size: 1, cachelines: 1, members: 1 */
	/* forced alignments: 1, forced holes: 1, sum forced holes: 1 */
	/* last cacheline: 1 bytes */
} __attribute__((__aligned__(1)));
struct Err {

	/* XXX 1 byte hole, try to pack */

	struct Error               __0 __attribute__((__aligned__(1))); /*     1     0 */

	/* size: 1, cachelines: 1, members: 1 */
	/* forced alignments: 1, forced holes: 1, sum forced holes: 1 */
	/* last cacheline: 1 bytes */
} __attribute__((__aligned__(1)));
struct Result<(), core::fmt::Error> {

	/* size: 1, cachelines: 0, members: 0 */
	/* padding: 1 */
	/* last cacheline: 1 bytes */
} __attribute__((__aligned__(1)));
struct None {

	/* size: 16, cachelines: 1, members: 0 */
	/* padding: 16 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Some {

	/* XXX 8 bytes hole, try to pack */

	usize                      __0 __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 1 */
	/* sum members: 8, holes: 1, sum holes: 8 */
	/* forced alignments: 1, forced holes: 1, sum forced holes: 8 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Option<usize> {

	/* size: 16, cachelines: 1, members: 0 */
	/* padding: 16 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Some {
	struct &[core::fmt::rt::v1::Argument] __0 __attribute__((__aligned__(8))); /*     0    16 */

	/* size: 16, cachelines: 1, members: 1 */
	/* forced alignments: 1 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Option<&[core::fmt::rt::v1::Argument]> {

	/* size: 16, cachelines: 1, members: 0 */
	/* padding: 16 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct &mut dyn core::fmt::Write {
	struct dyn core::fmt::Write * pointer __attribute__((__aligned__(8))); /*     0     8 */
	usize *                    vtable __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct dyn core::fmt::Write {

	/* size: 0, cachelines: 0, members: 0 */
} __attribute__((__aligned__(1)));
struct &[&str] {
	struct &str *              data_ptr __attribute__((__aligned__(8))); /*     0     8 */
	usize                      length __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct &str {
	u8 *                       data_ptr __attribute__((__aligned__(8))); /*     0     8 */
	usize                      length __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct &[core::fmt::rt::v1::Argument] {
	struct Argument *          data_ptr __attribute__((__aligned__(8))); /*     0     8 */
	usize                      length __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct &[core::fmt::ArgumentV1] {
	struct ArgumentV1 *        data_ptr __attribute__((__aligned__(8))); /*     0     8 */
	usize                      length __attribute__((__aligned__(8))); /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
struct Point<i32> {
	i32                        x __attribute__((__aligned__(4))); /*     0     4 */
	i32                        y __attribute__((__aligned__(4))); /*     4     4 */

	/* size: 8, cachelines: 1, members: 2 */
	/* forced alignments: 2 */
	/* last cacheline: 8 bytes */
} __attribute__((__aligned__(4)));
⬢[acme@toolbox pahole]$

I'll cut version 1.25 with what is in the 'master' and 'next' now as
binutils 2.40 is out and emitting DW_TAG_unspecified_type, that isn't
supported by pahole 1.24 and also Alan's work on optimized functions,
etc.

- Arnaldo

diff --git a/dwarf_loader.c b/dwarf_loader.c
index a77598dc3affca88..b767bcaa9322c95a 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -1093,6 +1093,18 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
 	return parm;
 }
 
+static struct template_type_parameter *template_type_parameter__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
+{
+	struct template_type_parameter *parm = tag__alloc(cu, sizeof(*parm));
+
+	if (parm != NULL) {
+		tag__init(&parm->tag, cu, die);
+		parm->name = attr_string(die, DW_AT_name, conf);
+	}
+
+	return parm;
+}
+
 static struct inline_expansion *inline_expansion__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
 {
 	struct inline_expansion *exp = tag__alloc(cu, sizeof(*exp));
@@ -1211,6 +1223,7 @@ static void ftype__init(struct ftype *ftype, Dwarf_Die *die, struct cu *cu)
 #endif
 	tag__init(&ftype->tag, cu, die);
 	INIT_LIST_HEAD(&ftype->parms);
+	INIT_LIST_HEAD(&ftype->template_type_parms);
 	ftype->nr_parms	    = 0;
 	ftype->unspec_parms = 0;
 }
@@ -1566,6 +1579,19 @@ static struct tag *die__create_new_parameter(Dwarf_Die *die,
 	return &parm->tag;
 }
 
+static struct tag *die__create_new_template_type_parameter(Dwarf_Die *die, struct ftype *ftype,
+							   struct cu *cu, struct conf_load *conf)
+{
+	struct template_type_parameter *parm = template_type_parameter__new(die, cu, conf);
+
+	if (parm == NULL)
+		return NULL;
+
+	ftype__add_template_type_parameter(ftype, parm);
+
+	return &parm->tag;
+}
+
 static struct tag *die__create_new_label(Dwarf_Die *die,
 					 struct lexblock *lexblock,
 					 struct cu *cu, struct conf_load *conf)
@@ -1989,6 +2015,8 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
 		case DW_TAG_GNU_template_template_param:
 #endif
 		case DW_TAG_template_type_parameter:
+			tag = die__create_new_template_type_parameter(die, ftype, cu, conf);
+			break;
 		case DW_TAG_template_value_parameter:
 			/* FIXME: probably we'll have to attach this as a list of
  			 * template parameters to use at class__fprintf time... 
diff --git a/dwarves.c b/dwarves.c
index b43031c93c5cab58..6a8feccc2d45c60d 100644
--- a/dwarves.c
+++ b/dwarves.c
@@ -1391,6 +1391,12 @@ void ftype__add_parameter(struct ftype *ftype, struct parameter *parm)
 	list_add_tail(&parm->tag.node, &ftype->parms);
 }
 
+void ftype__add_template_type_parameter(struct ftype *ftype, struct template_type_parameter *parm)
+{
+	++ftype->nr_template_type_parms;
+	list_add_tail(&parm->tag.node, &ftype->template_type_parms);
+}
+
 void lexblock__add_tag(struct lexblock *block, struct tag *tag)
 {
 	list_add_tail(&tag->node, &block->tags);
diff --git a/dwarves.h b/dwarves.h
index 24a1909a60389dee..5c9952bfddaed301 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -825,13 +825,30 @@ static inline const char *parameter__name(const struct parameter *parm)
 	return parm->name;
 }
 
+struct template_type_parameter {
+	struct tag tag;
+	const char *name;
+};
+
+static inline struct template_type_parameter *tag__template_type_parameter(const struct tag *tag)
+{
+	return (struct template_type_parameter *)tag;
+}
+
+static inline const char *template_type_parameter__name(const struct template_type_parameter *parm)
+{
+	return parm->name;
+}
+
 /*
  * tag.tag can be DW_TAG_subprogram_type or DW_TAG_subroutine_type.
  */
 struct ftype {
 	struct tag	 tag;
 	struct list_head parms;
+	struct list_head template_type_parms;
 	uint16_t	 nr_parms;
+	uint16_t	 nr_template_type_parms;
 	uint8_t		 unspec_parms:1; /* just one bit is needed */
 	uint8_t		 optimized_parms:1;
 	uint8_t		 processed:1;
@@ -872,6 +889,8 @@ void ftype__delete(struct ftype *ftype);
 	list_for_each_entry_safe_reverse(pos, n, &(ftype)->parms, tag.node)
 
 void ftype__add_parameter(struct ftype *ftype, struct parameter *parm);
+void ftype__add_template_type_parameter(struct ftype *ftype, struct template_type_parameter *parm);
+
 size_t ftype__fprintf(const struct ftype *ftype, const struct cu *cu,
 		      const char *name, const int inlined,
 		      const int is_pointer, const int type_spacing, bool is_prototype,
Arnaldo Carvalho de Melo Feb. 13, 2023, 9:53 p.m. UTC | #5 
Em Mon, Feb 13, 2023 at 12:53:38PM +0000, Eric Curtin escreveu:
> On Mon, 13 Feb 2023 at 12:45, Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > Hi Arnaldo,
> >
> > On Mon, Feb 13, 2023 at 1:09 PM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > The namespace.o seems to be ok:
> >
> > I saw the other message too -- this looks great, thanks a ton.
> >
> > > The core one needs work:
> >
> > If `core.o` works, then I think it is likely other things will work :)
> 
> Hi Guys,
> 
> I'll leave this to the experts, but if we get this to the point where
> we are happy to enable again for Rust CUs, could we request another
> version bump? It just makes it easier to integrate with the kernel

Sure, as we improve the support for encoding BTF from DWARF generated
for Rust code, and the subset that is used in the kernel is handled, we
will just adjust scripts/pahole-flags.sh to skip Rust DWARF if the
version is >= the version where excluding DWARF generated from some
languages (Rust in this case) but < the version where we're confortable
with generating BTF for Rust DWARF.

- Arnaldo

> scripts when we want to enable again.
> 
> >
> > I can try to extract the cases for those into simpler `.o` files, if
> > you would find simpler test cases useful (perhaps for the test suite
> > etc.).
> >
> > Cheers,
> > Miguel
> >
>
Arnaldo Carvalho de Melo Feb. 14, 2023, 9:22 p.m. UTC | #6 
Em Fri, Feb 10, 2023 at 05:48:36PM -0300, Arnaldo Carvalho de Melo escreveu:
> Hi Miguel, after a long winter, I'm trying to get Rust properly
> supported on pahole, please check that this specific use case is working
> for you as well.
> 
> I'll go thru the others to see if they are easy (or at least restricted
> to Rust CUs) as this one.

I needed to add this one on top:

From 1231b6b9b4d88e0084bef4254eb1a05eb9935c99 Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Tue, 14 Feb 2023 18:13:59 -0300
Subject: [PATCH 1/1] dwarf_loader: Fix sorting of Rust structs

We may have structs with fields on the same offset, don't move anything
in that case, otherwise we get into an eternal loop, doh.

Tested with the Linux kernel built with CONFIG_RUST + all the code
examples, first Rust struct where this happened was:

  (gdb) p type->namespace.name
  $2 = 0x7fffda938497 "((), char)"
  (gdb) p type->nr_members
  $3 = 2
  (gdb) p current_member->name
  $4 = 0x7fffda918f36 "__1"
  (gdb) p next_member->name
  $5 = 0x7fffda91765c "__0"
  (gdb) p current_member->byte_offset
  $6 = 0
  (gdb) p next_member->byte_offset
  $7 = 0

But this shows that --lang_exclude should be better positioned as we're
now needlessly loading all the tags for Rust DWARF to then just trow it
away at the cu__filter() in pahole :-\

Too late in the 1.25 release cycle for that, optimize it in 1.26.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 dwarf_loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index a77598dc3affca88..acdb68d5dd33f148 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2857,7 +2857,7 @@ restart:
 
 		struct class_member *next_member = list_entry(current_member->tag.node.next, typeof(*current_member), tag.node);
 
-		if (current_member->byte_offset < next_member->byte_offset)
+		if (current_member->byte_offset <= next_member->byte_offset)
 			continue;
 
 		list_del(&current_member->tag.node);
diff mbox series

Patch

diff --git a/dwarf_loader.c b/dwarf_loader.c
index 253c5efaf3b55a93..a77598dc3affca88 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2835,9 +2835,51 @@  static int class_member__cache_byte_size(struct tag *tag, struct cu *cu,
 	return 0;
 }
 
+static bool cu__language_reorders_offsets(const struct cu *cu)
+{
+	return cu->language == DW_LANG_Rust;
+}
+
+static int type__sort_by_offset(struct tag *tag, struct cu *cu, void *cookie __maybe_unused)
+{
+	if (!tag__is_type(tag))
+		return 0;
+
+	struct type *type = tag__type(tag);
+	struct class_member *current_member;
+
+	// There may be more than DW_TAG_members entries in the type tags, so do a simple
+	// bubble sort for now, so that the other non tags stay where they are.
+restart:
+	type__for_each_data_member(type, current_member) {
+		if (list_is_last(&current_member->tag.node, &type->namespace.tags))
+		       break;
+
+		struct class_member *next_member = list_entry(current_member->tag.node.next, typeof(*current_member), tag.node);
+
+		if (current_member->byte_offset < next_member->byte_offset)
+			continue;
+
+		list_del(&current_member->tag.node);
+		list_add(&current_member->tag.node, &next_member->tag.node);
+		goto restart;
+	}
+
+	return 0;
+}
+
+static void cu__sort_types_by_offset(struct cu *cu, struct conf_load *conf)
+{
+	cu__for_all_tags(cu, type__sort_by_offset, conf);
+}
+
 static int cu__finalize(struct cu *cu, struct conf_load *conf, void *thr_data)
 {
 	cu__for_all_tags(cu, class_member__cache_byte_size, conf);
+
+	if (cu__language_reorders_offsets(cu))
+		cu__sort_types_by_offset(cu, conf);
+
 	if (conf && conf->steal) {
 		return conf->steal(cu, conf, thr_data);
 	}