Re: [PATCH v2] gdb/hppa-tdep.c: Fix logical working flow typo issue

[Chen Gang <gang dot chen dot 5i5j at gmail dot com>]

Hello all:

Is this patch OK, it is part of checking saving registers in the stack,
it tries to recognize the saving instructions "stb, stw(m), or std", so
can continue checking.

Excuse me, I have no related environments for a test, I assumed that the
original author knew about the working flow, then wrote the related
function.

If need additional analyzing, please let me know (better to let me know
what I need try, next).


Thanks.

On 10/22/14 23:38, Chen Gang wrote:
> inst_saves_gr() wants to recognize 'st??' instruction, according to the
> code we know it include 'stb' (for store byte), 'stw(m)' (for store
> word), and 'std' (for store double word).
> 
> They should be in the same format, and have neighbour numbers:
> especially, 'stw(m)' need be in the middle of 'stb' and 'std'.
> 
>  - for ((inst >> 26) != 0x3):
> 
>    stb: 0x18, or 0x19,
>    stw: 0x1a, stwm: 0x1b,
>    std: 0x1c.
> 
>  - else ((inst >> 26) == 0x3), need check:
> 
>    stb: 0x08, or 0x09,
>    stw: 0x0a,
>    std: 0x0b.
> 
> For clearer reason, not combine the logical comparation code together.
> 
> 2014-10-22  Chen Gang  <gang.chen.5i5j@gmail.com>
> 
>         * hppa-tdep.c (inst_saves_gr): Fix logical working flow typo
>         issue.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  gdb/hppa-tdep.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
> index 627f31a..4363ab4 100644
> --- a/gdb/hppa-tdep.c
> +++ b/gdb/hppa-tdep.c
> @@ -1376,16 +1376,35 @@ is_branch (unsigned long inst)
>  }
>  
>  /* Return the register number for a GR which is saved by INST or
> -   zero it INST does not save a GR.  */
> +   zero it INST does not save a GR.
> +
> +   inst_saves_gr() wants to recognize 'st??' instruction, it include 'stb' (for
> +   store byte), 'stw(m)' (for store word), and 'std' (for store double word).
> +
> +   They should be in the same format, and have neighbour numbers: especially,
> +   'stw(m)' need be in the middle of 'stb' and 'std'.
> +
> +    - for ((inst >> 26) != 0x3):
> +
> +      stb: 0x18, or 0x19,
> +      stw: 0x1a, stwm: 0x1b,
> +      std: 0x1c.
> +
> +    - else ((inst >> 26) == 0x3), need check :
> +
> +      stb: 0x08, or 0x09,
> +      stw: 0x0a,
> +      std: 0x0b.
> +*/
>  
>  static int
>  inst_saves_gr (unsigned long inst)
>  {
> -  /* Does it look like a stw?  */
> +  /* Does it look like a stw(m)?  */
>    if ((inst >> 26) == 0x1a || (inst >> 26) == 0x1b
>        || (inst >> 26) == 0x1f
> -      || ((inst >> 26) == 0x1f
> -	  && ((inst >> 6) == 0xa)))
> +      || ((inst >> 26) == 0x3
> +	  && ((inst >> 6) & 0xf) == 0xa))
>      return hppa_extract_5R_store (inst);
>  
>    /* Does it look like a std?  */
> @@ -1394,16 +1413,12 @@ inst_saves_gr (unsigned long inst)
>  	  && ((inst >> 6) & 0xf) == 0xb))
>      return hppa_extract_5R_store (inst);
>  
> -  /* Does it look like a stwm?  GCC & HPC may use this in prologues.  */
> -  if ((inst >> 26) == 0x1b)
> -    return hppa_extract_5R_store (inst);
> -
>    /* Does it look like sth or stb?  HPC versions 9.0 and later use these
>       too.  */
>    if ((inst >> 26) == 0x19 || (inst >> 26) == 0x18
>        || ((inst >> 26) == 0x3
>  	  && (((inst >> 6) & 0xf) == 0x8
> -	      || (inst >> 6) & 0xf) == 0x9))
> +	      || ((inst >> 6) & 0xf) == 0x9)))
>      return hppa_extract_5R_store (inst);
>  
>    return 0;
> 

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed