[PATCH 1/2] In cp_print_value_fields() obey dont_print_statmem=1 explicitly passed from cp_print_static_field()

[PATCH 1/2] In cp_print_value_fields() obey dont_print_statmem=1 explicitly passed from cp_print_static_field()

[Patrick Frants]

This fixes Bug gdb/13669 (https://sourceware.org/bugzilla/show_bug.cgi?id=13669)

cp_print_value_fields() in cp-valprint.c optionally skips static members based on options->static_field_print. Additionally cp_print_value_fields() has a parameter dont_print_statmem, which instructs the current invocation to skip static members. The "if () continue" statement (line 236) fails to take into account this parameter and therefore gdb gets into an infinite recursion involving cp_print_value_fields() and cp_print_static_field().

Sample backtrace (note dont_print_statmem=1):
(gdb) bt 30
#0  0x00007ffff754737d in __libc_sigaction () from target:/lib64/libc.so.6
#1  0x0000000000527c65 in gdb_demangle (name=0xd97683 "m_blendColour", options=3) at cp-support.c:1524
#2  0x0000000000646a6f in fprintf_symbol_filtered (stream=0x7fffffffd580, name=0xd97683 "m_blendColour", lang=<optimized out>, arg_mode=3) at utils.c:2433
#3  0x000000000052866f in cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16284, val=0x221f040, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:267
#4  0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16284, stream=0x7fffffffd580, val=0x221f040, type=<optimized out>) at cp-valprint.c:672
#5  cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16283, val=0x221eb90, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#6  0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16283, stream=0x7fffffffd580, val=0x221eb90, type=<optimized out>) at cp-valprint.c:672
#7  cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16282, val=0x221e710, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#8  0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16282, stream=0x7fffffffd580, val=0x221e710, type=<optimized out>) at cp-valprint.c:672
#9  cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16281, val=0x221f7b0, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#10 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16281, stream=0x7fffffffd580, val=0x221f7b0, type=<optimized out>) at cp-valprint.c:672
#11 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16280, val=0x221e100, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#12 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16280, stream=0x7fffffffd580, val=0x221e100, type=<optimized out>) at cp-valprint.c:672
#13 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16279, val=0x2213190, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#14 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16279, stream=0x7fffffffd580, val=0x2213190, type=<optimized out>) at cp-valprint.c:672
#15 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16278, val=0x2212d20, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#16 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16278, stream=0x7fffffffd580, val=0x2212d20, type=<optimized out>) at cp-valprint.c:672
#17 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16277, val=0x2212860, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#18 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16277, stream=0x7fffffffd580, val=0x2212860, type=<optimized out>) at cp-valprint.c:672
#19 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16276, val=0x2213660, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#20 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16276, stream=0x7fffffffd580, val=0x2213660, type=<optimized out>) at cp-valprint.c:672
#21 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16275, val=0x22121d0, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#22 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16275, stream=0x7fffffffd580, val=0x22121d0, type=<optimized out>) at cp-valprint.c:672
#23 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16274, val=0x2211e10, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#24 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16274, stream=0x7fffffffd580, val=0x2211e10, type=<optimized out>) at cp-valprint.c:672
#25 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16273, val=0x2214210, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#26 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16273, stream=0x7fffffffd580, val=0x2214210, type=<optimized out>) at cp-valprint.c:672
#27 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16272, val=0x2213d60, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
#28 0x0000000000529301 in cp_print_static_field (options=0x7fffffffcda0, recurse=16272, stream=0x7fffffffd580, val=0x2213d60, type=<optimized out>) at cp-valprint.c:672
#29 cp_print_value_fields (type=<optimized out>, type@entry=0xdc90c0, real_type=<optimized out>, offset=offset@entry=0, address=address@entry=6295612, stream=stream@entry=0x7fffffffd580, recurse=recurse@entry=16271, val=0x22137d0, options=0x7fffffffcda0, dont_print_vb=0x0, dont_print_statmem=1) at cp-valprint.c:333
(More stack frames follow...)

------------------------------------------------
REPRODUCTION:
A unittest has been added as two files:
  binutils-gdb/gdb/testsuite/gdb.cp/printstaticrecursion.exp
  binutils-gdb/gdb/testsuite/gdb.cp/printstaticrecursion.cc

---
 gdb/cp-valprint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index fb9bfd904f..9dda6e2e5c 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -230,7 +230,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 	  const gdb_byte *valaddr = value_contents_for_printing (val);
 
 	  /* If requested, skip printing of static fields.  */
-	  if (!options->static_field_print
+	  if ( (!options->static_field_print || dont_print_statmem)
 	      && field_is_static (&TYPE_FIELD (type, i)))
 	    continue;
 
-- 
2.14.0

[PATCH 2/2] Add unit test for bug 13669 "Infinite recursion in cp_print_value_fields"

[Patrick Frants]

---
 gdb/testsuite/gdb.cp/printstaticrecursion.cc  | 65 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/printstaticrecursion.exp | 41 +++++++++++++++++
 2 files changed, 106 insertions(+)
 create mode 100644 gdb/testsuite/gdb.cp/printstaticrecursion.cc
 create mode 100644 gdb/testsuite/gdb.cp/printstaticrecursion.exp

diff --git a/gdb/testsuite/gdb.cp/printstaticrecursion.cc b/gdb/testsuite/gdb.cp/printstaticrecursion.cc
new file mode 100644
index 0000000000..17312e6982
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/printstaticrecursion.cc
@@ -0,0 +1,65 @@
+#include <new>
+using namespace std;
+//// Pixel
+
+class Pixel
+{
+    int m_red;
+
+public:
+    Pixel()
+        : m_red(0)
+    {}
+
+    // Static instances of Pixel
+    static const Pixel sm_black;
+};
+
+const Pixel Pixel::sm_black;
+
+
+//// BlendState
+
+class BlendState
+{
+    Pixel m_blendColour;
+
+    // Static instances of BlendState
+    static const BlendState sm_alphaBlending;
+};
+
+const BlendState BlendState::sm_alphaBlending;
+
+
+//// Context
+
+class Context
+{
+    BlendState m_blendStack;
+
+    // Static instances of Context
+    static Context * sm_instance;
+public:
+    static Context & instance();
+};
+
+Context * Context::sm_instance;
+
+Context & Context::instance()
+{
+    sm_instance = new Context;
+
+    Context & ref = *sm_instance;
+    return ref;  /* break-here */
+}
+
+// + }}}
+
+
+
+int main()
+{
+    Context & context = Context::instance();
+
+    return 0;
+}
\ 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..37e3fed05f
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/printstaticrecursion.exp
@@ -0,0 +1,41 @@
+# 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_breakpoint ${srcfile}:[gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "Continue to first breakpoint"
+gdb_test "finish"
+gdb_exit
+return 0
-- 
2.14.0

Re: [PATCH 1/2] In cp_print_value_fields() obey dont_print_statmem=1 explicitly passed from cp_print_static_field()

[Simon Marchi]

On 2017-10-18 09:17 AM, Patrick Frants wrote:
> This fixes Bug gdb/13669 (https://sourceware.org/bugzilla/show_bug.cgi?id=13669)
> 
> cp_print_value_fields() in cp-valprint.c optionally skips static members based on options->static_field_print. Additionally cp_print_value_fields() has a parameter dont_print_statmem, which instructs the current invocation to skip static members. The "if () continue" statement (line 236) fails to take into account this parameter and therefore gdb gets into an infinite recursion involving cp_print_value_fields() and cp_print_static_field().

Hi Patrick,

Thanks for taking the time to provide a patch and test case, it's really appreciated.

I haven't looked at the patch in details, as I would need more time to get familiar with that code, but I ran the gdb.cp tests and noticed two failures in gdb.cp/classes.exp:

-PASS: gdb.cp/classes.exp: print csi with static members
-PASS: gdb.cp/classes.exp: print cnsi with static members
+FAIL: gdb.cp/classes.exp: print csi with static members
+FAIL: gdb.cp/classes.exp: print cnsi with static members

The problem seems to be with the message "same as static member..." not appearing.  Before:

 2377 print csi^M
 2378 $39 = {x = 10, y = 20, static null = {x = 0, y = 0, static null = <same as static member of an already seen type>}}^M
 2379 (gdb) PASS: gdb.cp/classes.exp: print csi with static members

After:

 2377 print csi^M
 2378 $39 = {x = 10, y = 20, static null = {x = 0, y = 0}}^M
 2379 (gdb) FAIL: gdb.cp/classes.exp: print csi with static members

You can try running the C++-specific tests by running this in the gdb/ dir:

  make check TESTS="gdb.cp/*.exp"

The test result summary is in testsuite/gdb.sum.  However, some tests are already
broken.  So what you should do is compare the gdb.sum of two test runs, without and
with your patch applied (don't forget to rebuild gdb in between).  To analyze
failures, look in testsuite/gdb.log.

More tips on running the testsuite here:

  https://sourceware.org/gdb/wiki/TestingGDB

Simon