Re: dwarf-frame.c question

[Mark Kettenis <kettenis at chello dot nl>]

   Date: Thu, 29 May 2003 15:54:12 -0400
   From: Andrew Cagney <ac131313@redhat.com>

   >    Date: Tue, 27 May 2003 17:18:35 +0200
   >    From: Michal Ludvig <mludvig@suse.cz>
   > 
   >    Hi Mark,
   > 
   >    why do you decrement unwound PC in dwarf_frame_cache() before using it?
   > 
   > The unwound PC is the return address, i.e. the instruction that will
   > be executed when the function returns.

   Yes, the resume address, or the next instruction that will be executed 
   when the frame resumes.

   frame_address_in_block() also tries to handle this.

Indeed.  When I noted the problems with frame_address_in_block()
before, we more or less agreed that we needed to add properties to our
frames instead of making decision based on a frames type.  One such a
property could whether to decrease the rerturn address or not.

   >  This is the instruction after
   > the call instruction.  The problem is that if the call instruction is
   > the last instruction of a function, the return address might point to
   > the next function:

   > foo:
   >    ...
   >    call abort
   > 
   > bar:
   >    push %ebp
   >    mov %esp, %ebp
   >    ...
   > 
   > That's why the GCC unwinder does the same thing.  Note that the
   > decrementing the PC is wrong for "interrupt frames", which is why the
   > if-statement is there in the code fragment you cite:
   > 
   >    dwarf-frame.c:
   >    478       /* Unwind the PC.  */
   >    479       fs->pc = frame_pc_unwind (next_frame);
   >    480       if (get_frame_type (next_frame) == NORMAL_FRAME
   >    481           && frame_relative_level (next_frame) >= 0)
   >    482         fs->pc--;

   First an FYI.  CFI has that return-address column.  I'm left wondering 
   if frame_pc_unwind() should try the frame for the unwound pc before 
   trying for registers.  However, there has so far been zero evidence 
   supporting this need so I think, until there is, let it be.  It also 
   wouldn't help with this case - it to would still point back to beyond 
   the function :-(

I'm not sure what you mean here, but dwarf-frame.c treats the
return-address column as the saved/unwound pc.

   Second, another FYI.  This isn't just a CFI problem.  There have been 
   earlier posts about how GDB, already gets confused by this - printing 
   out the wrong function address for instance.  This problem is generic.

Yes indeed it is.

   Anyway, is it safe to always decrement the resume address before looking 
   for the CFI info?  Given a more complex sequence like:

	   1: setup call
	   2: call xxx with lots of side effects
	   3: delay slot saved r0++
	   4: discard call

   then the CFI info for 4 could be very different to that for 2/3.

Well, the GCC unwinder seems to think it is safe to do this.  Note
that we only use the decremented resume address to find the FDE, and
that we use the real return address for calculating the CFI.  This
means that as long as the compiler doesn't store the CFI for 3 and 4
in different FDE's, we'll always have the correct CFI.

    >  Andrew, it seems that we should tweak the frame code
    > to make sure that frame_unwind_by_pc is always passed a PC *within* the
    > function.

   True, but how?  It would effectively be frame_unwind_address_in_block() 
   but how reliably/where could it be used?

Oops, I meant frame_unwind_find_by_pc instead of frame_unwind_by_pc.
Anyway, every invocation of get_frame_pc() is a potential candidate.

It seems to me that this deserves a bit more thought.  Should I
postpone moving the CFI unwinder over to mainline until this is
resolved?  Alternatively I can comment out the bit of code that
decrements the return-address.  That would be "acceptable" since the
bug it tries to solve is present in the current code too.

Mark