Fix lldb-mi disable/enable breakpoints commands

[• faxue]

Summary

1. Make the enable breakpoint command actually enable
2. Remove unnecessary output from disable/enable command which isn't following mi spec

[ChuckR]

@abidh @ki.stfu Do you have any feedback? We would like to check this in and I want to get more sign off then just me.

[ki.stfu]

Hi Chuck! Sorry for delay, didn't see this CL. I'll check it tomorrow, so pls wait one more day.

[ki.stfu]

@faxue, thank you for your patch! The fixes look good, but if you don't mind I gonna improve your tests. After that I'll commit this patch.

[ki.stfu]

add checks for =breakpoint-modified

packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py

def test_lldbmi_break_insert_settings(self):
         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\"")
+
+    @skipIfWindows #llvm.org/pr24452: Get lldb-mi tests working on Windows
+    @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    def test_lldbmi_break_enable_disable(self):
+        """Test that 'lldb-mi --interpreter' works for enabling / disabling breakpoints."""
+
+        self.spawnLldbMi(args = None)
+
+        self.runCmd("-file-exec-and-symbols %s" % self.myexe)
+        self.expect("\^done")
+
+        self.runCmd("-break-insert main")
+        self.expect("\^done,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"main\"")
+        self.expect("=breakpoint-modified,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"main\",file=\"main\.cpp\",fullname=\".+?main\.cpp\",line=\"\d+\",times=\"0\",original-location=\"main\"}")
+
+        self.runCmd("-exec-run")
+        self.expect("\^running")
+        self.expect("=breakpoint-modified,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"main\",file=\"main\.cpp\",fullname=\".+?main\.cpp\",line=\"\d+\",times=\"0\",original-location=\"main\"}")
+        self.expect("\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"1\"")
+
+        self.runCmd("-break-insert ns::foo1")
+        self.expect("\^done,bkpt={number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"ns::foo1\(\)\"")
+        self.expect("=breakpoint-modified,bkpt={number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"ns::foo1\(\)\",file=\"main\.cpp\",fullname=\".+?main\.cpp\",line=\"\d+\",times=\"0\",original-location=\"ns::foo1\"}")
+
+        self.runCmd("-break-insert ns::foo2")
+        self.expect("\^done,bkpt={number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"ns::foo2\(\)\"")
+        self.expect("=breakpoint-modified,bkpt={number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"ns::foo2\(\)\",file=\"main\.cpp\",fullname=\".+?main\.cpp\",line=\"\d+\",times=\"0\",original-location=\"ns::foo2\"}")
+
+        # disable the 2nd breakpoint
+        self.runCmd("-break-disable 2")
+        self.expect("\^done")
+        self.expect("=breakpoint-modified,bkpt={number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"n\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"ns::foo1\(\)\",file=\"main\.cpp\",fullname=\".+?main\.cpp\",line=\"\d+\",times=\"0\",original-location=\"ns::foo1\"}")
+
+        # disable the 3rd breakpoint and re-enable
+        self.runCmd("-break-disable 3")
+        self.expect("\^done")
+        self.expect("=breakpoint-modified,bkpt={number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"n\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"ns::foo2\(\)\",file=\"main\.cpp\",fullname=\".+?main\.cpp\",line=\"\d+\",times=\"0\",original-location=\"ns::foo2\"}")
+
+        self.runCmd("-break-enable 3")
+        self.expect("\^done")
+        self.expect("=breakpoint-modified,bkpt={number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\"ns::foo2\(\)\",file=\"main\.cpp\",fullname=\".+?main\.cpp\",line=\"\d+\",times=\"0\",original-location=\"ns::foo2\"}")
+
+        self.runCmd("-exec-continue")
+        self.expect("\^running")
+        self.expect("\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"3\"")

tools/lldb-mi/MICmdCmdBreak.cpp

 //          MIstatus::failure - Functional failed.
 // Throws:  None.
 //--
 bool
 CMICmdCmdBreakDisable::Acknowledge()
 {
     if (m_bBrkPtDisabledOk)
     {
-        const CMICmnMIValueConst miValueConst(CMIUtilString::Format("%d", m_nBrkPtId));
-        const CMICmnMIValueResult miValueResult("number", miValueConst);
-        CMICmnMIValueTuple miValueTuple(miValueResult);
-        const CMICmnMIValueConst miValueConst2("n");
-        const CMICmnMIValueResult miValueResult2("enabled", miValueConst2);
-        miValueTuple.Add(miValueResult2);
-        const CMICmnMIValueResult miValueResult3("bkpt", miValueTuple);
-        const CMICmnMIOutOfBandRecord miOutOfBandRecord(CMICmnMIOutOfBandRecord::eOutOfBand_BreakPointModified, miValueResult3);
-        bool bOk = CMICmnStreamStdout::TextToStdout(miOutOfBandRecord.GetString());
-
         const CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Done);
         m_miResultRecord = miRecordResult;
-        return bOk;
+        return MIstatus::success;
     }
 
     const CMIUtilString strBrkPtId(CMIUtilString::Format("%d", m_nBrkPtId));
     const CMICmnMIValueConst miValueConst(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_BRKPT_INVALID), strBrkPtId.c_str()));
     const CMICmnMIValueResult miValueResult("msg", miValueConst);
     const CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Error, miValueResult);
     m_miResultRecord = miRecordResult;
 
if (!pArgBrkPt->GetExpectedOption<CMICmdArgValNumber, MIuint64>(nBrk))
         return MIstatus::failure;
     }
 
     CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance());
     lldb::SBBreakpoint brkPt = rSessionInfo.GetTarget().FindBreakpointByID(static_cast<lldb::break_id_t>(nBrk));
     if (brkPt.IsValid())
     {
         m_bBrkPtEnabledOk = true;
-        brkPt.SetEnabled(false);
+        brkPt.SetEnabled(true);
         m_nBrkPtId = nBrk;
     }
 
     return MIstatus::success;
 }
 
 //++ ------------------------------------------------------------------------------------
 // Details: The invoker requires this function. The command prepares a MI Record Result
 //          for the work carried out in the Execute().
 // Type:    Overridden.
 // Args:    None.
 // Return:  MIstatus::success - Functional succeeded.
 //          MIstatus::failure - Functional failed.
 // Throws:  None.
 //--
 bool
 CMICmdCmdBreakEnable::Acknowledge()
 {
     if (m_bBrkPtEnabledOk)
     {
-        const CMICmnMIValueConst miValueConst(CMIUtilString::Format("%d", m_nBrkPtId));
-        const CMICmnMIValueResult miValueResult("number", miValueConst);
-        CMICmnMIValueTuple miValueTuple(miValueResult);
-        const CMICmnMIValueConst miValueConst2("y");
-        const CMICmnMIValueResult miValueResult2("enabled", miValueConst2);
-        miValueTuple.Add(miValueResult2);
-        const CMICmnMIValueResult miValueResult3("bkpt", miValueTuple);
-        const CMICmnMIOutOfBandRecord miOutOfBandRecord(CMICmnMIOutOfBandRecord::eOutOfBand_BreakPointModified, miValueResult3);
-        bool bOk = CMICmnStreamStdout::TextToStdout(miOutOfBandRecord.GetString());
-
         const CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Done);
         m_miResultRecord = miRecordResult;
-        return bOk;
+        return MIstatus::success;
     }
 
     const CMIUtilString strBrkPtId(CMIUtilString::Format("%d", m_nBrkPtId));
     const CMICmnMIValueConst miValueConst(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_BRKPT_INVALID), strBrkPtId.c_str()));
     const CMICmnMIValueResult miValueResult("msg", miValueConst);
     const CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Error, miValueResult);
     m_miResultRecord = miRecordResult;