Re: [PATCH v1 31/58] perf gecko: Port gecko to use python module

sashiko-bot
2026-04-20T00:20:09Z

Sashiko AI review found 5 potential issue(s):
- [High] The HTTP server binds to all network interfaces, exposing the current working directory to the local network.
- [Medium] Unresolved symbols are formatted as literal 'None' strings instead of the intended '[unknown]'.
- [Medium] The script uses a hardcoded port (8000) without handling 'Address already in use' errors, causing silent hangs.
- [Medium] Using `uname -op` crashes the script entirely on non-Linux platforms (e.g., macOS).
- [Low] Race condition between HTTP server initialization and browser request.
--

commit dc4b7f657c6a2aa78c3372c06ef2745b8abfea67
Author: Ian Rogers <irogers@google.com>

perf gecko: Port gecko to use python module

This commit adds a port of the gecko script that uses the perf python
module directly. This approach is significantly faster than using perf
script callbacks as it avoids creating intermediate dictionaries.

> diff --git a/tools/perf/python/gecko.py b/tools/perf/python/gecko.py
> new file mode 100755
> index 0000000000000..5d55bcb8ac012
> --- /dev/null
> +++ b/tools/perf/python/gecko.py
[ ... ]
> class GeckoCLI:
> """Command-line interface for converting perf data to Gecko format."""
> def __init__(self, args):
> self.args = args
> self.tid_to_thread: Dict[int, Thread] = {}
> self.start_time_ms: Optional[float] = None
> self.session = None
> self.product = subprocess.check_output(['uname', '-op']).decode().strip()

Can this crash on non-Linux platforms like macOS or BSD? The -o and -p
options are GNU coreutils extensions and might raise a
subprocess.CalledProcessError on systems where they are invalid.

[ ... ]
> else:
> # Fallback if no callchain is present
> try:
> # If the perf module exposes symbol/dso directly on sample
> # when callchain is missing, we use them.
> symbol = getattr(sample, 'symbol', '[unknown]')
> dso = getattr(sample, 'dso', '[unknown]')
> stack.append(f"{symbol} (in {dso})")

If the C extension explicitly returns Python's None for unresolved symbols
rather than raising an AttributeError, will getattr successfully return None
here? This would cause the f-string to evaluate to the literal string
"None (in None)" rather than the intended [unknown] fallback.

[ ... ]
> def _write_and_launch(self, profile: Dict, filename: str) -> None:
> """Write the profile to a file and launch the Firefox profiler."""
> print("Starting Firefox Profiler on your default browser...")
> with open(filename, 'w', encoding='utf-8') as f:
> json.dump(profile, f, indent=2)
>
> # Start HTTP server in a daemon thread
> def start_server():
> server_address = ('', 8000)

Does this configuration expose the current working directory to the entire
local network? By binding to an empty string, it listens on 0.0.0.0 (all
available interfaces) rather than restricting it securely to 127.0.0.1.

> httpd = HTTPServer(server_address, CORSRequestHandler)

What happens if port 8000 is already in use by another process? It looks
like HTTPServer will raise an OSError, causing the daemon thread to crash
silently while leaving the main thread trapped in an infinite while True
loop.

> httpd.serve_forever()
>
> thread = threading.Thread(target=start_server, daemon=True)
> thread.start()
>
> # Open the browser
> safe_string = urllib.parse.quote_plus(f'http://localhost:8000/{filename}')
> url = f'https://profiler.firefox.com/from-url/{safe_string}'
> webbrowser.open(url)

Is there a race condition here where the browser might attempt to fetch the
profile before the newly spawned server thread has actually bound to the
port and started listening?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260419235911.2186050-1-irogers@google.com?part=31