[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG13978643b678: [lldb] implement up and 
down shortcuts in lldb gui (authored by llunak).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541

Files:
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -3243,6 +3243,8 @@
 {'f', "Step out (finish)"},
 {'s', "Step in (source line)"},
 {'S', "Step in (single instruction)"},
+{'u', "Frame up"},
+{'d', "Frame down"},
 {',', "Page up"},
 {'.', "Page down"},
 {'\0', nullptr}};
@@ -3856,6 +3858,26 @@
 }
   return eKeyHandled;
 
+case 'u': // 'u' == frame up
+case 'd': // 'd' == frame down
+{
+  ExecutionContext exe_ctx =
+  m_debugger.GetCommandInterpreter().GetExecutionContext();
+  if (exe_ctx.HasThreadScope()) {
+Thread *thread = exe_ctx.GetThreadPtr();
+uint32_t frame_idx = thread->GetSelectedFrameIndex();
+if (frame_idx == UINT32_MAX)
+  frame_idx = 0;
+if (c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
+  ++frame_idx;
+else if (c == 'd' && frame_idx > 0)
+  --frame_idx;
+if (thread->SetSelectedFrameByIndex(frame_idx, true))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+  }
+}
+  return eKeyHandled;
+
 case 'h':
   window.CreateHelpSubwindow();
   return eKeyHandled;
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
@@ -0,0 +1,7 @@
+extern int func();
+
+int main(int argc, char **argv) {
+  func(); // Break here
+  func(); // Second
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
@@ -0,0 +1,3 @@
+int func() {
+  return 1; // In function
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -0,0 +1,42 @@
+"""
+Test the 'gui' shortcuts 's','n','f','u','d' (step in, step over, step out, up, down)
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestGuiBasicDebugCommandTest(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+self.expect('br set -f main.c -p "// Break here"', substrs=["Breakpoint 1", "address ="])
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI and close the welcome window.
+self.child.sendline("gui")
+self.child.send(escape_key)
+
+# Simulate a simple debugging session.
+self.child.send("s") # step
+self.child.expect("return 1; // In function[^\r\n]+<<< Thread 1: step in")
+self.child.send("u") # up
+self.child.expect_exact("func(); // Break here")
+self.child.send("d") # down
+self.child.expect_exact("return 1; // In function")
+self.child.send("f") # finish
+self.child.expect("func\(\); // Break here[^\r\n]+<<< Thread 1: step out")
+self.child.send("s") # move onto the second one
+self.child.expect("func\(\); // Second[^\r\n]+<<< Thread 1: step in")
+self.child.send("n") # step over
+self.child.expect("return 0;[^\r\n]+<<< Thread 1: step over")
+
+self.quit()
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c func.c
+include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Thanks again for working on this. I will review any subsequent patches as they 
come in if you have any more contributions. Sorry for the long delay again, I 
had 3 months that I was out due to the head injury, hopefully this won't happen 
again!


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak updated this revision to Diff 281705.
llunak added a comment.

Updated for current git, and clang-format-ed.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541

Files:
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -3243,6 +3243,8 @@
 {'f', "Step out (finish)"},
 {'s', "Step in (source line)"},
 {'S', "Step in (single instruction)"},
+{'u', "Frame up"},
+{'d', "Frame down"},
 {',', "Page up"},
 {'.', "Page down"},
 {'\0', nullptr}};
@@ -3856,6 +3858,26 @@
 }
   return eKeyHandled;
 
+case 'u': // 'u' == frame up
+case 'd': // 'd' == frame down
+{
+  ExecutionContext exe_ctx =
+  m_debugger.GetCommandInterpreter().GetExecutionContext();
+  if (exe_ctx.HasThreadScope()) {
+Thread *thread = exe_ctx.GetThreadPtr();
+uint32_t frame_idx = thread->GetSelectedFrameIndex();
+if (frame_idx == UINT32_MAX)
+  frame_idx = 0;
+if (c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
+  ++frame_idx;
+else if (c == 'd' && frame_idx > 0)
+  --frame_idx;
+if (thread->SetSelectedFrameByIndex(frame_idx, true))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+  }
+}
+  return eKeyHandled;
+
 case 'h':
   window.CreateHelpSubwindow();
   return eKeyHandled;
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
@@ -0,0 +1,7 @@
+extern int func();
+
+int main(int argc, char **argv) {
+  func(); // Break here
+  func(); // Second
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
@@ -0,0 +1,3 @@
+int func() {
+  return 1; // In function
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -0,0 +1,42 @@
+"""
+Test the 'gui' shortcuts 's','n','f','u','d' (step in, step over, step out, up, down)
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestGuiBasicDebugCommandTest(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+self.expect('br set -f main.c -p "// Break here"', substrs=["Breakpoint 1", "address ="])
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI and close the welcome window.
+self.child.sendline("gui")
+self.child.send(escape_key)
+
+# Simulate a simple debugging session.
+self.child.send("s") # step
+self.child.expect("return 1; // In function[^\r\n]+<<< Thread 1: step in")
+self.child.send("u") # up
+self.child.expect_exact("func(); // Break here")
+self.child.send("d") # down
+self.child.expect_exact("return 1; // In function")
+self.child.send("f") # finish
+self.child.expect("func\(\); // Break here[^\r\n]+<<< Thread 1: step out")
+self.child.send("s") # move onto the second one
+self.child.expect("func\(\); // Second[^\r\n]+<<< Thread 1: step in")
+self.child.send("n") # step over
+self.child.expect("return 0;[^\r\n]+<<< Thread 1: step over")
+
+self.quit()
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c func.c
+include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

My usual experience with Clang submissions is that I have to be really 
persistent to get a reaction, let alone an approval, so I assumed it was the 
same here and I didn't feel like pushing this that much. I'll update the 
patches to match current git and have a look at the review that still has 
questions.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Yikes, sorry for not responding for so long. In the fall I was out on medical 
leave due to a head injury. Please feel free to ping more often if I do this 
again. I commented in the other patches (one accepted, and questions in the 
other). Let me know your thoughts.




Comment at: lldb/source/Core/IOHandler.cpp:4433
+--frame_idx;
+if( thread->SetSelectedFrameByIndex( frame_idx, true ))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());

Clang format this?

```
if (thread->SetSelectedFrameByIndex(frame_idx, true))
```



Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

What is the state of the patch? Does lldb support cgdb-style u/d/f/b etc now?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D68541#1710489 , @llunak wrote:

> Can you please ACK also the other patches linked above that this one depends 
> on? Or can I take the discussion here as being enough?


Ping?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py:31
+#  which is not necessarily the order on the screen.)
+self.child.timeout = 2
+self.child.send("s") # step

llunak wrote:
> labath wrote:
> > What's the reason for that? It'd be best to not mess with the timeout in 
> > the test, and particularly to not set such a small time out as it will 
> > likely lead to flakyness on slow/heavily loaded machines.
> I added it because writing and debugging the unittest without this was a 
> pain, but fair enough.
> 
Yeah, the pexpect tests leave a lot to be desired, and there are ways to avoid 
waiting for the entire timeout period when the output does not match (in the 
common cases), but it needs someone willing to implement it.

The deficiencies of pexpect become particularly obivous in the "gui" tests as 
there the raw stream of bytes coming out of the pty is way too low-level to be 
able to match it reliably. If we take the `[^\r\n]` pattern above, it encodes a 
lot of assumptions about how lldb and ncurses choose to print to the screen -- 
ncurses could perfectly well decide to insert some line breaks after printing 
the source code, but then go back (via ansi cursor movement codes), and print 
the stop reason. The result would be exactly the same as far as the user is 
concerned, but the test would fail.

For testing the gui mode, I think we should build (or reuse, if something 
suitable exists) a very simple "terminal emulator" which "understands" cursor 
movement codes (and maybe some other stuff). Then we could assert that the 
contents of the line as the user would see it instead of how it happened to be 
printed out.

Anyway, I don't think you have to do that now, but it is something to think 
about, should you want to invest more heavily into the lldb "gui".


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D68541#1710031 , @clayborg wrote:

> Looks good!


Can you please ACK also the other patches linked above that this one depends 
on? Or can I take the discussion here as being enough?

> With a little work, "gui" mode can really become great. I hope to see more 
> patches? Happy to discuss next stops on the mailing list if you are 
> interested!

That's kind of the plan. I rather miss a variables view in gdb tui and I've 
decided that it should be simpler to implement a command prompt in lldb gui 
than a variables view in gdb tui. But I'm still evaluating, I've got other 
things on my plate, only so much time and it's a question if it'd be really 
just a little work to make lldb meet my needs (and I've already run into few 
lame problems such as one of those harmless signals like SIGTSTP simply killing 
the debugging without any useful feedback). We'll see how this goes.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Looks good! With a little work, "gui" mode can really become great. I hope to 
see more patches? Happy to discuss next stops on the mailing list if you are 
interested!


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak updated this revision to Diff 225102.
llunak added a comment.

Adjusted the testcase.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541

Files:
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
  lldb/source/Core/IOHandler.cpp

Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -3801,6 +3801,8 @@
 {'f', "Step out (finish)"},
 {'s', "Step in (source line)"},
 {'S', "Step in (single instruction)"},
+{'u', "Frame up"},
+{'d', "Frame down"},
 {',', "Page up"},
 {'.', "Page down"},
 {'\0', nullptr}};
@@ -4414,6 +4416,26 @@
 }
   return eKeyHandled;
 
+case 'u': // 'u' == frame up
+case 'd': // 'd' == frame down
+{
+  ExecutionContext exe_ctx =
+  m_debugger.GetCommandInterpreter().GetExecutionContext();
+  if (exe_ctx.HasThreadScope()) {
+Thread* thread = exe_ctx.GetThreadPtr();
+uint32_t frame_idx = thread->GetSelectedFrameIndex();
+if (frame_idx == UINT32_MAX)
+  frame_idx = 0;
+if( c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
+++frame_idx;
+else if (c =='d' && frame_idx > 0)
+--frame_idx;
+if( thread->SetSelectedFrameByIndex( frame_idx, true ))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+  }
+}
+  return eKeyHandled;
+
 case 'h':
   window.CreateHelpSubwindow();
   return eKeyHandled;
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
@@ -0,0 +1,7 @@
+extern int func();
+
+int main(int argc, char **argv) {
+func(); // Break here
+func(); // Second
+return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
@@ -0,0 +1,3 @@
+int func() {
+return 1; // In function
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -0,0 +1,42 @@
+"""
+Test the 'gui' shortcuts 's','n','f','u','d' (step in, step over, step out, up, down)
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestGuiBasicDebugCommandTest(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+self.expect('br set -f main.c -p "// Break here"', substrs=["Breakpoint 1", "address ="])
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI and close the welcome window.
+self.child.sendline("gui")
+self.child.send(escape_key)
+
+# Simulate a simple debugging session.
+self.child.send("s") # step
+self.child.expect("return 1; // In function[^\r\n]+<<< Thread 1: step in")
+self.child.send("u") # up
+self.child.expect_exact("func(); // Break here")
+self.child.send("d") # down
+self.child.expect_exact("return 1; // In function")
+self.child.send("f") # finish
+self.child.expect("func\(\); // Break here[^\r\n]+<<< Thread 1: step out")
+self.child.send("s") # move onto the second one
+self.child.expect("func\(\); // Second[^\r\n]+<<< Thread 1: step in")
+self.child.send("n") # step over
+self.child.expect("return 0;[^\r\n]+<<< Thread 1: step over")
+
+self.quit()
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c func.c
+include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py:32
+self.child.send("s") # step
+self.child.expect_exact("return 1; // In function")
+self.child.expect_exact("Thread 1: step in")

Just checking for the source is probably not enough here. We need to check for 
the source _and_ the stop reason and ensure they are on the same line. If the 
output to the screen was:

```
return 1; // In function
other code here;  <<< Thread 1: step in
```

We would want this to fail (it will currently succeed), but we want it to 
succeed if we get it all on one line:

```
return 1; // In function   <<< Thread 1: step in
```

This applies to all expect_exact calls below as well.

So this should probably be something that uses a regex like:

```
self.child.expect("return 1; // In function[^\r\n]+<<< Thread 1: step in")
```





Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak updated this revision to Diff 225064.
llunak added a comment.

Removed the explicit test timeout.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541

Files:
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
  lldb/source/Core/IOHandler.cpp

Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -3771,6 +3771,8 @@
 {'f', "Step out (finish)"},
 {'s', "Step in (source line)"},
 {'S', "Step in (single instruction)"},
+{'u', "Frame up"},
+{'d', "Frame down"},
 {',', "Page up"},
 {'.', "Page down"},
 {'\0', nullptr}};
@@ -4384,6 +4386,26 @@
 }
   return eKeyHandled;
 
+case 'u': // 'u' == frame up
+case 'd': // 'd' == frame down
+{
+  ExecutionContext exe_ctx =
+  m_debugger.GetCommandInterpreter().GetExecutionContext();
+  if (exe_ctx.HasThreadScope()) {
+Thread* thread = exe_ctx.GetThreadPtr();
+uint32_t frame_idx = thread->GetSelectedFrameIndex();
+if (frame_idx == UINT32_MAX)
+  frame_idx = 0;
+if( c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
+++frame_idx;
+else if (c =='d' && frame_idx > 0)
+--frame_idx;
+if( thread->SetSelectedFrameByIndex( frame_idx, true ))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+  }
+}
+  return eKeyHandled;
+
 case 'h':
   window.CreateHelpSubwindow();
   return eKeyHandled;
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
@@ -0,0 +1,7 @@
+extern int func();
+
+int main(int argc, char **argv) {
+func(); // Break here
+func(); // Second
+return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
@@ -0,0 +1,3 @@
+int func() {
+return 1; // In function
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -0,0 +1,45 @@
+"""
+Test the 'gui' shortcuts 's','n','f','u','d' (step in, step over, step out, up, down)
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestGuiBasicDebugCommandTest(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+self.expect('br set -f main.c -p "// Break here"', substrs=["Breakpoint 1", "address ="])
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI and close the welcome window.
+self.child.sendline("gui")
+self.child.send(escape_key)
+
+# Simulate a simple debugging session.
+# (Note: The order of responses depend on how lldb/curses print them,
+#  which is not necessarily the order on the screen.)
+self.child.send("s") # step
+self.child.expect_exact("return 1; // In function")
+self.child.expect_exact("Thread 1: step in")
+self.child.send("u") # up
+self.child.expect_exact("func(); // Break here")
+self.child.send("d") # down
+self.child.expect_exact("return 1; // In function")
+self.child.send("f") # finish
+self.child.expect_exact("Thread 1: step out")
+self.child.expect_exact("func(); // Second")
+self.child.send("n") # step over
+self.child.expect_exact("return 0;")
+self.child.expect_exact("Thread 1: step over")
+
+self.quit()
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c func.c
+include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak marked an inline comment as done.
llunak added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py:31
+#  which is not necessarily the order on the screen.)
+self.child.timeout = 2
+self.child.send("s") # step

labath wrote:
> What's the reason for that? It'd be best to not mess with the timeout in the 
> test, and particularly to not set such a small time out as it will likely 
> lead to flakyness on slow/heavily loaded machines.
I added it because writing and debugging the unittest without this was a pain, 
but fair enough.



Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py:31
+#  which is not necessarily the order on the screen.)
+self.child.timeout = 2
+self.child.send("s") # step

What's the reason for that? It'd be best to not mess with the timeout in the 
test, and particularly to not set such a small time out as it will likely lead 
to flakyness on slow/heavily loaded machines.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak updated this revision to Diff 224737.
llunak edited the summary of this revision.
llunak added a comment.

Added a unittest.

Also, the other discussed changes are at https://reviews.llvm.org/D68908 and 
https://reviews.llvm.org/D68909 .


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541

Files:
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
  lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
  lldb/source/Core/IOHandler.cpp

Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -3771,6 +3771,8 @@
 {'f', "Step out (finish)"},
 {'s', "Step in (source line)"},
 {'S', "Step in (single instruction)"},
+{'u', "Frame up"},
+{'d', "Frame down"},
 {',', "Page up"},
 {'.', "Page down"},
 {'\0', nullptr}};
@@ -4384,6 +4386,26 @@
 }
   return eKeyHandled;
 
+case 'u': // 'u' == frame up
+case 'd': // 'd' == frame down
+{
+  ExecutionContext exe_ctx =
+  m_debugger.GetCommandInterpreter().GetExecutionContext();
+  if (exe_ctx.HasThreadScope()) {
+Thread* thread = exe_ctx.GetThreadPtr();
+uint32_t frame_idx = thread->GetSelectedFrameIndex();
+if (frame_idx == UINT32_MAX)
+  frame_idx = 0;
+if( c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
+++frame_idx;
+else if (c =='d' && frame_idx > 0)
+--frame_idx;
+if( thread->SetSelectedFrameByIndex( frame_idx, true ))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+  }
+}
+  return eKeyHandled;
+
 case 'h':
   window.CreateHelpSubwindow();
   return eKeyHandled;
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/main.c
@@ -0,0 +1,7 @@
+extern int func();
+
+int main(int argc, char **argv) {
+func(); // Break here
+func(); // Second
+return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/func.c
@@ -0,0 +1,3 @@
+int func() {
+return 1; // In function
+}
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -0,0 +1,46 @@
+"""
+Test the 'gui' shortcuts 's','n','f','u','d' (step in, step over, step out, up, down)
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestGuiBasicDebugCommandTest(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+self.expect('br set -f main.c -p "// Break here"', substrs=["Breakpoint 1", "address ="])
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI and close the welcome window.
+self.child.sendline("gui")
+self.child.send(escape_key)
+
+# Simulate a simple debugging session.
+# (Note: The order of responses depend on how lldb/curses print them,
+#  which is not necessarily the order on the screen.)
+self.child.timeout = 2
+self.child.send("s") # step
+self.child.expect_exact("return 1; // In function")
+self.child.expect_exact("Thread 1: step in")
+self.child.send("u") # up
+self.child.expect_exact("func(); // Break here")
+self.child.send("d") # down
+self.child.expect_exact("return 1; // In function")
+self.child.send("f") # finish
+self.child.expect_exact("Thread 1: step out")
+self.child.expect_exact("func(); // Second")
+self.child.send("n") # step over
+self.child.expect_exact("return 0;")
+self.child.expect_exact("Thread 1: step over")
+
+self.quit()
Index: lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c func.c
+include Makefile.rules

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D68541#1700398 , @llunak wrote:

> In D68541#1700359 , @clayborg wrote:
>
> > It would be fine to remove the 'd' shortcut for detach and resume if we 
> > need to by moving this functionality up into the menu bar. Right now the 
> > Process menu has "Detach" only, so it might be good to first change the 
> > "Process" menu to have "Detach" or "Detach suspended".  I agree that 
> > accidentally hitting 'd' would be bad.
>
>
> Ok, I can try to do that. And 'k' for "Kill process" should probably move to 
> the menu as well.


Sounds fine to me!

> But another shortcut I'd like to change is 'o' for "Step out", as that one is 
> 'f' for "Finish" in gdb tui, and I'm rather used to that.

I am fine with this as well.

> Unless you'd be willing to accept a patch changing that, can you comment on 
> the idea of making the shortcuts configurable?

Configurability would be nice, happy to accept patches that implement that.

I created "gui" mode many many years ago and hoped people would jump in and 
start making patches, so I am very happy to see this patch and look forward to 
more. It won't take too much work for us to make "gui" mode really useful, just 
a bit of work!


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D68541#1700359 , @clayborg wrote:

> It would be fine to remove the 'd' shortcut for detach and resume if we need 
> to by moving this functionality up into the menu bar. Right now the Process 
> menu has "Detach" only, so it might be good to first change the "Process" 
> menu to have "Detach" or "Detach suspended".  I agree that accidentally 
> hitting 'd' would be bad.


Ok, I can try to do that. And 'k' for "Kill process" should probably move to 
the menu as well.

But another shortcut I'd like to change is 'o' for "Step out", as that one is 
'f' for "Finish" in gdb tui, and I'm rather used to that. Unless you'd be 
willing to accept a patch changing that, can you comment on the idea of making 
the shortcuts configurable?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

It would be fine to remove the 'd' shortcut for detach and resume if we need to 
by moving this functionality up into the menu bar. Right now the Process menu 
has "Detach" only, so it might be good to first change the "Process" menu to 
have "Detach" or "Detach suspended".  I agree that accidentally hitting 'd' 
would be bad.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D68541#1698127 , @JDevlieghere 
wrote:

> Would it be possible to add a test for this? Maybe you can extend 
> `TestGuiBasic.py`.


I can, although to me it looks like a new test would be better. But first of 
all I wanted some feedback on this, as the patch in this form doesn't even 
compile. I'd actually personally prefer to just nuke the "Detach and resume 
process" shortcut, as I not only fail to see its usefulness but I also find it 
dangerous (I certainly don't want to hit it by mistake and ruin my debugging 
session), but then I assume there probably was a reason why it was added.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Would it be possible to add a test for this? Maybe you can extend 
`TestGuiBasic.py`.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68541/new/

https://reviews.llvm.org/D68541



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak created this revision.
llunak added a reviewer: clayborg.
llunak added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.

The attached patch implements 'u' and 'd' keyboard shortcuts in lldb gui, 
similar to gdb tui's shortcuts.

One obvious problem is that 'd' is already taken. Would something like 
'settings set gui.shortcut. ' be an acceptable solution for that?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68541

Files:
  lldb/source/Core/IOHandler.cpp


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -3771,6 +3771,8 @@
 {'o', "Step out"},
 {'s', "Step in (source line)"},
 {'S', "Step in (single instruction)"},
+{'u', "Frame up"},
+{'d', "Frame down"},
 {',', "Page up"},
 {'.', "Page down"},
 {'\0', nullptr}};
@@ -4395,6 +4397,26 @@
 }
   return eKeyHandled;
 
+case 'u': // 'u' == frame up
+case 'd': // 'd' == frame down
+{
+  ExecutionContext exe_ctx =
+  m_debugger.GetCommandInterpreter().GetExecutionContext();
+  if (exe_ctx.HasThreadScope()) {
+Thread* thread = exe_ctx.GetThreadPtr();
+uint32_t frame_idx = thread->GetSelectedFrameIndex();
+if (frame_idx == UINT32_MAX)
+  frame_idx = 0;
+if( c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
+++frame_idx;
+else if (c =='d' && frame_idx > 0)
+--frame_idx;
+if( thread->SetSelectedFrameByIndex( frame_idx, true ))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+  }
+}
+  return eKeyHandled;
+
 case 'h':
   window.CreateHelpSubwindow();
   return eKeyHandled;


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -3771,6 +3771,8 @@
 {'o', "Step out"},
 {'s', "Step in (source line)"},
 {'S', "Step in (single instruction)"},
+{'u', "Frame up"},
+{'d', "Frame down"},
 {',', "Page up"},
 {'.', "Page down"},
 {'\0', nullptr}};
@@ -4395,6 +4397,26 @@
 }
   return eKeyHandled;
 
+case 'u': // 'u' == frame up
+case 'd': // 'd' == frame down
+{
+  ExecutionContext exe_ctx =
+  m_debugger.GetCommandInterpreter().GetExecutionContext();
+  if (exe_ctx.HasThreadScope()) {
+Thread* thread = exe_ctx.GetThreadPtr();
+uint32_t frame_idx = thread->GetSelectedFrameIndex();
+if (frame_idx == UINT32_MAX)
+  frame_idx = 0;
+if( c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
+++frame_idx;
+else if (c =='d' && frame_idx > 0)
+--frame_idx;
+if( thread->SetSelectedFrameByIndex( frame_idx, true ))
+  exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+  }
+}
+  return eKeyHandled;
+
 case 'h':
   window.CreateHelpSubwindow();
   return eKeyHandled;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits