This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
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.
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Patrick Frants <osscontribute at gmail dot com>, <gdb-patches at sourceware dot org>
- Date: Mon, 23 Oct 2017 15:37:38 -0400
- Subject: 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.
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon dot marchi at ericsson dot com;
- References: <1508405381-16638-1-git-send-email-osscontribute@gmail.com> <a983ffc6-25d8-0553-3e4d-69dc3fec72df@ericsson.com> <0d612f89-a3a9-afea-965b-7c364b660235@ericsson.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
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