Address PR issues

Author: WardenGnaw

src/MIDebugEngine/AD7.Impl/AD7BoundBreakpoint.cs

@@ -5,6 +5,7 @@
 using Microsoft.VisualStudio.Debugger.Interop;
 using System;
 using System.Diagnostics;
+using System.Threading.Tasks;
 
 namespace Microsoft.MIDebugEngine
 {
@@ -162,15 +163,18 @@ int IDebugBoundBreakpoint2.SetCondition(BP_CONDITION bpCondition)
         int IDebugBoundBreakpoint2.SetHitCount(uint dwHitCount)
         {
             _bp.SetHitCount(dwHitCount);
+            _pendingBreakpoint?.RecomputeBreakAfter(dwHitCount);
+
             return Constants.S_OK;
         }
 
         /// <summary>
-        /// Syncs the hit count from GDB's "times" field in =breakpoint-modified events.
+        /// Syncs the hit count from GDB's "times" field using a delta
+        /// to preserve any user-initiated hit count reset.
         /// </summary>
         internal void SetHitCount(uint hitCount)
         {
-            _bp.SetHitCount(hitCount);
+            _bp.SetGdbHitCount(hitCount);
         }
 
         // This is used to specify the breakpoint hit count condition.
@@ -183,6 +187,8 @@ int IDebugBoundBreakpoint2.SetPassCount(BP_PASSCOUNT bpPassCount)
 
         #endregion
 
+        internal uint HitCount => _bp.HitCount;
+
         internal void IncrementHitCount()
         {
             _bp.IncrementHitCount();
@@ -211,6 +217,33 @@ internal bool ShouldBreak()
             }
         }
 
+        /// <summary>
+        /// Re-sends -break-after to GDB after a pass count breakpoint fires.
+        /// MOD: skips passCount-1 hits. EQUAL: clears the ignore count.
+        /// </summary>
+        internal async Task RearmBreakAfterAsync()
+        {
+            uint ignoreCount;
+            switch (_passCountStyle)
+            {
+                case enum_BP_PASSCOUNT_STYLE.BP_PASSCOUNT_MOD:
+                    if (_passCountValue == 0) return;
+                    ignoreCount = _passCountValue - 1;
+                    break;
+                case enum_BP_PASSCOUNT_STYLE.BP_PASSCOUNT_EQUAL:
+                    ignoreCount = 0;
+                    break;
+                default:
+                    return;
+            }
+
+            PendingBreakpoint bp = _pendingBreakpoint?.PendingBreakpoint;
+            if (bp != null && _engine?.DebuggedProcess != null)
+            {
+                await bp.SetBreakAfterAsync(ignoreCount, _engine.DebuggedProcess);
+            }
+        }
+
         internal void UpdateAddr(ulong addr)
         {
             _bp.Addr = addr;

src/MIDebugEngine/AD7.Impl/AD7PendingBreakpoint.cs

@@ -697,23 +697,27 @@ int IDebugPendingBreakpoint2.SetPassCount(BP_PASSCOUNT bpPassCount)
                 }
             }
 
-            // Re-send -break-after to GDB with the updated ignore count, accounting
-            // for the current hit count so the breakpoint fires at the right time.
-            if (bp != null && bpPassCount.stylePassCount != enum_BP_PASSCOUNT_STYLE.BP_PASSCOUNT_NONE)
+            // When the pass count is cleared (NONE), send ignore count 0 to clear
+            // any stale GDB ignore count from the previous condition.
+            if (bp != null)
             {
-                uint currentHits = 0;
-                lock (_boundBreakpoints)
+                uint ignoreCount = 0;
+                if (bpPassCount.stylePassCount != enum_BP_PASSCOUNT_STYLE.BP_PASSCOUNT_NONE)
                 {
-                    foreach (AD7BoundBreakpoint boundBp in _boundBreakpoints)
+                    uint currentHits = 0;
+                    lock (_boundBreakpoints)
                     {
-                        uint hc;
-                        if (((IDebugBoundBreakpoint2)boundBp).GetHitCount(out hc) == Constants.S_OK && hc > currentHits)
+                        foreach (AD7BoundBreakpoint boundBp in _boundBreakpoints)
                         {
-                            currentHits = hc;
+                            uint hc;
+                            if (((IDebugBoundBreakpoint2)boundBp).GetHitCount(out hc) == Constants.S_OK && hc > currentHits)
+                            {
+                                currentHits = hc;
+                            }
                         }
                     }
+                    ignoreCount = ComputeIgnoreCount(bpPassCount.stylePassCount, bpPassCount.dwPassCount, currentHits);
                 }
-                uint ignoreCount = ComputeIgnoreCount(bpPassCount.stylePassCount, bpPassCount.dwPassCount, currentHits);
                 _engine.DebuggedProcess.WorkerThread.RunOperation(() =>
                 {
                     _engine.DebuggedProcess.AddInternalBreakAction(
@@ -724,6 +728,28 @@ int IDebugPendingBreakpoint2.SetPassCount(BP_PASSCOUNT bpPassCount)
             return Constants.S_OK;
         }
 
+        /// <summary>
+        /// Re-sends -break-after to GDB after a hit count reset.
+        /// </summary>
+        internal void RecomputeBreakAfter(uint currentHits)
+        {
+            if (_bp == null
+                || (_bpRequestInfo.dwFields & enum_BPREQI_FIELDS.BPREQI_PASSCOUNT) == 0
+                || _bpRequestInfo.bpPassCount.stylePassCount == enum_BP_PASSCOUNT_STYLE.BP_PASSCOUNT_NONE)
+            {
+                return;
+            }
+
+            PendingBreakpoint bp = _bp;
+            uint ignoreCount = ComputeIgnoreCount(_bpRequestInfo.bpPassCount.stylePassCount, _bpRequestInfo.bpPassCount.dwPassCount, currentHits);
+            _engine.DebuggedProcess.WorkerThread.RunOperation(() =>
+            {
+                _engine.DebuggedProcess.AddInternalBreakAction(
+                    () => bp.SetBreakAfterAsync(ignoreCount, _engine.DebuggedProcess)
+                );
+            });
+        }
+
         // Toggles the virtualized state of this pending breakpoint. When a pending breakpoint is virtualized, 
         // the debug engine will attempt to bind it every time new code loads into the program.
         // The sample engine will does not support this.

src/MIDebugEngine/Engine.Impl/BreakpointManager.cs

@@ -80,7 +80,16 @@ public async Task BreakpointModified(object sender, EventArgs args)
                 {
                     if (boundBp.HasPassCount)
                     {
+                        uint previousHitCount = boundBp.HitCount;
                         boundBp.SetHitCount(times);
+
+                        // Re-arm GDB's ignore count so it skips to the next target hit.
+                        // Guard on hit count change to avoid looping: -break-after
+                        // itself triggers =breakpoint-modified.
+                        if (boundBp.HitCount != previousHitCount)
+                        {
+                            await boundBp.RearmBreakAfterAsync();
+                        }
                     }
                 }
             }

src/MIDebugEngine/Engine.Impl/Breakpoints.cs

@@ -375,6 +375,8 @@ internal class BoundBreakpoint
         /*OPTIONAL*/
         public string FunctionName { get; private set; }
         internal uint HitCount { get; private set; }
+        private uint _rawGdbHitCount;
+        private readonly object _hitCountLock = new object();
         internal bool Enabled { get; set; }
         internal bool IsDataBreakpoint { get { return _parent.AD7breakpoint.IsDataBreakpoint; } }
         private MITextPosition _textPosition;
@@ -450,12 +452,34 @@ internal uint Line
 
         internal void IncrementHitCount()
         {
-            HitCount++;
+            lock (_hitCountLock)
+            {
+                HitCount++;
+            }
         }
 
         internal void SetHitCount(uint count)
         {
-            HitCount = count;
+            lock (_hitCountLock)
+            {
+                HitCount = count;
+            }
+        }
+
+        /// <summary>
+        /// Applies the delta from GDB's "times" field, preserving user-initiated resets.
+        /// </summary>
+        internal void SetGdbHitCount(uint gdbTimes)
+        {
+            lock (_hitCountLock)
+            {
+                if (gdbTimes >= _rawGdbHitCount)
+                {
+                    uint delta = gdbTimes - _rawGdbHitCount;
+                    HitCount += delta;
+                }
+                _rawGdbHitCount = gdbTimes;
+            }
         }
     }
 }

src/OpenDebugAD7/AD7Impl/AD7BreakPointRequest.cs

@@ -197,10 +197,6 @@ public bool SetLogMessage(string logMessage)
 
         #region Hit Conditions
 
-        /// <summary>
-        /// Attempts to parse the hit condition string into a pass count style and value.
-        /// Returns true if HitCondition is null/empty (no condition) or if it is a valid hit condition.
-        /// </summary>
         /// <summary>
         /// Updates the hit condition string so that subsequent <see cref="TryParseHitCondition"/>
         /// calls return the new pass count, and future comparisons see the updated value.
@@ -210,6 +206,10 @@ internal void UpdateHitCondition(string hitCondition)
             HitCondition = hitCondition;
         }
 
+        /// <summary>
+        /// Attempts to parse the hit condition string into a pass count style and value.
+        /// Returns true if HitCondition is null/empty (no condition) or if it is a valid hit condition.
+        /// </summary>
         internal bool TryParseHitCondition(out enum_BP_PASSCOUNT_STYLE style, out uint passCount)
         {
             style = enum_BP_PASSCOUNT_STYLE.BP_PASSCOUNT_NONE;