[patch] varobj.c c_describe_child

[patch] varobj.c c_describe_child

[Aleksandar Ristovski]

[-- 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 *

Re: [patch] varobj.c c_describe_child

[Tom Tromey]

>>>>> "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

Re: [patch] varobj.c c_describe_child

[Aleksandar Ristovski]

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