Re: [PATCH 3/3] GDB: New target s12z

[Simon Marchi <simark@simark.ca>]

On 2018-08-26 1:41 p.m., John Darrington wrote:
> Hi Simon,
> 
> Thanks for the review.
> 
> On Sun, Aug 26, 2018 at 01:19:29PM -0400, Simon Marchi wrote:
>      Hi John,
>      
>      I did a first pass review and noted a few minor nits.  I didn't get too deep in the
>      frame_id/unwind code because I'm not too familiar with that.
> 
> Pity.  I was hoping someone could help me there :{P

I just haven't written enough of those to be able to spot bugs off-hand in them.

Maybe just one insight, it seems (I'm not sure) that s12z_frame_cache uses the SP value of the
current frame (the one for which we compute the id) in the frame id.

The usual thing to do (I looked at a few other arches) is to use the same value as the
canonical frame address as defined by DWARF (Section 6.4 in DWARF5.pdf), which is the
value of the stack pointer just before that frame was created.

This is of course not mandatory, but I suppose that adhering to this de facto rule could
make things clearer.

> Many of your comments and questions stem from the code which I used as a
> pattern, so I'm unsure of the answers.  But I will find out as best I
> can and prepare a new patch within the next few days.
> 
> J'

Thanks,

Simon