Re: your turn :-)

[Andrew Cagney <cagney at gnu dot org>]

> I'll see if I can take care of this. Right now, GDB is completely
> broken again. This change sort of patch things up a little bit
> by preventing a SEGV. But we get some other problems that just
> screw the testsuite run.
>
>
>     Index: mips-tdep.c
>     ===================================================================
>     RCS file: /cvs/src/src/gdb/mips-tdep.c,v
>     retrieving revision 1.320
>     diff -u -p -r1.320 mips-tdep.c
>     --- mips-tdep.c	4 Sep 2004 00:16:56 -0000	1.320
>     +++ mips-tdep.c	4 Sep 2004 18:59:18 -0000
>     @@ -2762,9 +2762,17 @@ mips32_heuristic_proc_desc (CORE_ADDR st
>      {
>        CORE_ADDR cur_pc;
>        CORE_ADDR frame_addr = 0;	/* Value of $r30. Used by gcc for frame-pointer */
>     +
>     +  /* FIXME: brobecker/2004-09-04: We're in the middle of a transition,
>     +     and this_cache may be NULL. In that case, then create a new one
>     +     just for now. It means a memory leak, but oh well, I don't care.  */
>     +  if (this_cache == NULL)
>     +    this_cache = FRAME_OBSTACK_ZALLOC (struct mips_frame_cache);

This shouldn't be needed, hmm:

>      restart:

oops, this should be deleted.  An earlier change modified set_reg_offset 
to ignore NULL pointers.

>     -  this_cache = xrealloc (this_cache, SIZEOF_FRAME_SAVED_REGS);
>     -  memset (this_cache, '\0', SIZEOF_FRAME_SAVED_REGS);


>     +  this_cache->saved_regs = xrealloc (this_cache->saved_regs,
>     +                                     SIZEOF_FRAME_SAVED_REGS);
>     +  memset (this_cache->saved_regs, '\0', SIZEOF_FRAME_SAVED_REGS);
>        PROC_FRAME_OFFSET (&temp_proc_desc) = 0;
>        PROC_FRAME_ADJUST (&temp_proc_desc) = 0;	/* offset of FP from SP */
>        for (cur_pc = start_pc; cur_pc < limit_pc; cur_pc += MIPS_INSTLEN)
>
> I guess you really don't want to see me incorporate my last RFA
> until the cleanup is done, eh?
>
>
> On Fri, Sep 03, 2004 at 08:11:36PM -0400, Andrew Cagney wrote:
>
> > > Joel
> > >
> > > The theory is to enable this:
> > >
> > > #ifdef NOT_YET
> > >     proc_desc = heuristic_proc_desc (start_addr, pc, next_frame, 
> > > this_cache);
> > > #else
> > >     proc_desc = heuristic_proc_desc (start_addr, pc, next_frame, NULL);
> > > #endif
>
>
> What should heuristic_proc_desc do if the cache given is NULL.
> Create a temporary one while scaning the function, and then
> trash it?

No, the code should survive a NULL this_cache.

> Another question that's troubling me: I can't see how mips_frame_cache
> structs are deallocated. I am guessing that this happens when the
> obstack is reset, but then that would mean that we leak the save_regs
> array. ???

Right.  The deallocate occures each time the frame cache is flushed and 
that occures over and over.

> > > Then:
> > >
> > > - delete the rest of the insn{32,16}_frame_cache code as its redundant, 
> > > the heuristic code will have already updated the cache
> > >
> > > - eliminate proc_desc from mips{32,16}_heuristic ... as its redundant, 
> > > the code only needs to update this_cache

This might get changed.

I've this hunch that heuristic_proc_desc may need to be changed to 
return the last instruction - indicating the end of the prologue.  But 
worry about this latter.

> > > - either inline mips{32,16}_heuristic into mips_insn{32,16}_frame_cache 
> > > or, instead, merge mips_insn{32,16}_frame_cache.
> > >
> > > want to try it (the test results don't even need to vaguely pass).
> > >
> > > Andrew

Andrew