fix software breakpoint handling issues

GleeBug/Debugger.Loop.Exception.cpp

@@ -6,7 +6,8 @@ namespace GleeBug
     void Debugger::exceptionBreakpoint(const EXCEPTION_RECORD & exceptionRecord, const bool firstChance)
     {
         //check if the breakpoint exists
-        auto foundInfo = mProcess->breakpoints.find({ BreakpointType::Software, ptr(exceptionRecord.ExceptionAddress) });
+        auto exceptionAddress = ptr(exceptionRecord.ExceptionAddress);
+        auto foundInfo = mProcess->breakpoints.find({ BreakpointType::Software, exceptionAddress });
         if(foundInfo == mProcess->breakpoints.end())
         {
             if(!this->mAttachedToProcess && !mProcess->systemBreakpoint) //handle system breakpoint
@@ -18,6 +19,20 @@ namespace GleeBug
                 //call the callback
                 cbSystemBreakpoint();
             }
+            else
+            {
+                //check if this was a deleted breakpoint
+                //if the byte at the exception address is not 0xCC, our breakpoint was deleted
+                //and we should set IP back and continue execution
+                uint8 currentByte = 0xCC;
+                if(mThread && mProcess->MemReadUnsafe(exceptionAddress, &currentByte, 1) && currentByte != 0xCC)
+                {
+                    //this was our deleted breakpoint, set IP back and continue
+                    Registers(mThread->hThread, CONTEXT_CONTROL).Gip = exceptionAddress;
+                    mContinueStatus = DBG_CONTINUE;
+                }
+                //else: byte is 0xCC, this is a real int3 in original code, let debuggee handle it
+            }
             return;
         }
 
@@ -30,12 +45,26 @@ namespace GleeBug
         Registers(mThread->hThread, CONTEXT_CONTROL).Gip = info.address;
 
         //restore the original breakpoint byte and do an internal step
-        mProcess->MemWriteUnsafe(info.address, info.internal.software.oldbytes, info.internal.software.size);
+        if(!mProcess->MemWriteUnsafe(info.address, info.internal.software.oldbytes, info.internal.software.size))
+        {
+            //failed to restore original byte, pass exception to debuggee
+            mContinueStatus = DBG_EXCEPTION_NOT_HANDLED;
+            return;
+        }
         mProcess->StepInternal([this, info]()
         {
             //only restore the bytes if the breakpoint still exists
-            if(mProcess->breakpoints.find({ BreakpointType::Software, info.address }) != mProcess->breakpoints.end())
-                mProcess->MemWriteUnsafe(info.address, info.internal.software.newbytes, info.internal.software.size);
+            auto foundBreakpoint = mProcess->breakpoints.find({ BreakpointType::Software, info.address });
+            if(foundBreakpoint != mProcess->breakpoints.end())
+            {
+                if(!mProcess->MemWriteUnsafe(info.address, info.internal.software.newbytes, info.internal.software.size))
+                {
+                    //failed to restore breakpoint byte, remove from maps to stay consistent
+                    mProcess->softwareBreakpointReferences.erase(info.address);
+                    mProcess->breakpoints.erase(foundBreakpoint);
+                    mProcess->breakpointCallbacks.erase({ BreakpointType::Software, info.address });
+                }
+            }
         });
 
         //call the generic callback

GleeBug/Debugger.Process.Memory.cpp

@@ -65,8 +65,58 @@ namespace GleeBug
 
     bool Process::MemWriteSafe(ptr address, const void* buffer, ptr size, ptr* bytesWritten)
     {
-        //TODO: correctly implement this
-        return MemWrite(address, buffer, size, bytesWritten, false);
+        if(size == 0)
+        {
+            if(bytesWritten)
+                *bytesWritten = 0;
+            return true;
+        }
+
+        std::vector<uint8> copy((const uint8*)buffer, (const uint8*)buffer + size);
+
+        auto start = address;
+        auto end = start + size;
+
+        //find overlapping software breakpoints and preserve their 0xCC bytes in the copy (so write doesn't remove breakpoints)
+        //as well as track what oldbytes values need updating after successful write
+        std::vector<std::tuple<uint8*, uint8, ptr>> pendingUpdates; //tuple: (pointer to oldbyte, new value, offset from start for partial write handling)
+
+        for(auto & breakpoint : breakpoints)
+        {
+            if(breakpoint.first.first != BreakpointType::Software)
+                continue;
+            auto & info = breakpoint.second;
+            auto curAddress = info.address;
+            for(ptr j = 0; j < info.internal.software.size; j++)
+            {
+                if(curAddress + j >= start && curAddress + j < end)
+                {
+                    auto offset = curAddress + j - start;
+                    pendingUpdates.emplace_back(&info.internal.software.oldbytes[j], copy[offset], offset);
+                    copy[offset] = info.internal.software.newbytes[j];
+                }
+            }
+        }
+
+        //write to memory (breakpoint bytes are preserved in the copy)
+        ptr written = 0;
+        if(!MemWriteUnsafe(address, copy.data(), size, &written))
+        {
+            if(bytesWritten)
+                *bytesWritten = written;
+            return false;
+        }
+
+        //apply oldbytes updates only for bytes that were actually written
+        for(const auto & update : pendingUpdates)
+        {
+            if(std::get<2>(update) < written)
+                *std::get<0>(update) = std::get<1>(update);
+        }
+
+        if(bytesWritten)
+            *bytesWritten = written;
+        return true;
     }
 
     bool Process::MemIsValidPtr(ptr address) const