[RFA/RFC] Tweak for a gdb.mi test.

[RFA/RFC] Tweak for a gdb.mi test.

[Michael Snyder]

I'm gonna ask for a second pair of eyes, since I don't know MI 
very well.

What this is -- the test is examining the stack, but it is
assuming that main is the last frame.  My change allows for
one extra frame below main (eg. for '_start').

OK to check in?

Michael

2002-05-07  Michael Snyder  <msnyder@redhat.com>

	* gdb.mi/mi-stack.exp (test_stack_args_listing): Allow for
	an optional frame for the caller of main (eg. '_start').
	(test_stack_info_depth): Ditto.
	* gdb.mi/mi0-stack.exp (test_stack_args_listing): Allow for
	an optional frame for the caller of main (eg. '_start').
	(test_stack_info_depth): Ditto.

Index: mi-stack.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-stack.exp,v
retrieving revision 1.8
diff -p -r1.8 mi-stack.exp
*** mi-stack.exp	19 Aug 2001 01:23:43 -0000	1.8
--- mi-stack.exp	8 May 2002 01:13:42 -0000
*************** proc test_stack_args_listing {} {
*** 101,108 ****
      # -stack-list-arguments 1 1 3
      # -stack-list-arguments 
  
      mi_gdb_test "231-stack-list-arguments 0" \
! 	    "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \
                  "stack args listing 0"
  
      mi_gdb_test "232-stack-list-arguments 0 1 1" \
--- 101,109 ----
      # -stack-list-arguments 1 1 3
      # -stack-list-arguments 
  
+     # Allow for an optional fifth frame ("_start" or whatever...)
      mi_gdb_test "231-stack-list-arguments 0" \
! 	    "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}(,frame=\{level=\"5\",args=\\\[\\\]\}|)\\\]" \
                  "stack args listing 0"
  
      mi_gdb_test "232-stack-list-arguments 0 1 1" \
*************** proc test_stack_args_listing {} {
*** 113,120 ****
  	    "233\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\}\\\]" \
                  "stack args listing 0 1 3"
  
      mi_gdb_test "231-stack-list-arguments 1" \
! 	    "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \
                  "stack args listing 1"
  
      mi_gdb_test "232-stack-list-arguments 1 1 1" \
--- 114,122 ----
  	    "233\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\}\\\]" \
                  "stack args listing 0 1 3"
  
+     # Allow for an optional fifth frame ("_start" or whatever...)
      mi_gdb_test "231-stack-list-arguments 1" \
! 	    "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}(,frame=\{level=\"5\",args=\\\[\\\]\}|)\\\]" \
                  "stack args listing 1"
  
      mi_gdb_test "232-stack-list-arguments 1 1 1" \
*************** proc test_stack_info_depth {} {
*** 140,155 ****
      # -stack-info-depth 3
      # -stack-info-depth 99
  
      mi_gdb_test "231-stack-info-depth" \
! 	    "231\\^done,depth=\"5\"" \
                  "stack info-depth"
  
      mi_gdb_test "231-stack-info-depth 3" \
  	    "231\\^done,depth=\"3\"" \
                  "stack info-depth 3"
  
      mi_gdb_test "231-stack-info-depth 99" \
! 	    "231\\^done,depth=\"5\"" \
                  "stack info-depth 99"
  
      mi_gdb_test "231-stack-info-depth 99 99" \
--- 142,159 ----
      # -stack-info-depth 3
      # -stack-info-depth 99
  
+     # Allow for an optional fifth frame ("_start" or whatever...)
      mi_gdb_test "231-stack-info-depth" \
! 	    "231\\^done,depth=\"(5|6)\"" \
                  "stack info-depth"
  
      mi_gdb_test "231-stack-info-depth 3" \
  	    "231\\^done,depth=\"3\"" \
                  "stack info-depth 3"
  
+     # Allow for an optional fifth frame ("_start" or whatever...)
      mi_gdb_test "231-stack-info-depth 99" \
! 	    "231\\^done,depth=\"(5|6)\"" \
                  "stack info-depth 99"
  
      mi_gdb_test "231-stack-info-depth 99 99" \
Index: mi0-stack.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi0-stack.exp,v
retrieving revision 1.4
diff -p -r1.4 mi0-stack.exp
*** mi0-stack.exp	19 Aug 2001 01:23:43 -0000	1.4
--- mi0-stack.exp	8 May 2002 01:13:42 -0000
*************** proc test_stack_args_listing {} {
*** 101,108 ****
      # -stack-list-arguments 1 1 3
      # -stack-list-arguments 
  
      mi_gdb_test "231-stack-list-arguments 0" \
! 	    "231\\^done,stack-args=\{frame=\{level=\"0\",args=\{\}\},frame=\{level=\"1\",args=\{name=\"strarg\"\}\},frame=\{level=\"2\",args=\{name=\"intarg\",name=\"strarg\"\}\},frame=\{level=\"3\",args=\{name=\"intarg\",name=\"strarg\",name=\"fltarg\"\}\},frame=\{level=\"4\",args=\{\}\}\}" \
                  "stack args listing 0"
  
      mi_gdb_test "232-stack-list-arguments 0 1 1" \
--- 101,109 ----
      # -stack-list-arguments 1 1 3
      # -stack-list-arguments 
  
+     # Allow for an optional fifth frame ("_start" or whatever...)
      mi_gdb_test "231-stack-list-arguments 0" \
! 	    "231\\^done,stack-args=\{frame=\{level=\"0\",args=\{\}\},frame=\{level=\"1\",args=\{name=\"strarg\"\}\},frame=\{level=\"2\",args=\{name=\"intarg\",name=\"strarg\"\}\},frame=\{level=\"3\",args=\{name=\"intarg\",name=\"strarg\",name=\"fltarg\"\}\},frame=\{level=\"4\",args=\{\}\}(,frame=\{level=\"5\",args=\{\}\}|)\}" \
                  "stack args listing 0"
  
      mi_gdb_test "232-stack-list-arguments 0 1 1" \
*************** proc test_stack_args_listing {} {
*** 113,120 ****
  	    "233\\^done,stack-args=\{frame=\{level=\"1\",args=\{name=\"strarg\"\}\},frame=\{level=\"2\",args=\{name=\"intarg\",name=\"strarg\"\}\},frame=\{level=\"3\",args=\{name=\"intarg\",name=\"strarg\",name=\"fltarg\"\}\}\}" \
                  "stack args listing 0 1 3"
  
      mi_gdb_test "231-stack-list-arguments 1" \
! 	    "231\\^done,stack-args=\{frame=\{level=\"0\",args=\{\}\},frame=\{level=\"1\",args=\{\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\}\},frame=\{level=\"2\",args=\{\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\}\},frame=\{level=\"3\",args=\{\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\}\},frame=\{level=\"4\",args=\{\}\}\}" \
                  "stack args listing 1"
  
      mi_gdb_test "232-stack-list-arguments 1 1 1" \
--- 114,122 ----
  	    "233\\^done,stack-args=\{frame=\{level=\"1\",args=\{name=\"strarg\"\}\},frame=\{level=\"2\",args=\{name=\"intarg\",name=\"strarg\"\}\},frame=\{level=\"3\",args=\{name=\"intarg\",name=\"strarg\",name=\"fltarg\"\}\}\}" \
                  "stack args listing 0 1 3"
  
+     # Allow for an optional fifth frame ("_start" or whatever...)
      mi_gdb_test "231-stack-list-arguments 1" \
! 	    "231\\^done,stack-args=\{frame=\{level=\"0\",args=\{\}\},frame=\{level=\"1\",args=\{\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\}\},frame=\{level=\"2\",args=\{\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\}\},frame=\{level=\"3\",args=\{\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\}\},frame=\{level=\"4\",args=\{\}\}(,frame=\{level=\"5\",args=\{\}\}|)\}" \
                  "stack args listing 1"
  
      mi_gdb_test "232-stack-list-arguments 1 1 1" \
*************** proc test_stack_info_depth {} {
*** 141,147 ****
      # -stack-info-depth 99
  
      mi_gdb_test "231-stack-info-depth" \
! 	    "231\\^done,depth=\"5\"" \
                  "stack info-depth"
  
      mi_gdb_test "231-stack-info-depth 3" \
--- 143,149 ----
      # -stack-info-depth 99
  
      mi_gdb_test "231-stack-info-depth" \
! 	    "231\\^done,depth=\"(5|6)\"" \
                  "stack info-depth"
  
      mi_gdb_test "231-stack-info-depth 3" \
*************** proc test_stack_info_depth {} {
*** 149,155 ****
                  "stack info-depth 3"
  
      mi_gdb_test "231-stack-info-depth 99" \
! 	    "231\\^done,depth=\"5\"" \
                  "stack info-depth 99"
  
      mi_gdb_test "231-stack-info-depth 99 99" \
--- 151,157 ----
                  "stack info-depth 3"
  
      mi_gdb_test "231-stack-info-depth 99" \
! 	    "231\\^done,depth=\"(5|6)\"" \
                  "stack info-depth 99"
  
      mi_gdb_test "231-stack-info-depth 99 99" \

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Daniel Jacobowitz]

On Tue, May 07, 2002 at 06:09:11PM -0700, Michael Snyder wrote:
> 
> I'm gonna ask for a second pair of eyes, since I don't know MI 
> very well.
> 
> What this is -- the test is examining the stack, but it is
> assuming that main is the last frame.  My change allows for
> one extra frame below main (eg. for '_start').
> 
> OK to check in?

Before you check this in, I would prefer to have a policy decision
in place about whether we should show that frame or not.  The relevant
macro is FRAME_CHAIN_VALID; I believe we should universally (or almost
universally) change this to stop at main.  I think that's
func_frame_chain_valid but don't trust my memory.

Some ports (HP/UX comes to mind) do wacky things in this macro/method. 
I'm not sure what they accomplish or whether they are really necessary. 
Most default to either file_ or func_, and we should standardize that
unless there is a good reason not to.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> On Tue, May 07, 2002 at 06:09:11PM -0700, Michael Snyder wrote:
> >
> > I'm gonna ask for a second pair of eyes, since I don't know MI
> > very well.
> >
> > What this is -- the test is examining the stack, but it is
> > assuming that main is the last frame.  My change allows for
> > one extra frame below main (eg. for '_start').
> >
> > OK to check in?
> 
> Before you check this in, I would prefer to have a policy decision
> in place about whether we should show that frame or not.  The relevant
> macro is FRAME_CHAIN_VALID; I believe we should universally (or almost
> universally) change this to stop at main.  I think that's
> func_frame_chain_valid but don't trust my memory.
> 
> Some ports (HP/UX comes to mind) do wacky things in this macro/method.
> I'm not sure what they accomplish or whether they are really necessary.
> Most default to either file_ or func_, and we should standardize that
> unless there is a good reason not to.

I don't think we can do that, Daniel -- that would force us to change
numerous existing target ports.  Retroactive requirements are not
generally a good idea.  AFAICT, we're stuck with the fact that this
has not been standardized in the past.  I would guess  that there are
just as many targets that display the _start frame as don't.

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Andrew Cagney]

> On Tue, May 07, 2002 at 06:09:11PM -0700, Michael Snyder wrote:
> 
>> 
>> I'm gonna ask for a second pair of eyes, since I don't know MI 
>> very well.
>> 
>> What this is -- the test is examining the stack, but it is
>> assuming that main is the last frame.  My change allows for
>> one extra frame below main (eg. for '_start').
>> 
>> OK to check in?
> 
> 
> Before you check this in, I would prefer to have a policy decision
> in place about whether we should show that frame or not.  The relevant
> macro is FRAME_CHAIN_VALID; I believe we should universally (or almost
> universally) change this to stop at main.  I think that's
> func_frame_chain_valid but don't trust my memory.

(don't remember which function either, but)
Yes, I don't think the backtrace should go past main so I think the 
change is wrong.

I remember much debate about the test at the time (it was Cygnus 
internal unfortunatly).  The thing that clinched the deal was the 
obeservation (made by a human factors person) that the behavour had to 
be consistent across platforms.  For a given OS (e.g. eCos, GNU/Linux, 
...) the backtrace should look identical, regardless of the ISA.

Having each ISA making independant, and somewhat arbitrary, decisions is 
wrong.

 From memory, a suggestion was to let people select the back-trace 
policy independant of the current architecture.

> Some ports (HP/UX comes to mind) do wacky things in this macro/method. 
> I'm not sure what they accomplish or whether they are really necessary. 
> Most default to either file_ or func_, and we should standardize that
> unless there is a good reason not to.

enjoy,
Andrew

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Michael Snyder]

Andrew Cagney wrote:
> 
> > On Tue, May 07, 2002 at 06:09:11PM -0700, Michael Snyder wrote:
> >
> >>
> >> I'm gonna ask for a second pair of eyes, since I don't know MI
> >> very well.
> >>
> >> What this is -- the test is examining the stack, but it is
> >> assuming that main is the last frame.  My change allows for
> >> one extra frame below main (eg. for '_start').
> >>
> >> OK to check in?
> >
> >
> > Before you check this in, I would prefer to have a policy decision
> > in place about whether we should show that frame or not.  The relevant
> > macro is FRAME_CHAIN_VALID; I believe we should universally (or almost
> > universally) change this to stop at main.  I think that's
> > func_frame_chain_valid but don't trust my memory.
> 
> (don't remember which function either, but)
> Yes, I don't think the backtrace should go past main so I think the
> change is wrong.
> 
> I remember much debate about the test at the time (it was Cygnus
> internal unfortunatly).  The thing that clinched the deal was the
> obeservation (made by a human factors person) that the behavour had to
> be consistent across platforms. 

1) It may have to be, but it isn't.
2) Why does it have to be?  Lots of behaviors differ between platforms.
3) My real reason for submitting this fix is that it breaks with the
"needs_status_wrapper" change that I submitted the other day.
Of course, technically it would not break if gdb always stopped the
trace at main, but it doesn't...


> For a given OS (e.g. eCos, GNU/Linux,
> ...) the backtrace should look identical, regardless of the ISA.
> 
> Having each ISA making independant, and somewhat arbitrary, decisions is
> wrong.

Whether that's so or not, they do.  Do you want to have this
always fail for current gdb ports?


>  From memory, a suggestion was to let people select the back-trace
> policy independant of the current architecture.

I thought we also had a policy of not inserting tests
that we knew would fail on some targets?  Something about
this being a regression test...

Sorry, don't mean to be snippy...

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Daniel Jacobowitz]

On Tue, May 07, 2002 at 06:50:48PM -0700, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> > 
> > On Tue, May 07, 2002 at 06:09:11PM -0700, Michael Snyder wrote:
> > >
> > > I'm gonna ask for a second pair of eyes, since I don't know MI
> > > very well.
> > >
> > > What this is -- the test is examining the stack, but it is
> > > assuming that main is the last frame.  My change allows for
> > > one extra frame below main (eg. for '_start').
> > >
> > > OK to check in?
> > 
> > Before you check this in, I would prefer to have a policy decision
> > in place about whether we should show that frame or not.  The relevant
> > macro is FRAME_CHAIN_VALID; I believe we should universally (or almost
> > universally) change this to stop at main.  I think that's
> > func_frame_chain_valid but don't trust my memory.
> > 
> > Some ports (HP/UX comes to mind) do wacky things in this macro/method.
> > I'm not sure what they accomplish or whether they are really necessary.
> > Most default to either file_ or func_, and we should standardize that
> > unless there is a good reason not to.
> 
> I don't think we can do that, Daniel -- that would force us to change
> numerous existing target ports.  Retroactive requirements are not
> generally a good idea.  AFAICT, we're stuck with the fact that this
> has not been standardized in the past.  I would guess  that there are
> just as many targets that display the _start frame as don't.

I don't see any argument not to change existing ports, actually.  This
is a behavioral improvement; I think it's a worthwhile one and that it
can't reasonably cause harm.

There are only five targets using custom frame_chain_valid functions;
all the others use a random mix of the five generic versions, some of
which are tangled up with dummy frames.  There are extra sanity checks
on the frame pointer in ARM and d10v and d30v and xstormy16, which
we might want to express independently of the stop-at-main setting;
and I can't even pretend to follow the HPPA version, but again it looks
as if it should be independent of the stop-at-main question.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Andrew Cagney]

>>  From memory, a suggestion was to let people select the back-trace
>> policy independant of the current architecture.
> 
> 
> I thought we also had a policy of not inserting tests
> that we knew would fail on some targets?  Something about
> this being a regression test...

My understanding of the ``the regression test'' is that it stops people 
adding ``feature'' tests that demonstrate bugs in things that have never 
worked (or are not even implemented).  It avoids, among other things, 
the problem of not knowing of a FAIL is a bug in GDB or in a testcase 
that never worked.

I look at this test as something similar to call-ar-st.  It has been 
demonstrate to work on one platform (I think the original was Arm/eCos 
but more recently GNU/Linux i386) and should work on other platforms.

enjoy,
Andrew

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Michael Snyder]

Andrew Cagney wrote:
> 
> >>  From memory, a suggestion was to let people select the back-trace
> >> policy independant of the current architecture.
> >
> >
> > I thought we also had a policy of not inserting tests
> > that we knew would fail on some targets?  Something about
> > this being a regression test...
> 
> My understanding of the ``the regression test'' is that it stops people
> adding ``feature'' tests that demonstrate bugs in things that have never
> worked (or are not even implemented).  It avoids, among other things,
> the problem of not knowing of a FAIL is a bug in GDB or in a testcase
> that never worked.
> 
> I look at this test as something similar to call-ar-st.  It has been
> demonstrate to work on one platform (I think the original was Arm/eCos
> but more recently GNU/Linux i386) and should work on other platforms.

OK, I yield.  Patch withdrawn.

Re: [RFA/RFC] Tweak for a gdb.mi test.

[Andrew Cagney]

> Andrew Cagney wrote:
> 
>> 
> 
>> >>  From memory, a suggestion was to let people select the back-trace
>> >> policy independant of the current architecture.
> 
>> >
>> >
>> > I thought we also had a policy of not inserting tests
>> > that we knew would fail on some targets?  Something about
>> > this being a regression test...
> 
>> 
>> My understanding of the ``the regression test'' is that it stops people
>> adding ``feature'' tests that demonstrate bugs in things that have never
>> worked (or are not even implemented).  It avoids, among other things,
>> the problem of not knowing of a FAIL is a bug in GDB or in a testcase
>> that never worked.
>> 
>> I look at this test as something similar to call-ar-st.  It has been
>> demonstrate to work on one platform (I think the original was Arm/eCos
>> but more recently GNU/Linux i386) and should work on other platforms.
> 
> 
> OK, I yield.  Patch withdrawn.

The sooner we get KFAIL the better!

Andrew