Opened 8 years ago

Closed 7 years ago

#8286 closed enhancement (fixed)

Debugger should show the address of stack variables

Reported by: yourpalal Owned by: anevilyak
Priority: low Milestone: Unscheduled
Component: Applications/Debugger Version: R1/Development
Keywords: Cc: bonefish
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

It would be very handy if Debugger could show the address of stack variables so that when entering into a method call, you could identify the object being manipulated by comparing the 'this' pointer to the address of the local variables that might be having their methods called.

Attachments (4)

variable_address.patch (2.4 KB ) - added by anevilyak 7 years ago.
debugger.png (131.6 KB ) - added by anevilyak 7 years ago.
Representative screenshot with address column
debugger_tooltip.2.patch (7.7 KB ) - added by anevilyak 7 years ago.
Implements support for tooltips and showing addresses of compound address nodes as value.
debugger_tooltip.png (139.2 KB ) - added by anevilyak 7 years ago.
Representative screenshot of tooltip support.

Download all attachments as: .zip

Change History (15)

comment:1 by anevilyak, 7 years ago

Owner: changed from bonefish to anevilyak
Status: newassigned

comment:2 by anevilyak, 7 years ago

Has a Patch: set

comment:3 by anevilyak, 7 years ago

Cc: bonefish added
Status: assignedin-progress

Attached a patch which adds an address column to the variables view. Thoughts? Alternatives?

by anevilyak, 7 years ago

Attachment: variable_address.patch added

comment:4 by bonefish, 7 years ago

I haven't applied and tested the patch, but given how little space there is for the variables view, a second column might not be the best solution. Currently the "Value" column is empty for compound types. I would simply show the address there -- maybe in a slightly different way (e.g. prefixed with an "@", in parentheses, or with a different font style) to make clear that it isn't a pointer type. That would cover the use case mentioned in the description.

Aside from that, it would also be nice to have the tool tip for the variable show additional information like the value location (there's also more space should it include multiple registers (we'll see that with x86-64 I suppose)) and the original type, when the displayed value has been casted (another missing feature).

in reply to:  4 ; comment:5 by anevilyak, 7 years ago

Replying to bonefish:

I haven't applied and tested the patch, but given how little space there is for the variables view, a second column might not be the best solution. Currently the "Value" column is empty for compound types. I would simply show the address there -- maybe in a slightly different way (e.g. prefixed with an "@", in parentheses, or with a different font style) to make clear that it isn't a pointer type. That would cover the use case mentioned in the description.

That's certainly a possibility. The main reasons I went with a column were 1) that allows one to see the address for all variables, not just compounds, 2) it's persistently visible unlike a tooltip which makes it more convenient for comparisons like mentioned in the original feature request, and 3) the column listview allows one to hide columns one isn't interested in anyways, so one could simply hide it and revert back to the 2 column view instead if preferred (that would need to be persisted in settings though, which it isn't yet with this patch). If it makes things easier, I can attach a representative screenshot later tonight. Another possibility for space reasons would be to simply default to hiding the column.

Aside from that, it would also be nice to have the tool tip for the variable show additional information like the value location (there's also more space should it include multiple registers (we'll see that with x86-64 I suppose)) and the original type, when the displayed value has been casted (another missing feature).

Indeed, I can implement that as well. Casting is also on my TODO list but I need to do a bit more studying of the value subsystem to be able to implement that one properly.

by anevilyak, 7 years ago

Attachment: debugger.png added

Representative screenshot with address column

in reply to:  4 comment:6 by anevilyak, 7 years ago

Replying to bonefish:

I would simply show the address there -- maybe in a slightly different way (e.g. prefixed with an "@", in parentheses, or with a different font style) to make clear that it isn't a pointer type. That would cover the use case mentioned in the description.

Aside from that, it would also be nice to have the tool tip for the variable show additional information like the value location.

I've implemented these two with another patch, see debugger_tooltip.png. For displaying the node address it simply reuses AddressValue though so there's no visual differentiator there, but in context it's IMHO not really ambiguous what the address being represented is. Let me know if this one's more to people's liking and I'll commit.

by anevilyak, 7 years ago

Attachment: debugger_tooltip.2.patch added

Implements support for tooltips and showing addresses of compound address nodes as value.

by anevilyak, 7 years ago

Attachment: debugger_tooltip.png added

Representative screenshot of tooltip support.

in reply to:  5 ; comment:7 by bonefish, 7 years ago

Replying to anevilyak:

Replying to bonefish:

I haven't applied and tested the patch, but given how little space there is for the variables view, a second column might not be the best solution. Currently the "Value" column is empty for compound types. I would simply show the address there -- maybe in a slightly different way (e.g. prefixed with an "@", in parentheses, or with a different font style) to make clear that it isn't a pointer type. That would cover the use case mentioned in the description.

That's certainly a possibility. The main reasons I went with a column were 1) that allows one to see the address for all variables, not just compounds,

Sure, I think it is more likely that one needs the address of a compound rather than a primitive value, though. And the tool tip still makes it possible to get the information.

2) it's persistently visible unlike a tooltip which makes it more convenient for comparisons like mentioned in the original feature request,

In the feature request values from two different stack frames need to be compared, so they can't be seen at the same time either way.

and 3) the column listview allows one to hide columns one isn't interested in anyways, so one could simply hide it and revert back to the 2 column view instead if preferred (that would need to be persisted in settings though, which it isn't yet with this patch).

I find the show/hide column feature nice, but more in the way to be able to configure your view. The address is something rarely needed, so having it always shown in the limited space doesn't seems a good setting. And showing the column temporarily just to get the address seems tedious. Showing the address in the "Value" column for compounds and having it in the tool tip for all types seems a reasonable compromise.

That reminds me, my original idea for the variables view was to have two parts: The table view and a details view for the selected variable (mostly for alternative representation, e.g. for a list the list of members, for a BMessage a dump, for a BBitmap the bitmap, etc.). That would also be a good place to show the address.

If it makes things easier, I can attach a representative screenshot later tonight. Another possibility for space reasons would be to simply default to hiding the column.

Thanks. I like the tool tip version. I would omit the "piece(s)" in the tool tip and, as mentioned before, use a different visualization of the address for compound types to avoid confusion with pointer types. Displaying the address in the "Value" column is purely a visualization feature. I wouldn't implement that in CompoundValueNode.

BTW, the cases in VariablesView::VariableTableModel::GetToolTipForTablePath() have incorrect indentation.

Casting is also on my TODO list but I need to do a bit more studying of the value subsystem to be able to implement that one properly.

The value subsystem part should be rather simple, at least for equally sized types (mainly pointers and references). One of the reasons why there are two node objects -- a ValueNodeChild and a ValueNode -- to represent a value object is to support casting. Currently the ValueNode on top of a ValueNodeChild is created automatically with the matching type, but it is possible to explicitly put a ValueNode for a different type on top of it. Obviously for free type conversions a type parser for the input is required and a way to resolve the respective type.

I don't recall the situation for differently sized types (e.g. int to double). That might need some explicit support. I think the pointer/reference casts are more important, though.

in reply to:  7 ; comment:8 by anevilyak, 7 years ago

Replying to bonefish:

Sure, I think it is more likely that one needs the address of a compound rather than a primitive value, though. And the tool tip still makes it possible to get the information.

Indeed, the more I look at it, the more I like the tooltip approach.

That reminds me, my original idea for the variables view was to have two parts: The table view and a details view for the selected variable (mostly for alternative representation, e.g. for a list the list of members, for a BMessage a dump, for a BBitmap the bitmap, etc.). That would also be a good place to show the address.

Just for clarification, was the idea there that the detail view would occupy the lower half of the variables view or something in that vein?

Thanks. I like the tool tip version. I would omit the "piece(s)" in the tool tip and, as mentioned before, use a different visualization of the address for compound types to avoid confusion with pointer types. Displaying the address in the "Value" column is purely a visualization feature. I wouldn't implement that in CompoundValueNode.

Will adjust, thanks for the feedback.

The value subsystem part should be rather simple, at least for equally sized types (mainly pointers and references). One of the reasons why there are two node objects -- a ValueNodeChild and a ValueNode -- to represent a value object is to support casting. Currently the ValueNode on top of a ValueNodeChild is created automatically with the matching type, but it is possible to explicitly put a ValueNode for a different type on top of it. Obviously for free type conversions a type parser for the input is required and a way to resolve the respective type.

Understood. As an initial implementation interface-wise, I had thought of maybe having a drop down list of available types for the user to choose from, perhaps with some option to type-ahead filter the list, since a freeform parser would be a bit more complex.

in reply to:  8 ; comment:9 by bonefish, 7 years ago

Replying to anevilyak:

Just for clarification, was the idea there that the detail view would occupy the lower half of the variables view or something in that vein?

Yes, though probably not the half, but significantly less (a split view anyway).

Understood. As an initial implementation interface-wise, I had thought of maybe having a drop down list of available types for the user to choose from, perhaps with some option to type-ahead filter the list, since a freeform parser would be a bit more complex.

All available types may be *a lot*, particularly when large libraries are involved. So free form with autocompletion would be nice (I would keep it simple first -- just typename + *s -- can be improved later). Very nice would be direct options to cast to the actual subclasses of the object (the interesting part is deducing the actual type of the object), since that is something that comes up often. Moreover a standard cast is pointer to array, at least for C/C++. Which reminds me, that the type cast feature needs to be source language driven, i.e. SourceLanguage needs to provide the cast options that make sense and also needs to be involved in parsing the free form input.

in reply to:  9 comment:10 by anevilyak, 7 years ago

Replying to bonefish:

All available types may be *a lot*, particularly when large libraries are involved. So free form with autocompletion would be nice (I would keep it simple first -- just typename + *s -- can be improved later). Very nice would be direct options to cast to the actual subclasses of the object (the interesting part is deducing the actual type of the object), since that is something that comes up often. Moreover a standard cast is pointer to array, at least for C/C++. Which reminds me, that the type cast feature needs to be source language driven, i.e. SourceLanguage needs to provide the cast options that make sense and also needs to be involved in parsing the free form input.

Sounds good! Added to TODO list.

comment:11 by anevilyak, 7 years ago

Resolution: fixed
Status: in-progressclosed

Implemented in hrev44351.

Note: See TracTickets for help on using tickets.