All Projects FPC fpcprojects fpprofilerfpGUI Lazarus Packages PatchesLazarus CCR Mantis pas2js

View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0038539LazarusDebuggerpublic2021-02-23 21:182021-02-27 22:37
ReporterChristo Crause Assigned ToMartin Friebe  
PrioritynormalSeverityminorReproducibilityN/A
Status assignedResolutionopen 
Product Version2.1 (SVN) 
Summary0038539: [Feature] LazDebuggerFP - track changes in registers
DescriptionLazDebuggerFp doesn't currently flag changed registers for the register view dialog to mark. Attached patch uses a simple approach to check for changed register value and set Modified if the value changed.
TagsNo tags attached.
Fixed in Revision
LazTarget-
Widgetset
Attached Files

Activities

Christo Crause

2021-02-23 21:18

reporter  

register-modified.patch (1,106 bytes)    
diff --git a/components/lazdebuggers/lazdebuggerfp/fpdebugdebugger.pas b/components/lazdebuggers/lazdebuggerfp/fpdebugdebugger.pas
index 5979336421..6505970eaf 100644
--- a/components/lazdebuggers/lazdebuggerfp/fpdebugdebugger.pas
+++ b/components/lazdebuggers/lazdebuggerfp/fpdebugdebugger.pas
@@ -1907,6 +1907,7 @@ var
   ARegisterValue: TRegisterValue;
   thr: TDbgThread;
   frm: TDbgCallstackEntry;
+  tmp: string;
 begin
   if not TFpDebugDebugger(Debugger).IsPausedAndValid then begin
     ARegisters.DataValidity:=ddsInvalid;
@@ -1936,9 +1937,14 @@ begin
   for i := 0 to ARegisterList.Count-1 do
     begin
     ARegisterValue := ARegisters.EntriesByName[ARegisterList[i].Name];
+    tmp := ARegisterValue.Value;
     ARegisterValue.ValueObj.SetAsNum(ARegisterList[i].NumValue, ARegisterList[i].Size);
     ARegisterValue.ValueObj.SetAsText(ARegisterList[i].StrValue);
     ARegisterValue.DataValidity:=ddsValid;
+    if tmp = ARegisterValue.Value then
+      ARegisterValue.Modified := False
+    else
+      ARegisterValue.Modified := True;
     end;
   ARegisters.DataValidity:=ddsValid;
 end;
register-modified.patch (1,106 bytes)   

Martin Friebe

2021-02-27 21:14

manager   ~0129213

Unfortunately the patch does not work.

- Debugger is paused and Some registers are marked at modified
- Users toggles a register between "Hex" and "raw"
=> modified flags for other registers are cleared.

Additionally in the above example, it can happen that the register that had its display format changed, will be marked as modified (even if it was not)

All that, without even running/stepping the debuggee.

----------------
It as also not a good idea to use the old values for this (they are marked as ddsUnknown).

This is the same state a register has, the very first time the app pauses, and all registers are inspected for the first time.
Currently they get all marked modified. But that is pure luck, on how the default display format acts on those registers. (I guess if flags was actually empty, it might not be marked, but have not tested)

Basing the decision on the old IDE-values also means that when and if (and yes that is a very big "if") the DebugIntf design for registers is changed this will break.

Further more, if the register window is closed, values are not updated, if it is opened again (maybe while stepping over a long running procedure) then the change indicator compares with what was shown when the window was last open, rather than to compare with what was when the last time the app was in pause. (having to ignore internal-pause / though on that, gdb-based may also not be exact...)

Christo Crause

2021-02-27 22:07

reporter   ~0129215

Martin, valid points. It seems to me that the biggest problem is to have a copy of the last valid state of the register values for the current thread to use for testing changed register values. In fpdebugdebugger the only historical information currently available is the ARegisters parameter passed to RequestData (and this is text based, not value based) as far as I can see.

There is currently a simple register cache in TDbgAvrThread, perhaps this can be extended for this purpose and moved up to TDbgThread? I can flesh out the idea if it sounds OK.

Martin Friebe

2021-02-27 22:34

manager   ~0129216

Last edited: 2021-02-27 22:37

View 3 revisions

 I would definitely think the fpdebug backend must keep the info on the last state. (or make it avail as on object, that then can be cached outside)
Because registers can be numbers, or flags, or.... so there needs to be competent code to compare it. That would be CPU specific code in fpdebug.

However fpdebug may stop/pause for many reasons, so the LazDebuggerXXX must trigger the capture. I.e. it must say fpDebug -> update the cache.
(fpdebug has to be clever enough to still know the modified state after that call, i.e. set a flag, and do the update before enter run state again)

---
IMHO state can be kept per thread / top level stack only.

State must be kept for any thread, for which the laz-debugger reads the registers (or at least reads the current modified state).
State can be kept for other threads if avail at no extra cost.

Issue History

Date Modified Username Field Change
2021-02-23 21:18 Christo Crause New Issue
2021-02-23 21:18 Christo Crause Status new => assigned
2021-02-23 21:18 Christo Crause Assigned To => Martin Friebe
2021-02-23 21:18 Christo Crause File Added: register-modified.patch
2021-02-27 21:14 Martin Friebe Status assigned => feedback
2021-02-27 21:14 Martin Friebe LazTarget => -
2021-02-27 21:14 Martin Friebe Note Added: 0129213
2021-02-27 22:07 Christo Crause Note Added: 0129215
2021-02-27 22:07 Christo Crause Status feedback => assigned
2021-02-27 22:34 Martin Friebe Note Added: 0129216
2021-02-27 22:37 Martin Friebe Note Edited: 0129216 View Revisions
2021-02-27 22:37 Martin Friebe Note Edited: 0129216 View Revisions