[lldb] Require paused process and frame for "register info" command

[DavidSpickett]

Prior to this the command would simply crash when run on a running process.

Of the three register commands, "info" was the only one missing these requirements. On some level it makes sense because you're not going to read a value or modify anything, but practically I think lldb assumes any time you're going to access register related stuff, the process should be paused.

I noticed this debugging with a remote gdb stub, so I've recreated that scenario using attach in a new test case.

[llvmbot]

@llvm/pr-subscribers-lldb

Changes

Prior to this the command would simply crash when run on a running process.

Of the three register commands, "info" was the only one missing these requirements. On some level it makes sense because you're not going to read a value or modify anything, but practically I think lldb assumes any time you're going to access register related stuff, the process should be paused.

I noticed this debugging with a remote gdb stub, so I've recreated that scenario using attach in a new test case.

---

Full diff: https://github.com/llvm/llvm-project/pull/67124.diff

2 Files Affected:

• (modified) lldb/source/Commands/CommandObjectRegister.cpp (+3-2)
• (modified) lldb/test/API/commands/register/register/register_command/TestRegisters.py (+13)

diff --git a/lldb/source/Commands/CommandObjectRegister.cpp b/lldb/source/Commands/CommandObjectRegister.cpp
index a0e88f6ab4ba27d..6e6071fd54606d0 100644
--- a/lldb/source/Commands/CommandObjectRegister.cpp
+++ b/lldb/source/Commands/CommandObjectRegister.cpp
@@ -406,8 +406,9 @@ class CommandObjectRegisterInfo : public CommandObjectParsed {
   CommandObjectRegisterInfo(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "register info",
                             "View information about a register.", nullptr,
-                            eCommandRequiresRegContext |
-                                eCommandProcessMustBeLaunched) {
+                            eCommandRequiresFrame | eCommandRequiresRegContext |
+                                eCommandProcessMustBeLaunched |
+                                eCommandProcessMustBePaused) {
     SetHelpLong(R"(
 Name             The name lldb uses for the register, optionally with an alias.
 Size             The size of the register in bytes and again in bits.
diff --git a/lldb/test/API/commands/register/register/register_command/TestRegisters.py b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
index 2e5c82a26cf1b9d..0ad9b8b24b3585c 100644
--- a/lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -659,3 +659,16 @@ def test_fs_gs_base(self):
             pthread_self_val.GetValueAsUnsigned(0),
             "fs_base does not equal to pthread_self() value.",
         )
+
+    def test_process_must_be_stopped(self):
+        """Check that all register commands error when the process is not stopped."""
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        pid = self.spawnSubprocess(exe, ["wait_for_attach"]).pid
+        self.setAsync(True)
+        self.runCmd("process attach --continue -p %d" % pid)
+
+        err_msg = "Command requires a process which is currently stopped."
+        self.expect("register read pc", substrs=[err_msg], error=True)
+        self.expect("register write pc 0", substrs=[err_msg], error=True)
+        self.expect("register info pc", substrs=[err_msg], error=True)

[jimingham]

process attach --continue won't return till the process stops when in sync mode. That's really what sync mode means: "commands that continue the target don't return till the target stops". But if the process is hitting a breakpoint or exiting and we aren't exiting the `process attach --continue` command in reaction to that, then that is a bug. Jim

…

On Sep 22, 2023, at 7:31 AM, David Spickett ***@***.***> wrote: @DavidSpickett commented on this pull request. In lldb/test/API/commands/register/register/register_command/TestRegisters.py <#67124 (comment)>: > + self.setAsync(True) + self.runCmd("process attach --continue -p %d" % pid) What I found was that process attach --continue would never return if you weren't in async mode. Whether that's a bug in itself I'm not sure. — Reply to this email directly, view it on GitHub <#67124 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVWYWCVHJNOT672DWAXLX3WOKRANCNFSM6AAAAAA5DBERUM>. You are receiving this because your review was requested.

[DavidSpickett]

Thanks for the explanation. Now I understand what I'm seeing is the expected behaviour, and async mode is the expected way to achieve what the test needs.