Re: [PATCH] Fix broken recursion detection when printing static members

[Patrick Frants <osscontribute@gmail.com>]

I am so sorry, looks like git send-email removed the first N lines. Here is
the complete changelog:

Fix broken recursion detection when printing static members

Recursion detection for static members was broken. The implementation uses
a growing (and shrinking) obstack object to simulate a stack of addresses
(CORE_ADDR). Pushing addresses is implemented by calling obstack_grow(),
while popping is implemented by calling obstack_free(). The latter is
problematic because obstack_free() expects a pointer to the base of an
object. When popping elements of the stack however, obstack_free() was
called with the new top, which potentially is not the same as the base of
the stack. This is unintended use and the effect is that obstack->next_free
and obstack->object_base members are assigned the value of the new top,
which equals an empty stack. Summary: popping elements would always result
in an empty stack, which breaks the recursion detection.

The fix shrinks the stack using obstack_blank_fast() with a negative value
as described at the bottom of this page:
https://gcc.gnu.org/onlinedocs/libiberty/Extra-Fast-Growing.html
"You can use obstack_blank_fast with a “negative” size argument to make the
current object smaller. Just don’t try to shrink it beyond zero
length—there’s no telling what will happen if you do that. Earlier versions
of obstacks allowed you to use obstack_blank to shrink objects. This will
no longer work."

A unit test (gdb.cp/printstaticrecursion.exp) was added. No new regression
has been observed in testsuite/gdb.cp/*.exp.



On Tue, Oct 24, 2017 at 12:55 PM, Patrick Frants <osscontribute@gmail.com>
wrote:

> The fix shrinks the stack using obstack_blank_fast() with a negative value
> as described at the bottom of this page: https://gcc.gnu.org/
> onlinedocs/libiberty/Extra-Fast-Growing.html
> "You can use obstack_blank_fast with a “negative” size argument to make
> the current object smaller. Just don’t try to shrink it beyond zero
> length—there’s no telling what will happen if you do that. Earlier versions
> of obstacks allowed you to use obstack_blank to shrink objects. This will
> no longer work."
>
> A unit test (gdb.cp/printstaticrecursion.exp) was added. No new
> regression has been observed in testsuite/gdb.cp/*.exp.
> ---
>  gdb/cp-valprint.c                             |  9 ++-----
>  gdb/testsuite/gdb.cp/printstaticrecursion.cc  | 18 +++++++++++++
>  gdb/testsuite/gdb.cp/printstaticrecursion.exp | 37
> +++++++++++++++++++++++++++
>  3 files changed, 57 insertions(+), 7 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/printstaticrecursion.cc
>  create mode 100644 gdb/testsuite/gdb.cp/printstaticrecursion.exp
>
> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
> index fb9bfd904f..927273f8a2 100644
> --- a/gdb/cp-valprint.c
> +++ b/gdb/cp-valprint.c
> @@ -371,13 +371,8 @@ 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);
> +             size_t shrink_bytes = statmem_obstack_initial_size -
> obstack_final_size;
> +             obstack_blank_fast (&dont_print_statmem_obstack,
> shrink_bytes);
>             }
>
>           if (last_set_recurse != recurse)
> diff --git a/gdb/testsuite/gdb.cp/printstaticrecursion.cc
> b/gdb/testsuite/gdb.cp/printstaticrecursion.cc
> new file mode 100644
> index 0000000000..40fd3c0c27
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/printstaticrecursion.cc
> @@ -0,0 +1,18 @@
> +struct Inner
> +{
> +  static Inner instance;
> +};
> +
> +struct Outer
> +{
> +  Inner inner;
> +  static Outer instance;
> +};
> +
> +Inner Inner::instance;
> +Outer Outer::instance;
> +
> +int main()
> +{
> +  return 0; /* break-here */
> +}
> \ No newline at end of file
> diff --git a/gdb/testsuite/gdb.cp/printstaticrecursion.exp
> b/gdb/testsuite/gdb.cp/printstaticrecursion.exp
> new file mode 100644
> index 0000000000..a71adc7d03
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/printstaticrecursion.exp
> @@ -0,0 +1,37 @@
> +# Copyright 2008-2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +
> +if { [skip_cplus_tests] } { continue }
> +
> +standard_testfile printstaticrecursion.cc
> +
> +# Create and source the file that provides information about the compiler
> +# used to compile the test case.
> +if [get_compiler_info "c++"] {
> +    untested "couldn't find a valid c++ compiler"
> +    return -1
> +}
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug
> c++}]} {
> +    return -1
> +}
> +
> +if ![runto_main] then {
> +    perror "couldn't run to main"
> +    continue
> +}
> +
> +gdb_test "print Outer::instance" "{inner = {static instance = {static
> instance = <same as static member of an already seen type>}}, static
> instance = {inner = {static instance = {static instance = <same as static
> member of an already seen type>}}, static instance = <same as static member
> of an already seen type>}}" "print recursive static member"
> --
> 2.14.0
>
>