pahole: Fix --unions bypassing the struct-only options

Split into three independent patches as you asked, so any of them can be picked up or dropped on its own:

1/3 --unions --packable segfault fix with the Fixes: tag 2/3 add flexible array options, to the struct only check 3/3 stop ignoring the struct-only filters.

v1: https://lore.kernel.org/dwarves/[email protected]/

Holes and padding in the structs inside a union can be found with 'pahole -a -A.' Details are in my reply on the v1 thread.

Would a '--recursive' modifier make sense? The hole and padding filters don't look inside member structs currently. Optionally with a depth like '--recursive=2'.

Aditya Dutt (3):
pahole: Fix --unions --packable segfault
pahole: Don't match unions with --with_flexible_array
pahole: Apply the struct-only filters to unions

 pahole.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

2.50.1

print_packable_info() reads c->priv. That field lives in 'struct class', but a union is allocated by as a 'struct type'. It reads past the end of the allocation.

  $ pahole --unions --packable
  Segmentation fault (core dumped)

So we return before the read.

---
 pahole.c | 4 ++++
 1 file changed, 4 insertions(+)

index a1b3d4a..7ec4a74 100644
@@ -462,6 +462,10 @@ static void class_formatter(struct class *class, struct cu *cu, uint32_t id)
 static void print_packable_info(struct class *c, struct cu *cu, uint32_t id)
 {
 	const struct tag *t = class__tag(c);
+
+	if (!tag__is_struct(t))
+		return;
+
 	const size_t orig_size = class__size(c);
 	const size_t new_size = class__size(c->priv);
 	const size_t savings = orig_size - new_size;

2.50.1

C doesn't allow a flexible array member in a union, but --with_flexible_array prints unions anyway.

  $ cat flex.c
  union wrap_u { int a; long b; };
  union wrap_u u;

  $ pahole --with_flexible_array flex.o
  union wrap_u {
        int                        a;                  /*     0     4 */
        long int                   b;                  /*     0     8 */
  };

--with_embedded_flexible_array doesn't have this problem because its option handler also sets just_structs.

Add both to the list so neither relies on that side effect.

After:

  $ pahole --with_flexible_array flex.o
  $

Structs with a flexible array are unaffected:

  $ cat has_flex.c
  struct has_flex { int n; char data[]; };
  struct has_flex *p;

  $ pahole --with_flexible_array has_flex.o
  struct has_flex {
        int                        n;                    /*     0     4 */
        char                       data[];               /*     4     0 */

        /* size: 4, cachelines: 1, members: 2 */
        /* last cacheline: 4 bytes */

};

---
 pahole.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 7ec4a74..d2dcd90 100644
@@ -734,7 +734,8 @@ static struct class *class__filter(struct class *class, struct cu *cu,
 	 */
 	if (!tag__is_struct(tag))
 		return (just_structs || show_packable || nr_holes || nr_bit_holes || hole_size_ge ||
-			end_padding_ge || end_padding) ? NULL : class;
+			end_padding_ge || end_padding || show_with_flexible_array ||
+			show_with_embedded_flexible_array) ? NULL : class;
 
 	if (tag->top_level)
 		class__find_holes(class);

2.50.1

class__filter() returns early with --unions before the struct only filters.

  $ pahole --unions -H 99 /sys/kernel/btf/vmlinux | wc -l
  1786

No union has 99 byte holes, so after this it prints 0.

  $ pahole --unions -H 99 /sys/kernel/btf/vmlinux | wc -l
  0

---
 pahole.c | 6 ------
 1 file changed, 6 deletions(-)

index d2dcd90..83ca321 100644
@@ -720,12 +720,6 @@ static struct class *class__filter(struct class *class, struct cu *cu,
 	     strncmp(decl_exclude_prefix, tag__decl_file(tag, cu),
 		     decl_exclude_prefix_len) == 0))
 		return NULL;
-	/*
-	 * if --unions was used and we got here, its a union and we satisfy the other
-	 * filters/options, so don't filter it.
-	 */
-	if (just_unions)
-		return class;
 	/*
 	 * The following only make sense for structs, i.e. 'struct class',
 	 * and as we can get here with a union, that is represented by a 'struct type',

2.50.1

I think --recursive-limit, longer, but conveys that this is a limit, not a request for some specific recursiveness.

Thanks, applied!

- Arnaldo