[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-24 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333205: [lldb-mi] Add possibility to set breakpoints without 
selecting a target. (authored by adrian, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46588?vs=148323&id=148436#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lldb/trunk/lit/lit.cfg
  lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test
  lldb/trunk/lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
  lldb/trunk/lit/tools/lldb-mi/breakpoint/lit.local.cfg
  lldb/trunk/tools/lldb-mi/MICmdCmdBreak.cpp
  lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: lldb/trunk/tools/lldb-mi/MICmdCmdBreak.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmdCmdBreak.cpp
+++ lldb/trunk/tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: lldb/trunk/lit/lit.cfg
===
--- lldb/trunk/lit/lit.cfg
+++ lldb/trunk/lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldbmi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldbmi', lldbmi + " --synchronous"))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
Index: lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test
===
--- lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test
+++ lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test
@@ -0,0 +1,15 @@
+# RUN: %cc %p/inputs/break-insert.c -g
+# RUN: %lldbmi < %s | FileCheck %s
+
+# Test that a breakpoint can be inserted before creating a target.
+
+-break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+
+-file-exec-and-symbols a.out
+# CHECK: ^done
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
Index: lldb/trunk/lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
===
--- lldb/trunk/lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
+++ lldb/trunk/lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
Index: lldb/trunk/lit/tools/lldb-mi/breakpoint/lit.local.cfg
===
--- lldb/trunk/lit/tools/lldb-mi/breakpoint/lit.local.cfg
+++ lldb/trunk/lit/tools/lldb-mi/breakpoint/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-23 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 148323.
polyakov.alex added a comment.

Removed underscore in the lldb-mi variable from lit.cfg.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lit/lit.cfg
  lit/tools/lldb-mi/breakpoint/break-insert.test
  lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
  lit/tools/lldb-mi/breakpoint/lit.local.cfg
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lit/tools/lldb-mi/breakpoint/lit.local.cfg
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Index: lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
Index: lit/tools/lldb-mi/breakpoint/break-insert.test
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/break-insert.test
@@ -0,0 +1,15 @@
+# RUN: %cc %p/inputs/break-insert.c -g
+# RUN: %lldbmi < %s | FileCheck %s
+
+# Test that a breakpoint can be inserted before creating a target.
+
+-break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+
+-file-exec-and-symbols a.out
+# CHECK: ^done
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldbmi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldbmi', lldbmi + " --synchronous"))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-23 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: lit/lit.cfg:61
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+

aprantl wrote:
> aprantl wrote:
> > It looks like "lldb-mi" may not be a valid substitution. On Darwin the 
> > command
> > 
> > `# RUN: %lldb_mi `
> > 
> > is expanded to
> > 
> > bin/lldb -S .../llvm/tools/lldb/lit/lit-lldb-init_mi < ...
> > 
> > So it behaves like `%lldb`+`_mi.`
> > 
> Can you see if you can figure out what's going on? Perhaps we need to rename 
> it to lldbmi without the underscore?
On linux it's ok. I'll remove underscore and be appreciated if you check it on 
Darwin.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-23 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lit/lit.cfg:61
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+

aprantl wrote:
> It looks like "lldb-mi" may not be a valid substitution. On Darwin the command
> 
> `# RUN: %lldb_mi `
> 
> is expanded to
> 
> bin/lldb -S .../llvm/tools/lldb/lit/lit-lldb-init_mi < ...
> 
> So it behaves like `%lldb`+`_mi.`
> 
Can you see if you can figure out what's going on? Perhaps we need to rename it 
to lldbmi without the underscore?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-23 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lit/lit.cfg:61
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+

It looks like "lldb-mi" may not be a valid substitution. On Darwin the command

`# RUN: %lldb_mi `

is expanded to

bin/lldb -S .../llvm/tools/lldb/lit/lit-lldb-init_mi < ...

So it behaves like `%lldb`+`_mi.`



Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-19 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added a comment.

LGTM!


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-19 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 147681.
polyakov.alex added a comment.

This version of commit uses the new lldb-mi option that I added to review 
https://reviews.llvm.org/D47110.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lit/lit.cfg
  lit/tools/lldb-mi/breakpoint/break-insert.test
  lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
  lit/tools/lldb-mi/breakpoint/lit.local.cfg
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lit/tools/lldb-mi/breakpoint/lit.local.cfg
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Index: lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
Index: lit/tools/lldb-mi/breakpoint/break-insert.test
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/break-insert.test
@@ -0,0 +1,15 @@
+# RUN: %cc %p/inputs/break-insert.c -g
+# RUN: %lldb_mi < %s | FileCheck %s
+
+# Test that a breakpoint can be inserted before creating a target.
+
+-break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+
+-file-exec-and-symbols a.out
+# CHECK: ^done
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldb_mi', lldb_mi + " --synchronous"))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Okay, that sounds promising! Then let's proceed this way:

- Add a new command line option to lldb-mi that is called `--synchronous` with 
a help text "Block until each command has finished executing. Used for testing 
only." and use it in this test
- continue writing as many lldb-mi tests as possible (everything where the 
commands don't depend on replies) using the lit/FileCheck approach
- gradually convert existing pexpect tests over to lit/FileCheck
- convert the the remaining tests to use synchronous mode, which might improve 
the reliability a tiny bit. We will still run into situations where on very 
loaded systems the pexpect timeout is triggered before a reply comes back from 
lldb-mi, but it will be less likely.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-17 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added a comment.

In https://reviews.llvm.org/D46588#1103237, @aprantl wrote:

> For the experiment you can probably just stick it into 
> `CMICmnLLDBDebugger::InitSBDebugger()`.


Yep, after that, test is passing with only `CHECK` directives.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In https://reviews.llvm.org/D46588#1103237, @aprantl wrote:

> For the experiment you can probably just stick it into 
> `CMICmnLLDBDebugger::InitSBDebugger()`.


But don't do it here permanently...


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

For the experiment you can probably just stick it into 
`CMICmnLLDBDebugger::InitSBDebugger()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-17 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added a comment.

In https://reviews.llvm.org/D46588#1103180, @aprantl wrote:

> My overall mental image of lldb-mi is very incomplete, but I'm imagining 
> lldb-mi as having one thread that wakes up every n milliseconds checks for 
> command input and then calls into the SBAPI to handle the commands. If that 
> is how it works, then one very simple thing we can try is to call 
> SBDebugger::SetAsync(false), thus forcing all the SBAPI calls to block until 
> completion.
>
> Alexander, can you see what happens to your lit test when you enable 
> synchronous mode in the SBAPI? Does it block now as expected, or does it 
> still finish early? If that experiment works, we could add a --async command 
> line option to lldb-mi to enable this behavior for running the test suite.


Where is the right place to call SBDebugger::SetAsync(true) to enable 
asynchronous mode?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

My overall mental image of lldb-mi is very incomplete, but I'm imagining 
lldb-mi as having one thread that wakes up every n milliseconds checks for 
command input and then calls into the SBAPI to handle the commands. If that is 
how it works, then one very simple thing we can try is to call 
SBDebugger::SetAsync(false), thus forcing all the SBAPI calls to block until 
completion.

Alexander, can you see what happens to your lit test when you enable 
synchronous mode in the SBAPI? Does it block now as expected, or does it still 
finish early? If that experiment works, we could add a --async command line 
option to lldb-mi to enable this behavior for running the test suite.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D46588#1102363, @aprantl wrote:

> >   The advantage of the second one is that we will have the ability to 
> > inject commands which depend on the results of previous commands (something 
> > that I think we will need, sooner or later).
>
> That is worth considering. To write good tests that depend on previous 
> results, we probably want to have SBAPI-like Python scripting for lldb-mi 
> available. To make that work we would need to introduce an 
> lldbmi.HandleCommand("-mycmd...") interface that runs one command, blocks on 
> it, and returns the output in a string. And then we would need to export that 
> to Python. Since the interface is string-in-string-out, we could probably do 
> it via C without even involving SWIG. As an escape hatch to access features 
> outside of the mi command set, we could implement the -interpreter-exec mi 
> command that passes its input to SBAPI's HandleCommand() if need be.
>  Alternatively we could use gtest to write unittest-style lldb-mi tests 
> directly in C++, if going via Python is too messy. Again, we would do that in 
> a blocking or synchronized I/O mode.


I agree with your analysis. I don't want to give an opinion on the direction, 
because:
a) it's not clear to me which way to go, each possibility has different 
tradeoffs involved
b) I don't see myself doing anything related to lldb-mi any time soon

What I can offer as an additional data point is the lldb-server test suite. 
lldb-server very similar to lldb-mi in a way, as it has an almost-text-based 
protocol as its main form of interaction with the world. We also have two ways 
of testing it: a python one (`test/testcases/tools/lldb-server` and a gtest one 
(`unittests/tools/lldb-server`). the python one is older, but a bit messy 
(IMO). The gtest one was started because we had a person interested in 
improving lldb-server testing situation. I supported starting a fresh sub-test 
suite, because I saw it as a way to solve a couple some fundamental issues in 
the old one:

- code duplication: the python test suite needs a complete reimplementation of 
the gdb-remote protocol. the c++ test can just hook it to the existing 
functionality at an appropriate layer.
- remote testing mess: a very convoluted setup is required to run tests on a 
remote (android) device. It's weird that you have to build a host lldb and run 
the tests from there, even though the lldb-server and the executables it debugs 
run on a different host/arch. The python part of the test needs to run on the 
host (there's no easily available python for android), so there's a lot of 
flakyness and complexity coming from the port forwarding/network communication).

The c++ based approach could solve these, as the gtest executable could run on 
the same device as the lldb-server under test, and it would be possible to run 
the tests via check-lldb(-server?) from the cross-build tree instead of copying 
things around between two build trees.

Sadly, the person who was supposed to do this left the team. I have tried to 
carry this idea on slowly, but haven't yet gotten around to implementing the 
most important part (running tests remotely), so I couldn't start migrating the 
tests. In retrospect, I am less excited about this idea than I was a year ago, 
though I still think it would offer a better testing story for lldb-server.

Overall, I am not sure how much of this relates to lldb-mi (for example, you 
probably aren't interested in running it in such a constrained environment), 
but I thought it might be interesting for you to look at these before deciding 
on a direction. I also don't want to hold up Alexander's internship over this, 
as I'm not sure this is what he signed up for. I wanted to bring up the testing 
story, because I think there are some issues there, but I'll leave it up to you 
(for some definition of "you") to decide on what to do about that.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-16 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

>   The advantage of the second one is that we will have the ability to inject 
> commands which depend on the results of previous commands (something that I 
> think we will need, sooner or later).

That is worth considering. To write good tests that depend on previous results, 
we probably want to have SBAPI-like Python scripting for lldb-mi available. To 
make that work we would need to introduce an lldbmi.HandleCommand("-mycmd...") 
interface that runs one command, blocks on it, and returns the output in a 
string. And then we would need to export that to Python. Since the interface is 
string-in-string-out, we could probably do it via C without even involving 
SWIG. As an escape hatch to access features outside of the mi command set, we 
could implement the -interpreter-exec mi command that passes its input to 
SBAPI's HandleCommand() if need be.
Alternatively we could use gtest to write unittest-style lldb-mi tests directly 
in C++, if going via Python is too messy. Again, we would do that in a blocking 
or synchronized I/O mode.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-16 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 147117.
polyakov.alex added a comment.

I have all the patches I was sending. I will follow this topic until you decide 
what to do with lldb-mi testing.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp


Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
@@ -143,6 +143,32 @@
 self.expect("\^running")
 self.expect("\*stopped,reason=\"breakpoint-hit\"")
 
+@skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows.
+@skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races.
+@skipIfRemote   # We do not currently support remote debugging via the MI.
+def test_lldbmi_break_insert_without_target(self):
+"""Test that 'lldb-mi --interpreter' can set breakpoints without
+selecting a valid target."""
+
+self.spawnLldbMi(args=None)
+
+# This command have to insert breakpoint to the dummy target.
+self.runCmd("-break-insert main")
+# FIXME function name is unknown on Darwin, fullname should be ??, 
line is -1
+self.expect(
+
"\^done,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\","
+
"addr=\"0x\",func=\"\?\?\",file=\"\?\?\",fullname=\"\?\?/\?\?\","
+
"line=\"0\",pending=\[\"main\"\],times=\"0\",original-location=\"main\"}"
+)
+
+# Add a valid target.
+self.runCmd("-file-exec-and-symbols %s" % self.myexe)
+self.expect("\^done")
+
+self.runCmd("-exec-run")
+self.expect("\^running")
+self.expect("\*stopped,reason=\"breakpoint-hit\"")
+
 @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
 @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races
 @skipIfRemote   # We do not currently support remote debugging via the MI.


Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

aprantl wrote:
> labath wrote:
> > aprantl wrote:
> > > polyakov.alex wrote:
> > > > aprantl wrote:
> > > > > CHECK-AFTER is not recognized by FileCheck:
> > > > > 
> > > > > https://www.llvm.org/docs/CommandGuide/FileCheck.html
> > > > > 
> > > > > You probably saw this in a testcase that ran FileCheck twice, one 
> > > > > time with the CHECK prefix and once with a custom 
> > > > > `--check-prefix=CHECK-AFTER` which is a common trick to have more 
> > > > > than one set of FileCheck directives in a single file.
> > > > Yes. There is no problem to write test using only `CHECK` and 
> > > > `CHECK-NOT`, but as I said, in lldb-mi's output we can't find any info 
> > > > about hitting breakpoint, so the question is: is it enough to check 
> > > > that breakpoint was set to a selected target?
> > > > in lldb-mi's output we can't find any info about hitting breakpoint,
> > > Is that how the gdb/mi protocol is supposed to work or is that a bug or 
> > > missing feature in lldb-mi?
> > > 
> > > > so the question is: is it enough to check that breakpoint was set to a 
> > > > selected target?
> > > If that's just how the protocol works then we'll have to make do with 
> > > what we got.
> > That's not "how the protocol works" in general. It's how lldb-mi behaves 
> > when it's control connection is closed. If you pipe its input from a file, 
> > lldb-mi will get an EOF as soon as it processes the last command,  
> > interpret that as the IDE closing the connection and exit (a perfectly 
> > reasonable behavior for its intended use case). So it will never get around 
> > to printing the breakpoint-hit message, because it will not wait long 
> > enough for that to happen. If you make sure the stdin does not get an EOF 
> > then (either by typing the same commands interactively, or by doing 
> > something like `cat break_insert.test - | lldb-mi`, you will see the "hit" 
> > message does get displayed (however, then the lldb-mi process will hang 
> > because it will expect more commands).
> To make sure I understand your point: Does lldb-mi consumes input 
> asynchronously from its command handler, so for example, when sending the mi 
> equivalent of `b myFunc`, `r`, `p foo` — will lldb-mi wait until the 
> breakpoint is hit or the target is terminated before consuming the next 
> command after `r` or will it consume and attempt to execute `p foo` as soon 
> as possible and thus potentially before the breakpoint is hit?
> 
> If this is the case, we could introduce a "synchronized" mode for testing 
> lldb-mi that behaves more like the SBAPI. Or we could pick up the idea you 
> mentioned a while ago and produce a separate lldb-mi test driver that can 
> send and get a reply for exactly one action at a time, similar to how the 
> SBAPI behaves.
> To make sure I understand your point: Does lldb-mi consumes input 
> asynchronously from its command handler, so for example, when sending the mi 
> equivalent of b myFunc, r, p foo — will lldb-mi wait until the breakpoint is 
> hit or the target is terminated before consuming the next command after r or 
> will it consume and attempt to execute p foo as soon as possible and thus 
> potentially before the breakpoint is hit?

I know very little about lldb-mi implementation or the gdb-mi protocol, but 
yes, that is what I think is happening. In general, you need the ability to 
issue asynchronous commands, if for nothing else, then to implement the 
equivalent of ^C. gdb-remote protocol also multiplexes reading from the socket 
and waiting on inferior state change for this reason (and would respond the 
same way if you tried to feed it input from a file).

BTW: SB API also has an asynchronous mode, and I think it is even the default. 
It's just that we switch it to synchronous mode for testing, as one does not 
care about the ability to interrupt the process in tests (most of the time).

> If this is the case, we could introduce a "synchronized" mode for testing 
> lldb-mi that behaves more like the SBAPI. Or we could pick up the idea you 
> mentioned a while ago and produce a separate lldb-mi test driver that can 
> send and get a reply for exactly one action at a time, similar to how the 
> SBAPI behaves.

I think both of these are valid options. Probably the simplest way to implement 
the first one would be to introduce a `-wait-for-target-to-stop` command (maybe 
there is one already ??). The advantage of the second one is that we will have 
the ability to inject commands which depend on the results of previous commands 
(something that I think we will need, sooner or later).


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-16 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Alexander, I don't want this discussion to block you progress too much. It's 
important that we sort out the best way to test lldb-mi early on (and I hope 
this doesn't take too long), but while we are debating this you could also land 
the patch with the classic expect-based testcase that you had and go back and 
convert it later on. It's a bit more work, but converting the testcases 
shouldn't be too difficult.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-16 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

labath wrote:
> aprantl wrote:
> > polyakov.alex wrote:
> > > aprantl wrote:
> > > > CHECK-AFTER is not recognized by FileCheck:
> > > > 
> > > > https://www.llvm.org/docs/CommandGuide/FileCheck.html
> > > > 
> > > > You probably saw this in a testcase that ran FileCheck twice, one time 
> > > > with the CHECK prefix and once with a custom 
> > > > `--check-prefix=CHECK-AFTER` which is a common trick to have more than 
> > > > one set of FileCheck directives in a single file.
> > > Yes. There is no problem to write test using only `CHECK` and 
> > > `CHECK-NOT`, but as I said, in lldb-mi's output we can't find any info 
> > > about hitting breakpoint, so the question is: is it enough to check that 
> > > breakpoint was set to a selected target?
> > > in lldb-mi's output we can't find any info about hitting breakpoint,
> > Is that how the gdb/mi protocol is supposed to work or is that a bug or 
> > missing feature in lldb-mi?
> > 
> > > so the question is: is it enough to check that breakpoint was set to a 
> > > selected target?
> > If that's just how the protocol works then we'll have to make do with what 
> > we got.
> That's not "how the protocol works" in general. It's how lldb-mi behaves when 
> it's control connection is closed. If you pipe its input from a file, lldb-mi 
> will get an EOF as soon as it processes the last command,  interpret that as 
> the IDE closing the connection and exit (a perfectly reasonable behavior for 
> its intended use case). So it will never get around to printing the 
> breakpoint-hit message, because it will not wait long enough for that to 
> happen. If you make sure the stdin does not get an EOF then (either by typing 
> the same commands interactively, or by doing something like `cat 
> break_insert.test - | lldb-mi`, you will see the "hit" message does get 
> displayed (however, then the lldb-mi process will hang because it will expect 
> more commands).
To make sure I understand your point: Does lldb-mi consumes input 
asynchronously from its command handler, so for example, when sending the mi 
equivalent of `b myFunc`, `r`, `p foo` — will lldb-mi wait until the breakpoint 
is hit or the target is terminated before consuming the next command after `r` 
or will it consume and attempt to execute `p foo` as soon as possible and thus 
potentially before the breakpoint is hit?

If this is the case, we could introduce a "synchronized" mode for testing 
lldb-mi that behaves more like the SBAPI. Or we could pick up the idea you 
mentioned a while ago and produce a separate lldb-mi test driver that can send 
and get a reply for exactly one action at a time, similar to how the SBAPI 
behaves.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

aprantl wrote:
> polyakov.alex wrote:
> > aprantl wrote:
> > > CHECK-AFTER is not recognized by FileCheck:
> > > 
> > > https://www.llvm.org/docs/CommandGuide/FileCheck.html
> > > 
> > > You probably saw this in a testcase that ran FileCheck twice, one time 
> > > with the CHECK prefix and once with a custom `--check-prefix=CHECK-AFTER` 
> > > which is a common trick to have more than one set of FileCheck directives 
> > > in a single file.
> > Yes. There is no problem to write test using only `CHECK` and `CHECK-NOT`, 
> > but as I said, in lldb-mi's output we can't find any info about hitting 
> > breakpoint, so the question is: is it enough to check that breakpoint was 
> > set to a selected target?
> > in lldb-mi's output we can't find any info about hitting breakpoint,
> Is that how the gdb/mi protocol is supposed to work or is that a bug or 
> missing feature in lldb-mi?
> 
> > so the question is: is it enough to check that breakpoint was set to a 
> > selected target?
> If that's just how the protocol works then we'll have to make do with what we 
> got.
That's not "how the protocol works" in general. It's how lldb-mi behaves when 
it's control connection is closed. If you pipe its input from a file, lldb-mi 
will get an EOF as soon as it processes the last command,  interpret that as 
the IDE closing the connection and exit (a perfectly reasonable behavior for 
its intended use case). So it will never get around to printing the 
breakpoint-hit message, because it will not wait long enough for that to 
happen. If you make sure the stdin does not get an EOF then (either by typing 
the same commands interactively, or by doing something like `cat 
break_insert.test - | lldb-mi`, you will see the "hit" message does get 
displayed (however, then the lldb-mi process will hang because it will expect 
more commands).


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

polyakov.alex wrote:
> aprantl wrote:
> > CHECK-AFTER is not recognized by FileCheck:
> > 
> > https://www.llvm.org/docs/CommandGuide/FileCheck.html
> > 
> > You probably saw this in a testcase that ran FileCheck twice, one time with 
> > the CHECK prefix and once with a custom `--check-prefix=CHECK-AFTER` which 
> > is a common trick to have more than one set of FileCheck directives in a 
> > single file.
> Yes. There is no problem to write test using only `CHECK` and `CHECK-NOT`, 
> but as I said, in lldb-mi's output we can't find any info about hitting 
> breakpoint, so the question is: is it enough to check that breakpoint was set 
> to a selected target?
> in lldb-mi's output we can't find any info about hitting breakpoint,
Is that how the gdb/mi protocol is supposed to work or is that a bug or missing 
feature in lldb-mi?

> so the question is: is it enough to check that breakpoint was set to a 
> selected target?
If that's just how the protocol works then we'll have to make do with what we 
got.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

aprantl wrote:
> CHECK-AFTER is not recognized by FileCheck:
> 
> https://www.llvm.org/docs/CommandGuide/FileCheck.html
> 
> You probably saw this in a testcase that ran FileCheck twice, one time with 
> the CHECK prefix and once with a custom `--check-prefix=CHECK-AFTER` which is 
> a common trick to have more than one set of FileCheck directives in a single 
> file.
Yes. There is no problem to write test using only `CHECK` and `CHECK-NOT`, but 
as I said, in lldb-mi's output we can't find any info about hitting breakpoint, 
so the question is: is it enough to check that breakpoint was set to a selected 
target?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

CHECK-AFTER is not recognized by FileCheck:

https://www.llvm.org/docs/CommandGuide/FileCheck.html

You probably saw this in a testcase that ran FileCheck twice, one time with the 
CHECK prefix and once with a custom `--check-prefix=CHECK-AFTER` which is a 
common trick to have more than one set of FileCheck directives in a single file.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10
+-file-exec-and-symbols a.out
+# CHECK-AFTER: ^done
+

labath wrote:
> polyakov.alex wrote:
> > labath wrote:
> > > polyakov.alex wrote:
> > > > labath wrote:
> > > > > I'm not familiar with this directive. Are you sure that this actually 
> > > > > does anything?
> > > > I tried to use only CHECK directive, but got errors like pattern not 
> > > > found, so I decided that it may be caused due to CHECK search features, 
> > > > for example, as I know, it finds pattern from the start of the file. If 
> > > > we want to check lldb-mi output, we should follow a specific order.
> > > > 
> > > > In our case, we should find "^done" string directly after -break-insert 
> > > > command's output.
> > > I think you got the FileCheck operation wrong.
> > > a `CHECK` should always start matching from the previous match. The 
> > > reason that this is passing for you now is that CHECK-AFTER is a 
> > > non-existing directive and FileCheck ignores it (try replacing it with a 
> > > bogus string and see if it still passes). if `CHECK` is not working for 
> > > you here then you probably have the pattern wrong.
> > You are right about the CHECK-AFTER. I wrote the test using only check and 
> > found an issue:
> > after executing of -exec-run command, we expect to see output like: 
> > *stopped,reason="breakpoint-hit"
> > but, I think, that FileCheck check file for the pattern, while a -exec-run 
> > hasn't finished yet. It means that there will not be expected output and 
> > we'll get the error: no such pattern.
> > 
> > Is there a mechanism to add some delay to FileCheck to wait until -exec-run 
> > finished?
> > 
> There shouldn't be a need for anything like that. FileCheck will wait until 
> EOF before doing anything. In fact, if you try running the test script 
> manually (`lldb-mi  does *not* print out the `*stopped` line. What I expect is happening here is 
> that lldb-mi reaches EOF, and then decides there is nothing left for it to do 
> and exits (a somewhat reasonable assumption given that it's expecting to be 
> talking via an interactive link).
> 
> This sounds like a fairly fundamental problem with the lit+FileCheck testing 
> strategy. If we're not able to come up with a command to make lldb-mi wait 
> until the target stops (maybe there is one already? I know very little about 
> lldb-mi), we may have to revisit the whole testing strategy...
I tried to run script manually (!!lldb-mi < break-insert.test!!) and got output 
which isn't useful for testing !!-break-insert!! command since there isn't any 
information about hitting breakpoint.

```
~/workspace/gsoc/build/bin/lldb-mi < ../break-insert.test 
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x",func="??",file="??",fullname="??/??",line="0",pending=["breakpoint"],times="0",original-location="breakpoint"}
(gdb)
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x",func="??",file="??",fullname="??/??",line="0",pending=["breakpoint"],times="0",original-location="breakpoint"}
(gdb)
^done
(gdb)
^done
(gdb)
=library-loaded,id="/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/a.out",target-name="/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/a.out",host-name="/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/a.out",symbols-loaded="0",loaded_addr="-",size="0"
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x004004c6",func="breakpoint",file="break-insert.c",fullname="/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/break-insert.c",line="2",pending=["breakpoint"],times="0",original-location="breakpoint"}
(gdb)
^done
(gdb)
^done
(gdb)
^running
=thread-group-started,id="i1",pid="13757"
(gdb)
=thread-created,id="1",group-id="i1"
=thread-selected,id="1"
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/ld-2.23.so",target-name="/lib/x86_64-linux-gnu/ld-2.23.so",host-name="/lib/x86_64-linux-gnu/ld-2.23.so",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/ld-2.23.so",loaded_addr="-",size="0"
(gdb)
=library-loaded,id="[vdso]",target-name="[vdso]",host-name="[vdso]",symbols-loaded="1",symbols-path="",loaded_addr="0x77ffa000",size="0"
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x004004c6",func="breakpoint",file="break-insert.c",fullname="/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/break-insert.c",line="2",pending=["breakpoint"],times="0",original-location="breakpoint"}
(gdb)
(gdb)
=library-loaded,id="/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/a.out",target-name="/home/alexande

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

For #2 we should be able to come up with something much more trivial than 
pexpect (and thereby more reliable) because we really aren't expecting 
patterns.  We are always sending a complete string reading a complete string 
back, then checking the string against some pattern.  It should be pretty 
straightforward to write a lit tool that does that.  IIUC that's what Davide 
did for the swift REPL tests in the github repo.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

In https://reviews.llvm.org/D46588#1099566, @clayborg wrote:

> In https://reviews.llvm.org/D46588#1099536, @aprantl wrote:
>
> > > If we're not able to come up with a command to make lldb-mi wait until 
> > > the target stops (maybe there is one already? I know very little about 
> > > lldb-mi), we may have to revisit the whole testing strategy...
> >
> > If one doesn't exist then I think it would be reasonable to invent one. 
> > Handling an additional command that is used in testing only should not 
> > break any existing clients.
>
>
> I am not sure I like the direction of "lets test lldb-mi in a way that 
> doesn't behave like a real debug session" by making new testing stuff so we 
> can text scrape.


There are two levels that we want to test lldb-mi on:

1. Test on the functionality / specification level: We want to make sure that a 
specific lldb-mi command triggers a specific action. To test this as reliably 
as possible getting rid of any temporal uncertainty is a good thing. This is 
what I'm concerned with here. The idea is to serialize an entire debug session 
with fixed inputs into one output file, which can then be checked via 
FileCheck. You are right that this is "scraping text", but I do think that that 
is the appropriate abstraction level for testing a textual protocol. The main 
befit of doing it this way is that there is no pexpect and timeouts involved 
(other than the per-test timeout imposed by the lit driver). This way we get a 
very reliable testsuite that is not affected by how much load the machine 
running it is under.

2. Test end-to-end that lldb-mi integrates well with an interactive client. I 
don't actually have any good ideas for how to do this beyond the existing 
pexpect-based method, which, as we found out, has some reliability issues. But 
perhaps reducing the number of interactive tests in favor of mostly 
specification (1) tests will reduce the chances of a random timeout occurring 
to a manageable number.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In https://reviews.llvm.org/D46588#1099536, @aprantl wrote:

> > If we're not able to come up with a command to make lldb-mi wait until the 
> > target stops (maybe there is one already? I know very little about 
> > lldb-mi), we may have to revisit the whole testing strategy...
>
> If one doesn't exist then I think it would be reasonable to invent one. 
> Handling an additional command that is used in testing only should not break 
> any existing clients.


I am not sure I like the direction of "lets test lldb-mi in a way that doesn't 
behave like a real debug session" by making new testing stuff so we can text 
scrape.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

> If we're not able to come up with a command to make lldb-mi wait until the 
> target stops (maybe there is one already? I know very little about lldb-mi), 
> we may have to revisit the whole testing strategy...

If one doesn't exist then I think it would be reasonable to invent one. 
Handling an additional command that is used in testing only should not break 
any existing clients.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10
+-file-exec-and-symbols a.out
+# CHECK-AFTER: ^done
+

polyakov.alex wrote:
> labath wrote:
> > polyakov.alex wrote:
> > > labath wrote:
> > > > I'm not familiar with this directive. Are you sure that this actually 
> > > > does anything?
> > > I tried to use only CHECK directive, but got errors like pattern not 
> > > found, so I decided that it may be caused due to CHECK search features, 
> > > for example, as I know, it finds pattern from the start of the file. If 
> > > we want to check lldb-mi output, we should follow a specific order.
> > > 
> > > In our case, we should find "^done" string directly after -break-insert 
> > > command's output.
> > I think you got the FileCheck operation wrong.
> > a `CHECK` should always start matching from the previous match. The reason 
> > that this is passing for you now is that CHECK-AFTER is a non-existing 
> > directive and FileCheck ignores it (try replacing it with a bogus string 
> > and see if it still passes). if `CHECK` is not working for you here then 
> > you probably have the pattern wrong.
> You are right about the CHECK-AFTER. I wrote the test using only check and 
> found an issue:
> after executing of -exec-run command, we expect to see output like: 
> *stopped,reason="breakpoint-hit"
> but, I think, that FileCheck check file for the pattern, while a -exec-run 
> hasn't finished yet. It means that there will not be expected output and 
> we'll get the error: no such pattern.
> 
> Is there a mechanism to add some delay to FileCheck to wait until -exec-run 
> finished?
> 
There shouldn't be a need for anything like that. FileCheck will wait until EOF 
before doing anything. In fact, if you try running the test script manually 
(`lldb-mi https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10
+-file-exec-and-symbols a.out
+# CHECK-AFTER: ^done
+

labath wrote:
> polyakov.alex wrote:
> > labath wrote:
> > > I'm not familiar with this directive. Are you sure that this actually 
> > > does anything?
> > I tried to use only CHECK directive, but got errors like pattern not found, 
> > so I decided that it may be caused due to CHECK search features, for 
> > example, as I know, it finds pattern from the start of the file. If we want 
> > to check lldb-mi output, we should follow a specific order.
> > 
> > In our case, we should find "^done" string directly after -break-insert 
> > command's output.
> I think you got the FileCheck operation wrong.
> a `CHECK` should always start matching from the previous match. The reason 
> that this is passing for you now is that CHECK-AFTER is a non-existing 
> directive and FileCheck ignores it (try replacing it with a bogus string and 
> see if it still passes). if `CHECK` is not working for you here then you 
> probably have the pattern wrong.
You are right about the CHECK-AFTER. I wrote the test using only check and 
found an issue:
after executing of -exec-run command, we expect to see output like: 
*stopped,reason="breakpoint-hit"
but, I think, that FileCheck check file for the pattern, while a -exec-run 
hasn't finished yet. It means that there will not be expected output and we'll 
get the error: no such pattern.

Is there a mechanism to add some delay to FileCheck to wait until -exec-run 
finished?



Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10
+-file-exec-and-symbols a.out
+# CHECK-AFTER: ^done
+

polyakov.alex wrote:
> labath wrote:
> > I'm not familiar with this directive. Are you sure that this actually does 
> > anything?
> I tried to use only CHECK directive, but got errors like pattern not found, 
> so I decided that it may be caused due to CHECK search features, for example, 
> as I know, it finds pattern from the start of the file. If we want to check 
> lldb-mi output, we should follow a specific order.
> 
> In our case, we should find "^done" string directly after -break-insert 
> command's output.
I think you got the FileCheck operation wrong.
a `CHECK` should always start matching from the previous match. The reason that 
this is passing for you now is that CHECK-AFTER is a non-existing directive and 
FileCheck ignores it (try replacing it with a bogus string and see if it still 
passes). if `CHECK` is not working for you here then you probably have the 
pattern wrong.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10
+-file-exec-and-symbols a.out
+# CHECK-AFTER: ^done
+

labath wrote:
> I'm not familiar with this directive. Are you sure that this actually does 
> anything?
I tried to use only CHECK directive, but got errors like pattern not found, so 
I decided that it may be caused due to CHECK search features, for example, as I 
know, it finds pattern from the start of the file. If we want to check lldb-mi 
output, we should follow a specific order.

In our case, we should find "^done" string directly after -break-insert 
command's output.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10
+-file-exec-and-symbols a.out
+# CHECK-AFTER: ^done
+

I'm not familiar with this directive. Are you sure that this actually does 
anything?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added a comment.

Also, I combined FileCheck commands and lldb-mi input in a single file.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 146777.
polyakov.alex added a comment.

Created separate directory for an lldb-mi tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lit/lit.cfg
  lit/tools/lldb-mi/breakpoint/break-insert.test
  lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
  lit/tools/lldb-mi/breakpoint/lit.local.cfg
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lit/tools/lldb-mi/breakpoint/lit.local.cfg
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Index: lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
Index: lit/tools/lldb-mi/breakpoint/break-insert.test
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/break-insert.test
@@ -0,0 +1,15 @@
+# RUN: %cc %p/inputs/break-insert.c -g
+# RUN: %lldb_mi < %s | FileCheck %s
+
+# Test that a breakpoint can be inserted before creating a target.
+
+-break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+
+-file-exec-and-symbols a.out
+# CHECK-AFTER: ^done
+
+-exec-run
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldb_mi', lldb_mi))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lit/Breakpoint/Inputs/break-insert.input:1-3
+-break-insert breakpoint
+-file-exec-and-symbols a.out
+-exec-run

Based on my experiments, lldb-mi seems to ignore lines starting with `#`. If 
that's true then we could put the contents of this file directly in the test, 
which will help readability as there is only one file to open to understand 
what a test does.



Comment at: lit/Breakpoint/break-insert.test:1
+# RUN: %cc %p/Inputs/break-insert.c -g 
+# RUN: %lldb_mi < %p/Inputs/break-insert.input | FileCheck %s

I think the lldb-mi tests should be kept separate. Could we move this test into 
a different directory? Maybe `lit/tools/lldb-mi` ?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-14 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: lit/Breakpoint/break-insert.test:5
+# cmd: -break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+

polyakov.alex wrote:
> aprantl wrote:
> > Very nice!
> > 
> > What does the actual output format of lldb-mi look like?
> > If every reply is on a single line, we could make this test even stricter 
> > by using `CHECK-NEXT` instead of `CHECK` to make sure that no other output 
> > appears between two matches. Alternatively you could also insert a 
> > `CHECK-NOT: ^done` line before every command to make the test even stricter.
> Unfortunately, lldb-mi output may look like:
> 
> (gdb)
> -exec-run
> ^running
> =thread-group-started,id="i1",pid="4632"
> =thread-created,id="1",group-id="i1"
> =thread-selected,id="1"
> (gdb)
> =library-loaded,id="/lib/x86_64-linux-gnu/ld-2.23.so",target-name="/lib/x86_64-linux-gnu/ld-2.23.so",host-name="/lib/x86_64-linux-gnu/ld-2.23.so",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/ld-2.23.so",loaded_addr="-",size="0"
> (gdb)
> =library-loaded,id="[vdso]",target-name="[vdso]",host-name="[vdso]",symbols-loaded="1",symbols-path="",loaded_addr="0x77ffa000",size="0"
> =breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0041eed0",func="??",file="??",fullname="??/??",line="0",pending=["main"],times="0",original-location="main"}
> (gdb)
> (gdb)
> (gdb)
> =library-loaded,id="/bin/bash",target-name="/bin/bash",host-name="/bin/bash",symbols-loaded="0",loaded_addr="-",size="0"
> (gdb)
> *running,thread-id="all"
> (gdb)
> (gdb)
> =library-loaded,id="/lib/x86_64-linux-gnu/libtinfo.so.5",target-name="/lib/x86_64-linux-gnu/libtinfo.so.5",host-name="/lib/x86_64-linux-gnu/libtinfo.so.5",symbols-loaded="0",loaded_addr="-",size="0"
> (gdb)
> =library-loaded,id="/lib/x86_64-linux-gnu/libdl.so.2",target-name="/lib/x86_64-linux-gnu/libdl.so.2",host-name="/lib/x86_64-linux-gnu/libdl.so.2",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libdl-2.23.so",loaded_addr="-",size="0"
> (gdb)
> =library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libc-2.23.so",loaded_addr="-",size="0"
> (gdb)
> =library-loaded,id="/lib/x86_64-linux-gnu/libtinfo.so.5",target-name="/lib/x86_64-linux-gnu/libtinfo.so.5",host-name="/lib/x86_64-linux-gnu/libtinfo.so.5",symbols-loaded="0",loaded_addr="-",size="0"
> =library-loaded,id="/lib/x86_64-linux-gnu/libdl.so.2",target-name="/lib/x86_64-linux-gnu/libdl.so.2",host-name="/lib/x86_64-linux-gnu/libdl.so.2",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libdl-2.23.so",loaded_addr="-",size="0"
> =library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libc-2.23.so",loaded_addr="-",size="0"
> (gdb)
> *stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={level="0",addr="0x0041eed0",func="main",args=[],file="??",fullname="??",line="-1"},thread-id="1",stopped-threads="all"
> (gdb)
> 
> So, I think, it's not a best idea to write 'CHECK-NOT' for each line.
Ah, my fault, there will not be such amount of lines if we use test's binary.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-14 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: lit/Breakpoint/break-insert.test:5
+# cmd: -break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+

aprantl wrote:
> Very nice!
> 
> What does the actual output format of lldb-mi look like?
> If every reply is on a single line, we could make this test even stricter by 
> using `CHECK-NEXT` instead of `CHECK` to make sure that no other output 
> appears between two matches. Alternatively you could also insert a 
> `CHECK-NOT: ^done` line before every command to make the test even stricter.
Unfortunately, lldb-mi output may look like:

(gdb)
-exec-run
^running
=thread-group-started,id="i1",pid="4632"
=thread-created,id="1",group-id="i1"
=thread-selected,id="1"
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/ld-2.23.so",target-name="/lib/x86_64-linux-gnu/ld-2.23.so",host-name="/lib/x86_64-linux-gnu/ld-2.23.so",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/ld-2.23.so",loaded_addr="-",size="0"
(gdb)
=library-loaded,id="[vdso]",target-name="[vdso]",host-name="[vdso]",symbols-loaded="1",symbols-path="",loaded_addr="0x77ffa000",size="0"
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0041eed0",func="??",file="??",fullname="??/??",line="0",pending=["main"],times="0",original-location="main"}
(gdb)
(gdb)
(gdb)
=library-loaded,id="/bin/bash",target-name="/bin/bash",host-name="/bin/bash",symbols-loaded="0",loaded_addr="-",size="0"
(gdb)
*running,thread-id="all"
(gdb)
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/libtinfo.so.5",target-name="/lib/x86_64-linux-gnu/libtinfo.so.5",host-name="/lib/x86_64-linux-gnu/libtinfo.so.5",symbols-loaded="0",loaded_addr="-",size="0"
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/libdl.so.2",target-name="/lib/x86_64-linux-gnu/libdl.so.2",host-name="/lib/x86_64-linux-gnu/libdl.so.2",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libdl-2.23.so",loaded_addr="-",size="0"
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libc-2.23.so",loaded_addr="-",size="0"
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/libtinfo.so.5",target-name="/lib/x86_64-linux-gnu/libtinfo.so.5",host-name="/lib/x86_64-linux-gnu/libtinfo.so.5",symbols-loaded="0",loaded_addr="-",size="0"
=library-loaded,id="/lib/x86_64-linux-gnu/libdl.so.2",target-name="/lib/x86_64-linux-gnu/libdl.so.2",host-name="/lib/x86_64-linux-gnu/libdl.so.2",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libdl-2.23.so",loaded_addr="-",size="0"
=library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libc-2.23.so",loaded_addr="-",size="0"
(gdb)
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={level="0",addr="0x0041eed0",func="main",args=[],file="??",fullname="??",line="-1"},thread-id="1",stopped-threads="all"
(gdb)

So, I think, it's not a best idea to write 'CHECK-NOT' for each line.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-14 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added a comment.

Thanks, LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-14 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 146693.
polyakov.alex added a comment.

Added more comments about the test.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lit/Breakpoint/Inputs/break-insert.c
  lit/Breakpoint/Inputs/break-insert.input
  lit/Breakpoint/break-insert.test
  lit/lit.cfg
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldb_mi', lldb_mi))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
Index: lit/Breakpoint/break-insert.test
===
--- /dev/null
+++ lit/Breakpoint/break-insert.test
@@ -0,0 +1,15 @@
+# RUN: %cc %p/Inputs/break-insert.c -g 
+# RUN: %lldb_mi < %p/Inputs/break-insert.input | FileCheck %s
+
+# Test that a breakpoint can be inserted before creating a target.
+#
+# cmd: -break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+#
+# cmd: -file-exec-and-symbols a.out
+# CHECK: ^done
+#
+# cmd: -exec-run
+# CHECK: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+
Index: lit/Breakpoint/Inputs/break-insert.input
===
--- /dev/null
+++ lit/Breakpoint/Inputs/break-insert.input
@@ -0,0 +1,3 @@
+-break-insert breakpoint
+-file-exec-and-symbols a.out
+-exec-run
Index: lit/Breakpoint/Inputs/break-insert.c
===
--- /dev/null
+++ lit/Breakpoint/Inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-14 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Thanks for converting the test!




Comment at: lit/Breakpoint/break-insert.test:2
+# RUN: %cc %p/Inputs/break-insert.c -g 
+# RUN: %lldb_mi < %p/Inputs/break-insert.input | FileCheck %s
+

Please add a comment like:
`# Test that a breakpoint can be inserted before creating a target.`




Comment at: lit/Breakpoint/break-insert.test:5
+# cmd: -break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+

Very nice!

What does the actual output format of lldb-mi look like?
If every reply is on a single line, we could make this test even stricter by 
using `CHECK-NEXT` instead of `CHECK` to make sure that no other output appears 
between two matches. Alternatively you could also insert a `CHECK-NOT: ^done` 
line before every command to make the test even stricter.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-12 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 146489.
polyakov.alex added a comment.

Added comments describing which lldb-mi command's output is currently parsing.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lit/Breakpoint/Inputs/break-insert.c
  lit/Breakpoint/Inputs/break-insert.input
  lit/Breakpoint/break-insert.test
  lit/lit.cfg
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldb_mi', lldb_mi))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
Index: lit/Breakpoint/break-insert.test
===
--- /dev/null
+++ lit/Breakpoint/break-insert.test
@@ -0,0 +1,13 @@
+# RUN: %cc %p/Inputs/break-insert.c -g 
+# RUN: %lldb_mi < %p/Inputs/break-insert.input | FileCheck %s
+
+# cmd: -break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+
+# cmd: -file-exec-and-symbols a.out
+# CHECK: ^done
+
+# cmd: -exec-run
+# CHECK: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+
Index: lit/Breakpoint/Inputs/break-insert.input
===
--- /dev/null
+++ lit/Breakpoint/Inputs/break-insert.input
@@ -0,0 +1,3 @@
+-break-insert breakpoint
+-file-exec-and-symbols a.out
+-exec-run
Index: lit/Breakpoint/Inputs/break-insert.c
===
--- /dev/null
+++ lit/Breakpoint/Inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-12 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 146487.
polyakov.alex added a comment.

Moved tests to lit.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lit/Breakpoint/Inputs/break-insert.c
  lit/Breakpoint/Inputs/break-insert.input
  lit/Breakpoint/break-insert.test
  lit/lit.cfg
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldb_mi', lldb_mi))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
Index: lit/Breakpoint/break-insert.test
===
--- /dev/null
+++ lit/Breakpoint/break-insert.test
@@ -0,0 +1,7 @@
+# RUN: %cc %p/Inputs/break-insert.c -g 
+# RUN: %lldb_mi < %p/Inputs/break-insert.input | FileCheck %s
+# CHECK: ^done,bkpt={number="1"
+# CHECK: ^done
+# CHECK: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+
Index: lit/Breakpoint/Inputs/break-insert.input
===
--- /dev/null
+++ lit/Breakpoint/Inputs/break-insert.input
@@ -0,0 +1,3 @@
+-break-insert breakpoint
+-file-exec-and-symbols a.out
+-exec-run
Index: lit/Breakpoint/Inputs/break-insert.c
===
--- /dev/null
+++ lit/Breakpoint/Inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py:157
+self.runCmd("-break-insert main")
+# FIXME function name is unknown on Darwin, fullname should be ??, 
line is -1
+self.expect(

since this comes form the dummy target, I suppose this doesn't matter?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

The code looks good — I think it would be worthwhile to try and convert this 
test to a LIT test. You can try to model it after 
`lit/Expr/TestCallUserDefinedFunction.test`. You will also need to modify 
`lit/lit.cfg` to define a `%lldb-mi` variable, but that should be 
straightforward. Let me know if that works. If we run into too many problems we 
can stick to the expect-style tests for now, but I'd like to do this experiment 
at the very beginning so all the subsequent patches can benefit from it.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-10 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added a comment.

So, what do you think about the patch? There is no any decision about lldb-mi 
tests and I do not know what to do with this.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Actually, the example I posted was not a good one. I'm fairly certain that that 
was de facto static information or otherwise not particularly important 
information.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Got it. We certainly do have dependent checks in our current tests:

  # Test that -data-info-line works for address
  self.runCmd("-data-info-line *%#x" % addr)
  self.expect(
  
"\^done,start=\"0x0*%x\",end=\"0x[0-9a-f]+\",file=\".+?main.cpp\",line=\"%d\"" %
  (addr, line))
  
  # Test that -data-info-line works for file:line
  self.runCmd("-data-info-line main.cpp:%d" % line)
  self.expect(
  
"\^done,start=\"0x0*%x\",end=\"0x[0-9a-f]+\",file=\".+?main.cpp\",line=\"%d\"" %
  (addr, line))


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D46588#1093267, @aprantl wrote:

> Can you give an example of what you mean by "specifying the lldb-mi commands 
> up-front"? it's not immediately obvious to me how that would look like.


In your example `lldb-mi-demo.input` is static text, right? No command depends 
on the result of any previous commands. That's what I mean by "up-front".

The opposite of that would be if you used the output of one command to 
construct the next one. Something like:

  result = execute("first command")
  value = extract_something_interesting(result)
  result2 = execute(create_second_command(value))

My point is that lit+FileCheck is good if all your tests look like your 
example, but it's not if you need to do something like the above. I haven't 
looked at lldb-mi in much detail, so I can't which of these two options is 
correct, although it seems that at least the majority of the tests could be 
written that way.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Can you give an example of what you mean by "specifying the lldb-mi commands 
up-front"? it's not immediately obvious to me how that would look like.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

My main question would be whether it is possible to specify all the lldb-mi 
commands up-front, which is kinda required for this approach. I was against 
this for the lldb-server tests, because there we often deal with values which 
are not sufficiently deterministic (for example, because they represent 
addresses in the inferior address space), so we needed the know the output of 
one command before we could specify the next one. I felt that trying to capture 
this statically would end up with a reinvention of a domain-specific scripting 
language.

*However*, the situation may be better in lldb-mi, as here we are sitting on 
top of layers which will enable us to reason about things symbolically. In a 
random sample of lldb-mi tests I did not find any inter-command dependencies, 
*except* the "-file-exec-and-symbols", where we pass the full path of the built 
executable. This can probably be worked around somehow.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Okay, that's no good. What would you think about writing tests using lit & 
FileCheck? The lldb-mi tests should not be in the debuginfo category anyway, 
since they only test the alternative driver.

  $ cat lldb-mi-demo.input
  -file-exec-and-symbols a.out
  -break-insert -f main
  ...
  
  $ cat lldb-mi-demo.test
  RUN: %cc test.c -g
  RUN: %lldb-mi https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D46588#1093054, @aprantl wrote:

> In https://reviews.llvm.org/D46588#1092884, @labath wrote:
>
> > Out of curiosity, are there any plans to improve the lldb-mi test 
> > reliability situation? As it stands now, most of the lldb-mi tests are 
> > disabled one way or another due to them being flaky.
>
>
> Thanks for bringing that up. I just looked at a few lldb-mi testcases and 
> they all seem to follow a pattern of `self.runCmd()` followed by 
> `self.expect()`. That in itself doesn't look like a particularly bad design 
> to me since it synchronizes commands and expected output tightly. Do we know 
> why the tests are flakey? Do they get out of sync, or is there something else?


Short answer: I don't know. Noone was motived enough to get to the bottom of 
that.

I tried looking at it once or twice, but I was put off by:

- when the test fails you get a dump of the pexpect state, which is hard to 
comprehend
- I tried enabling some logging but the log was full of useless messages 
because lldb-mi has a thread which does a 1 millisecond polls for events.

I think it would help if instead of pexpect, we used some driver which is aware 
of the basics of lldb-mi protocol. This would make logging the execution of a 
test easier and it would solve the XFAIL issue -- right now, if lldb-mi does 
not produce expected output, pexpect will just hang until it times out, hoping 
that the right output might come eventually. And it would also make it possible 
to run the tests on windows. The 1ms wait also sounds like something that 
should be fixed regardless of whether it is causing any flakyness or not.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

In https://reviews.llvm.org/D46588#1092884, @labath wrote:

> Out of curiosity, are there any plans to improve the lldb-mi test reliability 
> situation? As it stands now, most of the lldb-mi tests are disabled one way 
> or another due to them being flaky.


Thanks for bringing that up. I just looked at a few lldb-mi testcases and they 
all seem to follow a pattern of `self.runCmd()` followed by `self.expect()`. 
That in itself doesn't look like a particularly bad design to me since it 
synchronizes commands and expected output tightly. Do we know why the tests are 
flakey? Do they get out of sync, or is there something else?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Out of curiosity, are there any plans to improve the lldb-mi test reliability 
situation? As it stands now, most of the lldb-mi tests are disabled one way or 
another due to them being flaky.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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


[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-09 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 145909.
polyakov.alex retitled this revision from "[WIP][LLDB-MI] Add possibility to 
set breakpoints without selecting a target." to "[LLDB][lldb-mi] Add 
possibility to set breakpoints without selecting a target.".
polyakov.alex edited the summary of this revision.
polyakov.alex added a comment.

Added testcase and updated revision.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp


Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
@@ -143,6 +143,32 @@
 self.expect("\^running")
 self.expect("\*stopped,reason=\"breakpoint-hit\"")
 
+@skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows.
+@skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races.
+@skipIfRemote   # We do not currently support remote debugging via the MI.
+def test_lldbmi_break_insert_without_target(self):
+"""Test that 'lldb-mi --interpreter' can set breakpoints without
+selecting a valid target."""
+
+self.spawnLldbMi(args=None)
+
+# This command have to insert breakpoint to the dummy target.
+self.runCmd("-break-insert main")
+# FIXME function name is unknown on Darwin, fullname should be ??, 
line is -1
+self.expect(
+
"\^done,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\","
+
"addr=\"0x\",func=\"\?\?\",file=\"\?\?\",fullname=\"\?\?/\?\?\","
+
"line=\"0\",pending=\[\"main\"\],times=\"0\",original-location=\"main\"}"
+)
+
+# Add a valid target.
+self.runCmd("-file-exec-and-symbols %s" % self.myexe)
+self.expect("\^done")
+
+self.runCmd("-exec-run")
+self.expect("\^running")
+self.expect("\*stopped,reason=\"breakpoint-hit\"")
+
 @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
 @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races
 @skipIfRemote   # We do not currently support remote debugging via the MI.


Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  retur