Dwarves debugging tools
 help / color / mirror / Atom feed
From: Aditya Dutt <duttaditya18@gmail.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Aditya Dutt <duttaditya18@gmail.com>,
	dwarves@vger.kernel.org, Alan Maguire <alan.maguire@oracle.com>
Subject: Re: [PATCH dwarves] pahole: --unions shouldn't bypass struct-only options
Date: Tue,  1 Sep 2026 13:21:35 +0000	[thread overview]
Message-ID: <20260901132137.1206269-1-duttaditya18@gmail.com> (raw)
In-Reply-To: <apXzlIKeb4tc8x88@x2>

On Mon, Aug 31, 2026 at 06:35:16PM -0300, Arnaldo Carvalho de Melo wrote:
> Please split this into multiple patches, so that we can cherry pick
> independent patches when we have some issue with one of the patches in
> the series.

I'll split this change to 3 patches.
1. Fixing only the segfault by adding the guard at the top of
print_packable_info. Contains the Fixes tag.
2. --with_flexible_array and --with_embedded_flexible_array guard if !struct.
3. In the case of unions, don't ignore struct-only filters.

Is this good, so that whatever is needed can be picked up?

> I can imagine looking for an union with bit holes:
>
> union x86_pmu_config {
> 	struct {
> 		u64                event:8;            /*     0: 0  8 */
> 		u64                umask:8;            /*     0: 8  8 */
> 		u64                usr:1;              /*     0:16  8 */
> 		u64                os:1;               /*     0:17  8 */
> 		u64                edge:1;             /*     0:18  8 */
> 		u64                pc:1;               /*     0:19  8 */
> 		u64                interrupt:1;        /*     0:20  8 */
> 		u64                __reserved1:1;      /*     0:21  8 */
> 		u64                en:1;               /*     0:22  8 */
> 		u64                inv:1;              /*     0:23  8 */
> 		u64                cmask:8;            /*     0:24  8 */
> 		u64                event2:4;           /*     0:32  8 */
> 		u64                __reserved2:4;      /*     0:36  8 */
> 		u64                go:1;               /*     0:40  8 */
> 		u64                ho:1;               /*     0:41  8 */
> 	} bits;                                        /*     0     8 */
> 	u64                        value;              /*     0     8 */
> };
>
> The sum of that bitfield is 42 bits, so we have a "padding"/hole of 22
> bits in that union, can't see quickly a use fase for this right now, but
> maybe someone can have this corner case need?

You can already get that using 'pahole -a -A' (both flags are needed).
For example:

	$ cat /tmp/a.c
	typedef unsigned long long u64;
	union u_hole {
		struct {
			u64 a:8;
			u64 b:8;
			u64 c:1;
		} bits;
		u64 value;
	};
	union u_hole u;
	$ gcc -c -g /tmp/a.c -o /tmp/a.o
	$ pahole -a -A /tmp/a.o
	struct {
		u64                        a:8;                  /*     0: 0  8 */
		u64                        b:8;                  /*     0: 8  8 */
		u64                        c:1;                  /*     0:16  8 */

		/* size: 8, cachelines: 1, members: 3 */
		/* bit_padding: 47 bits */
		/* last cacheline: 8 bytes */
	};
	union u_hole {
		struct {
			u64                a:8;                /*     0: 0  8 */
			u64                b:8;                /*     0: 8  8 */
			u64                c:1;                /*     0:16  8 */
		} bits;                                        /*     0     8 */
		u64                        value;              /*     0     8 */
	};

The padding is for the anonymous struct and not the union itself.
Does it make sense for '--unions' to show the results for the anonymous
structs inside a union?

Maybe a '--recursive' modifier is better? Which looks inside
structs/unions and unions when applying the filters, so that the outer
struct/union returned if a filter applies to any inner struct/union.

> > --with_flexible_array and --with_embedded_flexible_array were missing from the
> > struct only list, so add them too.
>
> Well spotted, please put this in a separate patch, with the
> corresponding fix.

Same thing here, a union cannot have a flexible array directly but a
union may have a struct inside containing a flexible array. Maybe a
'--recursive' modifier makes sense? And optionally, you can specify the
depth to which we search '--recursive=2' etc.

-- 

Thanks,
Aditya Dutt

      reply	other threads:[~2026-09-01 13:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 18:56 [PATCH dwarves] pahole: --unions shouldn't bypass struct-only options Aditya Dutt
2026-08-31 21:35 ` Arnaldo Carvalho de Melo
2026-09-01 13:21   ` Aditya Dutt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260901132137.1206269-1-duttaditya18@gmail.com \
    --to=duttaditya18@gmail.com \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=dwarves@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox