Patch for PR 10728

Patch for PR 10728

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 238 bytes --]

The bug was basically in the mis-use of obstacks in cp-valprint.c

The patch includes testcase source files and and an expects file.

The gdb.sum diff between the unpatched and patched make check is also 
attached.

Chris Moller,
Red Hat

[-- Attachment #2: pr9067.patch --]
[-- Type: text/plain, Size: 8230 bytes --]

? gdb/testsuite/gdb.cp/pr9067
Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.11315
diff -u -r1.11315 ChangeLog
--- gdb/ChangeLog	2 Feb 2010 16:47:13 -0000	1.11315
+++ gdb/ChangeLog	2 Feb 2010 21:53:37 -0000
@@ -1,3 +1,14 @@
+Tue Feb  2 16:44:56 2010  Chris Moller  <moller@mollerware.com>
+
+	PR gdb/9067
+	* c-lang.h:
+	* c-valprint.c (c_value_print): Added initialisation and cleanup
+	for static member obstack.
+	* cp-valprint.c (p_initialise_statmem_stack) New.
+	(cp_free_statmem_stack) New.
+	(cp_print_value_fields) Fix use of obstacks.
+	(cp_print_static_field)  Fix use of obstacks.
+
 2010-02-02  Tom Tromey  <tromey@redhat.com>
 
 	* m2-typeprint.c (m2_record_fields): Don't use
Index: gdb/c-lang.h
===================================================================
RCS file: /cvs/src/src/gdb/c-lang.h,v
retrieving revision 1.27
diff -u -r1.27 c-lang.h
--- gdb/c-lang.h	2 Feb 2010 16:45:16 -0000	1.27
+++ gdb/c-lang.h	2 Feb 2010 21:53:37 -0000
@@ -112,6 +112,9 @@
 
 extern int cp_is_vtbl_member (struct type *);
 
+extern void cp_initialise_statmem_stack();
+extern void cp_free_statmem_stack();
+
 /* These are in c-valprint.c.  */
 
 extern int c_textual_element_type (struct type *, char);
Index: gdb/c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.67
diff -u -r1.67 c-valprint.c
--- gdb/c-valprint.c	2 Feb 2010 16:45:16 -0000	1.67
+++ gdb/c-valprint.c	2 Feb 2010 21:53:37 -0000
@@ -710,8 +710,17 @@
       /* Otherwise, we end up at the return outside this "if" */
     }
 
-  return val_print (val_type, value_contents_all (val),
+  {
+    int rc;
+
+    cp_initialise_statmem_stack();
+    
+    rc = val_print (val_type, value_contents_all (val),
 		    value_embedded_offset (val),
 		    value_address (val),
 		    stream, 0, &opts, current_language);
+
+    cp_free_statmem_stack();
+    return rc;
+  }
 }
Index: gdb/cp-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-valprint.c,v
retrieving revision 1.63
diff -u -r1.63 cp-valprint.c
--- gdb/cp-valprint.c	2 Feb 2010 16:45:16 -0000	1.63
+++ gdb/cp-valprint.c	2 Feb 2010 21:53:38 -0000
@@ -134,6 +134,22 @@
   return 0;
 }
 
+/* initialise to empty the static member stack */
+void
+cp_initialise_statmem_stack()
+{
+  obstack_init (&dont_print_statmem_obstack);
+}
+
+/* initialise to free the static member stack */
+void
+cp_free_statmem_stack()
+{
+  if (obstack_object_size (&dont_print_statmem_obstack) > 0)
+    obstack_free (&dont_print_statmem_obstack, NULL);
+}
+
+
 /* Mutually recursive subroutines of cp_print_value and c_val_print to
    print out a structure's fields: cp_print_value_fields and cp_print_value.
 
@@ -154,7 +170,6 @@
 		       struct type **dont_print_vb, int dont_print_statmem)
 {
   int i, len, n_baseclasses;
-  char *last_dont_print = obstack_next_free (&dont_print_statmem_obstack);
   int fields_seen = 0;
 
   CHECK_TYPEDEF (type);
@@ -177,14 +192,13 @@
     fprintf_filtered (stream, "<No data fields>");
   else
     {
-      struct obstack tmp_obstack = dont_print_statmem_obstack;
-
+      void * statmem_obstack_top = NULL;
+      
       if (dont_print_statmem == 0)
 	{
-	  /* If we're at top level, carve out a completely fresh
-	     chunk of the obstack and use that until this particular
-	     invocation returns.  */
-	  obstack_finish (&dont_print_statmem_obstack);
+	  /* set the current printed-statics stack top */
+	  statmem_obstack_top
+	    = obstack_next_free (&dont_print_statmem_obstack);
 	}
 
       for (i = n_baseclasses; i < len; i++)
@@ -305,10 +319,9 @@
 
       if (dont_print_statmem == 0)
 	{
-	  /* Free the space used to deal with the printing
-	     of the members from top level.  */
-	  obstack_free (&dont_print_statmem_obstack, last_dont_print);
-	  dont_print_statmem_obstack = tmp_obstack;
+	  /* In effect, a pop of the printed-statics stack */
+	  if (obstack_object_size (&dont_print_statmem_obstack) > 0) 
+	    obstack_free (&dont_print_statmem_obstack, statmem_obstack_top);
 	}
 
       if (options->pretty)
@@ -513,10 +526,10 @@
       CORE_ADDR addr;
       int i;
 
-      first_dont_print
-	= (CORE_ADDR *) obstack_base (&dont_print_statmem_obstack);
-      i = (CORE_ADDR *) obstack_next_free (&dont_print_statmem_obstack)
-	- first_dont_print;
+      first_dont_print =
+	(CORE_ADDR *)obstack_base (&dont_print_statmem_obstack);
+      i	=
+	obstack_object_size (&dont_print_statmem_obstack) / sizeof(CORE_ADDR);
 
       while (--i >= 0)
 	{
Index: gdb/testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.2119
diff -u -r1.2119 ChangeLog
--- gdb/testsuite/ChangeLog	2 Feb 2010 16:47:14 -0000	1.2119
+++ gdb/testsuite/ChangeLog	2 Feb 2010 21:53:55 -0000
@@ -1,3 +1,10 @@
+2010-0-02  Chris Moller  <cmoller@redhat.com>
+
+       PR gdb/9067
+       * gdb.cp/pr9067.exp:  New
+       * gdb.cp/pr9067.cc:   New
+       * gdb.cp/Makefile.in (EXECUTABLES): Add pr9067
+
 2010-02-02  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/member-ptr-forwardref.exp: Update expected result for
Index: gdb/testsuite/gdb.cp/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/Makefile.in,v
retrieving revision 1.7
diff -u -r1.7 Makefile.in
--- gdb/testsuite/gdb.cp/Makefile.in	10 Dec 2009 20:57:10 -0000	1.7
+++ gdb/testsuite/gdb.cp/Makefile.in	2 Feb 2010 21:53:56 -0000
@@ -4,7 +4,7 @@
 EXECUTABLES = ambiguous annota2 anon-union cplusfuncs cttiadd \
 	derivation inherit local member-ptr method misc \
         overload ovldbreak ref-typ ref-typ2 templates userdef virtfunc namespace \
-	ref-types ref-params method2 pr9594 gdb2495 virtfunc2
+	ref-types ref-params method2 pr9594 gdb2495 virtfunc2 pr9067
 
 all info install-info dvi install uninstall installcheck check:
 	@echo "Nothing to be done for $@..."
Index: gdb/testsuite/gdb.cp/pr9067.cc
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr9067.cc
diff -N gdb/testsuite/gdb.cp/pr9067.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr9067.cc	2 Feb 2010 21:53:56 -0000
@@ -0,0 +1,17 @@
+struct B;
+
+struct A {
+  static B b;
+};
+
+struct B {
+  A a;
+};
+
+B A::b;
+B b;
+
+int main(int,char **)
+{
+  return 0;
+}
Index: gdb/testsuite/gdb.cp/pr9067.exp
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr9067.exp
diff -N gdb/testsuite/gdb.cp/pr9067.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr9067.exp	2 Feb 2010 21:53:56 -0000
@@ -0,0 +1,53 @@
+# Copyright 2009, 2010 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/>.
+
+set nl		"\[\r\n\]+"
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib "cp-support.exp"
+
+set testfile "pr9067"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {c++ debug}] != "" } {
+     untested pr9067.exp
+     return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+send_gdb "print b\n"
+
+gdb_expect {
+    -re "same as static member" {
+	pass "print b"
+    }
+    timeout { fail "(timeout) print b" }
+}
+
+
+gdb_exit
+return 0
+

[-- Attachment #3: pr9067-summary.diff --]
[-- Type: text/plain, Size: 1306 bytes --]

--- gdb-virgin.sum	2010-02-02 16:07:43.278040349 -0500
+++ gdb-patched.sum	2010-02-02 16:41:59.925040099 -0500
@@ -1,4 +1,4 @@
-Test Run By moller on Tue Feb  2 15:58:30 2010
+Test Run By moller on Tue Feb  2 16:32:45 2010
 Native configuration is i686-pc-linux-gnu
 
 		=== gdb tests ===
@@ -11501,6 +11501,8 @@
 Running ../../../src/gdb/testsuite/gdb.cp/pr-574.exp ...
 PASS: gdb.cp/pr-574.exp: continue to breakpoint: end of constructors
 PASS: gdb.cp/pr-574.exp: PR gdb/574
+Running ../../../src/gdb/testsuite/gdb.cp/pr9067.exp ...
+PASS: gdb.cp/pr9067.exp: print b
 Running ../../../src/gdb/testsuite/gdb.cp/pr9631.exp ...
 PASS: gdb.cp/pr9631.exp: continue to breakpoint: after bar tender is initialized
 PASS: gdb.cp/pr9631.exp: print tender
@@ -15509,7 +15511,7 @@
 PASS: gdb.threads/watchthreads2.exp: all threads started
 PASS: gdb.threads/watchthreads2.exp: watch x
 PASS: gdb.threads/watchthreads2.exp: set var test_ready = 1
-PASS: gdb.threads/watchthreads2.exp: all threads incremented x
+KFAIL: gdb.threads/watchthreads2.exp: gdb can drop watchpoints in multithreaded app (PRMS: gdb/10116)
 Running ../../../src/gdb/testsuite/gdb.trace/actions.exp ...
 PASS: gdb.trace/actions.exp: 5.1a: set three tracepoints, no actions
 PASS: gdb.trace/actions.exp: 5.1b: set actions for first tracepoint

Re: Patch for PR 10728

[Tom Tromey]

Chris> +	* c-lang.h:

Empty entry; it is customary to list the new names, e.g., from an
earlier patch:

	* c-lang.h (cp_print_value_fields_rtti): Declare.

Chris> +extern void cp_initialise_statmem_stack();
Chris> +extern void cp_free_statmem_stack();

Space before open parens.  Use (void), not ().
These occur several times.

Chris> -  return val_print (val_type, value_contents_all (val),
Chris> +  {
Chris> +    int rc;
Chris> +
Chris> +    cp_initialise_statmem_stack();

initialize, with a z.

Chris> +    rc = val_print (val_type, value_contents_all (val),
Chris>  		    value_embedded_offset (val),
Chris>  		    value_address (val),
Chris>  		    stream, 0, &opts, current_language);
Chris> +
Chris> +    cp_free_statmem_stack();

There are other calls to val_print in this function.
It seems like they should all be wrapped.

If cp_free_statmem_stack must be called, then you need a cleanup.
However, does it really need to be called?  IMO it is ok to keep a
little data on this obstack in between calls as long as it is cleared at
the next call (so that we don't leak an unbounded amount over time).

It isn't clear to me from this patch or the explanation that 
cp_initialise_statmem_stack is called at every relevant entry point.
In fact it would be better not to have to do that, because auditing
this call hierarchy is tricky.

E.g., m2-valprint.c has a call to cp_print_value_fields.  Does that
matter?

I didn't read the old code in depth; is there a way to make this
self-initializing?

Chris> +/* initialise to empty the static member stack */

Comments should start with a capital and end with a period & two
spaces.  See the GNU Coding Standards.

Chris> +      void * statmem_obstack_top = NULL;

No space after the "*".

Chris> -      first_dont_print
Chris> -	= (CORE_ADDR *) obstack_base (&dont_print_statmem_obstack);
Chris> -      i = (CORE_ADDR *) obstack_next_free (&dont_print_statmem_obstack)
Chris> -	- first_dont_print;
Chris> +      first_dont_print =
Chris> +	(CORE_ADDR *)obstack_base (&dont_print_statmem_obstack);
Chris> +      i	=
Chris> +	obstack_object_size (&dont_print_statmem_obstack) / sizeof(CORE_ADDR);
 
The formatting is weird here; the first statement doesn't appear to have
changed, but has gratuitous formatting changes, and the second statement
has a tab before the "=".

Chris> +send_gdb "print b\n"
Chris> +
Chris> +gdb_expect {
Chris> +    -re "same as static member" {
Chris> +	pass "print b"
Chris> +    }
Chris> +    timeout { fail "(timeout) print b" }
Chris> +}

Don't use sent + gdb_expect.  Use gdb_test.

Tom

Re: Patch for PR 10728

[Chris Moller]

On 02/02/10 18:57, Tom Tromey wrote:
>
> There are other calls to val_print in this function.
> It seems like they should all be wrapped.
>    

I was basically applying the minimum-tinkering rule, but I'll wrap the 
other instances.

> If cp_free_statmem_stack must be called, then you need a cleanup.
> However, does it really need to be called?  IMO it is ok to keep a
> little data on this obstack in between calls as long as it is cleared at
> the next call (so that we don't leak an unbounded amount over time).
>    

It's not a matter of leaving a little data around, it's that the obstack 
has to be guaranteed to have been initialised (obstack_init()) and that 
it's empty.

I guess the obstack_init could by moved to _initialize_cp_valprint() as 
part of and obstack_begin(), and, actually that would likely be a good 
idea.  I'd mis-remembered that obstack_init doesn't allocate anything 
until the first grow--I just checked and it actually does allocate space 
which, so far as I can tell, can only be recovered with an 
obstack_finish.  But there still has to be a means of guaranteeing the 
obstack is empty.

Is there a cleanup for stuff that gets done in _initialize_cp_valprint()?

> It isn't clear to me from this patch or the explanation that
> cp_initialise_statmem_stack is called at every relevant entry point.
>    

The problem is I don't know which eps are relevant.  Does anything other 
than C/C++ have the statics problem?

> In fact it would be better not to have to do that, because auditing
> this call hierarchy is tricky.
>
> E.g., m2-valprint.c has a call to cp_print_value_fields.  Does that
> matter?
>    

I honestly don't know--all this arm-waving is explicitly to fix statics 
in C, and possibly exclusively C++.   I haven't much of a clue what the 
m2-* code is for (Modular 2?) and less of an idea if it needs to deal 
with statics.

> I didn't read the old code in depth; is there a way to make this
> self-initializing?
>    

I tried to find a way to do that, but just a couple of steps downstream 
from where I have the initialisation (at cp_print_value_fields) the code 
starts recursing.  The obstack stuff /has/ to be initialised and/or 
guaranteed clear, and that has to happen before the recursion starts.   
I could have stuck the initialisation in c_val_print, but that gets 
called from scm-valprint.c and jv-valprint.c, but I didn't want to 
tinker with otherwise unaffected code paths.

> Chris>  -      first_dont_print
> Chris>  -	= (CORE_ADDR *) obstack_base (&dont_print_statmem_obstack);
> Chris>  -      i = (CORE_ADDR *) obstack_next_free (&dont_print_statmem_obstack)
> Chris>  -	- first_dont_print;
> Chris>  +      first_dont_print =
> Chris>  +	(CORE_ADDR *)obstack_base (&dont_print_statmem_obstack);
> Chris>  +      i	=
> Chris>  +	obstack_object_size (&dont_print_statmem_obstack) / sizeof(CORE_ADDR);
>
> The formatting is weird here; the first statement doesn't appear to have
> changed, but has gratuitous formatting changes,

I just thought it made the stmts more readable.

Re: Patch for PR 10728

[Chris Moller]

On 02/02/10 18:57, Tom Tromey wrote:
> I didn't read the old code in depth; is there a way to make this
> self-initializing?
>    

Actually, it just occurred to me, the top level of the recursion is at 
cp_print_value_fields and the only time the recurse parm is zero is 
probably at top-level.  I expect I /can/ make it self-initialising and 
self-cleanering-uppering...

Re: Patch for PR 10728

[Tom Tromey]

Chris> Is there a cleanup for stuff that gets done in
Chris> _initialize_cp_valprint()?

There may be, but I wouldn't bother with it.
It is ok to initialize an obstack and then leave it around forever.

Chris> The problem is I don't know which eps are relevant.  Does anything
Chris> other than C/C++ have the statics problem?

Yeah, Java does at least.  I'm not sure about other languages, but I
assume so.

However, Java doesn't call into the code you're modifying, so it
shouldn't matter for this patch.

>> The formatting is weird here; the first statement doesn't appear to have
>> changed, but has gratuitous formatting changes,

Chris> I just thought it made the stmts more readable.

Usually we recommend formatting changes as separate patches.  Personally
I'm more inclined to just leave bad formatting alone, to avoid adding
noise to the history, but that's just me.  Also, the result still has to
conform to the GNU standards.

Tom

Re: Patch for PR 10728

[Tom Tromey]

>>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes:

Chris> +2010-01-25  Chris Moller  <cmoller@redhat.com>
Chris> +
Chris> +	PR gdb/10728:
Chris> +	* valarith.c (value_ptrdiff): Added a test for a zero type length,
Chris> +	warn if found, and assume length = 1.

Ok, thanks.

Tom

Re: Patch for PR 10728

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 255 bytes --]

Originally sent on 28 January.

On 01/28/10 15:03, Tom Tromey wrote:
>>>>>> "Chris" == Chris Moller<cmoller@redhat.com>  writes:
>>>>>>
>
> Chris>  Do you want me to send you a new patch?  Or just commit it?
>
> Send a new patch.
>

Here 'tis.

> Tom
>



[-- Attachment #2: pr10728.patch --]
[-- Type: text/plain, Size: 6012 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.11289
diff -u -r1.11289 ChangeLog
--- gdb/ChangeLog	25 Jan 2010 19:31:23 -0000	1.11289
+++ gdb/ChangeLog	28 Jan 2010 20:19:38 -0000
@@ -1,3 +1,9 @@
+2010-01-25  Chris Moller  <cmoller@redhat.com>
+
+	PR gdb/10728:
+	* valarith.c (value_ptrdiff): Added a test for a zero type length,
+	warn if found, and assume length = 1.
+
 2010-01-25  Tom Tromey  <tromey@redhat.com>
 
 	PR gdb/11049:
Index: gdb/valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.79
diff -u -r1.79 valarith.c
--- gdb/valarith.c	20 Jan 2010 18:06:15 -0000	1.79
+++ gdb/valarith.c	28 Jan 2010 20:19:39 -0000
@@ -122,6 +122,13 @@
 an integer nor a pointer of the same type."));
 
   sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE (type1)));
+  if (sz == 0) 
+    {
+      warning (_("Type size unknown, assuming 1. "
+               "Try casting to a known type, or void *."));
+      sz = 1;
+    }
+
   return (value_as_long (arg1) - value_as_long (arg2)) / sz;
 }
 
Index: gdb/testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.2101
diff -u -r1.2101 ChangeLog
--- gdb/testsuite/ChangeLog	25 Jan 2010 19:31:23 -0000	1.2101
+++ gdb/testsuite/ChangeLog	28 Jan 2010 20:19:55 -0000
@@ -1,3 +1,12 @@
+2010-01-26  Chris Moller  <cmoller@redhat.com>
+
+	PR gdb/10728
+	* gdb.cp/pr10728-x.h: New file.
+	* gdb.cp/pr10728-x.cc: New file.
+	* gdb.cp/pr10728-y.cc: New file.
+	* gdb.cp/pr10728.exp: New file.
+	* gdb.cp/Makefile.in (EXECUTABLES): Add pr10728
+	
 2010-01-25  Tom Tromey  <tromey@redhat.com>
 
 	PR gdb/11049:
Index: gdb/testsuite/gdb.cp/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/Makefile.in,v
retrieving revision 1.7
diff -u -r1.7 Makefile.in
--- gdb/testsuite/gdb.cp/Makefile.in	10 Dec 2009 20:57:10 -0000	1.7
+++ gdb/testsuite/gdb.cp/Makefile.in	28 Jan 2010 20:19:56 -0000
@@ -4,7 +4,7 @@
 EXECUTABLES = ambiguous annota2 anon-union cplusfuncs cttiadd \
 	derivation inherit local member-ptr method misc \
         overload ovldbreak ref-typ ref-typ2 templates userdef virtfunc namespace \
-	ref-types ref-params method2 pr9594 gdb2495 virtfunc2
+	ref-types ref-params method2 pr9594 gdb2495 virtfunc2 pr10728
 
 all info install-info dvi install uninstall installcheck check:
 	@echo "Nothing to be done for $@..."
Index: gdb/testsuite/gdb.cp/pr10728-x.cc
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728-x.cc
diff -N gdb/testsuite/gdb.cp/pr10728-x.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728-x.cc	28 Jan 2010 20:19:56 -0000
@@ -0,0 +1,7 @@
+#include "pr10728-x.h"
+
+int main()
+{
+  X* x = y();
+  return 0;		// marker 1
+}
Index: gdb/testsuite/gdb.cp/pr10728-x.h
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728-x.h
diff -N gdb/testsuite/gdb.cp/pr10728-x.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728-x.h	28 Jan 2010 20:19:56 -0000
@@ -0,0 +1,9 @@
+struct Y;
+struct X
+{
+  Y* y1;
+  Y* y2;
+};
+
+X* y();
+
Index: gdb/testsuite/gdb.cp/pr10728-y.cc
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728-y.cc
diff -N gdb/testsuite/gdb.cp/pr10728-y.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728-y.cc	28 Jan 2010 20:19:56 -0000
@@ -0,0 +1,11 @@
+#include "pr10728-x.h"
+struct Y{};
+
+X* y()
+{
+  static X xx;
+  static Y yy;
+  xx.y1 = &yy;
+  xx.y2 = xx.y1+1;
+  return &xx;
+}
Index: gdb/testsuite/gdb.cp/pr10728.exp
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728.exp
diff -N gdb/testsuite/gdb.cp/pr10728.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728.exp	28 Jan 2010 20:19:56 -0000
@@ -0,0 +1,66 @@
+# Copyright 2010 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/>.
+
+# This file is part of the gdb testsuite
+
+set nl		"\[\r\n\]+"
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib "cp-support.exp"
+
+set testfile "pr10728"
+set srcfile ${testfile}-x.cc
+set tfx ${testfile}-x
+set tfy ${testfile}-y
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${tfy}.cc" "${tfy}.o" object {c++}] != "" } {
+     untested pr10728.exp
+     return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${tfx}.cc" "${tfx}.o" object {debug c++}] != "" } {
+     untested pr10728.exp
+     return -1
+}
+
+if  { [gdb_compile "${tfx}.o ${tfy}.o" ${binfile} executable {debug c++}] != "" } {
+     untested pr10728.exp
+     return -1
+}
+
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# set a breakpoint at the return stmt
+
+gdb_breakpoint [gdb_get_line_number "marker 1"]
+gdb_continue_to_breakpoint "marker 1"
+
+gdb_test "print x->y2 - x->y1" "warning: Type size unknown, assuming 1\. Try casting to a known type, or void \*\.\[^=\]*= 1"
+
+gdb_exit
+return 0
+
+

Re: Patch for PR 10728

[Tom Tromey]

>>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes:

Chris> Do you want me to send you a new patch?  Or just commit it?

Send a new patch.

Tom

Re: Patch for PR 10728

[Chris Moller]

On 01/28/10 13:09, Tom Tromey wrote:
>>>>>> "Chris" == Chris Moller<cmoller@redhat.com>  writes:
>>>>>>              
>
>    

> Chris>  +     untested psmang.exp
>
> Wrong name here, this occurs a few times.
>    

Oops--the down side of cut'n'paste...

> Chris>  +send_gdb "print x->y2 - x->y1\n"
> Chris>  +
> Chris>  +gdb_expect {
>
> I think we are trying to avoid send_gdb+gdb_expect, as much as possible.
> This test can comfortably be written using gdb_test, so do that.
>    

I tried gdb_test to start with, but it didn't seem to be doing the right 
thing.  I just went back to it and it's doing okay now--my original 
regex was probably screwed up.

Do you want me to send you a new patch?  Or just commit it?

> Tom
>    

Re: Patch for PR 10728

[Tom Tromey]

>>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes:

Chris> The attached stuff is a patch for PR 10728:
Chris> http://sourceware.org/bugzilla/show_bug.cgi?id=10728 "infinite loop
Chris> evaluating pointer difference w/o complete debug info"

Thanks.

Chris> The bug was in valarith.c : value_ptrdiff--if the type structs showed
Chris> size == 0, things came unstuck.  This patch fixes that by issuing a
Chris> warning and assuming size = 1.

This seems reasonable enough to me.  Maybe other maintainers would
prefer an error?  I think there are precedents either way.

Chris> Index: gdb/testsuite/gdb.cp/pr10728.exp

Chris> +     untested psmang.exp

Wrong name here, this occurs a few times.

Chris> +send_gdb "print x->y2 - x->y1\n"
Chris> +
Chris> +gdb_expect {

I think we are trying to avoid send_gdb+gdb_expect, as much as possible.
This test can comfortably be written using gdb_test, so do that.

Tom

Patch for PR 10728

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

The attached stuff is a patch for PR 10728: 
http://sourceware.org/bugzilla/show_bug.cgi?id=10728 "infinite loop 
evaluating pointer difference w/o complete debug info"

The bug was in valarith.c : value_ptrdiff--if the type structs showed 
size == 0, things came unstuck.  This patch fixes that by issuing a 
warning and assuming size = 1.

The patch includes testcase source files and and an expects file.

The gdb.sum diff between the unpatched and patched make check is also 
attached.

Chris Moller,
Red Hat

[-- Attachment #2: pr10728.patch --]
[-- Type: text/plain, Size: 6128 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.11289
diff -u -r1.11289 ChangeLog
--- gdb/ChangeLog	25 Jan 2010 19:31:23 -0000	1.11289
+++ gdb/ChangeLog	27 Jan 2010 18:56:27 -0000
@@ -1,3 +1,9 @@
+2010-01-25  Chris Moller  <cmoller@redhat.com>
+
+	PR gdb/10728:
+	* valarith.c (value_ptrdiff): Added a test for a zero type length,
+	warn if found, and assume length = 1.
+
 2010-01-25  Tom Tromey  <tromey@redhat.com>
 
 	PR gdb/11049:
Index: gdb/valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.79
diff -u -r1.79 valarith.c
--- gdb/valarith.c	20 Jan 2010 18:06:15 -0000	1.79
+++ gdb/valarith.c	27 Jan 2010 18:56:29 -0000
@@ -122,6 +122,13 @@
 an integer nor a pointer of the same type."));
 
   sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE (type1)));
+  if (sz == 0) 
+    {
+      warning (_("Type size unknown, assuming 1. "
+               "Try casting to a known type, or void *."));
+      sz = 1;
+    }
+
   return (value_as_long (arg1) - value_as_long (arg2)) / sz;
 }
 
Index: gdb/testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.2101
diff -u -r1.2101 ChangeLog
--- gdb/testsuite/ChangeLog	25 Jan 2010 19:31:23 -0000	1.2101
+++ gdb/testsuite/ChangeLog	27 Jan 2010 18:56:45 -0000
@@ -1,3 +1,12 @@
+2010-01-26  Chris Moller  <cmoller@redhat.com>
+
+	PR gdb/10728
+	* gdb.cp/pr10728-x.h: New file.
+	* gdb.cp/pr10728-x.cc: New file.
+	* gdb.cp/pr10728-y.cc: New file.
+	* gdb.cp/pr10728.exp: New file.
+	* gdb.cp/Makefile.in (EXECUTABLES): Add pr10728
+	
 2010-01-25  Tom Tromey  <tromey@redhat.com>
 
 	PR gdb/11049:
Index: gdb/testsuite/gdb.cp/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/Makefile.in,v
retrieving revision 1.7
diff -u -r1.7 Makefile.in
--- gdb/testsuite/gdb.cp/Makefile.in	10 Dec 2009 20:57:10 -0000	1.7
+++ gdb/testsuite/gdb.cp/Makefile.in	27 Jan 2010 18:56:45 -0000
@@ -4,7 +4,7 @@
 EXECUTABLES = ambiguous annota2 anon-union cplusfuncs cttiadd \
 	derivation inherit local member-ptr method misc \
         overload ovldbreak ref-typ ref-typ2 templates userdef virtfunc namespace \
-	ref-types ref-params method2 pr9594 gdb2495 virtfunc2
+	ref-types ref-params method2 pr9594 gdb2495 virtfunc2 pr10728
 
 all info install-info dvi install uninstall installcheck check:
 	@echo "Nothing to be done for $@..."
Index: gdb/testsuite/gdb.cp/pr10728-x.cc
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728-x.cc
diff -N gdb/testsuite/gdb.cp/pr10728-x.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728-x.cc	27 Jan 2010 18:56:45 -0000
@@ -0,0 +1,7 @@
+#include "pr10728-x.h"
+
+int main()
+{
+  X* x = y();
+  return 0;		// marker 1
+}
Index: gdb/testsuite/gdb.cp/pr10728-x.h
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728-x.h
diff -N gdb/testsuite/gdb.cp/pr10728-x.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728-x.h	27 Jan 2010 18:56:45 -0000
@@ -0,0 +1,9 @@
+struct Y;
+struct X
+{
+  Y* y1;
+  Y* y2;
+};
+
+X* y();
+
Index: gdb/testsuite/gdb.cp/pr10728-y.cc
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728-y.cc
diff -N gdb/testsuite/gdb.cp/pr10728-y.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728-y.cc	27 Jan 2010 18:56:45 -0000
@@ -0,0 +1,11 @@
+#include "pr10728-x.h"
+struct Y{};
+
+X* y()
+{
+  static X xx;
+  static Y yy;
+  xx.y1 = &yy;
+  xx.y2 = xx.y1+1;
+  return &xx;
+}
Index: gdb/testsuite/gdb.cp/pr10728.exp
===================================================================
RCS file: gdb/testsuite/gdb.cp/pr10728.exp
diff -N gdb/testsuite/gdb.cp/pr10728.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/pr10728.exp	27 Jan 2010 18:56:45 -0000
@@ -0,0 +1,73 @@
+# Copyright 2010 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/>.
+
+# This file is part of the gdb testsuite
+
+set nl		"\[\r\n\]+"
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib "cp-support.exp"
+
+set testfile "pr10728"
+set srcfile ${testfile}-x.cc
+set tfx ${testfile}-x
+set tfy ${testfile}-y
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${tfy}.cc" "${tfy}.o" object {c++}] != "" } {
+     untested psmang.exp
+     return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${tfx}.cc" "${tfx}.o" object {debug c++}] != "" } {
+     untested psmang.exp
+     return -1
+}
+
+if  { [gdb_compile "${tfx}.o ${tfy}.o" ${binfile} executable {debug c++}] != "" } {
+     untested psmang.exp
+     return -1
+}
+
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# set a breakpoint at the return stmt
+
+gdb_breakpoint [gdb_get_line_number "marker 1"]
+gdb_continue_to_breakpoint "marker 1"
+
+send_gdb "print x->y2 - x->y1\n"
+
+gdb_expect {
+  -re "warning: Type size unknown, assuming 1\. Try casting to a known type, or void \*\.\[^=\]*= 1" {
+     pass "print x->y2 - x->y1"
+  }
+  timeout { fail "(timeout) print x->y2 - x->y1" }
+}
+
+gdb_exit
+return 0
+
+

[-- Attachment #3: pr10728-check.diff --]
[-- Type: text/plain, Size: 2504 bytes --]

--- gdb-virgin.sum	2010-01-25 22:03:19.916165023 -0500
+++ gdb-patched2.sum	2010-01-26 22:24:47.539164893 -0500
@@ -1,4 +1,4 @@
-Test Run By moller on Mon Jan 25 21:55:54 2010
+Test Run By moller on Tue Jan 26 22:06:34 2010
 Native configuration is i686-pc-linux-gnu
 
 		=== gdb tests ===
@@ -11460,6 +11460,9 @@
 Running ../../../src/gdb/testsuite/gdb.cp/pr-574.exp ...
 PASS: gdb.cp/pr-574.exp: continue to breakpoint: end of constructors
 PASS: gdb.cp/pr-574.exp: PR gdb/574
+Running ../../../src/gdb/testsuite/gdb.cp/pr10728.exp ...
+PASS: gdb.cp/pr10728.exp: continue to breakpoint: marker 1
+PASS: gdb.cp/pr10728.exp: print x->y2 - x->y1
 Running ../../../src/gdb/testsuite/gdb.cp/pr9631.exp ...
 PASS: gdb.cp/pr9631.exp: continue to breakpoint: after bar tender is initialized
 PASS: gdb.cp/pr9631.exp: print tender
@@ -12530,7 +12533,7 @@
 PASS: gdb.mi/mi-nsmoribund.exp: resume all, thread specific breakpoint
 PASS: gdb.mi/mi-nsmoribund.exp: hit thread specific breakpoint
 PASS: gdb.mi/mi-nsmoribund.exp: thread state: all running except the breakpoint thread
-FAIL: gdb.mi/mi-nsmoribund.exp: unexpected stop
+PASS: gdb.mi/mi-nsmoribund.exp: resume all, program exited normally
 Running ../../../src/gdb/testsuite/gdb.mi/mi-nsthrexec.exp ...
 PASS: gdb.mi/mi-nsthrexec.exp: successfully compiled posix threads test case
 PASS: gdb.mi/mi-nsthrexec.exp: breakpoint at main
@@ -15083,7 +15086,8 @@
 PASS: gdb.threads/local-watch-wrong-thread.exp: local watchpoint still triggers
 PASS: gdb.threads/local-watch-wrong-thread.exp: let thread_function0 return
 PASS: gdb.threads/local-watch-wrong-thread.exp: breakpoint on thread_function0's caller
-PASS: gdb.threads/local-watch-wrong-thread.exp: local watchpoint automatically deleted
+ERROR: Process no longer exists
+UNRESOLVED: gdb.threads/local-watch-wrong-thread.exp: local watchpoint automatically deleted
 Running ../../../src/gdb/testsuite/gdb.threads/manythreads.exp ...
 PASS: gdb.threads/manythreads.exp: successfully compiled posix threads test case
 PASS: gdb.threads/manythreads.exp: set print sevenbit-strings
@@ -15640,11 +15644,11 @@
 
 		=== gdb Summary ===
 
-# of expected passes		14847
-# of unexpected failures	26
+# of expected passes		14849
+# of unexpected failures	25
 # of expected failures		41
 # of untested testcases		3
-# of unresolved testcases	1
+# of unresolved testcases	2
 # of unsupported tests		61
 /home/moller/tinkering/fsfgdb-10728/build/gdb/testsuite/../../gdb/gdb version  7.0.50.20100125-cvs -nw -nx