public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
* [Bug mi/29554] New: --simple-values does not take reference types into account
@ 2022-09-07 15:46 grees at undo dot io
2022-09-08 6:55 ` [Bug mi/29554] " grees at undo dot io
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: grees at undo dot io @ 2022-09-07 15:46 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=29554
Bug ID: 29554
Summary: --simple-values does not take reference types into
account
Product: gdb
Version: HEAD
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: mi
Assignee: unassigned at sourceware dot org
Reporter: grees at undo dot io
Target Milestone: ---
Created attachment 14321
--> https://sourceware.org/bugzilla/attachment.cgi?id=14321&action=edit
Patch against 154f2735ad4
SUMMARY
The '--simple-values' argument to '-stack-list-arguments' and similar
GDB/MI commands does not take reference types into account, so that
references to arbitrarily large structures are considered "simple" and
printed. This means that the '--simple-values' argument cannot be used
by IDEs when tracing the stack due to the time taken to print large
structures passed by reference.
DETAILS
Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals',
'-stack-list-variables' and so on) take a PRINT-VALUES argument which
may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2).
In the '--simple-values' case, the command is supposed to print the
name, type, and value of variables with simple types, and print only the
name and type of variables with compound types.
The '--simple-values' argument ought to be suitable for IDEs that need
to update their user interface with the program's call stack every time
the program stops. However, it does not take C++ reference types into
account, and this makes the argument unsuitable for this purpose.
For example, consider the following C++ program:
struct s {
int v[10];
};
int
sum(const struct s &s)
{
int total = 0;
for (int i = 0; i < 10; ++i) total += s.v[i];
return total;
}
int
main(void)
{
struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } };
return sum(s);
}
If we start GDB in MI mode and continue to 'sum', the behaviour of
'-stack-list-arguments' is as follows:
(gdb)
-stack-list-arguments --simple-values
^done,stack-args=[frame={level="0",args=[{name="s",type="const s
&",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9,
10}}"}]},frame={level="1",args=[]}]
Note that the value of the argument 's' was printed, even though 's' is
a reference to a structure, which is not a simple value.
See https://github.com/microsoft/MIEngine/pull/673 for a case where this
behaviour caused Microsoft to avoid the use of '--simple-values' in
their MIEngine debug adapter, because it caused Visual Studio Code to
take too long to refresh the call stack in the user interface.
SOLUTIONS
There are two ways we could fix this problem, depending on whether we
consider the current behaviour to be a bug.
1. If the current behaviour is a bug, then we can update the behaviour
of '--simple-values' so that it takes reference types into account:
that is, a value is simple if it is neither an array, struct, or
union, nor a reference to an array, struct or union.
In this case we must add a feature to the '-list-features' command so
that IDEs can detect that it is safe to use the '--simple-values'
argument when refreshing the call stack.
2. If the current behaviour is not a bug, then we can add a new option
for the PRINT-VALUES argument, for example, '--simplest-values' (3),
that would be suitable for use by IDEs.
In this case we must add a feature to the '-list-features' command so
that IDEs can detect that the '--simplest-values' argument is
available for use when refreshing the call stack.
PATCH
The attached patch implements solution (1) as I think the current
behaviour of not printing structures, but printing references to
structures, is surprising. However, if you prefer solution (2) I would
be happy to implement that instead.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug mi/29554] --simple-values does not take reference types into account
2022-09-07 15:46 [Bug mi/29554] New: --simple-values does not take reference types into account grees at undo dot io
@ 2022-09-08 6:55 ` grees at undo dot io
2022-09-09 8:09 ` grees at undo dot io
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: grees at undo dot io @ 2022-09-08 6:55 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=29554
Gareth Rees <grees at undo dot io> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #14321|0 |1
is obsolete| |
--- Comment #1 from Gareth Rees <grees at undo dot io> ---
Created attachment 14324
--> https://sourceware.org/bugzilla/attachment.cgi?id=14324&action=edit
Corrected patch against 154f2735ad
My first patch was backwards due to getting the "git diff" command wrong. New
patch is the right way round.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug mi/29554] --simple-values does not take reference types into account
2022-09-07 15:46 [Bug mi/29554] New: --simple-values does not take reference types into account grees at undo dot io
2022-09-08 6:55 ` [Bug mi/29554] " grees at undo dot io
@ 2022-09-09 8:09 ` grees at undo dot io
2022-10-04 9:05 ` grees at undo dot io
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: grees at undo dot io @ 2022-09-09 8:09 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=29554
--- Comment #2 from Gareth Rees <grees at undo dot io> ---
Created attachment 14329
--> https://sourceware.org/bugzilla/attachment.cgi?id=14329&action=edit
Patch against 154f2735ad implementing solution (2)
Here's an alternative patch implementing solution (2), adding a
'--scalar-values' option for the PRINT-VALUES argument to
'-stack-list-arguments' and similar commands. This option prints the
value only for scalars and so matches the behaviour of the 'scalars'
argument to the 'set print frame-arguments' command. References to
structures are not scalars, and so the option is suitable for use by
IDEs.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug mi/29554] --simple-values does not take reference types into account
2022-09-07 15:46 [Bug mi/29554] New: --simple-values does not take reference types into account grees at undo dot io
2022-09-08 6:55 ` [Bug mi/29554] " grees at undo dot io
2022-09-09 8:09 ` grees at undo dot io
@ 2022-10-04 9:05 ` grees at undo dot io
2022-10-20 17:49 ` grees at undo dot io
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: grees at undo dot io @ 2022-10-04 9:05 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=29554
Gareth Rees <grees at undo dot io> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #14329|0 |1
is obsolete| |
--- Comment #3 from Gareth Rees <grees at undo dot io> ---
Created attachment 14379
--> https://sourceware.org/bugzilla/attachment.cgi?id=14379&action=edit
Corrected patch against 154f2735ad implementing solution (2)
Here's a patch implementing solution (2), corrected after review by Eli
Zaretskii.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug mi/29554] --simple-values does not take reference types into account
2022-09-07 15:46 [Bug mi/29554] New: --simple-values does not take reference types into account grees at undo dot io
` (2 preceding siblings ...)
2022-10-04 9:05 ` grees at undo dot io
@ 2022-10-20 17:49 ` grees at undo dot io
2023-05-04 15:08 ` cvs-commit at gcc dot gnu.org
2023-05-04 16:32 ` tromey at sourceware dot org
5 siblings, 0 replies; 7+ messages in thread
From: grees at undo dot io @ 2022-10-20 17:49 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=29554
Gareth Rees <grees at undo dot io> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #14379|0 |1
is obsolete| |
--- Comment #4 from Gareth Rees <grees at undo dot io> ---
Created attachment 14408
--> https://sourceware.org/bugzilla/attachment.cgi?id=14408&action=edit
Corrected patch against 154f2735ad implementing solution (2)
Here's a patch implementing solution (2), corrected after review by Eli
Zaretskii and Andrew Burgess.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug mi/29554] --simple-values does not take reference types into account
2022-09-07 15:46 [Bug mi/29554] New: --simple-values does not take reference types into account grees at undo dot io
` (3 preceding siblings ...)
2022-10-20 17:49 ` grees at undo dot io
@ 2023-05-04 15:08 ` cvs-commit at gcc dot gnu.org
2023-05-04 16:32 ` tromey at sourceware dot org
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-04 15:08 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=29554
--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=51f8dafba8175a8d59716f220a46de4e626e5073
commit 51f8dafba8175a8d59716f220a46de4e626e5073
Author: Gareth Rees <grees@undo.io>
Date: Sat Mar 11 11:49:34 2023 +0000
Don't treat references to compound values as "simple".
SUMMARY
The '--simple-values' argument to '-stack-list-arguments' and similar
GDB/MI commands does not take reference types into account, so that
references to arbitrarily large structures are considered "simple" and
printed. This means that the '--simple-values' argument cannot be used
by IDEs when tracing the stack due to the time taken to print large
structures passed by reference.
DETAILS
Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals',
'-stack-list-variables' and so on) take a PRINT-VALUES argument which
may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2).
In the '--simple-values' case, the command is supposed to print the
name, type, and value of variables with simple types, and print only the
name and type of variables with compound types.
The '--simple-values' argument ought to be suitable for IDEs that need
to update their user interface with the program's call stack every time
the program stops. However, it does not take C++ reference types into
account, and this makes the argument unsuitable for this purpose.
For example, consider the following C++ program:
struct s {
int v[10];
};
int
sum(const struct s &s)
{
int total = 0;
for (int i = 0; i < 10; ++i) total += s.v[i];
return total;
}
int
main(void)
{
struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } };
return sum(s);
}
If we start GDB in MI mode and continue to 'sum', the behaviour of
'-stack-list-arguments' is as follows:
(gdb)
-stack-list-arguments --simple-values
^done,stack-args=[frame={level="0",args=[{name="s",type="const s
&",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9,
10}}"}]},frame={level="1",args=[]}]
Note that the value of the argument 's' was printed, even though 's' is
a reference to a structure, which is not a simple value.
See https://github.com/microsoft/MIEngine/pull/673 for a case where this
behaviour caused Microsoft to avoid the use of '--simple-values' in
their MIEngine debug adapter, because it caused Visual Studio Code to
take too long to refresh the call stack in the user interface.
SOLUTIONS
There are two ways we could fix this problem, depending on whether we
consider the current behaviour to be a bug.
1. If the current behaviour is a bug, then we can update the behaviour
of '--simple-values' so that it takes reference types into account:
that is, a value is simple if it is neither an array, struct, or
union, nor a reference to an array, struct or union.
In this case we must add a feature to the '-list-features' command so
that IDEs can detect that it is safe to use the '--simple-values'
argument when refreshing the call stack.
2. If the current behaviour is not a bug, then we can add a new option
for the PRINT-VALUES argument, for example, '--scalar-values' (3),
that would be suitable for use by IDEs.
In this case we must add a feature to the '-list-features' command
so that IDEs can detect that the '--scalar-values' argument is
available for use when refreshing the call stack.
PATCH
This patch implements solution (1) as I think the current behaviour of
not printing structures, but printing references to structures, is
contrary to reasonable expectation.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug mi/29554] --simple-values does not take reference types into account
2022-09-07 15:46 [Bug mi/29554] New: --simple-values does not take reference types into account grees at undo dot io
` (4 preceding siblings ...)
2023-05-04 15:08 ` cvs-commit at gcc dot gnu.org
@ 2023-05-04 16:32 ` tromey at sourceware dot org
5 siblings, 0 replies; 7+ messages in thread
From: tromey at sourceware dot org @ 2023-05-04 16:32 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=29554
Tom Tromey <tromey at sourceware dot org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
CC| |tromey at sourceware dot org
Target Milestone|--- |14.1
--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
I think this is fixed now.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-05-04 16:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-07 15:46 [Bug mi/29554] New: --simple-values does not take reference types into account grees at undo dot io
2022-09-08 6:55 ` [Bug mi/29554] " grees at undo dot io
2022-09-09 8:09 ` grees at undo dot io
2022-10-04 9:05 ` grees at undo dot io
2022-10-20 17:49 ` grees at undo dot io
2023-05-04 15:08 ` cvs-commit at gcc dot gnu.org
2023-05-04 16:32 ` tromey at sourceware dot org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).