Conversation
There was a problem hiding this comment.
Pull request overview
This PR shifts remote debugging workflows from GEF’s gef-remote command toward GDB’s native target remote/extended-remote, adds GDB target hooks to initialize GEF’s remote session automatically, and updates/extends the test suite and CI coverage generation accordingly.
Changes:
- Deprecates
gef-remotebehavior by routing it totarget remoteand introducestarget remotepre/post hooks to initializegef.session.remote. - Updates tests to use
target remoteand adds new remote-mode API/regression coverage (includinggdbserver --multiand qemu-user scenarios). - Tweaks coverage generation (pytest invocation) and CI workflow steps (dependencies + building test binaries).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
tests/utils.py |
Uses shutil.which, adds gdbserver multi helpers, configurable startup delay, qemu-user args support, and a random-port helper used across tests. |
tests/regressions/gdbserver_connection.py |
Migrates regression test from gef-remote to target remote. |
tests/regressions/1131_target_remote_registers.py |
Adds a new slow regression test reproducer for target-remote register display. |
tests/commands/gef_remote.py |
Updates command tests to validate sessions via target remote and new RemoteSession(...) string format. |
tests/base.py |
Centralizes port selection via get_random_port() and runs tests under gdb-multiarch. |
tests/api/gef_session.py |
Updates session API tests to use target remote and shared port helper. |
tests/api/gef_remote.py |
Adds API-level tests for detecting remote/extended-remote and differentiating gdbserver multi vs qemu-user. |
tests/api/gef_memory.py |
Updates remote memory-maps tests to use target remote and shared port helper. |
scripts/generate-coverage-docs.sh |
Adjusts pytest invocation for coverage runs (--forked, verbosity, marker selection). |
gef.py |
Introduces target-remote detection helpers, new remote-mode handling, hooks for target remote, and reworks/limits legacy gef-remote behavior. |
docs/commands/gef-remote.md |
Updates docs to emphasize gef-remote deprecation. |
.github/workflows/coverage.yml |
Installs additional requirements, builds binaries, and runs updated coverage generation script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| python${{ env.PY_VER }} -m pip install --user --upgrade -r tests/requirements.txt -r docs/requirements.txt --quiet | ||
| current_score=$(curl --silent https://hugsy.github.io/gef/coverage/gef_py.html | grep pc_cov | sed 's?.*<span class="pc_cov">\([^%]*\)%</span>?\1?g') | ||
| make -C tests/binaries | ||
| bash scripts/generate-coverage-docs.sh |
There was a problem hiding this comment.
The workflow now runs the full test suite for coverage, and this PR adds a test that shells out to wget/unzip. Those packages are not installed in the job currently (only curl is). Either avoid those external dependencies in the test, or install the required packages here to prevent CI failures.
| path = evt.new_objfile.filename or "" | ||
| elif progspace: | ||
| path = progspace.filename or "" | ||
| else: | ||
| raise RuntimeError("Cannot determine file path") | ||
|
|
||
| assert path | ||
| try: |
There was a problem hiding this comment.
new_objfile_handler() now does assert path after pulling evt.new_objfile.filename/progspace.filename. In practice GDB can emit objfile events with an empty/None filename; asserting here will crash GEF instead of safely ignoring the event. Please replace the assert with an explicit guard (e.g., early-return when not path) and keep handling of special pseudo-paths (like "system-supplied DSO") intact.
| def target_remote_hook(): | ||
| # disable the context until the session has been fully established | ||
| gef.temp["context_old_value"] = gef.config["context.enable"] | ||
| gef.config["context.enable"] = False | ||
|
|
||
|
|
||
| def target_remote_posthook(): | ||
| if gef.session.remote_initializing: | ||
| return | ||
| conn = gdb.selected_inferior().connection | ||
| if not isinstance(conn, gdb.RemoteTargetConnection): | ||
| raise TypeError("Expected type gdb.RemoteTargetConnection") | ||
| assert is_target_remote_or_extended(conn), "Target is not remote" | ||
| gef.session.remote = GefRemoteSessionManager(conn) | ||
|
|
||
| # switch back context to its old context | ||
| gef.config["context.enable"] = gef.temp.pop("context_old_value", True) | ||
|
|
There was a problem hiding this comment.
target_remote_hook() disables context.enable before the target remote command runs, but if the command fails (bad host/port, auth, etc.) the post-hook may not execute and context will remain disabled for the rest of the session. Consider restoring the previous value on failure (e.g., wrap the post-hook logic in a defensive try/except that always resets config when the connection isn’t remote, or avoid changing the setting in the pre-hook).
| @@ -11639,77 +11645,25 @@ | |||
| reset_architecture() | |||
| return True | |||
There was a problem hiding this comment.
GefRemoteSessionManager.setup() unconditionally does gef.binary = Elf(self.lfile) after setting self.__local_root_path to / for several modes. If the remote executable is not present locally (which is a common target remote workflow), this will raise and break the connection post-hook. Please guard this with an existence check / try-except and fall back to leaving gef.binary unchanged (or use progspace.filename only when it points to a local file).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Frank Dana <ferdnyc@gmail.com>
Description
This PR obsoletes
gef-remoteto be completely replaced by pre and post hooks ontarget remotedirectly.We use some heuristics to determine what type of remote we're connected to reliably.
In some old qemu, we still have to mock a memory layout, but the whole
syncthing is not necessary any longer.Fixes #1080
Fixes #1128
Checklist