Re: pahole vs isatty

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Tue, Jun 29, 2021 at 09:38:41AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jun 29, 2021 at 10:13:38AM +0200, Bernd Buschinski escreveu:
> > It seems the output is now only available if it is a real tty, which
 
> if stdin is a real tty, stdout can be a real tty or be redirected, as
> before.
 
> > doesn't work for my scripts.
 
> Sorry about that, I should have added a explicit command line option,
> like with 'perf', where '-' means stdin. As this is a relatively new
> feature I guess I'll do just that, i.e. stop unconditionally checking
> for isatty(0) and only use the pretty printer when --printer is used.
 
> > So, just as a question: Is this change really intentional?
> > Is there any easy way to restore the old behavior?

> > FYI: my scripts are using perl and python, I do no really favor
> > changing them, but if there is no other way.. I will do it :)
 
> Well, you'll at least need to update pahole to 1.22, or, in the
> meantime, use a patch, I'm working on it now, thanks for the report!

So, while fixing this I ran into bugs, fixed those and at the end I
committed the patch at the end of this message.

Please try building it from the tmp.master branch and please let me know
if your scripts are back working.

There is quite a lot of refactorings in this branch, as I'm paving the
way for multithreading DWARF loading and BTF encoding, so if you find
anything you find suspicious, please, please report here.

Thanks,

- Arnaldo

commit c71cbe9918c40cad2ac8ae982aa8001e7766dd97
Author: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Date:   Tue Jun 29 13:22:00 2021 -0300

    pahole: Introduce --prettify option
    
    The use of isatty(0) to switch into pretty printing is problematic as
    reported by Bernd Buschinski, that ran into problems with his scripts:
    
    ========================================================================
      I am using pahole 1.21 and I recently noticed that I no longer have
      any pahole output in several scripts.
    
      Using (on the command line):
    
        $ pahole -V -E -C my_struct /path/to/my/debug.o
    
      works fine and gives the expected output.
    
      But:
    
        $ parallel -j 1 pahole -V -E -C my_struct ::: /path/to/my/debug.o
    
      gives nothing, no stderr, no stdout and ret code 0.
    
      After testing some versions, it works fine in 1.17 and no longer works in 1.18.
    ========================================================================
    
    Since the pretty printer broke existing scripts, and its a relatively
    new feature, lets switch to using a explicit command line option to
    activate the pretty printer, i.e. where we used:
    
      $ pahole --header elf64_hdr < /bin/bash
    
    We now use one of:
    
      ⬢[acme@toolbox pahole]$ pahole --header elf64_hdr --prettify=/bin/bash
      {
            .e_ident = { 127, 69, 76, 70, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            .e_type = 3,
            .e_machine = 62,
            .e_version = 1,
            .e_entry = 204016,
            .e_phoff = 64,
            .e_shoff = 1388096,
            .e_flags = 0,
            .e_ehsize = 64,
            .e_phentsize = 56,
            .e_phnum = 13,
            .e_shentsize = 64,
            .e_shnum = 31,
            .e_shstrndx = 30,
      },
      ⬢[acme@toolbox pahole]$ pahole --header elf64_hdr --prettify /bin/bash
      {
            .e_ident = { 127, 69, 76, 70, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            .e_type = 3,
            .e_machine = 62,
            .e_version = 1,
            .e_entry = 204016,
            .e_phoff = 64,
            .e_shoff = 1388096,
            .e_flags = 0,
            .e_ehsize = 64,
            .e_phentsize = 56,
            .e_phnum = 13,
            .e_shentsize = 64,
            .e_shnum = 31,
            .e_shstrndx = 30,
      },
      ⬢[acme@toolbox pahole]$ pahole --header elf64_hdr --prettify - < /bin/bash
      {
            .e_ident = { 127, 69, 76, 70, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            .e_type = 3,
            .e_machine = 62,
            .e_version = 1,
            .e_entry = 204016,
            .e_phoff = 64,
            .e_shoff = 1388096,
            .e_flags = 0,
            .e_ehsize = 64,
            .e_phentsize = 56,
            .e_phnum = 13,
            .e_shentsize = 64,
            .e_shnum = 31,
            .e_shstrndx = 30,
      },
      ⬢[acme@toolbox pahole]$ pahole --header elf64_hdr --prettify=- < /bin/bash
      {
            .e_ident = { 127, 69, 76, 70, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            .e_type = 3,
            .e_machine = 62,
            .e_version = 1,
            .e_entry = 204016,
            .e_phoff = 64,
            .e_shoff = 1388096,
            .e_flags = 0,
            .e_ehsize = 64,
            .e_phentsize = 56,
            .e_phnum = 13,
            .e_shentsize = 64,
            .e_shnum = 31,
            .e_shstrndx = 30,
      },
      ⬢[acme@toolbox pahole]$
    
    Reported-by: Bernd Buschinski <b.buschinski@xxxxxxxxxxxxxx>
    Report-Link: https://lore.kernel.org/dwarves/CACN-hLVoz2tWrtgDLabOv6S1-H_8RD2fh8SV6EnADF1ikMxrmw@xxxxxxxxxxxxxx/
    Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
index 5cb356b9f8064139..a2bb920bc13bf250 100644
--- a/man-pages/pahole.1
+++ b/man-pages/pahole.1
@@ -21,7 +21,7 @@ It also uses these structure layouts to pretty print data feed to its standard
 input, e.g.:
 .PP
 .nf
-$ pahole --header elf64_hdr < /lib/modules/5.8.0-rc6+/build/vmlinux
+$ pahole --header elf64_hdr --prettify /lib/modules/5.8.0-rc6+/build/vmlinux
 {
 	.e_ident = { 127, 69, 76, 70, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
 	.e_type = 2,
@@ -566,8 +566,8 @@ $
 
 .SH PRETTY PRINTING
 .P
-pahole can also use the data structure types to pretty print raw data coming
-from its standard input.
+pahole can also use the data structure types to pretty print raw data specified via --prettify.
+To consume raw data from the standard input, just use '--prettify -'
 .P
 It can also pretty print raw data from stdin according to the type specified:
 .PP
@@ -585,7 +585,7 @@ $
 $ ls -la versions
 -rw-rw-r--. 1 acme acme 7616 Jun 25 11:33 versions
 $
-$ pahole --count 3 -C modversion_info drivers/scsi/sg.ko < versions
+$ pahole --count 3 -C modversion_info drivers/scsi/sg.ko --prettify versions
 {
       .crc = 0x8dabd84,
       .name = "module_layout",
@@ -599,7 +599,7 @@ $ pahole --count 3 -C modversion_info drivers/scsi/sg.ko < versions
       .name = "param_ops_int",
 },
 $
-$ pahole --skip 1 --count 2 -C modversion_info drivers/scsi/sg.ko < versions
+$ pahole --skip 1 --count 2 -C modversion_info drivers/scsi/sg.ko --prettify - < versions
 {
       .crc = 0x45e4617b,
       .name = "no_llseek",
@@ -611,7 +611,7 @@ $ pahole --skip 1 --count 2 -C modversion_info drivers/scsi/sg.ko < versions
 $
 This is equivalent to:
 
-$ pahole --seek_bytes 64 --count 1 -C modversion_info drivers/scsi/sg.ko < versions
+$ pahole --seek_bytes 64 --count 1 -C modversion_info drivers/scsi/sg.ko --prettify versions
 {
 	.crc = 0x45e4617b,
 	.name = "no_llseek",
@@ -662,7 +662,7 @@ $
 Now we can use this to show the first record from offset zero:
 .PP
 .nf
-$ pahole -C elf64_hdr --count 1 < /lib/modules/5.8.0-rc3+/build/vmlinux
+$ pahole -C elf64_hdr --count 1 --prettify /lib/modules/5.8.0-rc3+/build/vmlinux
 {
 	.e_ident = { 127, 69, 76, 70, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
 	.e_type = 2,
@@ -685,7 +685,7 @@ $
 This is equivalent to:
 .PP
 .nf
-$ pahole --header elf64_hdr < /lib/modules/5.8.0-rc3+/build/vmlinux
+$ pahole --header elf64_hdr --prettify /lib/modules/5.8.0-rc3+/build/vmlinux
 .fi
 .P
 The --header option also allows reference in other command line options to fields in the header.
@@ -693,7 +693,7 @@ This is useful when one wants to show multiple records in a file and the range w
 are located is specified in header fields, such as for perf.data files:
 .PP
 .nf
-$ pahole --hex ~/bin/perf --header perf_file_header < perf.data
+$ pahole --hex ~/bin/perf --header perf_file_header --prettify perf.data
 {
 	.magic = 0x32454c4946524550,
 	.size = 0x68,
@@ -718,7 +718,7 @@ $
 So to display the cgroups records in the perf_file_header.data section we can use:
 .PP
 .nf
-$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,type_enum=perf_event_type,filter=type==PERF_RECORD_CGROUP)' < perf.data
+$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,type_enum=perf_event_type,filter=type==PERF_RECORD_CGROUP)' --prettify perf.data
 {
 	.header = {
 		.type = PERF_RECORD_CGROUP,
@@ -770,7 +770,7 @@ $
 For the common case of the header having a member that has the 'offset' and 'size' members, it is possible to use this more compact form:
 .PP
 .nf
-$ pahole ~/bin/perf --header=perf_file_header --range=data -C 'perf_event_header(sizeof,type,type_enum=perf_event_type,filter=type==PERF_RECORD_CGROUP)' < perf.data
+$ pahole ~/bin/perf --header=perf_file_header --range=data -C 'perf_event_header(sizeof,type,type_enum=perf_event_type,filter=type==PERF_RECORD_CGROUP)' --prettify perf.data
 .fi
 .P
 This uses ~/bin/perf to get the type definitions, the defines 'struct perf_file_header' as the header,
@@ -844,7 +844,7 @@ If we remove that type_enum=perf_event_type, we will lose the conversion of 'str
 more descriptive 'struct perf_record_cgroup', and also the beautification of the header.type field:
 .PP
 .nf
-$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,filter=type==19)' < perf.data
+$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,filter=type==19)' --prettify perf.data
 {
 	.type = 19,
 	.misc = 0,
@@ -876,7 +876,7 @@ $
 Some of the records are not found in 'type_enum=perf_event_type' so some of the records don't get converted to a type that fully shows its contents. For perf we know that those are in another enumeration, 'enum perf_user_event_type', so, for these cases, we can create a 'virtual enum', i.e. the sum of two enums and then get all those entries decoded and properly casted, first few records with just 'enum perf_event_type':
 .PP
 .nf
-$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,type_enum=perf_event_type)' --count 4 < perf.data
+$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,type_enum=perf_event_type)' --count 4 --prettify perf.data
 {
 	.type = 79,
 	.misc = 0,
@@ -907,7 +907,7 @@ $
 Now with both enumerations, i.e. with 'type_enum=perf_event_type+perf_user_event_type':
 .PP
 .nf
-$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,type_enum=perf_event_type+perf_user_event_type)' --count 5 < perf.data
+$ pahole ~/bin/perf --header=perf_file_header --seek_bytes '$header.data.offset' --size_bytes='$header.data.size' -C 'perf_event_header(sizeof,type,type_enum=perf_event_type+perf_user_event_type)' --count 5 --prettify perf.data
 {
 	.header = {
 		.type = PERF_RECORD_TIME_CONV,
@@ -966,7 +966,7 @@ data range with the following command:
 .PP
 .nf
 pahole ~/bin/perf --header=perf_file_header \
-         -C 'perf_file_attr(range=attrs),perf_event_header(range=data,sizeof,type,type_enum=perf_event_type+perf_user_event_type)' < perf.data
+         -C 'perf_file_attr(range=attrs),perf_event_header(range=data,sizeof,type,type_enum=perf_event_type+perf_user_event_type)' --prettify perf.data
 .fi
 
 .SH SEE ALSO
diff --git a/pahole.c b/pahole.c
index 06c4025549396fbf..520ddef93494d84f 100644
--- a/pahole.c
+++ b/pahole.c
@@ -35,6 +35,9 @@ static bool skip_encoding_btf_vars;
 static bool btf_encode_force;
 static const char *base_btf_file;
 
+static const char *prettify_input_filename;
+static FILE *prettify_input;
+
 static uint8_t class__include_anonymous;
 static uint8_t class__include_nested_anonymous;
 static uint8_t word_size, original_word_size;
@@ -854,6 +857,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
 #define ARGP_with_flexible_array   324
 #define ARGP_kabi_prefix	   325
 #define ARGP_btf_encode_detached   326
+#define ARGP_prettify_input_filename 327
 
 static const struct argp_option pahole__options[] = {
 	{
@@ -1202,6 +1206,12 @@ static const struct argp_option pahole__options[] = {
 		.key  = ARGP_numeric_version,
 		.doc  = "Print a numeric version, i.e. 119 instead of v1.19"
 	},
+	{
+		.name = "prettify",
+		.key  = ARGP_prettify_input_filename,
+		.arg  = "PATH",
+		.doc  = "Path to the raw data to pretty print",
+	},
 	{
 		.name = NULL,
 	}
@@ -1332,6 +1342,8 @@ static error_t pahole__options_parser(int key, char *arg,
 		btf_gen_floats = true;			break;
 	case ARGP_with_flexible_array:
 		show_with_flexible_array = true;	break;
+	case ARGP_prettify_input_filename:
+		prettify_input_filename = arg;		break;
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}
@@ -2586,7 +2598,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
 			class_id = 0;
 		}
 
-		if (!isatty(0)) {
+		if (prettify_input) {
 			prototype->class = class;
 			prototype->cu	 = cu;
 			continue;
@@ -2624,7 +2636,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
 
 	// If we got here with pretty printing is because we have everything solved except for type_enum or --header
 
-	if (!isatty(0)) {
+	if (prettify_input) {
 		// Check if we need to continue loading CUs to get those type_enum= and --header resolved
 		if (header == NULL && conf.header_type)
 			return LSK__KEEPIT;
@@ -2637,7 +2649,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
 		// All set, pretty print it!
 		list_for_each_entry_safe(prototype, n, &class_names, node) {
 			list_del_init(&prototype->node);
-			if (prototype__stdio_fprintf_value(prototype, header, stdin, stdout) < 0)
+			if (prototype__stdio_fprintf_value(prototype, header, prettify_input, stdout) < 0)
 				break;
 		}
 
@@ -2783,9 +2795,6 @@ int main(int argc, char *argv[])
 {
 	int err, remaining, rc = EXIT_FAILURE;
 
-	if (!isatty(0))
-		conf.hex_fmt = 0;
-
 	if (argp_parse(&pahole__argp, argc, argv, 0, &remaining, NULL)) {
 		argp_help(&pahole__argp, stderr, ARGP_HELP_SEE, argv[0]);
 		goto out;
@@ -2801,6 +2810,19 @@ int main(int argc, char *argv[])
 		goto out;
 	}
 
+	if (prettify_input_filename) {
+		if (strcmp(prettify_input_filename, "-") == 0) {
+			prettify_input = stdin;
+		} else {
+			prettify_input = fopen(prettify_input_filename, "r");
+			if (prettify_input == NULL) {
+				fprintf(stderr, "Failed to read input '%s': %s\n",
+					prettify_input_filename, strerror(errno));
+				goto out_dwarves_exit;
+			}
+		}
+	}
+
 	if (base_btf_file) {
 		conf_load.base_btf = btf__parse(base_btf_file, NULL);
 		if (libbpf_get_error(conf_load.base_btf)) {
@@ -2825,7 +2847,7 @@ int main(int argc, char *argv[])
 	conf_load.steal = pahole_stealer;
 
 	// Make 'pahole --header type < file' a shorter form of 'pahole -C type --count 1 < file'
-	if (conf.header_type && !class_name && !isatty(0)) {
+	if (conf.header_type && !class_name && prettify_input) {
 		conf.count = 1;
 		class_name = conf.header_type;
 		conf.header_type = 0; // so that we don't read it and then try to read the -C type
@@ -2923,6 +2945,10 @@ out_cus_delete:
 	conf_load.base_btf = NULL;
 #endif
 out_dwarves_exit:
+	if (prettify_input && prettify_input != stdin) {
+		fclose(prettify_input);
+		prettify_input = NULL;
+	}
 #ifdef DEBUG_CHECK_LEAKS
 	dwarves__exit();
 #endif



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux