* [patch] varobj.c c_describe_child
@ 2010-03-22 19:42 Aleksandar Ristovski
2010-03-25 18:29 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Aleksandar Ristovski @ 2010-03-22 19:42 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 950 bytes --]
Hello,
When printing children of a variable created with a pointer
type, gdb generates a copy of value of target type for each
child. This quickly exhausts memory in certain cases.
In my case, customer has a struct with 1600+ fields and of
size approx 900K. Listing children of such struct via a
variable created with a pointer to it, crashes gdb (gdb
version 6.8) due to running out of memory. The patch fixes
this by removing intermediate value and freeing it.
Note that with latest gdb from HEAD it doesn't crash, I
believe due to recent changes in lazy allocating the values,
but it still seems wrong to keep all these intermediate
values generated by 'adjust_value_for_child_access' in
'all_values' list.
Thanks,
--
Aleksandar Ristovski
QNX Software Systems
ChangeLog:
2010-03-22 Aleksandar Ristovski <aristovski@qnx.com>
* varobj.c (c_describe_child): Do not keep temporary parent
value
while fetching child's value.
[-- Attachment #2: varobj-removetempvalue-20100322-ChangeLog.patch --]
[-- Type: text/plain, Size: 155 bytes --]
2010-03-22 Aleksandar Ristovski <aristovski@qnx.com>
* varobj.c (c_describe_child): Do not keep temporary parent value
while fetching child's value.
[-- Attachment #3: varobj-removetempvalue-20100322.patch --]
[-- Type: text/plain, Size: 933 bytes --]
Index: gdb/varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.156
diff -u -p -r1.156 varobj.c
--- gdb/varobj.c 3 Mar 2010 18:32:44 -0000 1.156
+++ gdb/varobj.c 22 Mar 2010 19:15:34 -0000
@@ -2821,6 +2821,12 @@ c_describe_child (struct varobj *parent,
parent_expression = varobj_get_path_expr (parent);
}
adjust_value_for_child_access (&value, &type, &was_ptr);
+ if (value != parent->value)
+ {
+ /* If value was created merely to later create child,
+ * do not keep it in value history. */
+ release_value (value);
+ }
switch (TYPE_CODE (type))
{
@@ -2902,6 +2908,9 @@ c_describe_child (struct varobj *parent,
*cfull_expression = xstrdup ("???");
/* Don't set value and type, we don't know then. */
}
+
+ if (value != parent->value)
+ value_free (value);
}
static char *
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] varobj.c c_describe_child
2010-03-22 19:42 [patch] varobj.c c_describe_child Aleksandar Ristovski
@ 2010-03-25 18:29 ` Tom Tromey
2010-03-25 19:20 ` Aleksandar Ristovski
0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2010-03-25 18:29 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: gdb-patches
>>>>> "Aleksandar" == Aleksandar Ristovski <aristovski@qnx.com> writes:
Aleksandar> Note that with latest gdb from HEAD it doesn't crash, I
Aleksandar> believe due to recent changes in lazy allocating the values,
Aleksandar> but it still seems wrong to keep all these intermediate
Aleksandar> values generated by adjust_value_for_child_access' in
Aleksandar> 'all_values' list.
I think these values will automatically be freed the next time an MI
command is executed. The call chain is mi_cmd_execute ->
prepare_execute_command -> free_all_values.
Or is the idea to limit the transient amount of memory used?
I can't tell from the patch.
In that case, I think it would be more typical to use
value_mark and value_free_to_mark.
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] varobj.c c_describe_child
2010-03-25 18:29 ` Tom Tromey
@ 2010-03-25 19:20 ` Aleksandar Ristovski
0 siblings, 0 replies; 3+ messages in thread
From: Aleksandar Ristovski @ 2010-03-25 19:20 UTC (permalink / raw)
To: gdb-patches
On 25/03/2010 2:29, Tom Tromey wrote:
>>>>>> "Aleksandar" == Aleksandar Ristovski<aristovski@qnx.com> writes:
>
> Aleksandar> Note that with latest gdb from HEAD it doesn't crash, I
> Aleksandar> believe due to recent changes in lazy allocating the values,
> Aleksandar> but it still seems wrong to keep all these intermediate
> Aleksandar> values generated by adjust_value_for_child_access' in
> Aleksandar> 'all_values' list.
>
> I think these values will automatically be freed the next time an MI
> command is executed. The call chain is mi_cmd_execute ->
> prepare_execute_command -> free_all_values.
Yes. However, in gdb 6.8, it will not get a chance since it
crashes before finishing creating children (on HEAD gdb it
doesn't crash, I believe due to lazy mechanism).
>
> Or is the idea to limit the transient amount of memory used?
> I can't tell from the patch.
Yes, the idea is to reduce amount of transient memory.
>
> In that case, I think it would be more typical to use
> value_mark and value_free_to_mark.
What is happening (in a loop for each field of the struct) is:
- create value for the parent type (huge struct)
- create value for the field
and then repeat all that. I believe value_mark would not
work since value that we do want to keep is interspersed
with the ones we can discard.
Maybe the right fix is to create a varobj of pointed-to type
as a child to pointer-to varobj, and then calculate children
for this new varobj?
Or introduce "target_value" in varobj struct?
Not sure. The patch I proposed reduces the issue but is not
solving it completely.
Thanks,
--
Aleksandar Ristovski
QNX Software Systems
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-03-25 19:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-22 19:42 [patch] varobj.c c_describe_child Aleksandar Ristovski
2010-03-25 18:29 ` Tom Tromey
2010-03-25 19:20 ` Aleksandar Ristovski
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).