Conditional breakpoints: add implementation and test
authorIgor Kulaychuk <igor.kulaychuk@gmail.com>
Fri, 5 Oct 2018 17:21:47 +0000 (20:21 +0300)
committerPetr Bred/AI Ecosystem Lab /SRR/Staff Engineer/삼성전자 <p.bred@samsung.com>
Sun, 14 Oct 2018 07:04:19 +0000 (10:04 +0300)
Signed-off-by: Alexander Aksenov <a.aksenov@samsung.com>
src/debug/netcoredbg/breakpoints.cpp
src/debug/netcoredbg/debugger.h
src/debug/netcoredbg/manageddebugger.cpp
src/debug/netcoredbg/manageddebugger.h
src/debug/netcoredbg/miprotocol.cpp
src/debug/netcoredbg/miprotocol.h
src/debug/netcoredbg/protocol.h
src/debug/netcoredbg/vscodeprotocol.cpp
tests/BreakpointAddRemoveTest/BreakpointAddRemoveTest.cs

index fe0c2e08c350af131521c7cf68738668b722af3b..fae365d3b1286279a90555d6497916c291512263 100644 (file)
@@ -23,12 +23,14 @@ void Breakpoints::ManagedBreakpoint::ToBreakpoint(Breakpoint &breakpoint)
 {
     breakpoint.id = this->id;
     breakpoint.verified = this->IsResolved();
+    breakpoint.condition = this->condition;
     breakpoint.source = Source(this->fullname);
     breakpoint.line = this->linenum;
     breakpoint.hitCount = this->times;
 }
 
 HRESULT Breakpoints::HitBreakpoint(
+    Debugger *debugger,
     ICorDebugThread *pThread,
     ICorDebugBreakpoint *pBreakpoint,
     Breakpoint &breakpoint,
@@ -74,6 +76,19 @@ HRESULT Breakpoints::HitBreakpoint(
         b.methodToken == methodToken &&
         b.enabled)
     {
+        if (!b.condition.empty())
+        {
+            DWORD threadId = 0;
+            IfFailRet(pThread->GetID(&threadId));
+            uint64_t frameId = StackFrame(threadId, 0, "").id;
+
+            Variable variable;
+            std::string output;
+            IfFailRet(debugger->Evaluate(frameId, b.condition, variable, output));
+
+            if (variable.type != "bool" || variable.value != "true")
+                return E_FAIL;
+        }
         ++b.times;
         b.ToBreakpoint(breakpoint);
         return S_OK;
@@ -354,21 +369,21 @@ HRESULT Breakpoints::ResolveBreakpoint(ManagedBreakpoint &bp)
 
 HRESULT ManagedDebugger::SetBreakpoints(
     std::string filename,
-    const std::vector<int> &lines,
+    const std::vector<SourceBreakpoint> &srcBreakpoints,
     std::vector<Breakpoint> &breakpoints)
 {
-    return m_breakpoints.SetBreakpoints(m_pProcess, filename, lines, breakpoints);
+    return m_breakpoints.SetBreakpoints(m_pProcess, filename, srcBreakpoints, breakpoints);
 }
 
 HRESULT Breakpoints::SetBreakpoints(
     ICorDebugProcess *pProcess,
     std::string filename,
-    const std::vector<int> &lines,
+    const std::vector<SourceBreakpoint> &srcBreakpoints,
     std::vector<Breakpoint> &breakpoints)
 {
     std::lock_guard<std::mutex> lock(m_breakpointsMutex);
 
-    if (lines.empty())
+    if (srcBreakpoints.empty())
     {
         auto it = m_breakpoints.find(filename);
         if (it != m_breakpoints.end())
@@ -380,8 +395,9 @@ HRESULT Breakpoints::SetBreakpoints(
 
     // Remove old breakpoints
     std::unordered_set<int> unchangedLines;
-    for (int line : lines)
+    for (const auto &sb : srcBreakpoints)
     {
+        int line = sb.line;
         if (breakpointsInSource.find(line) != breakpointsInSource.end())
             unchangedLines.insert(line);
     }
@@ -396,8 +412,9 @@ HRESULT Breakpoints::SetBreakpoints(
 
     // Export breakpoints
 
-    for (int line : lines)
+    for (const auto &sb : srcBreakpoints)
     {
+        int line = sb.line;
         Breakpoint breakpoint;
 
         auto b = breakpointsInSource.find(line);
@@ -408,6 +425,7 @@ HRESULT Breakpoints::SetBreakpoints(
             bp.id = m_nextBreakpointId++;
             bp.fullname = filename;
             bp.linenum = line;
+            bp.condition = sb.condition;
 
             if (pProcess)
                 ResolveBreakpoint(bp);
@@ -418,7 +436,9 @@ HRESULT Breakpoints::SetBreakpoints(
         else
         {
             // Existing breakpoint
-            b->second.ToBreakpoint(breakpoint);
+            ManagedBreakpoint &bp = b->second;
+            bp.condition = sb.condition;
+            bp.ToBreakpoint(breakpoint);
         }
 
         breakpoints.push_back(breakpoint);
index f5a3154558a998f6fdc1890ea80ef8fc1f21f538..5b4107068b20ff67e1e3db692c4643ae884d3e0b 100644 (file)
@@ -49,7 +49,7 @@ public:
     virtual HRESULT Continue() = 0;
     virtual HRESULT Pause() = 0;
     virtual HRESULT GetThreads(std::vector<Thread> &threads) = 0;
-    virtual HRESULT SetBreakpoints(std::string filename, const std::vector<int> &lines, std::vector<Breakpoint> &breakpoints) = 0;
+    virtual HRESULT SetBreakpoints(std::string filename, const std::vector<SourceBreakpoint> &srcBreakpoints, std::vector<Breakpoint> &breakpoints) = 0;
     virtual void InsertExceptionBreakpoint(const std::string &name, Breakpoint &breakpoint) = 0;
     virtual HRESULT GetStackTrace(int threadId, int startFrame, int levels, std::vector<StackFrame> &stackFrames, int &totalFrames) = 0;
     virtual HRESULT StepCommand(int threadId, StepType stepType) = 0;
index e756e09b8bdf79704999855f635426da465632a7..82e7767862c348820398de16c0c6f90f1b2c3708 100644 (file)
@@ -339,26 +339,44 @@ public:
                 return S_OK;
             }
 
-            DWORD threadId = 0;
-            pThread->GetID(&threadId);
-
-            bool atEntry = false;
-            StoppedEvent event(StopBreakpoint, threadId);
-            if (FAILED(m_debugger.m_breakpoints.HitBreakpoint(pThread, pBreakpoint, event.breakpoint, atEntry)))
+            ToRelease<ICorDebugAppDomain> callbackAppDomain(pAppDomain);
+            pAppDomain->AddRef();
+            ToRelease<ICorDebugThread> callbackThread(pThread);
+            pThread->AddRef();
+            ToRelease<ICorDebugBreakpoint> callbackBreakpoint(pBreakpoint);
+            pBreakpoint->AddRef();
+
+            std::thread([this](
+                ICorDebugAppDomain *pAppDomain,
+                ICorDebugThread *pThread,
+                ICorDebugBreakpoint *pBreakpoint)
             {
-                pAppDomain->Continue(0);
-                return S_OK;
-            }
+                DWORD threadId = 0;
+                pThread->GetID(&threadId);
+
+                bool atEntry = false;
+                StoppedEvent event(StopBreakpoint, threadId);
+                if (FAILED(m_debugger.m_breakpoints.HitBreakpoint(&m_debugger, pThread, pBreakpoint, event.breakpoint, atEntry)))
+                {
+                    pAppDomain->Continue(0);
+                    return;
+                }
 
-            if (atEntry)
-                event.reason = StopEntry;
+                if (atEntry)
+                    event.reason = StopEntry;
 
-            ToRelease<ICorDebugFrame> pFrame;
-            if (SUCCEEDED(pThread->GetActiveFrame(&pFrame)) && pFrame != nullptr)
-                m_debugger.GetFrameLocation(pFrame, threadId, 0, event.frame);
+                ToRelease<ICorDebugFrame> pFrame;
+                if (SUCCEEDED(pThread->GetActiveFrame(&pFrame)) && pFrame != nullptr)
+                    m_debugger.GetFrameLocation(pFrame, threadId, 0, event.frame);
+
+                m_debugger.SetLastStoppedThread(pThread);
+                m_debugger.m_protocol->EmitStoppedEvent(event);
 
-            m_debugger.SetLastStoppedThread(pThread);
-            m_debugger.m_protocol->EmitStoppedEvent(event);
+            },
+                std::move(callbackAppDomain),
+                std::move(callbackThread),
+                std::move(callbackBreakpoint)
+            ).detach();
 
             return S_OK;
         }
index 88e168963cb4162786a8212c77778c716021c8ce..75b11895ef4c405f8998701ef53ccad35b378200 100644 (file)
@@ -184,6 +184,7 @@ class Breakpoints
         ToRelease<ICorDebugBreakpoint> breakpoint;
         bool enabled;
         ULONG32 times;
+        std::string condition;
 
         bool IsResolved() const { return modAddress != 0; }
 
@@ -216,6 +217,7 @@ public:
         m_modules(modules), m_nextBreakpointId(1), m_stopAtEntry(false), m_entryPoint(mdMethodDefNil) {}
 
     HRESULT HitBreakpoint(
+        Debugger *debugger,
         ICorDebugThread *pThread,
         ICorDebugBreakpoint *pBreakpoint,
         Breakpoint &breakpoint,
@@ -230,7 +232,7 @@ public:
     HRESULT SetBreakpoints(
         ICorDebugProcess *pProcess,
         std::string filename,
-        const std::vector<int> &lines,
+        const std::vector<SourceBreakpoint> &srcBreakpoints,
         std::vector<Breakpoint> &breakpoints);
 
     void SetStopAtEntry(bool stopAtEntry);
@@ -471,7 +473,7 @@ public:
     HRESULT Continue() override;
     HRESULT Pause() override;
     HRESULT GetThreads(std::vector<Thread> &threads) override;
-    HRESULT SetBreakpoints(std::string filename, const std::vector<int> &lines, std::vector<Breakpoint> &breakpoints) override;
+    HRESULT SetBreakpoints(std::string filename, const std::vector<SourceBreakpoint> &srcBreakpoints, std::vector<Breakpoint> &breakpoints) override;
     void InsertExceptionBreakpoint(const std::string &name, Breakpoint &breakpoint) override;
     HRESULT GetStackTrace(int threadId, int startFrame, int levels, std::vector<StackFrame> &stackFrames, int &totalFrames) override;
     HRESULT StepCommand(int threadId, StepType stepType) override;
index 9047b1e4e1233f91a50fb9fa169c0de6d8b36888..6243f2f1805b0a7299c5b12971d7454ddb03b5b5 100644 (file)
@@ -88,7 +88,11 @@ static bool GetIndices(const std::vector<std::string> &args, int &index1, int &i
     return true;
 }
 
-bool ParseBreakpoint(const std::vector<std::string> &args_orig, std::string &filename, unsigned int &linenum)
+bool ParseBreakpoint(
+    const std::vector<std::string> &args_orig,
+    std::string &filename,
+    unsigned int &linenum,
+    std::string &condition)
 {
     std::vector<std::string> args = args_orig;
     StripArgs(args);
@@ -103,6 +107,14 @@ bool ParseBreakpoint(const std::vector<std::string> &args_orig, std::string &fil
             return false;
     }
 
+    if (args.at(0) == "-c")
+    {
+        if (args.size() < 3)
+            return false;
+        condition = args.at(1);
+        args.erase(args.begin(), args.begin() + 2);
+    }
+
     std::size_t i = args.at(0).rfind(':');
 
     if (i == std::string::npos)
@@ -415,27 +427,31 @@ HRESULT MIProtocol::ListChildren(int threadId, int level, int childStart, int ch
     return S_OK;
 }
 
-HRESULT MIProtocol::SetBreakpoint(const std::string &filename, int linenum, Breakpoint &breakpoint)
+HRESULT MIProtocol::SetBreakpoint(
+    const std::string &filename,
+    int linenum,
+    const std::string &condition,
+    Breakpoint &breakpoint)
 {
     HRESULT Status;
 
     auto &breakpointsInSource = m_breakpoints[filename];
-    std::vector<int> lines;
+    std::vector<SourceBreakpoint> srcBreakpoints;
     for (auto it : breakpointsInSource)
     {
-        lines.push_back(it.second);
+        srcBreakpoints.push_back(it.second);
     }
-    lines.push_back(linenum);
+    srcBreakpoints.emplace_back(linenum, condition);
 
     std::vector<Breakpoint> breakpoints;
-    IfFailRet(m_debugger->SetBreakpoints(filename, lines, breakpoints));
+    IfFailRet(m_debugger->SetBreakpoints(filename, srcBreakpoints, breakpoints));
 
     for (const Breakpoint& b : breakpoints)
     {
         if (b.line != linenum)
             continue;
 
-        breakpointsInSource.insert(std::make_pair(b.id, b.line));
+        breakpointsInSource.insert(std::make_pair(b.id, SourceBreakpoint{ b.line, b.condition }));
         breakpoint = b;
         return S_OK;
     }
@@ -443,23 +459,54 @@ HRESULT MIProtocol::SetBreakpoint(const std::string &filename, int linenum, Brea
     return E_FAIL;
 }
 
+HRESULT MIProtocol::SetBreakpointCondition(uint32_t id, const std::string &condition)
+{
+    HRESULT Status;
+
+    // For each file
+    for (auto &breakpointsIter : m_breakpoints)
+    {
+        std::unordered_map<uint32_t, SourceBreakpoint> &fileBreakpoints = breakpointsIter.second;
+
+        // Find breakpoint with specified id in this file
+        const auto &sbIter = fileBreakpoints.find(id);
+        if (sbIter == fileBreakpoints.end())
+            continue;
+
+        // Modify breakpoint condition
+        sbIter->second.condition = condition;
+
+        // Gather all breakpoints in this file
+        std::vector<SourceBreakpoint> existingBreakpoints;
+        for (const auto &it : fileBreakpoints)
+            existingBreakpoints.emplace_back(it.second);
+
+        // Update breakpoints data for this file
+        const std::string &filename = breakpointsIter.first;
+        std::vector<Breakpoint> tmpBreakpoints;
+        return m_debugger->SetBreakpoints(filename, existingBreakpoints, tmpBreakpoints);
+    }
+
+    return E_FAIL;
+}
+
 void MIProtocol::DeleteBreakpoints(const std::unordered_set<uint32_t> &ids)
 {
     for (auto &breakpointsIter : m_breakpoints)
     {
-        std::vector<int> remainingLines;
+        std::vector<SourceBreakpoint> remainingBreakpoints;
         for (auto it : breakpointsIter.second)
         {
             if (ids.find(it.first) == ids.end())
-                remainingLines.push_back(it.second);
+                remainingBreakpoints.push_back(it.second);
         }
-        if (remainingLines.size() == breakpointsIter.second.size())
+        if (remainingBreakpoints.size() == breakpointsIter.second.size())
             continue;
 
         std::string filename = breakpointsIter.first;
 
         std::vector<Breakpoint> tmpBreakpoints;
-        m_debugger->SetBreakpoints(filename, remainingLines, tmpBreakpoints);
+        m_debugger->SetBreakpoints(filename, remainingBreakpoints, tmpBreakpoints);
     }
 }
 
@@ -613,9 +660,10 @@ HRESULT MIProtocol::HandleCommand(std::string command,
     { "break-insert", [this](const std::vector<std::string> &args, std::string &output) -> HRESULT {
         std::string filename;
         unsigned int linenum;
+        std::string condition;
         Breakpoint breakpoint;
-        if (ParseBreakpoint(args, filename, linenum)
-            && SUCCEEDED(SetBreakpoint(filename, linenum, breakpoint)))
+        if (ParseBreakpoint(args, filename, linenum, condition)
+            && SUCCEEDED(SetBreakpoint(filename, linenum, condition, breakpoint)))
         {
             PrintBreakpoint(breakpoint, output);
             return S_OK;
@@ -636,6 +684,25 @@ HRESULT MIProtocol::HandleCommand(std::string command,
         DeleteBreakpoints(ids);
         return S_OK;
     } },
+    { "break-condition", [this](const std::vector<std::string> &args, std::string &output) -> HRESULT {
+        HRESULT Status;
+
+        if (args.size() < 2)
+        {
+            output = "Command requires at least 2 arguments";
+            return E_FAIL;
+        }
+
+        bool ok;
+        int id = ParseInt(args.at(0), ok);
+        if (!ok)
+        {
+            output = "Unknown breakpoint id";
+            return E_FAIL;
+        }
+
+        return SetBreakpointCondition(id, args.at(1));
+    } },
     { "exec-step", [this](const std::vector<std::string> &args, std::string &output) -> HRESULT {
         return StepCommand(args, output, Debugger::STEP_IN);
     }},
@@ -985,8 +1052,15 @@ static bool ParseLine(const std::string &str,
     if (cmd == "var-assign")
     {
         tokenizer.Next(result);
-        args.push_back(result);
-        args.push_back(tokenizer.Remain());
+        args.push_back(result); // name
+        args.push_back(tokenizer.Remain()); // expression
+        return true;
+    }
+    else if (cmd == "break-condition")
+    {
+        tokenizer.Next(result);
+        args.push_back(result); // id
+        args.push_back(tokenizer.Remain()); // expression
         return true;
     }
 
index 684a240bb0b25b037e9ee938eaa31b072372046c..8c99db429517c2d3ca1eb1b51ada35195b69d0ad 100644 (file)
@@ -21,7 +21,7 @@ class MIProtocol : public Protocol
 
     unsigned int m_varCounter;
     std::unordered_map<std::string, Variable> m_vars;
-    std::unordered_map<std::string, std::unordered_map<int32_t, int> > m_breakpoints;
+    std::unordered_map<std::string, std::unordered_map<uint32_t, SourceBreakpoint> > m_breakpoints;
 
     static std::string EscapeMIValue(const std::string &str);
     static HRESULT PrintBreakpoint(const Breakpoint &b, std::string &output);
@@ -71,7 +71,8 @@ private:
     void PrintChildren(std::vector<Variable> &children, int threadId, int print_values, bool has_more, std::string &output);
     void PrintNewVar(std::string varobjName, Variable &v, int threadId, int print_values, std::string &output);
     HRESULT ListChildren(int threadId, int level, int childStart, int childEnd, const std::string &varName, int print_values, std::string &output);
-    HRESULT SetBreakpoint(const std::string &filename, int linenum, Breakpoint &breakpoints);
+    HRESULT SetBreakpoint(const std::string &filename, int linenum, const std::string &condition, Breakpoint &breakpoints);
+    HRESULT SetBreakpointCondition(uint32_t id, const std::string &condition);
     void DeleteBreakpoints(const std::unordered_set<uint32_t> &ids);
     static HRESULT PrintFrameLocation(const StackFrame &stackFrame, std::string &output);
 };
index d73c1391ca83d4ab064a51e77826d28f1cbbea52..608c6dd2050f56083a9635f4c467f8c1162139f6 100644 (file)
@@ -74,6 +74,7 @@ struct Breakpoint
     uint32_t id;
     bool verified;
     std::string message;
+    std::string condition;
     Source source;
     int line;
 
@@ -241,3 +242,11 @@ enum VariablesFilter
     VariablesIndexed,
     VariablesBoth
 };
+
+struct SourceBreakpoint
+{
+    int line;
+    std::string condition;
+
+    SourceBreakpoint(int linenum, const std::string &cond = std::string()) : line(linenum), condition(cond) {}
+};
index 995734815b0dd36ee9021f4115ac2a7cef94bca6..f12656434ce557bad7e4cb6c8a0fa16f30e9e109 100644 (file)
@@ -258,6 +258,7 @@ HRESULT VSCodeProtocol::HandleCommand(const std::string &command, const json &ar
         m_debugger->Initialize();
         body["supportsConfigurationDoneRequest"] = true;
         body["supportTerminateDebuggee"] = true;
+        body["supportsConditionalBreakpoints"] = true;
         return S_OK;
     } },
     { "configurationDone", [this](const json &arguments, json &body){
@@ -266,12 +267,12 @@ HRESULT VSCodeProtocol::HandleCommand(const std::string &command, const json &ar
     { "setBreakpoints", [this](const json &arguments, json &body){
         HRESULT Status;
 
-        std::vector<int> lines;
+        std::vector<SourceBreakpoint> srcBreakpoints;
         for (auto &b : arguments.at("breakpoints"))
-            lines.push_back(b.at("line"));
+            srcBreakpoints.emplace_back(b.at("line"), b.value("condition", std::string()));
 
         std::vector<Breakpoint> breakpoints;
-        IfFailRet(m_debugger->SetBreakpoints(arguments.at("source").at("path"), lines, breakpoints));
+        IfFailRet(m_debugger->SetBreakpoints(arguments.at("source").at("path"), srcBreakpoints, breakpoints));
 
         body["breakpoints"] = breakpoints;
 
index d09b8230f6bd5fd7092a775fad81a5654a5934cc..c3ceb87c7108748999882964ef1dc82322aad413 100644 (file)
@@ -13,12 +13,18 @@ Send(String.Format("4-break-insert -f {0}:{1}", filename, Lines["BREAK2"]));
 r = Expect("4^done");
 int id2 = r.Find("bkpt").FindInt("number");
 
-Send("5-exec-run");
+Send(String.Format("5-break-insert -f -c \"x>20\" {0}:{1}", filename, Lines["BREAK3"]));
+r = Expect("5^done");
+int id3 = r.Find("bkpt").FindInt("number");
+
+Send("-exec-run");
 */
 using System;
 
 namespace BreakpointAddRemoveTest
 {
+
+
     class Program
     {
         static void Main(string[] args)
@@ -39,12 +45,29 @@ Assert.Equal(id1, r.FindInt("bkptno"));
 
 Send(String.Format("7-break-delete {0}", id2));
 Expect("7^done");
-
 Send("8-exec-continue");
+*/
+                TestFunc(10);
+                TestFunc(21);
+                TestFunc(9);
+
+                Console.WriteLine("Hello World!"); // //@BREAK2@
+        }
+
+        static void TestFunc(int x)
+        {
+                x++;                            // //@BREAK3@
+/*
 r = Expect("*stopped");
-Assert.Equal("exited", r.FindString("reason"));
+Assert.Equal("breakpoint-hit", r.FindString("reason"));
+Assert.Equal(Lines["BREAK3"], r.Find("frame").FindInt("line"));
+Assert.Equal(id3, r.FindInt("bkptno"));
+Send("9-exec-continue");
 */
-            Console.WriteLine("Hello World!"); // //@BREAK2@
         }
     }
+/*
+r = Expect("*stopped");
+Assert.Equal("exited", r.FindString("reason"));
+*/
 }