Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Cagney <cagney@gnu.org>
To: Joel Brobecker <brobecker@gnat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: your turn :-)
Date: Sat, 04 Sep 2004 23:58:00 -0000	[thread overview]
Message-ID: <413A5649.8070205@gnu.org> (raw)
In-Reply-To: <20040904190742.GW1216@gnat.com>

> 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


  parent reply	other threads:[~2004-09-04 23:58 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-04  0:12 Andrew Cagney
2004-09-04 19:07 ` Joel Brobecker
2004-09-04 23:30   ` Joel Brobecker
2004-09-04 23:58   ` Andrew Cagney [this message]
2004-09-05  0:14     ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=413A5649.8070205@gnu.org \
    --to=cagney@gnu.org \
    --cc=brobecker@gnat.com \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox