Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch/testsuite/c++] test script for PR c++/186
@ 2003-11-23 22:05 Michael Elizabeth Chastain
  2003-11-23 22:08 ` Daniel Jacobowitz
  2003-11-24 17:23 ` David Carlton
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Elizabeth Chastain @ 2003-11-23 22:05 UTC (permalink / raw)
  To: gdb-patches

This is a new test script.  It's for PR gdb/186:

  http://sources.redhat.com/gdb/bugs/186
  gdb have problems with C++ casting

Here is a summary of the code:

  class A;
  class B : public A;

  int main (void)
  {
    B beta, *beta_p;
    beta_p = β
    ...
  } // breakpoint here

And here is a summary of the *.exp file:

  (gdb) break "breakpoint here"
  (gdb) print beta
  ... okay ...
  (gdb) print * beta_p
  ... gdb prints bogus data ...

The issue is that the user is accessing a destroyed object through a
pointer.  Because the object has been destroyed, its vptr has been
degraded to its most primitive base class.  gdb gets confused and prints
incorrect data, which is always wrong.

Testing: I tested this on native i686-pc-linux-gnu with:

  gcc 2.95.3 -gdwarf-2
  gcc 2.95.3 -gstabs+
  gcc 3.3.2 -gdwarf-2
  gcc 3.3.2 -gstabs+

The results are all PASS and KFAIL, with no FAIL.

I'll put this up for a few days for comment, especially from David C.
Then I'll commit it.

Michael C

===

2003-11-23  Michael Chastain  <mec.gnu@mindspring.com>

	New test case for PR c++/186.
	* gdb.cp/gdb186.cc: New file.
	* gdb.cp/gdb186.exp: New file.

===

/* This testcase is part of GDB, the GNU debugger.

   Copyright 2003 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 2 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, write to the Free Software
   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.

   Please email any bugs, comments, and/or additions to this file to:
   bug-gdb@prep.ai.mit.edu  */

struct A
{
  virtual ~A ();
  int a1;
};

A::~A()
{
  ;
}

struct B : public A
{
  virtual ~B ();
  int b1;
  int b2;
};

B::~B()
{
  ;
}

void marker1 (A *)
{
  ;
}

int main (void)
{
  A *alpha_p;
  B beta, *beta_p;
  beta.a1 = 100;
  beta.b1 = 200;
  beta.b2 = 201;
  alpha_p = &beta;
  beta_p  = (B *) alpha_p;
  marker1 (alpha_p);
  marker1 (beta_p);
  return 0;  // marker return 0
} // marker close brace

===

# Copyright 2003 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 2 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, write to the Free Software
# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  

# Test for PR gdb/186.
#
# This is a problem with C++ casting on a dead object.  The object's
# type (its vptr) has been degraded by its destructors.

if $tracelevel then {
    strace $tracelevel
    }

if { [skip_cplus_tests] } { continue }

set prms_id 0
set bug_id 0

set testfile "gdb186"
set srcfile ${testfile}.cc
set binfile ${objdir}/${subdir}/${testfile}

if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
    gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
}

gdb_exit
gdb_start
gdb_reinitialize_dir $srcdir/$subdir
gdb_load ${binfile}

if ![runto_main] then {
    perror "couldn't run to main"
    continue
}

gdb_test "break [gdb_get_line_number "marker return 0"]" \
    "Breakpoint.*at.* file .*" ""

gdb_test "continue" "Breakpoint .* at .*" ""

# This part should be simple and good.

gdb_test "print beta" \
    "= {.*a1 = 100.*b1 = 200.*b2 = 201}" \
    "print beta at marker return 0"

gdb_test "print * beta_p" \
    "= {.*a1 = 100.*b1 = 200.*b2 = 201}" \
    "print * beta_p at marker return 0"

gdb_test "print * (B *) alpha_p" \
    "= {.*a1 = 100.*b1 = 200.*b2 = 201}" \
    "print * (B *) alpha_p at marker return 0"

gdb_test "print * alpha_p" \
    "= {.*a1 = 100.*}" \
    "print * alpha_p at marker return 0"

# All that again, at the close brace.
#
# This is where the real problem happens.  "beta" has been destroyed!
# The gcc code for B::~B changes the type (the vptr) of "beta" from
# "B" to "A" so that B::~B can call A::~A.  Every C++ compiler has to
# do something like this, in case A::~A calls a virtual function.
#
# It would also be legal for a C++ compiler to put some kind of sentinel
# value in beta.vptr so that it's not even an "A" any more.  But I am
# not seeing that with gcc.
#
# So, the user is trying to print a structure of type "B", but there is
# only an "A" in memory, and the user might not even realize that.

gdb_test "break [gdb_get_line_number "marker close brace"]" \
    "Breakpoint.*at.* file .*" ""

gdb_test "continue" "Breakpoint .* at .*" ""

gdb_test_multiple "print beta"  "print beta at marker close brace" {
    -re "= {.*a1 = 100.*b1 = 200.*b2 = 201}\r\n$gdb_prompt "  {
	# the data fields should still be okay even though beta is dead
	pass "print beta at marker close brace"
    }
}

# "* beta_p" has static type "B", but there is probably only an "A"
# in memory after B::~B does its thing.  The user may not realize
# this so gdb should be careful to be informative here.
#
# TODO: add another PASS case for this depending on what David C
# thinks the correct output should be.

gdb_test_multiple "print * beta_p" "print * beta_p at marker close brace" {
    -re "= {.*a1 = 100.*b1 = 200.*b2 = 201}\r\n$gdb_prompt $" {
	# This is not happening, but it would be okay.
	pass "print * beta_p at marker close brace"
    }
    -re "= {.*a1 = 100}\r\n$gdb_prompt $" {
	# This is not happening, but it would be okay,
	# although a little surprising.
	pass "print * beta_p at marker close brace"
    }
    -re "= {.*a1 = 100.*b1 = .*b2 = .*}\r\n$gdb_prompt $" {
	# gcc 2.95.3 -gstabs+
	# gcc 3.3.2 -gdwarf-2
	# gcc 3.3.2 -gstabs+
	# Someone might argue that gdb is not buggy here but I think it is.
	kfail "gdb/186" "print * beta_p at marker close brace #1"
    }
    -re "= {.*a1 = .*b1 = .*b2 = .*}\r\n$gdb_prompt $" {
	# gcc 2.95.3 -gdwarf-2
	# gdb does not even get the value of a1 right!
	kfail "gdb/186" "print * beta_p at marker close brace #2"
    }
}

# Another way to pose the same question.

gdb_test_multiple "print * (B *) alpha_p " "print * (B *) alpha_p at marker close brace" {
    -re "= {.*a1 = 100.*b1 = 200.*b2 = 201}\r\n$gdb_prompt $" {
	# This is not happening, but it would be okay.
	pass "print * (B *) alpha_p at marker close brace"
    }
    -re "= {.*a1 = 100}\r\n$gdb_prompt $" {
	# This is not happening, but it would be okay,
	# although a little surprising.
	pass "print * (B *) alpha_p at marker close brace"
    }
    -re "= {.*a1 = 100.*b1 = .*b2 = .*}\r\n$gdb_prompt $" {
	# gcc 2.95.3 -gstabs+
	# gcc 3.3.2 -gdwarf-2
	# gcc 3.3.2 -gstabs+
	# Someone might argue that gdb is not buggy here but I think it is.
	kfail "gdb/186" "print * (B *) alpha_p at marker close brace #1"
    }
    -re "= {.*a1 = .*b1 = .*b2 = .*}\r\n$gdb_prompt $" {
	# gcc 2.95.3 -gdwarf-2
	# gdb does not even get the value of a1 right!
	kfail "gdb/186" "print * (B *) alpha_p at marker close brace #2"
    }
}

# This should work better, as long as beta is still an "A" at least.

gdb_test_multiple "print * alpha_p" "print * alpha_p at marker close brace" {
    -re "= {.*a1 = 100.*b1 = 200.*b2 = 201}\r\n$gdb_prompt $" {
	# this should not happen but if it did, it would be okay
	pass "print * alpha_p at marker close brace"
    }
    -re "= {.*a1 = .*b1 = .*b2 = .*}\r\n$gdb_prompt $" {
	# if gdb prints any values and they are WRONG, that is not okay
	kfail "gdb/186" "print * alpha_p at marker close brace #1"
    }
    -re "= {.*a1 = 100.*}\r\n$gdb_prompt $" {
	# gcc 2.95.3 -gstabs+
	# gcc 3.3.2 -gdwarf-2
	# gcc 3.3.2 -gstabs+
	# this is okay
	pass "print * alpha_p at marker close brace"
    }
    -re "= {.*a1 = .*}\r\n$gdb_prompt $" {
	# gcc 2.95.3 -gdwarf-2
	kfail "gdb/186" "print * alpha_p at marker close brace #2"
    }
}


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch/testsuite/c++] test script for PR c++/186
@ 2003-11-23 23:00 Michael Elizabeth Chastain
  2003-11-24 17:25 ` David Carlton
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Elizabeth Chastain @ 2003-11-23 23:00 UTC (permalink / raw)
  To: drow, gdb-patches

Well, you have a point.  The gdb user is attempting to look at an object
after its last access from the source code.  A C++ compiler has always
had the right to fool around with such memory (so does a C compiler;
gcc optimizes away local variables often).

A test script can only observe behavior.  When gdb prints
"b1 = 12345678", the test script doesn't know if it's dealing with a
conforming compiler that scribbled on the memory, or a gdb bug that
causes gdb to print a value that is not in memory.

We kinda rely on compilers being simple.  As compilers grow
more optimizations we get breakages like store.exp and have to fix them.
Imagine a bug-catching compiler that scribbles bait-patterns into dead
memory -- that would break this script.

Hmmm, my test would be more clear if I made B::~B write something into
the data fields.  I will do that.  And write some more comments about
this.

I will also add some tests like this:

  A alpha;
  ...
  (gdb) print (B *) &alpha;

I think that's actually the heart and these issues with destructors
and scope are peripheral.  (But the destructor issue is interesting
because the program is doing something important in memory that is not
obvious to the programmer).

Michael C


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch/testsuite/c++] test script for PR c++/186
@ 2003-11-24 16:41 Michael Elizabeth Chastain
  2003-11-24 17:27 ` David Carlton
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Elizabeth Chastain @ 2003-11-24 16:41 UTC (permalink / raw)
  To: gdb-patches

Sigh, I have to put this patch on hold for a little while.

I was adding more tests to it and I ran into a test where
gdb really does the wrong thing with gcc 2.95.3.  That is:

  (gdb) ptype alpha
  type = class A {
    public:
      int a1;

      virtual ~A(void);
  }
  (gdb) print alpha
  $1 = {a1 = 100, _vptr.A = 0x804e198}
  (gdb) print *&alpha
  $2 = {a1 = 942760548, _vptr.A = 0x63632e36}
  (gdb) print *&alpha
  $3 = {a1 = -1073743984, _vptr.A = 0x8}
  (gdb) print *&alpha
  $4 = {a1 = -1073750008, _vptr.A = 0x2c}

I think the problem is somewhere around where gnuv2_value_rtti_type
calls lookup_typename.  Before David's big namespace patch on
2003-09-11, lookup_typename returned a TYPE_CODE_STRUCT type to
gnuv2_value_rtti_type.  Now lookup_typename is returning a
TYPE_CODE_NAMESPACE type and as you can see, gdb is printing random
weird values when it goes through gnuv2_value_rtti_type.

So I am gonna pursue that bug now, because it is simpler,
and put this patch on hold.

Michael C

===

2003-11-23  Michael Chastain  <mec.gnu@mindspring.com>

	New test case for PR c++/186.
	* gdb.cp/gdb186.cc: New file.
	* gdb.cp/gdb186.exp: New file.


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch/testsuite/c++] test script for PR c++/186
@ 2003-11-24 18:13 Michael Elizabeth Chastain
  2003-11-24 20:02 ` David Carlton
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Elizabeth Chastain @ 2003-11-24 18:13 UTC (permalink / raw)
  To: carlton; +Cc: gdb-patches

Hi David,
> I wouldn't necessarily call this incorrect/wrong, but it is somewhat
> unfortunate.  I confess, though, that the correct fix isn't at all
> obvious to me, given that normally the dynamic type is more useful
> than the static type.  Should GDB try to somehow take the supremum of
> the static type and the dynamic type?  (And what if there is no
> supremum?)  Should GDB try to remember when the user explicitly casts?
> (If so, exactly how do we want to remember that?)  Should there be an
> option to turn of RTTI usage?  (But users won't know about that.)

Well, gdb does have "set print object", which defaults to "off".

There are really two points here.  One is that gdb has to decide
which type to print.  There is the static type of the expression,
and the dynamic type in memory, and the value of "set print object"
to consider.

But beyond that level, gdb is printing bogus values.  Specifically: I've
got these two classes, "A" and "B".  "B" is derived from "A".  I've got
an object that used to be a "B", but thanks to B::~B, the dynamic type
is actually "A" by now.  When I say "print *pb", gdb prints values for
the fields of B.  But it prints bogus data, and it prints *different*
bogus data on successive calls.  It's not like gdb is choosing an
infelicitous type and forcing the target memory into that type, it's like 
gdb is dereferencing wild pointers internally.

I suspect that gdb is using the static type to decide "let's print the B
type", but is using the dynamic type to find things, without any error
checking.  Something like:

  for (i = 0; i < static_type->nfields; i++)
    {
      ... dynamic_type->fields[i] ...
    }

The dynamic type is a parent of the static type (thanks to the
destructor) so it has fewer fields.

> Maybe the best thing to do would be as follows, if A is the dynamic
> type and B is the static type, then we check to make sure that A is
> more specialized than B.

Hmmm, that would be one way of fixing the problem, I think.  Code like
the above actually works if you can assert that dynamic_type is derived
from static_type.

But I would rather that gdb have a more clear notion of which type
information it is using -- pick one type and then stick with it.
It's just asking for trouble if there is code like the above.

Michael C


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2003-11-24 20:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-11-23 22:05 [patch/testsuite/c++] test script for PR c++/186 Michael Elizabeth Chastain
2003-11-23 22:08 ` Daniel Jacobowitz
2003-11-23 22:22   ` Daniel Jacobowitz
2003-11-24 17:23 ` David Carlton
2003-11-23 23:00 Michael Elizabeth Chastain
2003-11-24 17:25 ` David Carlton
2003-11-24 16:41 Michael Elizabeth Chastain
2003-11-24 17:27 ` David Carlton
2003-11-24 18:13 Michael Elizabeth Chastain
2003-11-24 20:02 ` David Carlton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox