[lldb-mi] Fix breakpoints on functions when C++ namespaces are used.

[dawn]

Summary

The command "-break-insert ns::foo" for function 'foo' in namespace 'ns'
was being parsed as file:function, and the command "-break-insert file:::foo" for global function 'foo' in file 'file' was being parsed as 'file::' + ':' + 'foo'. This patch fixes these cases by adding checks for '::' and ':::'. (Note: '::func' is not parsed correctly by lldb, but that's a different bug...)

[dawn]

This patch isn't quite right as it doesn't handle foo.c:std::string. will post new patch...

[dawn]

This revision fixes the case that we have 'file:ns::func' and enables a test case.

[brucem]

Could you please add a test for the file: + ::func case?

[dawn]

In D11396#209412, @brucem wrote:

Could you please add a test for the file: + ::func case?

Setting breakpoints on ::func doesn't work in lldb! It needs to be fixed there first. If/when it is fixed, I will add a test.

(lldb) br set --name ::func
Breakpoint 6: no locations (pending).
WARNING:  Unable to resolve breakpoint to any actual locations.

[ki.stfu]

In D11396#209959, @dawn wrote:

In D11396#209412, @brucem wrote:

Could you please add a test for the file: + ::func case?

Setting breakpoints on ::func doesn't work in lldb! It needs to be fixed there first. If/when it is fixed, I will add a test.

(lldb) br set --name ::func
Breakpoint 6: no locations (pending).
WARNING:  Unable to resolve breakpoint to any actual locations.

Please add this test and comment it out with FIXME explanation why it's disabled. Also, could you please create a bug for this and add a link in FIXME?

[dawn]

In D11396#210455, @ki.stfu wrote:

Please add this test and comment it out with FIXME explanation why it's disabled. Also, could you please create a bug for this and add a link in FIXME?

I don't have an account on bugzilla and don't want to create one due to the warnings about public e-mails. Can you add the bug for me please?

I will add a test and comment it out (but won't bother to resubmit new patch just for that minor addition). Ok to commit?

[dawn]

Comment about "::func" syntax not working in lldb added as requested. Please accept this patch - it fixes namespace tokens used in lldb-mi breakpoints like "-break-insert ns::func". Patches to lldb and other issues are not intended to be part of this patch and should be handled separately. Thank you.

[ki.stfu]

Add a link to the bug please and go ahead.

test/tools/lldb-mi/breakpoint/TestMiBreak.py

79 ↗

(On Diff #30609)

Add a link to the bug please: "llvm.org/pr24271"

lldb/trunk/test/tools/lldb-mi/breakpoint/TestMiBreak.py

def test_lldbmi_break_insert_function(self):
         self.runCmd("-break-insert unknown_func")
         self.expect("\^error,msg=\"Command 'break-insert'. Breakpoint location 'unknown_func' not found\"")
 
         # Test that non-pending BP was set correctly
         self.runCmd("-exec-continue")
         self.expect("\^running")
         self.expect("\*stopped,reason=\"breakpoint-hit\"")
 
+        #FIXME: this test is disabled due to lldb bug llvm.org/pr24271.
+        # Test that we can set a BP using the global namespace token
+        #self.runCmd("-break-insert ::main")
+        #self.expect("\^done,bkpt={number=\"3\"")
+
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
     def test_lldbmi_break_insert_file_line_pending(self):
         """Test that 'lldb-mi --interpreter' works for pending file:line breakpoints."""
 
         self.spawnLldbMi(args = None)
 
def test_lldbmi_break_insert_settings(self):
         self.expect("\^running")
         self.expect("\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"4\"")
 
         # Test that BP #3 is hit
         self.runCmd("-exec-continue")
         self.expect("\^running")
         self.expect("\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"3\"")
 
-        # Test that the target.language=pascal setting works and that BP #5 is not set
+        # Test that the target.language=pascal setting works and that BP #5 is NOT set
         self.runCmd("-interpreter-exec console \"settings set target.language c\"")
         self.expect("\^done")
         self.runCmd("-break-insert ns.foo1")
         self.expect("\^error")
 
         # Test that the target.language=c++ setting works and that BP #6 is hit
-        # FIXME: lldb-mi interprets 'ns::func' as file:func where file='ns:'.
-        #self.runCmd("-interpreter-exec console \"settings set target.language c++\"")
-        #self.expect("\^done")
-        #self.runCmd("-break-insert ns::foo1")
-        #self.expect("\^done,bkpt={number=\"6\"")
-        #self.runCmd("-exec-run")
-        #self.expect("\^running")
-        #self.expect("\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"6\"")
+        self.runCmd("-interpreter-exec console \"settings set target.language c++\"")
+        self.expect("\^done")
+        self.runCmd("-break-insert ns::foo1")
+        self.expect("\^done,bkpt={number=\"6\"")
+        self.runCmd("-exec-continue")
+        self.expect("\^running")
+        self.expect("\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"6\"")
 
         # Test that BP #1 and #2 weren't set by running to program exit
         self.runCmd("-exec-continue")
         self.expect("\^running")
         self.expect("\*stopped,reason=\"exited-normally\"")
 
 if __name__ == '__main__':
     unittest2.main()

lldb/trunk/tools/lldb-mi/MICmdCmdBreak.cpp

m_setCmdArgs.Add(*(new CMICmdArgValOptionShort(m_constStrArgNamedRestrictBrkPtToThreadId, false, true,
                                                    CMICmdArgValListBase::eArgValType_Number, 1)));
     m_setCmdArgs.Add(*(new CMICmdArgValString(m_constStrArgNamedLocation, false, true)));
     m_setCmdArgs.Add(
         *(new CMICmdArgValOptionLong(m_constStrArgNamedThreadGroup, false, true, CMICmdArgValListBase::eArgValType_ThreadGrp, 1)));
     return ParseValidateCmdOptions();
 }
 
 //++ ------------------------------------------------------------------------------------
+// Helper function for CMICmdCmdBreakInsert::Execute(void).
+//
+// Given a string, return the position of the ':' separator in 'file:func'
+// or 'file:line', if any.  If not found, return npos.  For example, return
+// 5 for 'foo.c:std::string'.
+//--
+static size_t findFileSeparatorPos(const std::string& x)
+{
+    // Full paths in windows can have ':' after a drive letter, so we
+    // search backwards, taking care to skip C++ namespace tokens '::'.
+    size_t n = x.find_last_of(':');
+    while (n != std::string::npos && n > 1 && x[n-1] == ':')
+    {
+        n = x.find_last_of(':', n - 2);
+    }
+    return n;
+}
+
+//++ ------------------------------------------------------------------------------------
 // Details: The invoker requires this function. The command does work in this function.
 //          The command is likely to communicate with the LLDB SBDebugger in here.
 // Type:    Overridden.
 // Args:    None.
 // Return:  MIstatus::success - Functional succeeded.
 //          MIstatus::failure - Functional failed.
 // Throws:  None.
 //--
CMICmdCmdBreakInsert::Execute(void)
     m_bBrkPtThreadId = pArgRestrictBrkPtToThreadId->GetFound();
     if (m_bBrkPtCondition)
     {
         pArgRestrictBrkPtToThreadId->GetExpectedOption<CMICmdArgValNumber, MIuint>(m_nBrkPtThreadId);
     }
 
     // Determine if break on a file line or at a function
     BreakPoint_e eBrkPtType = eBreakPoint_NotDefineYet;
-    const CMIUtilString cColon = ":";
     CMIUtilString fileName;
     MIuint nFileLine = 0;
     CMIUtilString strFileFn;
     CMIUtilString rStrLineOrFn;
-    // Full path in windows can have : after drive letter. So look for the
-    // last colon
-    const size_t nPosColon = m_brkName.find_last_of(cColon);
+    // Is the string in the form 'file:func' or 'file:line'?
+    // If so, find the position of the ':' separator.
+    const size_t nPosColon = findFileSeparatorPos(m_brkName);
     if (nPosColon != std::string::npos)
     {
-        // extract file name and line number from it
+        // Extract file name and line number from it
         fileName = m_brkName.substr(0, nPosColon);
         rStrLineOrFn = m_brkName.substr(nPosColon + 1, m_brkName.size() - nPosColon - 1);
 
         if (rStrLineOrFn.empty())
             eBrkPtType = eBreakPoint_ByName;
         else
         {
             MIint64 nValue = 0;