Re: [RFA/RFC] fix problems with unwinder on mips-irix

[Joel Brobecker <brobecker@gnat.com>]

Andrew,

> Yes, sometimes inlineing doesn't help, here it does.  There's really no 
> value in trying to preserve this code so be brutal.

Thanks a lot for the detailed message. Really appreciated. I'm trying to
find my way through all this. Let's first look at inlining find_proc_desc.
I'll work on the rest as soon as I have this one figured out.

Reading your last commit to this file, I discovered that you added frame
sniffers, so, if I understand correctly, we can now more or less predict
the circumstances under which find_proc_desc should be called (heuristic
vs non-heuristic). Is that right?

find_proc_desc is called by 4 routines:

  1. mips_mdebug_frame_cache

      /* Get the mdebug proc descriptor.  */
      proc_desc = find_proc_desc (frame_pc_unwind (next_frame), next_frame, 1);

     In that case, I think this call can be replaced by a call to
     non_heuristic_proc_desc? How about the handling this case:

      /* IF this is the topmost frame AND
       * (this proc does not have debugging information OR
       * the PC is in the procedure prologue)
       * THEN create a "heuristic" proc_desc (by analyzing
       * the actual code) to replace the "official" proc_desc.
       */
  
  2. mips_insn16_frame_cache
  3. mips_insn32_frame_cache

     In these two cases, the call to find_proc_desc can be reduced to
     the case where the heuristics have to be used. You said it can be
     inline using something like this:

      if (startaddr == 0)
        startaddr = heuristic_proc_start (pc);

      proc_desc = heuristic_proc_desc (startaddr, pc, next_frame, cur_frame);

     I see that linked_proc_desc_table is never used, which explains
     why we can get rid of:

      /* Is linked_proc_desc_table really necessary?  It only seems to be used
         by procedure call dummys.  However, the procedures being called ought
         to have their own proc_descs, and even if they don't,
         heuristic_proc_desc knows how to create them! */

      struct linked_proc_info *link;

      for (link = linked_proc_desc_table; link; link = link->next)
        if (PROC_LOW_ADDR (&link->info) <= pc
            && PROC_HIGH_ADDR (&link->info) > pc)
          return &link->info;
  
  4. after_prologue

     So far so good. But there there is the case of after_prologue:

        /* Pass cur_frame == 0 to find_proc_desc.  We should not attempt
           to read the stack pointer from the current machine state, because
           the current machine state has nothing to do with the information
           we need from the proc_desc; and the process may or may not exist
           right now.  */
        if (!proc_desc)
          proc_desc = find_proc_desc (pc, NULL, 0);

     The only place where this function is called is in mips_skip_prologue:

        CORE_ADDR post_prologue_pc = after_prologue (pc, NULL);


     So arguably we could remove this extra parameter from
     after_prologue. Should we do this?

     Back to find_proc_desc, I suppose the above code should be replaced
     by something like this:

        if (!proc_desc)
          proc_desc = non_heuristic_proc_desc (pc, &startaddr);

        if (!proc_desc)
          proc_desc = heuristic_proc_desc (pc);
          
     Would that be right? Same question as in point 1 above, actually.
     
Thanks,
-- 
Joel