[PATCH 2/2] tools: perf: flamegraph.py: fix "Error reading report header"

[Stephen Brennan]

When running "perf script flamegraph" in a directory without perf.data,
the script succeeds but prints out the following error:

    # perf script flamegraph -ag -F999 -- sleep 5
    failed to open perf.data: No such file or directory  (try 'perf record' first)
    Error reading report header: Command '['perf', 'report', '--header-only']' returned non-zero exit status 255.
    dumping data to flamegraph.html

If a "perf.data" file were present in the working directory, then the
error would not occur, but flamegraph.py would silently include an
incorrect header as the context for the flamegraph.

The issue is with the "-i" option and how it interacts with option
parsing in the flamegraph-report shell script. Below are the different
flamegraph-report script calls in recent git history:

[1]: perf script [...] -- "$@"
[2]: perf script [...] -i - -- "$@"
[3]: perf script [...] "$@"

[1] is prior to commit 23e0a63c6dd3f ("perf script: force stdin for
flamegraph in live mode"), [2] is after the same commit, and [3] is
after commit 29bab85418efd ("perf script: Fix hangup in offline
flamegraph report"). As noted in the latter commit, for the "perf script
flamegraph" invocation, the parent perf process which sets up the pipe
will set the appropriate "-i -" option when it invokes
flamegraph-report. Thus, the third form is the correct way to ensure
perf itself processes the "-i" option and thus processes the correct
input file.

As a result, for cases 1 and 2, there was an input file string passed to
flamegraph.py -- though it did not necessarily match the one that perf
was processing. For the current case (#3), the script doesn't receive an
input file string and thus assumes that "perf.data" is the input, even
when stdin is being processed.

All of this is fragile and works around what we really want: to know
what input file perf is processing. The previous commit implements a
helper for this, so drop the "-i" option entirely and use the helper to
properly detect the filename.

Signed-off-by: Stephen Brennan <stephen.s.brennan@xxxxxxxxxx>
---
 tools/perf/scripts/python/flamegraph.py | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/tools/perf/scripts/python/flamegraph.py b/tools/perf/scripts/python/flamegraph.py
index ad735990c5bec..d49b96a324a6f 100755
--- a/tools/perf/scripts/python/flamegraph.py
+++ b/tools/perf/scripts/python/flamegraph.py
@@ -28,6 +28,8 @@ import sys
 from typing import Dict, Optional, Union
 import urllib.request
 
+from perf_trace_context import perf_input_name, perf_script_context
+
 MINIMAL_HTML = """<head>
   <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/d3-flamegraph.css";>
 </head>
@@ -122,20 +124,14 @@ class FlameGraphCLI:
         node.value += 1
 
     def get_report_header(self) -> str:
-        if self.args.input == "-":
+        if self.input_file_name == "-":
             # when this script is invoked with "perf script flamegraph",
             # no perf.data is created and we cannot read the header of it
             return ""
 
         try:
-            # if the file name other than perf.data is given,
-            # we read the header of that file
-            if self.args.input:
-                output = subprocess.check_output(["perf", "report", "--header-only",
-                                                  "-i", self.args.input])
-            else:
-                output = subprocess.check_output(["perf", "report", "--header-only"])
-
+            output = subprocess.check_output(["perf", "report", "--header-only",
+                                              "-i", self.input_file_name])
             result = output.decode("utf-8")
             if self.args.event_name:
                 result += "\nFocused event: " + self.args.event_name
@@ -144,6 +140,9 @@ class FlameGraphCLI:
             print(f"Error reading report header: {err}", file=sys.stderr)
             return ""
 
+    def trace_begin(self) -> None:
+        self.input_file_name = perf_input_name(perf_script_context) or "-"
+
     def trace_end(self) -> None:
         stacks_json = json.dumps(self.stack, default=lambda x: x.to_json())
 
@@ -167,7 +166,7 @@ js-d3-flame-graph or libjs-d3-flame-graph, specify an existing flame
 graph template (--template PATH) or use another output format (--format
 FORMAT).""",
                               file=sys.stderr)
-                        if self.args.input == "-":
+                        if self.input_file_name == "-":
                             print(
 """Not attempting to download Flame Graph template as script command line
 input is disabled due to using live mode. If you want to download the
@@ -248,8 +247,6 @@ if __name__ == "__main__":
                         default="blue-green",
                         help="flame graph color scheme",
                         choices=["blue-green", "orange"])
-    parser.add_argument("-i", "--input",
-                        help=argparse.SUPPRESS)
     parser.add_argument("--allow-download",
                         default=False,
                         action="store_true",
@@ -265,3 +262,4 @@ if __name__ == "__main__":
 
     process_event = cli.process_event
     trace_end = cli.trace_end
+    trace_begin = cli.trace_begin
-- 
2.52.0