Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debug: use TCL-RPC to fetch results of OpenOCD commands instead of parsing log file #522

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 38 additions & 29 deletions debug/testlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import time
import traceback

import tty
import pexpect
import yaml

Expand Down Expand Up @@ -308,20 +309,16 @@ def __init__(self, server_cmd=None, config=None, debug=False, timeout=60,
cmd = shlex.split(server_cmd)
else:
cmd = ["openocd"]
if debug:
cmd.append("-d")

# This command needs to come before any config scripts on the command
# line, since they are executed in order.
cmd += [
# Tell OpenOCD to bind gdb to an unused, ephemeral port.
"--command", "gdb_port 0",
# We don't use the TCL server.
"--command", "tcl_port disabled",
# Put the regular command prompt in stdin. Don't listen on a port
# because it will conflict if multiple OpenOCD instances are running
# at the same time.
"--command", "telnet_port pipe",
# We create a socket for OpenOCD command line (TCL-RPC)
"--command", "tcl_port 0",
# don't use telnet
"--command", "telnet_port disabled",
]

if config:
Expand All @@ -331,6 +328,7 @@ def __init__(self, server_cmd=None, config=None, debug=False, timeout=60,
sys.exit(1)

cmd += ["-f", self.config_file]

if debug:
cmd.append("-d")

Expand Down Expand Up @@ -366,28 +364,47 @@ def __init__(self, server_cmd=None, config=None, debug=False, timeout=60,
logfile.flush()

self.gdb_ports = []
self.tclrpc_port = None
self.start(cmd, logfile, extra_env)

self.openocd_cli = pexpect.spawn(f"nc localhost {self.tclrpc_port}")
# TCL-RPC uses \x1a as a watermark for end of message. We set raw
# pty mode to disable translation of \x1a to EOF
tty.setraw(self.openocd_cli.child_fd)

def start(self, cmd, logfile, extra_env):
combined_env = {**os.environ, **extra_env}
# pylint: disable-next=consider-using-with
self.process = subprocess.Popen(cmd, stdin=subprocess.PIPE,
self.process = subprocess.Popen(cmd, stdin=None,
stdout=logfile, stderr=logfile, env=combined_env)

try:
# Wait for OpenOCD to have made it through riscv_examine(). When
# using OpenOCD to communicate with a simulator this may take a
# long time, and gdb will time out when trying to connect if we
# attempt too early.

while True:
m = self.expect(
rb"(Listening on port (\d+) for gdb connections|"
rb"tcl server disabled)",
rb"Listening on port (?P<port>\d+) for "
rb"(?P<server>(?:gdb)|(?:tcl)) connections",
message="Waiting for OpenOCD to start up...")
if b"gdb" in m.group(1):
self.gdb_ports.append(int(m.group(2)))
else:
if m["server"] == b"gdb":
self.gdb_ports.append(int(m["port"]))
elif m["server"] == b"tcl":
if self.tclrpc_port:
raise TestLibError(
"unexpected re-definition of TCL-RPC port")
self.tclrpc_port = int(m["port"])
# WARNING! WARNING! WARNING!
# The condition below works properly only if OpenOCD reports
# gdb/tcl ports in a specific order. Namely, it requires the
# gdb ports to be reported before the tcl one. At the moment
# this comment was written OpenOCD reports these ports in the
# required order if we have a call to `init` statement in
# either target configuration file or command-line parameter.
# All configuration files used in testing include a call to
# `init`
if self.tclrpc_port and (len(self.gdb_ports) > 0):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is not introduced by this patch, but I think it worth pointing out. This condition relies on all GDB ports being reported by OpenOCD prior to TCL-RPC port.
In turn this depends on whether the call to init is implicit or explicit:

> openocd ... -c "gdb_port 0" -c "tcl_port 0"  -c "telnet_port disabled" |& grep 'Listening'
Info : Listening on port 41587 for tcl connections
Info : Listening on port 40911 for gdb connections
Info : Listening on port 33639 for gdb connections
> openocd ... -c "gdb_port 0" -c "tcl_port 0"  -c "telnet_port disabled" -c init |& grep 'Listening'
Info : Listening on port 40575 for gdb connections
Info : Listening on port 38511 for gdb connections
Info : Listening on port 35989 for tcl connections

Currently, this works, since all the configs in debug/targets/RISC-V call init explicitly.
Wouldn't it be better to add some comment regarding this behavior?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not add anything unnecessary to this patch. It's fairly urgent since without this fix, OpenOCD development is hamstrung. It's better to merge this soon than it is to disable the tests while we add more improvements to this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already addressed this. Just added a comment

break

if self.debug_openocd:
Expand Down Expand Up @@ -420,20 +437,12 @@ def smp(self):
return False

def command(self, cmd):
"""Write the command to OpenOCD's stdin. Return the output of the
command, minus the prompt."""
self.process.stdin.write(f"{cmd}\n".encode())
self.process.stdin.flush()
m = self.expect(re.escape(f"{cmd}\n".encode()))

# The prompt isn't flushed to the log, so send a unique command that
# lets us find where output of the last command ends.
magic = f"# {self.command_count}x".encode()
self.command_count += 1
self.process.stdin.write(magic + b"\n")
self.process.stdin.flush()
m = self.expect(rb"(.*)^>\s*" + re.escape(magic))
return m.group(1)
"""Send the command to OpenOCD's TCL-RPC server. Return the output of
the command, minus the prompt."""
self.openocd_cli.write(f"{cmd}\n\x1a")
self.openocd_cli.expect(rb"(.*)\x1a")
m = self.openocd_cli.match.group(1)
return m

def expect(self, regex, message=None):
"""Wait for the regex to match the log, and return the match object. If
Expand Down
Loading