[RFA] mips-tdep.c: Fix inferior function call breakage

[RFA] mips-tdep.c: Fix inferior function call breakage

[Kevin Buettner]

Without the following patch in place, I've found that inferior function
calls are badly broken for the mips linux targets that I've been testing.
(The warning in heuristic_proc_start() is tripped and things degrade quickly
from there...)

Okay?

	* mips-tdep.c (find_proc_desc): Return early if in a dummy frame.

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.160
diff -u -p -r1.160 mips-tdep.c
--- mips-tdep.c	19 Jan 2003 04:06:46 -0000	1.160
+++ mips-tdep.c	23 Jan 2003 07:20:26 -0000
@@ -2350,6 +2350,9 @@ find_proc_desc (CORE_ADDR pc, struct fra
   mips_extra_func_info_t proc_desc;
   CORE_ADDR startaddr = 0;
 
+  if (pc_in_dummy_frame (pc))
+    return NULL;
+
   proc_desc = non_heuristic_proc_desc (pc, &startaddr);
 
   if (proc_desc)

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Daniel Jacobowitz]

On Thu, Jan 23, 2003 at 12:26:43AM -0700, Kevin Buettner wrote:
> Without the following patch in place, I've found that inferior function
> calls are badly broken for the mips linux targets that I've been testing.
> (The warning in heuristic_proc_start() is tripped and things degrade quickly
> from there...)
> 
> Okay?

In the pile of MIPS patches I really need to get around to submitting
(sorry!) I did this via avoiding the call to find_proc_desc from
mips_init_extra_frame_info.  I think that's functionally equivalent, so
this looks good to me (although Andrew's the one that needs to approve
it of course).

I also short-circuited the rest of mips_init_extra_frame_info if
PC_IN_CALL_DUMMY but that's a noop given the control flow in that
function if proc_desc is NULL.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Andrew Cagney]

> Without the following patch in place, I've found that inferior function
> calls are badly broken for the mips linux targets that I've been testing.
> (The warning in heuristic_proc_start() is tripped and things degrade quickly
> from there...)
> 
> Okay?
> 
> 	* mips-tdep.c (find_proc_desc): Return early if in a dummy frame.

Hmm, explicit tests for PC in call-dummy set of alarm bells.

Kevin, can you post a backtrace showing how the program came to be 
there?  Two alternative possabilities (in addition to this patch) are:

- the code should not have been called (and the dummy-frame flow should 
have gone elsewhere)

- the code needs to be made per-frame

Andrew


> Index: mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.160
> diff -u -p -r1.160 mips-tdep.c
> --- mips-tdep.c	19 Jan 2003 04:06:46 -0000	1.160
> +++ mips-tdep.c	23 Jan 2003 07:20:26 -0000
> @@ -2350,6 +2350,9 @@ find_proc_desc (CORE_ADDR pc, struct fra
>    mips_extra_func_info_t proc_desc;
>    CORE_ADDR startaddr = 0;
>  
> +  if (pc_in_dummy_frame (pc))
> +    return NULL;
> +
>    proc_desc = non_heuristic_proc_desc (pc, &startaddr);
>  
>    if (proc_desc)
> 

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Daniel Jacobowitz]

On Thu, Jan 23, 2003 at 04:00:32PM -0500, Andrew Cagney wrote:
> >Without the following patch in place, I've found that inferior function
> >calls are badly broken for the mips linux targets that I've been testing.
> >(The warning in heuristic_proc_start() is tripped and things degrade 
> >quickly
> >from there...)
> >
> >Okay?
> >
> >	* mips-tdep.c (find_proc_desc): Return early if in a dummy frame.
> 
> Hmm, explicit tests for PC in call-dummy set of alarm bells.
> 
> Kevin, can you post a backtrace showing how the program came to be 
> there?  Two alternative possabilities (in addition to this patch) are:
> 
> - the code should not have been called (and the dummy-frame flow should 
> have gone elsewhere)
> 
> - the code needs to be made per-frame

How is via INIT_EXTRA_FRAME_INFO, already a legacy mechanism. 
Presumably someone could convert MIPS; although if no one's going to do
that, we might want Kevin's change in the interim.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Kevin Buettner]

On Jan 23,  4:00pm, Andrew Cagney wrote:

> Kevin, can you post a backtrace showing how the program came to be 
> there?

#0  heuristic_proc_start (pc=268437552)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/mips-tdep.c:1794
#1  0x080ead9f in find_proc_desc (pc=268437552, next_frame=0x0, cur_frame=1)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/mips-tdep.c:2399
#2  0x080eaf49 in mips_init_extra_frame_info (fromleaf=0, fci=0x83f8990)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/mips-tdep.c:2473
#3  0x080e0484 in gdbarch_init_extra_frame_info (gdbarch=0x8424428, 
    fromleaf=0, frame=0x83f8990)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/gdbarch.c:4122
#4  0x0809f589 in create_new_frame (addr=1099511626816, pc=268437552)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/blockframe.c:273
#5  0x080cd089 in run_stack_dummy (addr=268438080, buffer=0x8460058)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/infcmd.c:1002
#6  0x080b6c90 in hand_function_call (function=0x8464a90, nargs=0, 
    args=0xbffff054)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/valops.c:1762
#7  0x080b2716 in evaluate_subexp_standard (expect_type=0x0, exp=0x8501c68, 
    pos=0xbffff2e4, noside=EVAL_NORMAL)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/eval.c:938
#8  0x080b0cbc in evaluate_subexp (expect_type=0x0, exp=0x8501c68, 
    pos=0xbffff2e4, noside=EVAL_NORMAL)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/eval.c:69
#9  0x080b0e5e in evaluate_expression (exp=0x8501c68)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/eval.c:158
#10 0x080be66c in print_command_1 (exp=0x83f2a35 "hello()", inspect=0, 
    voidprint=0)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/printcmd.c:926
#11 0x080be7d6 in call_command (exp=0x83f2a35 "hello()", from_tty=1)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/printcmd.c:988
#12 0x0808d0f2 in do_cfunc (c=0x83fdb60, args=0x83f2a35 "hello()", from_tty=1)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/cli/cli-decode.c:53
#13 0x0808e96e in cmd_func (cmd=0x83fdb60, args=0x83f2a35 "hello()", 
    from_tty=1)
    at /ocotillo2/mips64linux-021009-branch/devo/gdb/cli/cli-decode.c:1527
...

Hmm, that's from a branch, but it happens with current sources too.  If
you wish, I can generate another backtrace with current sources...

Kevin

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Andrew Cagney]

Thanks,

> #3  0x080e0484 in gdbarch_init_extra_frame_info (gdbarch=0x8424428, 
>     fromleaf=0, frame=0x83f8990)
>     at /ocotillo2/mips64linux-021009-branch/devo/gdb/gdbarch.c:4122
> #4  0x0809f589 in create_new_frame (addr=1099511626816, pc=268437552)
>     at /ocotillo2/mips64linux-021009-branch/devo/gdb/blockframe.c:273

Things went wrong here (with the below calling the above).  However, ...

> #5  0x080cd089 in run_stack_dummy (addr=268438080, buffer=0x8460058)
>     at /ocotillo2/mips64linux-021009-branch/devo/gdb/infcmd.c:1002
> #6  0x080b6c90 in hand_function_call (function=0x8464a90, nargs=0, 
>     args=0xbffff054)
>     at /ocotillo2/mips64linux-021009-branch/devo/gdb/valops.c:1762

... I've eliminated that call from "infcmd.c"!

> Hmm, that's from a branch, but it happens with current sources too.  If
> you wish, I can generate another backtrace with current sources...

Yes.

Andrew

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Kevin Buettner]

On Jan 27,  2:21pm, Andrew Cagney wrote:
> Subject: Re: [RFA] mips-tdep.c: Fix inferior function call breakage
> Thanks,
> 
> > #3  0x080e0484 in gdbarch_init_extra_frame_info (gdbarch=0x8424428, 
> >     fromleaf=0, frame=0x83f8990)
> >     at /ocotillo2/mips64linux-021009-branch/devo/gdb/gdbarch.c:4122
> > #4  0x0809f589 in create_new_frame (addr=1099511626816, pc=268437552)
> >     at /ocotillo2/mips64linux-021009-branch/devo/gdb/blockframe.c:273
> 
> Things went wrong here (with the below calling the above).  However, ...
> 
> > #5  0x080cd089 in run_stack_dummy (addr=268438080, buffer=0x8460058)
> >     at /ocotillo2/mips64linux-021009-branch/devo/gdb/infcmd.c:1002
> > #6  0x080b6c90 in hand_function_call (function=0x8464a90, nargs=0, 
> >     args=0xbffff054)
> >     at /ocotillo2/mips64linux-021009-branch/devo/gdb/valops.c:1762
> 
> ... I've eliminated that call from "infcmd.c"!
> 
> > Hmm, that's from a branch, but it happens with current sources too.  If
> > you wish, I can generate another backtrace with current sources...
> 
> Yes.

#0  heuristic_proc_start (pc=268437552)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/mips-tdep.c:1796
#1  0x080d1e3b in find_proc_desc (pc=268437552, next_frame=0x0, cur_frame=1)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/mips-tdep.c:2406
#2  0x080d1feb in mips_init_extra_frame_info (fromleaf=0, fci=0x82ceed8)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/mips-tdep.c:2478
#3  0x080c670c in gdbarch_init_extra_frame_info (gdbarch=0x82d91e0, 
    fromleaf=0, frame=0x82ceed8)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/gdbarch.c:4422
#4  0x08106e57 in create_new_frame (addr=1099511626816, pc=268437552)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/frame.c:890
#5  0x081066b5 in get_current_frame ()
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/frame.c:506
#6  0x08089261 in bpstat_stop_status (pc=0x82b2f20, not_a_sw_breakpoint=0)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/breakpoint.c:2724
#7  0x080b621f in handle_inferior_event (ecs=0xbfffee00)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/infrun.c:1805
#8  0x080b51fe in wait_for_inferior ()
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/infrun.c:1006
#9  0x080b5025 in proceed (addr=268438080, siggnal=TARGET_SIGNAL_0, step=0)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/infrun.c:804
#10 0x080b2e95 in run_stack_dummy (addr=268438080, buffer=0x837a228)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/infcmd.c:1039
#11 0x0809c735 in hand_function_call (function=0x8316e68, nargs=0, 
    args=0xbffff064)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/valops.c:1672
#12 0x0809844e in evaluate_subexp_standard (expect_type=0x0, exp=0x885fec0, 
    pos=0xbffff2f4, noside=EVAL_NORMAL)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/eval.c:940
#13 0x080969f0 in evaluate_subexp (expect_type=0x0, exp=0x885fec0, 
    pos=0xbffff2f4, noside=EVAL_NORMAL)
    at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/eval.c:70
...

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Andrew Cagney]

Hmm, this a different, but related, stack dump.  The old backtrace was 
for before the inferior function call had even been started.  The below 
is for when the target has stopped in the dummy frame code after the 
inferior function returns.

Read the next bit bottom up:

> #3  0x080c670c in gdbarch_init_extra_frame_info (gdbarch=0x82d91e0, 
>     fromleaf=0, frame=0x82ceed8)
>     at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/gdbarch.c:4422
> #4  0x08106e57 in create_new_frame (addr=1099511626816, pc=268437552)
>     at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/frame.c:890

2. GDB starts to go wrong with the above call.

However, I've just committed the sentinel-frame patch and 
create_new_frame() is no longer called.  Instead get_prev_frame() will 
call init_extra_frame_info directly (the call goes with a nasty comment 
:-)) and, hence, the problem will still occure.

> #5  0x081066b5 in get_current_frame ()
>     at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/frame.c:506

1. Ok so far.

> #6  0x08089261 in bpstat_stop_status (pc=0x82b2f20, not_a_sw_breakpoint=0)
>     at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/breakpoint.c:2724
> #7  0x080b621f in handle_inferior_event (ecs=0xbfffee00)
>     at /ocotillo2/sourceware-mips64/ocotillo-cross-mips64/../src/gdb/infrun.c:1805

I can see two options:

- Modify get_prev_frame() to not call init extra info when in a dummy 
frame.  Not to sure about this.

- Modify mips_init_extra_frame_info() to return immediatly when a dummy 
frame.

Note that there is ment to be only one call to pc_in_dummy_frame() (in 
"frame.c") and even that is going away - I'm going to deprecate that 
function.

Andrew

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Kevin Buettner]

On Jan 27,  5:58pm, Andrew Cagney wrote:

> I can see two options:
> 
> - Modify get_prev_frame() to not call init extra info when in a dummy 
> frame.  Not to sure about this.

This might be okay, but it needs to be studied.

> - Modify mips_init_extra_frame_info() to return immediatly when a dummy 
> frame.

I'm more confident in this solution.  See below.  Okay?

	* mips-tdep.c (mips_init_extra_frame_info): Return early for
	dummy frames.

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.160
diff -u -p -r1.160 mips-tdep.c
--- mips-tdep.c	19 Jan 2003 04:06:46 -0000	1.160
+++ mips-tdep.c	27 Jan 2003 23:51:21 -0000
@@ -2468,9 +2468,13 @@ static void
 mips_init_extra_frame_info (int fromleaf, struct frame_info *fci)
 {
   int regnum;
+  mips_extra_func_info_t proc_desc;
+
+  if (get_frame_type (fci) == DUMMY_FRAME)
+    return;
 
   /* Use proc_desc calculated in frame_chain */
-  mips_extra_func_info_t proc_desc =
+  proc_desc =
     get_next_frame (fci)
     ? cached_proc_desc
     : find_proc_desc (get_frame_pc (fci), get_next_frame (fci), 1);

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Andrew Cagney]

> On Jan 27,  5:58pm, Andrew Cagney wrote:
> 
> 
>> I can see two options:
>> 
>> - Modify get_prev_frame() to not call init extra info when in a dummy 
>> frame.  Not to sure about this.
> 
> 
> This might be okay, but it needs to be studied.

Yep.  Study it more when most of the mainstream targets have been 
overhauled.  The PPC (which uses an unusual frame arangement) is 
especially troubling.

>> - Modify mips_init_extra_frame_info() to return immediatly when a dummy 
>> frame.
> 
> 
> I'm more confident in this solution.  See below.  Okay?

Yes.

Andrew


> 	* mips-tdep.c (mips_init_extra_frame_info): Return early for
> 	dummy frames.
> 
> Index: mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.160
> diff -u -p -r1.160 mips-tdep.c
> --- mips-tdep.c	19 Jan 2003 04:06:46 -0000	1.160
> +++ mips-tdep.c	27 Jan 2003 23:51:21 -0000
> @@ -2468,9 +2468,13 @@ static void
>  mips_init_extra_frame_info (int fromleaf, struct frame_info *fci)
>  {
>    int regnum;
> +  mips_extra_func_info_t proc_desc;
> +
> +  if (get_frame_type (fci) == DUMMY_FRAME)
> +    return;
>  
>    /* Use proc_desc calculated in frame_chain */
> -  mips_extra_func_info_t proc_desc =
> +  proc_desc =
>      get_next_frame (fci)
>      ? cached_proc_desc
>      : find_proc_desc (get_frame_pc (fci), get_next_frame (fci), 1);
> 
> 

Re: [RFA] mips-tdep.c: Fix inferior function call breakage

[Kevin Buettner]

On Jan 28,  8:14am, Andrew Cagney wrote:

> >> - Modify mips_init_extra_frame_info() to return immediatly when a dummy 
> >> frame.
> > 
> > I'm more confident in this solution.  See below.  Okay?
> 
> Yes.

Committed.