[rfc] trad-frame change

[rfc] trad-frame change

[Andrew Cagney]

Think of this as a pseudo patch.

I'm looking to extend the trad-frame code so that it includes the interface:

   trad_frame_append (gdbarch, frame_type, frame_sniffer, frame_cache);

This would do all the housekeeping necessary to create a FRAME_TYPE 
unwinder implemented with FRAME_SNIFFER and FRAME_CACHE functions.  The 
second function would have the interface:

   struct trad_frame_cache
   {
     struct frame_id this_id;
     CORE_ADDR this_base;
     struct trad_frame *saved_regs;
   };

   void frame_cache (struct frame_info *next_frame, struct 
trad_frame_cache *this_cache);

and would be called _once_ to populate the entire trad-frame cache. 
After that frame ID and register requests would be handled directly.

My rationale is simple.  Having just churned out unwinders for two 
architectures I'm seeing a pattern that suggests this would make life 
easier:

- people in a hurry can use interface

- people wanting to tune their unwinder can use the lower-level interface

thoughts?
Andrew

PS: This has been suggested before, but not as a new layer above the 
existing unwinder code

[rfc] trad-frame change

[Andrew Cagney]

Think of this as a pseudo patch.

I'm looking to extend the trad-frame code so that it includes the interface:

   trad_frame_append (gdbarch, frame_type, frame_sniffer, frame_cache);

This would do all the housekeeping necessary to create a FRAME_TYPE 
unwinder implemented with FRAME_SNIFFER and FRAME_CACHE functions.  The 
second function would have the interface:

   struct trad_frame_cache
   {
     struct frame_id this_id;
     CORE_ADDR this_base;
     struct trad_frame *saved_regs;
   };

   void frame_cache (struct frame_info *next_frame, struct 
trad_frame_cache *this_cache);

and would be called _once_ to populate the entire trad-frame cache. 
After that frame ID and register requests would be handled directly.

My rationale is simple.  Having just churned out unwinders for two 
architectures I'm seeing a pattern that suggests this would make life 
easier:

- people in a hurry can use interface

- people wanting to tune their unwinder can use the lower-level interface

thoughts?
Andrew

PS: This has been suggested before, but not as a new layer above the 
existing unwinder code

Re: [rfc] trad-frame change

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 11:43:20AM -0500, Andrew Cagney wrote:
> Think of this as a pseudo patch.
> 
> I'm looking to extend the trad-frame code so that it includes the interface:
> 
>   trad_frame_append (gdbarch, frame_type, frame_sniffer, frame_cache);
> 
> This would do all the housekeeping necessary to create a FRAME_TYPE 
> unwinder implemented with FRAME_SNIFFER and FRAME_CACHE functions.  The 
> second function would have the interface:
> 
>   struct trad_frame_cache
>   {
>     struct frame_id this_id;
>     CORE_ADDR this_base;
>     struct trad_frame *saved_regs;
>   };
> 
>   void frame_cache (struct frame_info *next_frame, struct 
> trad_frame_cache *this_cache);
> 
> and would be called _once_ to populate the entire trad-frame cache. 
> After that frame ID and register requests would be handled directly.
> 
> My rationale is simple.  Having just churned out unwinders for two 
> architectures I'm seeing a pattern that suggests this would make life 
> easier:

Sounds pretty nice to me.  For what it's worth, I'm testing a sigtramp
unwinder on MIPS/Linux that could almost but not quite use this:

+struct mips_prologue_cache
+{
+  /* The stack pointer at the time this frame was created; i.e. the
+     caller's stack pointer when this function was called.  It is used
+     to identify this frame.  */
+  CORE_ADDR prev_sp;
+
+  CORE_ADDR tramp_start;
+
+  int kind;
+
+  /* Saved register offsets.  */
+  struct trad_frame_saved_reg *saved_regs;
+};

(so that the frame ID is constant for both instructions of the
trampoline).

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfc] trad-frame change

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 11:43:20AM -0500, Andrew Cagney wrote:
> Think of this as a pseudo patch.
> 
> I'm looking to extend the trad-frame code so that it includes the interface:
> 
>   trad_frame_append (gdbarch, frame_type, frame_sniffer, frame_cache);
> 
> This would do all the housekeeping necessary to create a FRAME_TYPE 
> unwinder implemented with FRAME_SNIFFER and FRAME_CACHE functions.  The 
> second function would have the interface:
> 
>   struct trad_frame_cache
>   {
>     struct frame_id this_id;
>     CORE_ADDR this_base;
>     struct trad_frame *saved_regs;
>   };
> 
>   void frame_cache (struct frame_info *next_frame, struct 
> trad_frame_cache *this_cache);
> 
> and would be called _once_ to populate the entire trad-frame cache. 
> After that frame ID and register requests would be handled directly.
> 
> My rationale is simple.  Having just churned out unwinders for two 
> architectures I'm seeing a pattern that suggests this would make life 
> easier:

Sounds pretty nice to me.  For what it's worth, I'm testing a sigtramp
unwinder on MIPS/Linux that could almost but not quite use this:

+struct mips_prologue_cache
+{
+  /* The stack pointer at the time this frame was created; i.e. the
+     caller's stack pointer when this function was called.  It is used
+     to identify this frame.  */
+  CORE_ADDR prev_sp;
+
+  CORE_ADDR tramp_start;
+
+  int kind;
+
+  /* Saved register offsets.  */
+  struct trad_frame_saved_reg *saved_regs;
+};

(so that the frame ID is constant for both instructions of the
trampoline).

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfc] trad-frame change

[Andrew Cagney]

> Sounds pretty nice to me.  For what it's worth, I'm testing a sigtramp
> unwinder on MIPS/Linux that could almost but not quite use this:
> 
> +struct mips_prologue_cache
> +{
> +  /* The stack pointer at the time this frame was created; i.e. the
> +     caller's stack pointer when this function was called.  It is used
> +     to identify this frame.  */
> +  CORE_ADDR prev_sp;
> +
> +  CORE_ADDR tramp_start;
> +
> +  int kind;
> +
> +  /* Saved register offsets.  */
> +  struct trad_frame_saved_reg *saved_regs;
> +};
> 
> (so that the frame ID is constant for both instructions of the
> trampoline).

.. and frame_id_unwind() looks something like:

	frame_id_build (cache->prev_sp, cache->tramp_start)?

the trad-frame chache instead has the field:

     struct frame_id this_id;

which is equivalent - the ID being constructed up front.

What is "kind"?

Andrew

Re: [rfc] trad-frame change

[Andrew Cagney]

> Sounds pretty nice to me.  For what it's worth, I'm testing a sigtramp
> unwinder on MIPS/Linux that could almost but not quite use this:
> 
> +struct mips_prologue_cache
> +{
> +  /* The stack pointer at the time this frame was created; i.e. the
> +     caller's stack pointer when this function was called.  It is used
> +     to identify this frame.  */
> +  CORE_ADDR prev_sp;
> +
> +  CORE_ADDR tramp_start;
> +
> +  int kind;
> +
> +  /* Saved register offsets.  */
> +  struct trad_frame_saved_reg *saved_regs;
> +};
> 
> (so that the frame ID is constant for both instructions of the
> trampoline).

.. and frame_id_unwind() looks something like:

	frame_id_build (cache->prev_sp, cache->tramp_start)?

the trad-frame chache instead has the field:

     struct frame_id this_id;

which is equivalent - the ID being constructed up front.

What is "kind"?

Andrew

Re: [rfc] trad-frame change

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 01:34:23PM -0500, Andrew Cagney wrote:
> >Sounds pretty nice to me.  For what it's worth, I'm testing a sigtramp
> >unwinder on MIPS/Linux that could almost but not quite use this:
> >
> >+struct mips_prologue_cache
> >+{
> >+  /* The stack pointer at the time this frame was created; i.e. the
> >+     caller's stack pointer when this function was called.  It is used
> >+     to identify this frame.  */
> >+  CORE_ADDR prev_sp;
> >+
> >+  CORE_ADDR tramp_start;
> >+
> >+  int kind;
> >+
> >+  /* Saved register offsets.  */
> >+  struct trad_frame_saved_reg *saved_regs;
> >+};
> >
> >(so that the frame ID is constant for both instructions of the
> >trampoline).
> 
> .. and frame_id_unwind() looks something like:
> 
> 	frame_id_build (cache->prev_sp, cache->tramp_start)?
> 
> the trad-frame chache instead has the field:
> 
>     struct frame_id this_id;
> 
> which is equivalent - the ID being constructed up front.

Yes, precisely - I missed seeing this_id in the bit you posted.

> What is "kind"?

There are four kinds of signal frames on MIPS/Linux that we have to
recognize:
  - o32 sigreturn
  - o32 rt_sigreturn
  - n32 rt_sigreturn
  - n64 rt_sigreturn

They're all basically the same but the offsets differ.  Kind could be
moved out of the cache, I think, since it isn't used after the cache is
filled.  So I could use your new mechanism after all.

What would _really_ be nice would be a way to pass the kind from the
sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
code... not have to read inferior memory to figure out which it is,
twice.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfc] trad-frame change

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 01:34:23PM -0500, Andrew Cagney wrote:
> >Sounds pretty nice to me.  For what it's worth, I'm testing a sigtramp
> >unwinder on MIPS/Linux that could almost but not quite use this:
> >
> >+struct mips_prologue_cache
> >+{
> >+  /* The stack pointer at the time this frame was created; i.e. the
> >+     caller's stack pointer when this function was called.  It is used
> >+     to identify this frame.  */
> >+  CORE_ADDR prev_sp;
> >+
> >+  CORE_ADDR tramp_start;
> >+
> >+  int kind;
> >+
> >+  /* Saved register offsets.  */
> >+  struct trad_frame_saved_reg *saved_regs;
> >+};
> >
> >(so that the frame ID is constant for both instructions of the
> >trampoline).
> 
> .. and frame_id_unwind() looks something like:
> 
> 	frame_id_build (cache->prev_sp, cache->tramp_start)?
> 
> the trad-frame chache instead has the field:
> 
>     struct frame_id this_id;
> 
> which is equivalent - the ID being constructed up front.

Yes, precisely - I missed seeing this_id in the bit you posted.

> What is "kind"?

There are four kinds of signal frames on MIPS/Linux that we have to
recognize:
  - o32 sigreturn
  - o32 rt_sigreturn
  - n32 rt_sigreturn
  - n64 rt_sigreturn

They're all basically the same but the offsets differ.  Kind could be
moved out of the cache, I think, since it isn't used after the cache is
filled.  So I could use your new mechanism after all.

What would _really_ be nice would be a way to pass the kind from the
sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
code... not have to read inferior memory to figure out which it is,
twice.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfc] trad-frame change

[Andrew Cagney]

> There are four kinds of signal frames on MIPS/Linux that we have to
> recognize:
>   - o32 sigreturn
>   - o32 rt_sigreturn
>   - n32 rt_sigreturn
>   - n64 rt_sigreturn
> 
> They're all basically the same but the offsets differ.  Kind could be
> moved out of the cache, I think, since it isn't used after the cache is
> filled.  So I could use your new mechanism after all.

So there are really two variants - o32sigreturn and rt_sigreturn (with 
constants computed from the architecture vector)?

> What would _really_ be nice would be a way to pass the kind from the
> sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
> code... not have to read inferior memory to figure out which it is,
> twice.

I'll think about that, I may need to change the unwinder object anyway.

Just remember that it is the number of unique addresses accessed (0 vs 1 
vs 2 ...) and not the number of times each address is accessed that is 
going to be important - multiple accesses to a single address can be 
handled with a cache.

Andrew

Re: [rfc] trad-frame change

[Andrew Cagney]

> There are four kinds of signal frames on MIPS/Linux that we have to
> recognize:
>   - o32 sigreturn
>   - o32 rt_sigreturn
>   - n32 rt_sigreturn
>   - n64 rt_sigreturn
> 
> They're all basically the same but the offsets differ.  Kind could be
> moved out of the cache, I think, since it isn't used after the cache is
> filled.  So I could use your new mechanism after all.

So there are really two variants - o32sigreturn and rt_sigreturn (with 
constants computed from the architecture vector)?

> What would _really_ be nice would be a way to pass the kind from the
> sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
> code... not have to read inferior memory to figure out which it is,
> twice.

I'll think about that, I may need to change the unwinder object anyway.

Just remember that it is the number of unique addresses accessed (0 vs 1 
vs 2 ...) and not the number of times each address is accessed that is 
going to be important - multiple accesses to a single address can be 
handled with a cache.

Andrew

Re: [rfc] trad-frame change

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 03:20:44PM -0500, Andrew Cagney wrote:
> >There are four kinds of signal frames on MIPS/Linux that we have to
> >recognize:
> >  - o32 sigreturn
> >  - o32 rt_sigreturn
> >  - n32 rt_sigreturn
> >  - n64 rt_sigreturn
> >
> >They're all basically the same but the offsets differ.  Kind could be
> >moved out of the cache, I think, since it isn't used after the cache is
> >filled.  So I could use your new mechanism after all.
> 
> So there are really two variants - o32sigreturn and rt_sigreturn (with 
> constants computed from the architecture vector)?

I don't see much point in distinguishing them that way, honestly.  The
difference between o32 sigreturn and o32 rt_sigreturn is about the same
as the difference between n32 and n64.  Sure, you could put the
constants in the architecture tdep vector... but why bother?

And it may be silly, but there's nothing stopping a theoretical
hand-written program from using a different ABI's sigreturn syscall.

> >What would _really_ be nice would be a way to pass the kind from the
> >sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
> >code... not have to read inferior memory to figure out which it is,
> >twice.
> 
> I'll think about that, I may need to change the unwinder object anyway.
> 
> Just remember that it is the number of unique addresses accessed (0 vs 1 
> vs 2 ...) and not the number of times each address is accessed that is 
> going to be important - multiple accesses to a single address can be 
> handled with a cache.

We really should turn on that cache someday.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfc] trad-frame change

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 03:20:44PM -0500, Andrew Cagney wrote:
> >There are four kinds of signal frames on MIPS/Linux that we have to
> >recognize:
> >  - o32 sigreturn
> >  - o32 rt_sigreturn
> >  - n32 rt_sigreturn
> >  - n64 rt_sigreturn
> >
> >They're all basically the same but the offsets differ.  Kind could be
> >moved out of the cache, I think, since it isn't used after the cache is
> >filled.  So I could use your new mechanism after all.
> 
> So there are really two variants - o32sigreturn and rt_sigreturn (with 
> constants computed from the architecture vector)?

I don't see much point in distinguishing them that way, honestly.  The
difference between o32 sigreturn and o32 rt_sigreturn is about the same
as the difference between n32 and n64.  Sure, you could put the
constants in the architecture tdep vector... but why bother?

And it may be silly, but there's nothing stopping a theoretical
hand-written program from using a different ABI's sigreturn syscall.

> >What would _really_ be nice would be a way to pass the kind from the
> >sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
> >code... not have to read inferior memory to figure out which it is,
> >twice.
> 
> I'll think about that, I may need to change the unwinder object anyway.
> 
> Just remember that it is the number of unique addresses accessed (0 vs 1 
> vs 2 ...) and not the number of times each address is accessed that is 
> going to be important - multiple accesses to a single address can be 
> handled with a cache.

We really should turn on that cache someday.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfc] trad-frame change

[Andrew Cagney]

>>> So there are really two variants - o32sigreturn and rt_sigreturn (with 
>>> constants computed from the architecture vector)?
> 
> 
> I don't see much point in distinguishing them that way, honestly.  The
> difference between o32 sigreturn and o32 rt_sigreturn is about the same
> as the difference between n32 and n64.  Sure, you could put the
> constants in the architecture tdep vector... but why bother?
> 
> And it may be silly, but there's nothing stopping a theoretical
> hand-written program from using a different ABI's sigreturn syscall.

Like you said, theoretical - program for what is, not what might be, but 
what ever.

>>>> >What would _really_ be nice would be a way to pass the kind from the
>>>> >sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
>>>> >code... not have to read inferior memory to figure out which it is,
>>>> >twice.
>>
>>> 
>>> I'll think about that, I may need to change the unwinder object anyway.
>>> 
>>> Just remember that it is the number of unique addresses accessed (0 vs 1 
>>> vs 2 ...) and not the number of times each address is accessed that is 
>>> going to be important - multiple accesses to a single address can be 
>>> handled with a cache.
> 
> 
> We really should turn on that cache someday.

Not that cache, a new one, yes, someday.

Andrew

Re: [rfc] trad-frame change

[Andrew Cagney]

>>> So there are really two variants - o32sigreturn and rt_sigreturn (with 
>>> constants computed from the architecture vector)?
> 
> 
> I don't see much point in distinguishing them that way, honestly.  The
> difference between o32 sigreturn and o32 rt_sigreturn is about the same
> as the difference between n32 and n64.  Sure, you could put the
> constants in the architecture tdep vector... but why bother?
> 
> And it may be silly, but there's nothing stopping a theoretical
> hand-written program from using a different ABI's sigreturn syscall.

Like you said, theoretical - program for what is, not what might be, but 
what ever.

>>>> >What would _really_ be nice would be a way to pass the kind from the
>>>> >sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
>>>> >code... not have to read inferior memory to figure out which it is,
>>>> >twice.
>>
>>> 
>>> I'll think about that, I may need to change the unwinder object anyway.
>>> 
>>> Just remember that it is the number of unique addresses accessed (0 vs 1 
>>> vs 2 ...) and not the number of times each address is accessed that is 
>>> going to be important - multiple accesses to a single address can be 
>>> handled with a cache.
> 
> 
> We really should turn on that cache someday.

Not that cache, a new one, yes, someday.

Andrew

Re: [rfc] trad-frame change

[Andrew Cagney]

>> What would _really_ be nice would be a way to pass the kind from the
>> sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
>> code... not have to read inferior memory to figure out which it is,
>> twice.
> 
> 
> I'll think about that, I may need to change the unwinder object anyway.
> 
> Just remember that it is the number of unique addresses accessed (0 vs 1 vs 2 ...) and not the number of times each address is accessed that is going to be important - multiple accesses to a single address can be handled with a cache. 

Just FYI, having poked it a bit more I've decided to leave this sit 
until after 6.1 ships - its too invasive an interface change.

Andrew

Re: [rfc] trad-frame change

[Andrew Cagney]

>> What would _really_ be nice would be a way to pass the kind from the
>> sniffer (which really just calls PC_IN_SIGTRAMP) to the frame creation
>> code... not have to read inferior memory to figure out which it is,
>> twice.
> 
> 
> I'll think about that, I may need to change the unwinder object anyway.
> 
> Just remember that it is the number of unique addresses accessed (0 vs 1 vs 2 ...) and not the number of times each address is accessed that is going to be important - multiple accesses to a single address can be handled with a cache. 

Just FYI, having poked it a bit more I've decided to leave this sit 
until after 6.1 ships - its too invasive an interface change.

Andrew