dwarves.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* pahole vs isatty
@ 2021-06-29  8:13 Bernd Buschinski
  2021-06-29 12:38 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 5+ messages in thread
From: Bernd Buschinski @ 2021-06-29  8:13 UTC (permalink / raw)
  To: dwarves

Hello,

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.

-> Git Bisect
$ git bisect start
$ git bisect good v1.17
$ git bisect bad v1.18
Bisecting: 54 revisions left to test after this (roughly 6 steps)
[0a97d6c143fcc92a335e532a991e1e1c62c9bd3e] pahole: Factor out parsing
class prototypes
$ git bisect bad
Bisecting: 26 revisions left to test after this (roughly 5 steps)
[a5bb31b86fbe53624165ecdf1bd18a19ac224b7c] pahole: Fix --skip for
variable sized records
$ git bisect bad
Bisecting: 13 revisions left to test after this (roughly 4 steps)
[fe284221448c950d2ba17bce91d19c484d6580a0] pahole: Introduce --seek_bytes
$ git bisect bad
Bisecting: 6 revisions left to test after this (roughly 3 steps)
[a231d00f8d08825ea1c9937cf5c932883ad28ecc] pahole: Print comma at the
end of field name + field value
$ git bisect bad
Bisecting: 2 revisions left to test after this (roughly 2 steps)
[cc65946e3068f7b68a0aa437261c748c20445986] dwarves: Adopt
tag__is_base_type() from ctrace.c
$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 1 step)
[d8079c6d373a5754164ecd784844ce2622d782b8] pahole: Hex dump a type
from stdio when it isn't a tty
$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[38109ab45fe0307dd21b96a866c1f9b62df64fd4] spec: Fix date
$ git bisect good
d8079c6d373a5754164ecd784844ce2622d782b8 is the first bad commit
commit d8079c6d373a5754164ecd784844ce2622d782b8
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Wed Jun 24 09:08:17 2020 -0300

    pahole: Hex dump a type from stdio when it isn't a tty

    For now the only aspect of the type that is considered is its size, so
    one line each sizeof(type) will be hex dumped, e.g.:

      $ objcopy -O binary --only-section=.altinstructions
~/git/build/v5.7-rc2+/vmlinux .altinstructions
      $ ls -la .altinstructions
      -rwxrwxr-x. 1 acme acme 18551 Jun 24 09:09 .altinstructions
      $ pahole -C alt_instr ~/git/build/v5.7-rc2+/vmlinux
      struct alt_instr {
            s32                        instr_offset;         /*     0     4 */
            s32                        repl_offset;          /*     4     4 */
            u16                        cpuid;                /*     8     2 */
            u8                         instrlen;             /*    10     1 */
            u8                         replacementlen;       /*    11     1 */
            u8                         padlen;               /*    12     1 */

            /* size: 13, cachelines: 1, members: 6 */
            /* last cacheline: 13 bytes */
      } __attribute__((__packed__));
      $ pahole -C alt_instr ~/git/build/v5.7-rc2+/vmlinux <
.altinstructions  | head
      0x90 0xe6 0xe0 0xff 0x73 0x48 0x00 0x00 0x70 0x00 0x05 0x05 0x00,
      0x83 0xe6 0xe0 0xff 0x6b 0x48 0x00 0x00 0x29 0x01 0x05 0x05 0x00,
      0x2a 0x72 0xc5 0xfe 0x63 0x48 0x00 0x00 0xeb 0x00 0x02 0x00 0x00,
      0x22 0x72 0xc5 0xfe 0x56 0x48 0x00 0x00 0x91 0x00 0x05 0x05 0x05,
      0x0c 0x73 0xc5 0xfe 0x4e 0x48 0x00 0x00 0xeb 0x00 0x02 0x00 0x00,
      0x04 0x73 0xc5 0xfe 0x41 0x48 0x00 0x00 0x91 0x00 0x02 0x00 0x00,
      0x72 0x73 0xc5 0xfe 0x34 0x48 0x00 0x00 0xf3 0x00 0x2b 0x2b 0x29,
      0xca 0x73 0xc5 0xfe 0x52 0x48 0x00 0x00 0xec 0x00 0x18 0x18 0x16,
      0xbd 0x73 0xc5 0xfe 0x5d 0x48 0x00 0x00 0xed 0x00 0x18 0x05 0x16,
      0xd3 0x7a 0xc5 0xfe 0x55 0x48 0x00 0x00 0x34 0x01 0x03 0x03 0x03,
      $

    Suggested-by: Joe Lawrence <joe.lawrence@redhat.com>
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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



It seems the output is now only available if it is a real tty, which
doesn't work for my scripts.
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 :)

-- Bernd

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: pahole vs isatty
  2021-06-29  8:13 pahole vs isatty Bernd Buschinski
@ 2021-06-29 12:38 ` Arnaldo Carvalho de Melo
  2021-06-29 16:43   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-29 12:38 UTC (permalink / raw)
  To: Bernd Buschinski; +Cc: dwarves

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!

- Arnaldo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: pahole vs isatty
  2021-06-29 12:38 ` Arnaldo Carvalho de Melo
@ 2021-06-29 16:43   ` Arnaldo Carvalho de Melo
  2021-06-30  6:13     ` Bernd Buschinski
  0 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-29 16:43 UTC (permalink / raw)
  To: Bernd Buschinski; +Cc: dwarves

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@redhat.com>
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@googlemail.com>
    Report-Link: https://lore.kernel.org/dwarves/CACN-hLVoz2tWrtgDLabOv6S1-H_8RD2fh8SV6EnADF1ikMxrmw@mail.gmail.com/
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: pahole vs isatty
  2021-06-29 16:43   ` Arnaldo Carvalho de Melo
@ 2021-06-30  6:13     ` Bernd Buschinski
  2021-06-30 12:26       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 5+ messages in thread
From: Bernd Buschinski @ 2021-06-30  6:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: dwarves

I tested tmp.master c71cbe9918c40cad2ac8ae982aa8001e7766dd97
and everything seems to work fine! Thanks! :)

Am Di., 29. Juni 2021 um 18:43 Uhr schrieb Arnaldo Carvalho de Melo
<acme@kernel.org>:
>
> 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@redhat.com>
> 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@googlemail.com>
>     Report-Link: https://lore.kernel.org/dwarves/CACN-hLVoz2tWrtgDLabOv6S1-H_8RD2fh8SV6EnADF1ikMxrmw@mail.gmail.com/
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: pahole vs isatty
  2021-06-30  6:13     ` Bernd Buschinski
@ 2021-06-30 12:26       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-30 12:26 UTC (permalink / raw)
  To: Bernd Buschinski; +Cc: dwarves

Em Wed, Jun 30, 2021 at 08:13:56AM +0200, Bernd Buschinski escreveu:
> I tested tmp.master c71cbe9918c40cad2ac8ae982aa8001e7766dd97
> and everything seems to work fine! Thanks! :)

added a "Tested-by: Bernd Buschinski <b.buschinski@googlemail.com>" to
the commit,

Please keep reporting, and if you have suggestions, please voice them
:-)

Thanks!

- Arnaldo
 
> Am Di., 29. Juni 2021 um 18:43 Uhr schrieb Arnaldo Carvalho de Melo
> <acme@kernel.org>:
> >
> > 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@redhat.com>
> > 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@googlemail.com>
> >     Report-Link: https://lore.kernel.org/dwarves/CACN-hLVoz2tWrtgDLabOv6S1-H_8RD2fh8SV6EnADF1ikMxrmw@mail.gmail.com/
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > 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

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-30 12:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-29  8:13 pahole vs isatty Bernd Buschinski
2021-06-29 12:38 ` Arnaldo Carvalho de Melo
2021-06-29 16:43   ` Arnaldo Carvalho de Melo
2021-06-30  6:13     ` Bernd Buschinski
2021-06-30 12:26       ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).