From: Igor Kulaychuk <i.kulaychuk@samsung.com>
Date: Wed, 17 Jan 2018 20:25:02 +0000 (+0300)
Subject: Refactor internal structure for handling breakpoints
X-Git-Tag: submit/tizen/20180620.071641~53
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6e410157c39d6e993f0303acad7d5ab38eca762d;p=sdk%2Ftools%2Fnetcoredbg.git

Refactor internal structure for handling breakpoints
---

diff --git a/src/debug/netcoredbg/breakpoints.cpp b/src/debug/netcoredbg/breakpoints.cpp
index 21f4c83..6ce65f8 100644
--- a/src/debug/netcoredbg/breakpoints.cpp
+++ b/src/debug/netcoredbg/breakpoints.cpp
@@ -4,65 +4,34 @@
 
 #include "common.h"
 
-#include <sstream>
 #include <mutex>
 #include <memory>
-#include <unordered_map>
-#include <vector>
-#include <map>
-#include <mutex>
-#include <condition_variable>
+#include <unordered_set>
 
 #include "debugger.h"
 #include "modules.h"
-#include "breakpoints.h"
-
-
-static std::mutex g_breakMutex;
-static ULONG32 g_breakIndex = 1;
-
-struct ManagedBreakpoint {
-    ULONG32 id;
-    CORDB_ADDRESS modAddress;
-    mdMethodDef methodToken;
-    ULONG32 ilOffset;
-    std::string fullname;
-    int linenum;
-    ToRelease<ICorDebugBreakpoint> breakpoint;
-    bool enabled;
-    ULONG32 times;
-
-    bool IsResolved() const
-    {
-        return modAddress != 0;
-    }
 
-    ManagedBreakpoint() :
-        id(0), modAddress(0), methodToken(0), ilOffset(0), linenum(0), breakpoint(nullptr), enabled(true), times(0) {}
 
-    ~ManagedBreakpoint()
-    {
-        if (breakpoint)
-            breakpoint->Activate(0);
-    }
-
-    void ToBreakpoint(Breakpoint &breakpoint)
-    {
-        breakpoint.id = this->id;
-        breakpoint.verified = this->IsResolved();
-        breakpoint.source = Source(this->fullname);
-        breakpoint.line = this->linenum;
-        breakpoint.hitCount = this->times;
-    }
-
-    ManagedBreakpoint(ManagedBreakpoint &&that) = default;
+Debugger::ManagedBreakpoint::ManagedBreakpoint() :
+    id(0), modAddress(0), methodToken(0), ilOffset(0), linenum(0), breakpoint(nullptr), enabled(true), times(0)
+{}
 
-    ManagedBreakpoint(const ManagedBreakpoint &that) = delete;
-};
+Debugger::ManagedBreakpoint::~ManagedBreakpoint()
+{
+    if (breakpoint)
+        breakpoint->Activate(0);
+}
 
-static std::map<ULONG32, ManagedBreakpoint> g_breaks;
+void Debugger::ManagedBreakpoint::ToBreakpoint(Breakpoint &breakpoint)
+{
+    breakpoint.id = this->id;
+    breakpoint.verified = this->IsResolved();
+    breakpoint.source = Source(this->fullname);
+    breakpoint.line = this->linenum;
+    breakpoint.hitCount = this->times;
+}
 
-HRESULT GetCurrentBreakpoint(ICorDebugThread *pThread, Breakpoint &breakpoint)
+HRESULT Debugger::HitBreakpoint(ICorDebugThread *pThread, Breakpoint &breakpoint)
 {
     HRESULT Status;
 
@@ -78,95 +47,45 @@ HRESULT GetCurrentBreakpoint(ICorDebugThread *pThread, Breakpoint &breakpoint)
 
     IfFailRet(Modules::GetFrameLocation(pFrame, ilOffset, sp));
 
-    std::lock_guard<std::mutex> lock(g_breakMutex);
+    std::lock_guard<std::mutex> lock(m_breakpointsMutex);
 
-    for (auto &it : g_breaks)
-    {
-        ManagedBreakpoint &b = it.second;
-
-        if (b.fullname == sp.document &&
-            b.ilOffset == ilOffset &&
-            b.methodToken == methodToken &&
-            b.linenum == sp.startLine &&
-            b.enabled)
-        {
-            b.ToBreakpoint(breakpoint);
-            return S_OK;
-        }
-    }
-
-    return E_FAIL;
-}
-
-HRESULT HitBreakpoint(ICorDebugThread *pThread, Breakpoint &breakpoint)
-{
-    HRESULT Status;
-
-    ULONG32 ilOffset;
-    Modules::SequencePoint sp;
-    mdMethodDef methodToken;
-
-    ToRelease<ICorDebugFrame> pFrame;
-    IfFailRet(pThread->GetActiveFrame(&pFrame));
-    if (pFrame == nullptr)
+    auto breakpoints = m_breakpoints.find(sp.document);
+    if (breakpoints == m_breakpoints.end())
         return E_FAIL;
-    IfFailRet(pFrame->GetFunctionToken(&methodToken));
 
-    IfFailRet(Modules::GetFrameLocation(pFrame, ilOffset, sp));
+    auto &breakpointsInSource = breakpoints->second;
+    auto it = breakpointsInSource.find(sp.startLine);
+    if (it == breakpointsInSource.end())
+        return E_FAIL;
 
-    std::lock_guard<std::mutex> lock(g_breakMutex);
+    ManagedBreakpoint &b = it->second;
 
-    for (auto &it : g_breaks)
+    if (b.ilOffset == ilOffset &&
+        b.methodToken == methodToken &&
+        b.enabled)
     {
-        ManagedBreakpoint &b = it.second;
-
-        if (b.fullname == sp.document &&
-            b.ilOffset == ilOffset &&
-            b.methodToken == methodToken &&
-            b.linenum == sp.startLine &&
-            b.enabled)
-        {
-            ++b.times;
-            b.ToBreakpoint(breakpoint);
-            return S_OK;
-        }
+        ++b.times;
+        b.ToBreakpoint(breakpoint);
+        return S_OK;
     }
 
     return E_FAIL;
 }
 
-static void InsertBreakpoint(ManagedBreakpoint &bp, Breakpoint &breakpoint)
-{
-    std::lock_guard<std::mutex> lock(g_breakMutex);
-    ULONG32 id = g_breakIndex++;
-    bp.id = id;
-    g_breaks.insert(std::make_pair(id, std::move(bp)));
-    bp.ToBreakpoint(breakpoint);
-}
-
-void InsertExceptionBreakpoint(const std::string &name, Breakpoint &breakpoint)
+void Debugger::InsertExceptionBreakpoint(const std::string &name, Breakpoint &breakpoint)
 {
-    ManagedBreakpoint bp;
-    InsertBreakpoint(bp, breakpoint);
-}
-
-HRESULT DeleteBreakpoint(ULONG32 id)
-{
-    std::lock_guard<std::mutex> lock(g_breakMutex);
-
-    g_breaks.erase(id);
-
-    return S_OK;
+    std::lock_guard<std::mutex> lock(m_breakpointsMutex);
+    m_nextBreakpointId++;
 }
 
-void DeleteAllBreakpoints()
+void Debugger::DeleteAllBreakpoints()
 {
-    std::lock_guard<std::mutex> lock(g_breakMutex);
+    std::lock_guard<std::mutex> lock(m_breakpointsMutex);
 
-    g_breaks.clear();
+    m_breakpoints.clear();
 }
 
-static HRESULT ResolveBreakpointInModule(ICorDebugModule *pModule, ManagedBreakpoint &bp)
+HRESULT Debugger::ResolveBreakpointInModule(ICorDebugModule *pModule, ManagedBreakpoint &bp)
 {
     HRESULT Status;
 
@@ -175,7 +94,8 @@ static HRESULT ResolveBreakpointInModule(ICorDebugModule *pModule, ManagedBreakp
     std::string fullname;
 
     IfFailRet(Modules::GetLocationInModule(
-        pModule, bp.fullname,
+        pModule,
+        bp.fullname,
         bp.linenum,
         ilOffset,
         methodToken,
@@ -202,7 +122,30 @@ static HRESULT ResolveBreakpointInModule(ICorDebugModule *pModule, ManagedBreakp
     return S_OK;
 }
 
-static HRESULT ResolveBreakpoint(ManagedBreakpoint &bp)
+void Debugger::TryResolveBreakpointsForModule(ICorDebugModule *pModule)
+{
+    std::lock_guard<std::mutex> lock(m_breakpointsMutex);
+
+    for (auto &breakpoints : m_breakpoints)
+    {
+        for (auto &it : breakpoints.second)
+        {
+            ManagedBreakpoint &b = it.second;
+
+            if (b.IsResolved())
+                continue;
+
+            if (SUCCEEDED(ResolveBreakpointInModule(pModule, b)))
+            {
+                Breakpoint breakpoint;
+                b.ToBreakpoint(breakpoint);
+                m_protocol->EmitBreakpointEvent(BreakpointEvent(BreakpointChanged, breakpoint));
+            }
+        }
+    }
+}
+
+HRESULT Debugger::ResolveBreakpoint(ManagedBreakpoint &bp)
 {
     HRESULT Status;
 
@@ -241,36 +184,69 @@ static HRESULT ResolveBreakpoint(ManagedBreakpoint &bp)
     return S_OK;
 }
 
-void Debugger::TryResolveBreakpointsForModule(ICorDebugModule *pModule)
+HRESULT Debugger::SetBreakpoints(std::string filename,
+                                 const std::vector<int> &lines,
+                                 std::vector<Breakpoint> &breakpoints)
 {
-    std::lock_guard<std::mutex> lock(g_breakMutex);
+    std::lock_guard<std::mutex> lock(m_breakpointsMutex);
 
-    for (auto &it : g_breaks)
+    if (lines.empty())
     {
-        ManagedBreakpoint &b = it.second;
+        auto it = m_breakpoints.find(filename);
+        if (it != m_breakpoints.end())
+            m_breakpoints.erase(it);
+        return S_OK;
+    }
 
-        if (b.IsResolved())
-            continue;
+    Source source(filename);
 
-        if (SUCCEEDED(ResolveBreakpointInModule(pModule, b)))
-        {
-            Breakpoint breakpoint;
-            b.ToBreakpoint(breakpoint);
-            m_protocol->EmitBreakpointEvent(BreakpointEvent(BreakpointChanged, breakpoint));
-        }
+    auto &breakpointsInSource = m_breakpoints[filename];
+
+    // Remove old breakpoints
+    std::unordered_set<int> unchangedLines;
+    for (int line : lines)
+    {
+        if (breakpointsInSource.find(line) != breakpointsInSource.end())
+            unchangedLines.insert(line);
     }
-}
 
-HRESULT InsertBreakpointInProcess(ICorDebugProcess *pProcess, std::string filename, int linenum, Breakpoint &breakpoint)
-{
-    ManagedBreakpoint bp;
-    bp.fullname = filename;
-    bp.linenum = linenum;
+    std::unordered_set<int> removedLines;
+    for (auto &b : breakpointsInSource)
+        if (unchangedLines.find(b.first) == unchangedLines.end())
+            removedLines.insert(b.first);
+
+    for (int line : removedLines)
+        breakpointsInSource.erase(line);
 
-    if (pProcess)
-        ResolveBreakpoint(bp);
+    // Export breakpoints
 
-    InsertBreakpoint(bp, breakpoint);
+    for (int line : lines)
+    {
+        Breakpoint breakpoint;
+
+        auto b = breakpointsInSource.find(line);
+        if (b == breakpointsInSource.end())
+        {
+            // New breakpoint
+            ManagedBreakpoint bp;
+            bp.id = m_nextBreakpointId++;
+            bp.fullname = filename;
+            bp.linenum = line;
+
+            if (m_pProcess)
+                ResolveBreakpoint(bp);
+
+            bp.ToBreakpoint(breakpoint);
+            breakpointsInSource.insert(std::make_pair(line, std::move(bp)));
+        }
+        else
+        {
+            // Existing breakpoint
+            b->second.ToBreakpoint(breakpoint);
+        }
+
+        breakpoints.push_back(breakpoint);
+    }
 
     return S_OK;
 }
diff --git a/src/debug/netcoredbg/breakpoints.h b/src/debug/netcoredbg/breakpoints.h
deleted file mode 100644
index c2446ca..0000000
--- a/src/debug/netcoredbg/breakpoints.h
+++ /dev/null
@@ -1,11 +0,0 @@
-// Copyright (c) 2017 Samsung Electronics Co., LTD
-// Distributed under the MIT License.
-// See the LICENSE file in the project root for more information.
-
-HRESULT DeleteBreakpoint(ULONG32 id);
-HRESULT InsertBreakpointInProcess(ICorDebugProcess *pProcess, std::string filename, int linenum, Breakpoint &breakpoint);
-void InsertExceptionBreakpoint(const std::string &name, Breakpoint &breakpoint);
-HRESULT GetCurrentBreakpoint(ICorDebugThread *pThread, Breakpoint &breakpoint);
-
-void DeleteAllBreakpoints();
-HRESULT HitBreakpoint(ICorDebugThread *pThread, Breakpoint &breakpoint);
diff --git a/src/debug/netcoredbg/commands.cpp b/src/debug/netcoredbg/commands.cpp
index 5062d70..9e802e1 100644
--- a/src/debug/netcoredbg/commands.cpp
+++ b/src/debug/netcoredbg/commands.cpp
@@ -19,7 +19,6 @@
 #include "platform.h"
 #include "debugger.h"
 #include "modules.h"
-#include "breakpoints.h"
 #include "frames.h"
 
 
@@ -151,7 +150,7 @@ void MIProtocol::EmitBreakpointEvent(BreakpointEvent event)
 {
     switch(event.reason)
     {
-        case StopBreakpoint:
+        case BreakpointChanged:
         {
             std::string output;
             PrintBreakpoint(event.breakpoint, output);
@@ -236,11 +235,6 @@ HRESULT Debugger::GetThreads(std::vector<Thread> &threads)
     return GetThreadsState(m_pProcess, threads);
 }
 
-HRESULT Debugger::SetBreakpoint(std::string filename, int linenum, Breakpoint &breakpoint)
-{
-    return InsertBreakpointInProcess(m_pProcess, filename, linenum, breakpoint);
-}
-
 HRESULT Debugger::GetStackTrace(int threadId, int lowFrame, int highFrame, std::vector<StackFrame> &stackFrames)
 {
     HRESULT Status;
@@ -421,6 +415,7 @@ void MIProtocol::Cleanup()
 {
     m_vars.clear();
     m_varCounter = 0;
+    m_breakpoints.clear();
 }
 
 void MIProtocol::PrintChildren(std::vector<Variable> &children, int threadId, int print_values, bool has_more, std::string &output)
@@ -478,6 +473,47 @@ 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 Status;
+
+    auto &breakpointsInSource = m_breakpoints[filename];
+    std::vector<int> lines;
+    for (auto it : breakpointsInSource)
+    {
+        lines.push_back(it.second);
+    }
+    lines.push_back(linenum);
+
+    std::vector<Breakpoint> breakpoints;
+    IfFailRet(m_debugger->SetBreakpoints(filename, lines, breakpoints));
+
+    breakpoint = breakpoints.back();
+    breakpointsInSource.insert(std::make_pair(breakpoint.id, linenum));
+
+    return S_OK;
+}
+
+void MIProtocol::DeleteBreakpoints(const std::unordered_set<uint32_t> &ids)
+{
+    for (auto &breakpointsIter : m_breakpoints)
+    {
+        std::vector<int> remainingLines;
+        for (auto it : breakpointsIter.second)
+        {
+            if (ids.find(it.first) == ids.end())
+                remainingLines.push_back(it.second);
+        }
+        if (remainingLines.size() == breakpointsIter.second.size())
+            continue;
+
+        std::string filename = breakpointsIter.first;
+
+        std::vector<Breakpoint> tmpBreakpoints;
+        m_debugger->SetBreakpoints(filename, remainingLines, tmpBreakpoints);
+    }
+}
+
 void MIProtocol::EmitStoppedEvent(StoppedEvent event)
 {
     HRESULT Status;
@@ -599,7 +635,7 @@ HRESULT MIProtocol::HandleCommand(std::string command,
         ULONG32 id;
         Breakpoint breakpoint;
         if (ParseBreakpoint(args, filename, linenum)
-            && SUCCEEDED(m_debugger->SetBreakpoint(filename, linenum, breakpoint)))
+            && SUCCEEDED(SetBreakpoint(filename, linenum, breakpoint)))
         {
             PrintBreakpoint(breakpoint, output);
             return S_OK;
@@ -608,14 +644,16 @@ HRESULT MIProtocol::HandleCommand(std::string command,
         output = "Unknown breakpoint location format";
         return E_FAIL;
     } },
-    { "break-delete", [](const std::vector<std::string> &args, std::string &) -> HRESULT {
+    { "break-delete", [this](const std::vector<std::string> &args, std::string &) -> HRESULT {
+        std::unordered_set<uint32_t> ids;
         for (const std::string &idStr : args)
         {
             bool ok;
             int id = ParseInt(idStr, ok);
             if (ok)
-                DeleteBreakpoint(id);
+                ids.insert(id);
         }
+        DeleteBreakpoints(ids);
         return S_OK;
     } },
     { "exec-step", [this](const std::vector<std::string> &args, std::string &output) -> HRESULT {
@@ -786,7 +824,7 @@ HRESULT MIProtocol::HandleCommand(std::string command,
     { "interpreter-exec", [this](const std::vector<std::string> &args, std::string &output) -> HRESULT {
         return S_OK;
     }},
-    { "break-exception-insert", [](const std::vector<std::string> &args, std::string &output) -> HRESULT {
+    { "break-exception-insert", [this](const std::vector<std::string> &args, std::string &output) -> HRESULT {
         if (args.empty())
             return E_FAIL;
         size_t i = 1;
@@ -799,7 +837,7 @@ HRESULT MIProtocol::HandleCommand(std::string command,
         for (; i < args.size(); i++)
         {
             Breakpoint b;
-            InsertExceptionBreakpoint(args.at(i), b);
+            m_debugger->InsertExceptionBreakpoint(args.at(i), b);
             ss << sep;
             sep = ",";
             ss << "{number=\"" << b.id << "\"}";
diff --git a/src/debug/netcoredbg/debugger.h b/src/debug/netcoredbg/debugger.h
index 7e1bd84..a0d031d 100644
--- a/src/debug/netcoredbg/debugger.h
+++ b/src/debug/netcoredbg/debugger.h
@@ -4,6 +4,9 @@
 
 #include "protocol.h"
 #include <unordered_map>
+#include <unordered_set>
+#include <vector>
+#include <condition_variable>
 
 class ManagedCallback;
 class Protocol;
@@ -84,6 +87,38 @@ private:
 
     void AddVariableReference(Variable &variable, uint64_t frameId, ICorDebugValue *value, ValueKind valueKind);
 
+    struct ManagedBreakpoint {
+        uint32_t id;
+        CORDB_ADDRESS modAddress;
+        mdMethodDef methodToken;
+        ULONG32 ilOffset;
+        std::string fullname;
+        int linenum;
+        ToRelease<ICorDebugBreakpoint> breakpoint;
+        bool enabled;
+        ULONG32 times;
+
+        bool IsResolved() const { return modAddress != 0; }
+
+        ManagedBreakpoint();
+        ~ManagedBreakpoint();
+
+        void ToBreakpoint(Breakpoint &breakpoint);
+
+        ManagedBreakpoint(ManagedBreakpoint &&that) = default;
+    private:
+        ManagedBreakpoint(const ManagedBreakpoint &that) = delete;
+    };
+
+    uint32_t m_nextBreakpointId;
+    std::mutex m_breakpointsMutex;
+    std::unordered_map<std::string, std::unordered_map<int, ManagedBreakpoint> > m_breakpoints;
+    HRESULT HitBreakpoint(ICorDebugThread *pThread, Breakpoint &breakpoint);
+    void DeleteAllBreakpoints();
+
+    HRESULT ResolveBreakpointInModule(ICorDebugModule *pModule, ManagedBreakpoint &bp);
+    HRESULT Debugger::ResolveBreakpoint(ManagedBreakpoint &bp);
+
     HRESULT CheckNoProcess();
 
     static VOID StartupCallback(IUnknown *pCordb, PVOID parameter, HRESULT hr);
@@ -143,7 +178,8 @@ public:
     HRESULT Continue();
     HRESULT Pause();
     HRESULT GetThreads(std::vector<Thread> &threads);
-    HRESULT SetBreakpoint(std::string filename, int linenum, Breakpoint &breakpoint);
+    HRESULT SetBreakpoints(std::string filename, const std::vector<int> &lines, std::vector<Breakpoint> &breakpoints);
+    void InsertExceptionBreakpoint(const std::string &name, Breakpoint &breakpoint);
     HRESULT GetStackTrace(int threadId, int lowFrame, int highFrame, std::vector<StackFrame> &stackFrames);
     HRESULT StepCommand(int threadId, StepType stepType);
     HRESULT GetScopes(uint64_t frameId, std::vector<Scope> &scopes);
@@ -176,6 +212,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;
 public:
     void SetDebugger(Debugger *debugger) { m_debugger = debugger; m_debugger->SetProtocol(this); }
     static std::string EscapeMIValue(const std::string &str);
@@ -206,6 +243,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);
+    void DeleteBreakpoints(const std::unordered_set<uint32_t> &ids);
 };
 
 HRESULT DisableAllSteppers(ICorDebugProcess *pProcess);
diff --git a/src/debug/netcoredbg/main.cpp b/src/debug/netcoredbg/main.cpp
index 44b5029..676ba38 100644
--- a/src/debug/netcoredbg/main.cpp
+++ b/src/debug/netcoredbg/main.cpp
@@ -20,8 +20,6 @@
 #include "debugger.h"
 #include "modules.h"
 #include "valuewalk.h"
-#include "breakpoints.h"
-#include "valuewalk.h"
 #include "frames.h"
 
 #define __in
@@ -188,9 +186,6 @@ static HRESULT DisableAllBreakpointsAndSteppersInAppDomain(ICorDebugAppDomain *p
         }
     }
 
-    // FIXME: Delete all or Release all?
-    DeleteAllBreakpoints();
-
     DisableAllSteppersInAppDomain(pAppDomain);
 
     return S_OK;
@@ -341,7 +336,7 @@ public:
             pThread->GetID(&threadId);
 
             StoppedEvent event(StopBreakpoint, threadId);
-            HitBreakpoint(pThread, event.breakpoint);
+            m_debugger->HitBreakpoint(pThread, event.breakpoint);
 
             ToRelease<ICorDebugFrame> pFrame;
             if (SUCCEEDED(pThread->GetActiveFrame(&pFrame)) && pFrame != nullptr)
@@ -685,7 +680,8 @@ Debugger::Debugger() :
         m_startupResult(S_OK),
         m_unregisterToken(nullptr),
         m_processId(0),
-        m_nextVariableReference(1)
+        m_nextVariableReference(1),
+        m_nextBreakpointId(1)
 {
     m_managedCallback->m_debugger = this;
 }
@@ -905,6 +901,7 @@ HRESULT Debugger::DetachFromProcess()
 
     if (SUCCEEDED(m_pProcess->Stop(0)))
     {
+        DeleteAllBreakpoints();
         DisableAllBreakpointsAndSteppers(m_pProcess);
         m_pProcess->Detach();
     }
diff --git a/src/debug/netcoredbg/protocol.h b/src/debug/netcoredbg/protocol.h
index 653031e..50c358e 100644
--- a/src/debug/netcoredbg/protocol.h
+++ b/src/debug/netcoredbg/protocol.h
@@ -6,6 +6,7 @@
 
 #include "platform.h"
 #include <string>
+#include <vector>
 
 // From https://github.com/Microsoft/vscode-debugadapter-node/blob/master/protocol/src/debugProtocol.ts