Feature Request/Suggestion in Memory Viewer

[440bx]

Hello,

Every time an instruction is stepped, the memory viewer goes back to the beginning of the memory block.

This is inconvenient because, one of the uses of the memory viewer is to watch how multiple writes to a memory block have accumulated.  The fact that the view scrolls back to the beginning forces having to scroll down to the previous location (which the programmer has to remember) to see what was written into memory every time the changes need to be inspected.

That's the first suggestion.

The second suggestion is, allow more than one memory viewer to be active.  This can be very useful when a destination block of memory is a composite made of the bytes in the source block plus some bytes.  e.g, inserting a block of characters somewhere in a line done by creating a new block that consists of the original line plus the block of characters inserted at a particular location.

While I'm at it...

It would be very nice if the "memory dump" view in the Watches window showed a typical memory dump (like the memory viewer does.)

Thank you for reading.

[Martin_fr]

The scrolling I had not paid attention too.

Multiple windows, are a thought. There is some old code that limits that...

Proper mem dump in the watches
1) You can use the "detail pane" and it shows multiline with 16 bytes. But its always byte based and hex, none of the options that you have in mem-viewer.

2) The bigger part, showing the mem dump multiline in the tree.
That is actually something that applies to all watches that have data which is cut off. One day the watches window should support variable height entries (not be expand/collapse, but for a single "row" that wraps line). But that again is likely still some time in the future.

[Martin_fr]

Code: Text  [Select][+][-]

1. ^byte( :flatten(   TObject(self) ,  ( TObject(^byte(:_)+16 ))      , [max=4] )  [0..4] ) [0..15]

This should give you a dump like experience (set displayformat to hex or as you like it). Set the digit count (display format) for each element to ensure alignment.

You can also use ^word and then 0..7

The details are a bit more complex.

:flatten needs a structure with an address in memory (well, I need to double check the exact requirements)
https://wiki.lazarus.freepascal.org/FpDebug-Watches-Intrinsic-Functions#Flatten/F_(LinkedList_or_Tree,_FieldName,_FieldName,_[opts])_{Display_as_array}


So then
- you are starting with the "pointer to address" (here the internal pointer in self), and cast it to an object.
- 2nd argument to flatten is the next element in the computed flat-list: an object with an address 16 bytes higher
- then how many addresses (disguised as TObject) to create: max=4

Outside of flatten.
- Convert to MAP-able array https://wiki.lazarus.freepascal.org/FpDebug-Watches-Intrinsic-Functions#Array_slice_MyArray[n..m]_with_mapping
- map each TObject into a ^byte
- for each: return a slice of 0..15 bytes

[Martin_fr]

about the scroll, please try

Code: Diff  [Select][+][-]

1. diff --git a/ide/packages/idedebugger/memviewerdlg.pas b/ide/packages/idedebugger/memviewerdlg.pas
2. index 8056e7ca5b..aaa4f85639 100644
3. --- a/ide/packages/idedebugger/memviewerdlg.pas
4. +++ b/ide/packages/idedebugger/memviewerdlg.pas
5. @@ -65,6 +65,8 @@ type
6.      FUpdatingAddr: boolean;
7.      FWatches: TCurrentWatches;
8.      FCurrentWatchValue: TIdeWatchValue;
9. +    FCurrentDisplayedWatchExpr: String;
10. +    FCurrentDisplayedWatchMemAddr: TDBGPtr;
11.      FCurrentWatchMode: (wmNone, wmInit, wmInitAddress, wmBefore, wmAfter);
12.      FTargetWidth: Integer;
13.      FCurrentMemAddress: TDBGPtr;     // Address for the actual known data i FCurrentMemData
14. @@ -301,11 +303,19 @@ const
15.  var
16.    s: String;
17.    CurLnIdx, CurLineChrIdx, mpos, mlen, b, line: Integer;
18. -  grp, g: Integer;
19. +  grp, g, CurTopLine: Integer;
20. +  CurCaret, CurSelB, CurSelE: TPoint;
21.  begin
22.    if FCurrentMemData = '' then
23.      exit;
24.  
25. +  if (edAddressBase.Text = FCurrentDisplayedWatchExpr) and
26. +     (FCurrentDisplayedWatchMemAddr = FCurrentMemAddress)
27. +  then
28. +    AKeepLines := True;
29. +  FCurrentDisplayedWatchExpr := edAddressBase.Text;
30. +  FCurrentDisplayedWatchMemAddr := FCurrentMemAddress;
31. +
32.    edMemViewer.BeginUpdate;
33.    if not AKeepLines then
34.      edMemViewer.Lines.Clear;
35.  

[440bx]

Good stuff... thank you Martin!

Please refer to the attachment.
 
The attachment has the "details" pane open for the null terminated string LpiOut, which is in the Watches as "@LpiOut[0]".  This gives a very usable view of the memory used by LpiOut.

The problem is that, just like the memory viewer, just about everything makes it scroll back to the top which means constantly having to scroll back to the bottom to see what has been added to it.

One thing in particular that would be very useful is if the view would not go back to the top when switching from one window to another.  In the screenshot, when switching from the memory viewer back to the Watches window, the display pane is back at the top.   Basically, it would be nice if the details pane did not change unless the user specifically changed it (IOW, it should remember its position and stay there.)

Essentially, the "Details" pane is functioning a bit like another memory viewer (albeit without the hex dump part but, that's not an issue in this case.)

Out of curiosity, would it be reasonably easy to enable having multiple "Watches" window ?  (using that and the "Details" pane would "emulate" multiple memory viewers.)

Lastly, I tried the Flatten you suggested but, I couldn't get it right.  Could you please post how you'd apply it to the LpiOut variable ? (Note: it's a pchar), thank you!

[Martin_fr]

Is that an ansi or shortstring:

If it is an ansistring (or pchar), then it should work by just putting it in place of "self". I just tried that, with ansistring.

Of course byte and TObject must be used in the app, so the debugger can find their definitions, or it will say "identifier byte not found".

For shortstring
 TObject( @short[1] )


What output did you get?


Have you tested the patch?

I will check the scroll of the detail pane / Might be towards end of the week....


Problem with multiple watches is, that they are mapped to an enum / Hold in an array, with a fixed spot for each... That is 20years old code...

Eventually I will have to deal with that...


Always best to report stuff like the scrolling.

I simply have never noted. I usually use other tricks to see parts of the data that I want.

Like instead of scrolling into MyPChar, I use ^char(@MyPchar[500]) 
Or maybe it was by using addition. Not sure right now.

And nowadays the array slices [500..600]

So usually I don't have to scroll, and I wasn't aware of the issue.

[440bx]

Life got in the way... I'll be back  in a couple of hours and give you an update.

[440bx]

Is that an ansi or shortstring:

If it is an ansistring (or pchar), then it should work by just putting it in place of "self". I just tried that, with ansistring.

Of course byte and TObject must be used in the app, so the debugger can find their definitions, or it will say "identifier byte not found".

It's a pchar.  I got the "identifier not found" because there is no TObject in my code.

Have you tested the patch?

I haven't yet. I'm in the middle of some code and I don't want to lose track of what I'm doing.  I'll do the patch "imminently".

I will check the scroll of the detail pane / Might be towards end of the week....

That would be great.  Thank you.

Problem with multiple watches is, that they are mapped to an enum / Hold in an array, with a fixed spot for each... That is 20years old code...

I suspected it would not be straightforward to have multiple Watch windows.

I simply have never noted. I usually use other tricks to see parts of the data that I want.

Like instead of scrolling into MyPChar, I use ^char(@MyPchar[500]) 
Or maybe it was by using addition. Not sure right now.

And nowadays the array slices [500..600]

I should have thought about that.  I changed the @LpiOut[0] to @LpiOut[LpiOutCount - 16] and that stays put.   Very convenient.  However, when switching windows and coming back to the Watches window by clicking in some empty space in the Watches window causes the watch item that was previously selected to be deselected.  It would be nice if when clicking on empty space in the Watch window, the last selected item would remain selected/active (the same behavior as if the caption had been clicked.)  IOW, clicking on empty space would behave the same as clicking on the caption (the selected item in the Watch window is _not_ deselected.)

So usually I don't have to scroll, and I wasn't aware of the issue.

No problem and, for the program I'm currently debugging your method of using array slices works very well therefore, I'm switching to that.

[Martin_fr]

I also just realized, stopping the scrolling of the detail pane requires some more thought...

The first part can be worked around:  While stepping the watch changes to "invalid" and "evaluating" both of which don't need a scrollbar. But the position can of course be cached.
And off topic, yes for stepping (if the step dose not last for longer times, like when stepping over a procedure that may take longer) then those updates may one day be skipped. But that wouldn't affect the issue, as steps may always take longer, and then the cache is still needed.

But the actual issue is that "not scrolling" depends on the value being "the same". E.g. if you have a watch "foo" and it's a very long string, then keep the scroll pos. But no you step in/out, and "foo" becomes a record/object. It's still long enough to scroll. But the old scroll pos is no longer relevant. That can even happen between 2 strings...

For the mem viewer, this is not an issue. It checks that
- the address remained the same
- the expression remained the same

If you step out, and the same expression still gets the same address => fine.

But for watches, an ansistring may change address. Other data displayed may not have an address at all (result of a calculation).
So that can't be used.

Ideas?

[Martin_fr]

On the multi watch window...

There are more considerations... They aren't technically mandated and also I haven't checked them all, or how much real impact they have.

The debugger used to be one big "all in one" piece of code. Backend, Frontend, IDE, ... all just one piece.

I started separating the backends. And the frontend also moved to a package. But the frontend is still tightly bound to the IDE. To a big extend that is ok, but still there should be a clearer cut. IMHO.

Multiple Windows will affect the IDE menu structure. And if I dive into that now, then I end up having to do that twice.
Because those menus shouldn't be hardcoded, they should get registered by the package.

So at the moment this is something I am pushing to a later date. As there are other bits and pieces that may be addressed before looking at the integration issues.

I may come to a point were I decide those changes shouldn't wait on each other. But currently not at that point.


Also, as for watches the windows, having more than one, requires the underlaying storage of the watches to be changed. And that also affects the history window.

Currently each watches window would show the same list. Which could be of interest, if they showed different stack frames, or difff history selection. But that too requires changes to the underlying data....

The memory window doesn't have the underlying data issue... It is just bound by being part of the entire eco system of debug windows.

[Martin_fr]

Btw, have you looked at the CPU view package? https://forum.lazarus.freepascal.org/index.php/topic,68962.0.html

That also has a mem viewer. So that would give you a second mem view window.

[440bx]

Ideas?

What comes to mind is to take the "simple route" which would be, if it is _easily_ possible to determine that the scroll position can be kept then keep it as it was.  If anything changes that makes it cumbersome or difficult to keep the scroll position then set it back at the top.

I think that in most cases such a simple approach will work well in most situations.  Basically, the idea is to make the feature dependent "on a best effort".

If I think of something "better" that would work in "all" cases, I'll definitely share.


The multiple "Watches" windows would be a "nice to have" feature but, it is not often that I've felt the need for it.  IOW, it is a feature that might be worth looking into again once the current code has gone through modularization with time and its implementation no longer requires a lot of significant changes in a lot of places.

I'd say that having more than one memory viewer window should definitely take precedence.  That's a feature I need very often and, I've been using Process Hacker when I need additional views into memory.


The CPU view package is something I think of quite often because of my wanting to step into assembly code.  I haven't looked at it yet.  It's high on my "todo" list.

[Martin_fr]

Any updates on the patch? Does it work as you would expect?

[440bx]

Any updates on the patch? Does it work as you would expect?

I'm sorry, I haven't applied the patch yet.  Part of the reason is that I have to apply it manually because I'm not setup to use git.  Another part of the reason is that your suggestion of using array slices took care of the problem and yet, another part is that I'm in the middle of updating some code and I really don't want to lose track of what I'm doing.

I will test the patch.  It might be in another 3 to 5 days.  I apologize for the delay.