[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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