[patch] [python] mi/12531

[patch] [python] mi/12531

[Phil Muldoon]

This patch fixes a case where an MI user could install a visualizer on a
synthetic varobj.  If we detect that the object is a CPLUS_FAKE_CHILD,
we do not install a visualizer.  I thought about raising an error, but
other MI behavior with fake children is just to move on if possible.  I
did the same.

Cheers

Phil

--

2011-04-13  Phil Muldoon  <pmuldoon@redhat.com>

	* varobj.c (install_default_visualizer): Do not install a
	visualizer if the varobj is CPLUS_FAKE_CHILD.
	(construct_visualizer): Likewise.


2011-04-13  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/py-mi.exp: Add CPLUS_FAKE_CHILD tests and a C++
	compile target.
	* gdb.python/py-prettyprint.exp: Add C++ object for
	CPLUS_FAKE_CHILD test.

--

diff --git a/gdb/testsuite/gdb.python/py-mi.exp b/gdb/testsuite/gdb.python/py-mi.exp
index 629417b..2adb449 100644
--- a/gdb/testsuite/gdb.python/py-mi.exp
+++ b/gdb/testsuite/gdb.python/py-mi.exp
@@ -283,4 +283,42 @@ mi_list_varobj_children nstype2 {
     { {nstype2.<error at 0>} {<error at 0>} 6 {char \[6\]} }
 } "list children after setting exception flag"
 
+# C++ MI tests
+gdb_exit
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++ additional_flags=-DMI}] != "" } {
+	untested "Couldn't compile ${srcfile} in c++ mode"
+	return -1
+    }
+
+if [mi_gdb_start] {
+    continue
+}
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+if {[lsearch -exact [mi_get_features] python] < 0} {
+    unsupported "python support is disabled"
+    return -1
+}
+
+mi_runto main
+mi_continue_to_line [gdb_get_line_number {break to inspect struct and union} ${testfile}.c] \
+  "step to breakpoint"
+
+# Test python/12531.  Install visualizer on a cplus_fake_child.
+mi_create_varobj fake fake \
+  "create fake varobj"
+
+mi_list_varobj_children fake {
+    { fake.private private 1 }
+} "list children of fake"
+
+mi_list_varobj_children fake.private {
+    { fake.private.sname sname 2 std::string }
+} "list children fake.private"
+
+mi_gdb_test "-var-set-visualizer fake.private gdb.default_visualizer" \
+    "\\^done" "Install visualizer on a cplus_fake_child"
+
 remote_file host delete ${remote_python_file}
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.c b/gdb/testsuite/gdb.python/py-prettyprint.c
index 5f98433..0b6d216 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.c
+++ b/gdb/testsuite/gdb.python/py-prettyprint.c
@@ -45,6 +45,8 @@ struct lazystring {
 };
 
 #ifdef __cplusplus
+#include <string>
+
 struct S : public s {
   int zs;
 };
@@ -90,6 +92,16 @@ class Derived : public Vbase1, public Vbase2, public Vbase3
     }
 };
 
+class Fake
+{
+  std::string sname;
+
+ public:
+  Fake (const std::string& name = ""):
+  sname (name)
+  {
+  }
+};
 #endif
 
 struct substruct {
@@ -262,6 +274,7 @@ main ()
 
   Derived derived;
   
+  Fake fake ("foo");
 #endif
 
   add_item (&c, 23);		/* MI breakpoint here */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index bfb3851..1c4c88f 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1397,6 +1397,11 @@ install_visualizer (struct varobj *var, PyObject *constructor,
 static void
 install_default_visualizer (struct varobj *var)
 {
+
+  /* Do not install a visualizer on a CPLUS_FAKE_CHILD.  */
+  if (CPLUS_FAKE_CHILD (var))
+    return;
+
   if (pretty_printing)
     {
       PyObject *pretty_printer = NULL;
@@ -1429,6 +1434,10 @@ construct_visualizer (struct varobj *var, PyObject *constructor)
 {
   PyObject *pretty_printer;
 
+  /* Do not install a visualizer on a CPLUS_FAKE_CHILD.  */
+  if (CPLUS_FAKE_CHILD (var))
+    return;
+
   Py_INCREF (constructor);
   if (constructor == Py_None)
     pretty_printer = NULL;

Re: [patch] [python] mi/12531

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> This patch fixes a case where an MI user could install a visualizer on a
Phil> synthetic varobj.  If we detect that the object is a CPLUS_FAKE_CHILD,
Phil> we do not install a visualizer.  I thought about raising an error, but
Phil> other MI behavior with fake children is just to move on if possible.  I
Phil> did the same.

Thanks for doing this.

I have some comments, but they are all minor things.

Phil> 2011-04-13  Phil Muldoon  <pmuldoon@redhat.com>
Phil> 	* varobj.c (install_default_visualizer): Do not install a
Phil> 	visualizer if the varobj is CPLUS_FAKE_CHILD.
Phil> 	(construct_visualizer): Likewise.

I thought there was a PR open for this.  If so, it should be mentioned
in the ChangeLog.  (If it is just in Red Hat bugzilla then it is
optional; but if you choose to use it, put the whole URL in.)

Phil> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++ additional_flags=-DMI}] != "" } {

This should be split somewhere.

Phil> +	untested "Couldn't compile ${srcfile} in c++ mode"
Phil> +	return -1

Wrong indentation, I think.

Phil> +    }

Definitely wrong indentation :)

Phil> +mi_continue_to_line [gdb_get_line_number {break to inspect struct and union} ${testfile}.c] \

Split this line somewhere.

Phil>  #ifdef __cplusplus
Phil> +#include <string>

It is best not to rely on libstdc++ in the test suite.

I think just an ordinary struct ought to exhibit the behavior you need,
as long as the file is compiled by the C++ compiler.  So, you probably
don't need any additional classes.  But, if you do, you have to write
them yourself :)

Phil>  static void
Phil>  install_default_visualizer (struct varobj *var)
Phil>  {
Phil> +
Phil> +  /* Do not install a visualizer on a CPLUS_FAKE_CHILD.  */
Phil> +  if (CPLUS_FAKE_CHILD (var))
Phil> +    return;

Spurious blank line after the "{".

Tom

Re: [patch] [python] mi/12531

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

> I have some comments, but they are all minor things.
>
> Phil> 2011-04-13  Phil Muldoon  <pmuldoon@redhat.com>
> Phil> 	* varobj.c (install_default_visualizer): Do not install a
> Phil> 	visualizer if the varobj is CPLUS_FAKE_CHILD.
> Phil> 	(construct_visualizer): Likewise.
>
> I thought there was a PR open for this.  If so, it should be mentioned
> in the ChangeLog.  (If it is just in Red Hat bugzilla then it is
> optional; but if you choose to use it, put the whole URL in.)

Nope, it is a sourceware bug.  I just forgot this time ;)

> Phil> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++ additional_flags=-DMI}] != "" } {
>
> This should be split somewhere.

Ok. And for other line wraps/indention fixes.  For some reason emacs
want to further indent the "executable ..." line  in py-mi.exp by
an extra tab.  It looks wrong, but I'll defer to emacs. ;)


> Phil>  #ifdef __cplusplus
> Phil> +#include <string>
>
> It is best not to rely on libstdc++ in the test suite.

Ack, good catch.  I can just use an int here.  I converted it to a struct
(compiled under C++) and for some reason, asking for the children of
private just tells me variable not found.  It works with a class though.

You did not give an explicit ok, so I am attaching the patch.  I won't
attach the ChangeLog again, but I will attach the relevant bz entries.

Also, as this tends to block Dodji's use of pretty-printing in Nemiver, is
it ok for the 7.3 branch too?

Cheers

Phil

--

diff --git a/gdb/testsuite/gdb.python/py-mi.exp b/gdb/testsuite/gdb.python/py-mi.exp
index 629417b..37359e9 100644
--- a/gdb/testsuite/gdb.python/py-mi.exp
+++ b/gdb/testsuite/gdb.python/py-mi.exp
@@ -283,4 +283,44 @@ mi_list_varobj_children nstype2 {
     { {nstype2.<error at 0>} {<error at 0>} 6 {char \[6\]} }
 } "list children after setting exception flag"
 
+# C++ MI tests
+gdb_exit
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	   executable {debug c++ additional_flags=-DMI}] != "" } {
+    untested "Couldn't compile ${srcfile} in c++ mode"
+    return -1
+}
+
+if [mi_gdb_start] {
+    continue
+}
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+if {[lsearch -exact [mi_get_features] python] < 0} {
+    unsupported "python support is disabled"
+    return -1
+}
+
+mi_runto main
+mi_continue_to_line \
+    [gdb_get_line_number {break to inspect struct and union} ${testfile}.c] \
+    "step to breakpoint"
+
+# Test python/12531.  Install visualizer on a cplus_fake_child.
+mi_create_varobj fake fake \
+  "create fake varobj"
+
+mi_list_varobj_children fake {
+    { fake.private private 1 }
+} "list children of fake"
+
+mi_list_varobj_children fake.private {
+    { fake.private.sname sname 0 int }
+} "list children fake.private"
+
+mi_gdb_test "-var-set-visualizer fake.private gdb.default_visualizer" \
+    "\\^done" "Install visualizer on a cplus_fake_child"
+
 remote_file host delete ${remote_python_file}
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.c b/gdb/testsuite/gdb.python/py-prettyprint.c
index 1d88b4e..b65a84fb 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.c
+++ b/gdb/testsuite/gdb.python/py-prettyprint.c
@@ -94,6 +94,16 @@ class Derived : public Vbase1, public Vbase2, public Vbase3
     }
 };
 
+class Fake
+{
+  int sname;
+  
+ public:
+  Fake (const int name = 0):
+  sname (name)
+  {
+  }
+};
 #endif
 
 struct substruct {
@@ -267,6 +277,7 @@ main ()
 
   Derived derived;
   
+  Fake fake (42);
 #endif
 
   add_item (&c, 23);		/* MI breakpoint here */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index bfb3851..e068823 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1397,6 +1397,10 @@ install_visualizer (struct varobj *var, PyObject *constructor,
 static void
 install_default_visualizer (struct varobj *var)
 {
+  /* Do not install a visualizer on a CPLUS_FAKE_CHILD.  */
+  if (CPLUS_FAKE_CHILD (var))
+    return;
+
   if (pretty_printing)
     {
       PyObject *pretty_printer = NULL;
@@ -1429,6 +1433,10 @@ construct_visualizer (struct varobj *var, PyObject *constructor)
 {
   PyObject *pretty_printer;
 
+  /* Do not install a visualizer on a CPLUS_FAKE_CHILD.  */
+  if (CPLUS_FAKE_CHILD (var))
+    return;
+
   Py_INCREF (constructor);
   if (constructor == Py_None)
     pretty_printer = NULL;

Re: [patch] [python] mi/12531

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> You did not give an explicit ok, so I am attaching the patch.  I won't
Phil> attach the ChangeLog again, but I will attach the relevant bz entries.

Ok.

Phil> Also, as this tends to block Dodji's use of pretty-printing in Nemiver, is
Phil> it ok for the 7.3 branch too?

Sure, thanks.

Tom

Re: [patch] [python] mi/12531

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> You did not give an explicit ok, so I am attaching the patch.  I won't
> Phil> attach the ChangeLog again, but I will attach the relevant bz entries.
>
> Ok.
>
> Phil> Also, as this tends to block Dodji's use of pretty-printing in Nemiver, is
> Phil> it ok for the 7.3 branch too?
>
> Sure, thanks.


Committed to both branch and head.

Cheers,

Phil