Re: [PATCH] Fix faulty use of obstack_free() to *shrink* dont_print_statmem_obstack. Instead use obstack_blank_fast() with a "negative" size. A real stack data structured would be appropriate here. Added unit test gdb/testsuite/gdb.cp/printstaticrecursion.exp.

[Simon Marchi <simon.marchi@ericsson.com>]

On 2017-10-23 01:17 PM, Simon Marchi wrote:
> On 2017-10-23 12:09 PM, Simon Marchi wrote:
>>> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
>>> index fb9bfd9..8f9658d 100644
>>> --- a/gdb/cp-valprint.c
>>> +++ b/gdb/cp-valprint.c
>>> @@ -370,14 +370,9 @@ cp_print_value_fields (struct type *type, struct type *real_type,
>>>  
>>>  	  if (obstack_final_size > statmem_obstack_initial_size)
>>>  	    {
>>> -	      /* In effect, a pop of the printed-statics stack.  */
>>> -
>>> -	      void *free_to_ptr =
>>> -		(char *) obstack_next_free (&dont_print_statmem_obstack) -
>>> -		(obstack_final_size - statmem_obstack_initial_size);
>>> -
>>> -	      obstack_free (&dont_print_statmem_obstack,
>>> -			    free_to_ptr);
>>> +          /* In effect, a pop of the printed-statics stack.  */
>>> +          size_t shrink_bytes = statmem_obstack_initial_size - obstack_final_size;
> 
> Hmm, size_t is unsigned, maybe it would be better to use ssize_t?
> 
>>> +          obstack_blank_fast(&dont_print_statmem_obstack, shrink_bytes);
>>
>> The indentation should be 1 tab + 6 spaces.
>>
>>>  	    }
>>>  
>>>  	  if (last_set_recurse != recurse)
>>
>> The code below that (which seems to be handling a similar situation, but for arrays) uses
>> obstack_next_free as well.  Is there the same problem there?
> 
> Never mind about this, I saw your other message after reading this one.
> 
> Simon
> 

I've stepped over the obstack_free call and noticed something strange, it changes the
value of obstack::object_base...

(top-gdb) p dont_print_statmem_obstack
$6 = {chunk_size = 256, chunk = 0x3b8b5a0, object_base = 0x3b8b5b0 "2\020`",
  next_free = 0x3b8sb5c0 "0\270\270\003", chunk_limit = 0x3b8b6a0 "p\265\270\003", temp = {i = 0,
    p = 0x0}, alignment_mask = 15, chunkfun = {plain = 0x798604 <xmalloc(size_t)>,
    extra = 0x798604 <xmalloc(size_t)>}, freefun = {plain = 0x798732 <xfree(void*)>,
    extra = 0x798732 <xfree(void*)>}, extra_arg = 0x0, use_extra_arg = 0, maybe_empty_object = 0,
  alloc_failed = 0}
(top-gdb) p free_to_ptr
$7 = (void *) 0x3b8b5b8
(top-gdb) n
383		  if (last_set_recurse != recurse)
(top-gdb) p dont_print_statmem_obstack
$8 = {chunk_size = 256, chunk = 0x3b8b5a0, object_base = 0x3b8b5b8 "1\020`",
  next_free = 0x3b8b5b8 "1\020`", chunk_limit = 0x3b8b6a0 "p\265\270\003", temp = {i = 0,
    p = 0x0}, alignment_mask = 15, chunkfun = {plain = 0x798604 <xmalloc(size_t)>,
    extra = 0x798604 <xmalloc(size_t)>}, freefun = {plain = 0x798732 <xfree(void*)>,
    extra = 0x798732 <xfree(void*)>}, extra_arg = 0x0, use_extra_arg = 0, maybe_empty_object = 0,
  alloc_failed = 0}


As you can see, object_base goes from 0x3b8b5b0 to 0x3b8b5b8.  And indeed, looking at obstack_free,
I see:

       if (__obj > (void *) __o->chunk && __obj < (void *) __o->chunk_limit)  \
	 __o->next_free = __o->object_base = (char *) __obj;		      \

So when you free, it resets object_base to that point... why does it do that?  It doesn't make sense
to me.

At least, that seems to explain why the obstack is empty after having called
obstack_free, even though we didn't ask it to free the whole obstack.

Simon