Skip to contentSkip to conversation

pahole: Fix -C -T segfault

PATCH Applied Aditya Dutt 2 participants 4 messages Sep 3 – Sep 4

lore
Share & tools
lore
#0 lore raw

cover pahole: Fix -C -T segfault

'formatter' is NULL when -T is given and the -C path calls it, which segfaults.
Patch 2 rejects that, like '-m -C' already does.
Patch 1 fixes the '-m -C' error, which names '--class' incorrectly.

-T has set 'formatter' to NULL since commit 286c7740fd98c4df but -C did not call formatter until f5857bd34b220a64 (used to call tag__fprintf before), so added Fixes tag referencing it.

I think support for the combinations '-T -C' and '-m -C' could be added in the future.

Problem with doing that right now is that the counting logic lives in the 'class_name == NULL' block that -C skips. And -C stops loading CUs once all classes are found.

Aditya Dutt (2):
pahole: Fix option name in the -m/-C error message
pahole: Fix -C -T segfault

 pahole.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Signature · 1 line
2.34.1
#1 lore raw

PATCH 1/2 pahole: Fix option name in the -m/-C error message

There is no '--class' option, the long form of -C is '--class_name'.

---
 pahole.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
pahole.c +1 -1
index 83ca321..aa04d98 100644
@@ -3662,7 +3662,7 @@ int main(int argc, char *argv[])
 		return rc;
 
 	if (class_name != NULL && stats_formatter == nr_methods_formatter) {
-		fputs("pahole: -m/nr_methods doesn't work with --class/-C, it shows all classes and the number of its methods\n", stderr);
+		fputs("pahole: -m/nr_methods doesn't work with --class_name/-C, it shows all classes and the number of its methods\n", stderr);
 		return rc;
 	}
 
Signature · 1 line
2.34.1
#2 lore raw

PATCH 2/2 pahole: Fix -C -T segfault

In reply to Aditya Dutt

-T/--nr_definitions sets stats_formatter and sets formatter to NULL. print_classes() checks for that before calling it.
The -C code path does not which results in a segfault.

Before:

  $ cat m.c
  struct named { int a; char b; };
  union u_named { int a; long b; };
  struct holey { char a; int b; };
  struct named s; union u_named u; struct holey h;
  $ pahole -C named -T m.o
  Segmentation fault (core dumped)

Reject the combination, similar to '-m -C'.

After:

  $ pahole -C named -T m.o
  pahole: -T/nr_definitions doesn't work with --class_name/-C, it shows all classes and the number of times they are defined
  $ echo $?
  1
  $ pahole -T m.o # unchanged
  named   1
  u_named 1
  holey   1
Fixes f5857bd34b220a64 ("pahole: Honour --compile when -C is used")
---
 pahole.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
pahole.c +6 -1
index aa04d98..b2047c1 100644
@@ -3461,7 +3461,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, struct conf_load *conf
 			print_containers(cu, class_id, 0);
 		else if (find_pointers_in_structs)
 			print_structs_with_pointer_to(cu, class_id);
-		else if (class) {
+		else if (class && formatter != NULL) {
 			/*
 			 * We don't need to print it for every compile unit
 			 * but the previous options need
@@ -3666,6 +3666,11 @@ int main(int argc, char *argv[])
 		return rc;
 	}
 
+	if (class_name != NULL && stats_formatter == nr_definitions_formatter) {
+		fputs("pahole: -T/nr_definitions doesn't work with --class_name/-C, it shows all classes and the number of times they are defined\n", stderr);
+		return rc;
+	}
+
 	if (print_numeric_version) {
 		dwarves_print_numeric_version(stdout);
 		return 0;
Signature · 1 line
2.34.1
#3 lore raw
In reply to Aditya Dutt
Quoted 9 lines · Aditya Dutt

'formatter' is NULL when -T is given and the -C path calls it, which segfaults.
Patch 2 rejects that, like '-m -C' already does.
Patch 1 fixes the '-m -C' error, which names '--class' incorrectly.

-T has set 'formatter' to NULL since commit 286c7740fd98c4df but -C did not call formatter until f5857bd34b220a64 (used to call tag__fprintf before), so added Fixes tag referencing it.

Quoted 6 lines

I think support for the combinations '-T -C' and '-m -C' could be added in the future.

Problem with doing that right now is that the counting logic lives in the 'class_name == NULL' block that -C skips. And -C stops loading CUs once all classes are found.

Thanks, applied, I'll now stop processing patches and release 1.32 tomorrow.

- Arnaldo