[Pdbg] [PATCH 06/10] libpdbg: Return if invalid register numbers are given

[Alistair Popple]

On Friday, 15 June 2018 4:22:28 PM AEST rashmica wrote:
> 
> On 15/06/18 11:33, Alistair Popple wrote:
> > On Thursday, 31 May 2018 3:29:11 PM AEST Rashmica Gupta wrote:
> >> Currently if an invalid register number (eg 44) is given to the
> >> opcode functions an error is printed and the instruction with
> >> the invalid register number is returned. This means that this
> >> invalid instruction is rammed into the machine and causes
> >> the machine to reboot.
> >>
> >> So if an invalid register number is supplied, exit out.
> > Sorry, but I'm not sure I like the idea of just exiting out from the library in
> > the case of user error. Could we check the register numbers are valid in the
> > ram_*() instructions and return an error there instead? Or perhaps check the
> > opcode return value prior to ramming the instructions.
> Or return INVALID_OPCODE where INVALID_OPCODE is defined with the other
> opcodes?

Yep, that's basically what I was getting at. Although you'd gave to loop over
all the opcodes to check that, although I guess that would be easy enough to do
in ram_instructions() (although I'd rather we didn't even attempt instruction
ramming if user input is invalid so make sure it's a seperate check if you go
that path).

- Alistair

> > - Alistair
> >
> >> Signed-off-by: Rashmica Gupta <rashmica.g at gmail.com>
> >> ---
> >>  libpdbg/chip.c | 46 +++++++++++++++++++++++++++++++++++-----------
> >>  1 file changed, 35 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/libpdbg/chip.c b/libpdbg/chip.c
> >> index f478526..b698bcd 100644
> >> --- a/libpdbg/chip.c
> >> +++ b/libpdbg/chip.c
> >> @@ -28,66 +28,84 @@
> >>  
> >>  static uint64_t mfspr(uint64_t reg, uint64_t spr)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mfspr\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return MFSPR_OPCODE | (reg << 21) | ((spr & 0x1f) << 16) | ((spr & 0x3e0) << 6);
> >>  }
> >>  
> >>  static uint64_t mtspr(uint64_t spr, uint64_t reg)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mtspr\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return MTSPR_OPCODE | (reg << 21) | ((spr & 0x1f) << 16) | ((spr & 0x3e0) << 6);
> >>  }
> >>  
> >>  static uint64_t mfocrf(uint64_t reg, uint64_t cr)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mfocrf\n");
> >> -	if (cr > 7)
> >> +		exit(1);
> >> +	}
> >> +	if (cr > 7) {
> >>  		PR_ERROR("Invalid CR field specified\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return MFOCRF_OPCODE | (reg << 21) | (1U << (12 + cr));
> >>  }
> >>  
> >>  static uint64_t mfnia(uint64_t reg)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mfnia\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return MFNIA_OPCODE | (reg << 21);
> >>  }
> >>  
> >>  static uint64_t mtnia(uint64_t reg)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mtnia\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return MTNIA_OPCODE | (reg << 21);
> >>  }
> >>  
> >>  static uint64_t mfmsr(uint64_t reg)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mfmsr\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return MFMSR_OPCODE | (reg << 21);
> >>  }
> >>  
> >>  static uint64_t mtmsr(uint64_t reg)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mtmsr\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return MTMSR_OPCODE | (reg << 21);
> >>  }
> >>  
> >>  static uint64_t mfxerf(uint64_t reg, uint64_t field)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mfxerf\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	switch(field) {
> >>  	case 0:
> >> @@ -100,14 +118,17 @@ static uint64_t mfxerf(uint64_t reg, uint64_t field)
> >>  		return MFXERF3_OPCODE | (reg << 21);
> >>  	default:
> >>  		PR_ERROR("Invalid XER field specified\n");
> >> +		exit(1);
> >>  	}
> >>  	return 0;
> >>  }
> >>  
> >>  static uint64_t mtxerf(uint64_t reg, uint64_t field)
> >>  {
> >> -	if (reg > 31)
> >> +	if (reg > 31) {
> >>  		PR_ERROR("Invalid register specified for mtxerf\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	switch(field) {
> >>  	case 0:
> >> @@ -120,14 +141,17 @@ static uint64_t mtxerf(uint64_t reg, uint64_t field)
> >>  		return MTXERF3_OPCODE | (reg << 21);
> >>  	default:
> >>  		PR_ERROR("Invalid XER field specified\n");
> >> +		exit(1);
> >>  	}
> >>  	return 0;
> >>  }
> >>  
> >>  static uint64_t ld(uint64_t rt, uint64_t ds, uint64_t ra)
> >>  {
> >> -	if ((rt > 31) | (ra > 31) | (ds > 0x3fff))
> >> +	if ((rt > 31) | (ra > 31) | (ds > 0x3fff)) {
> >>  		PR_ERROR("Invalid register specified\n");
> >> +		exit(1);
> >> +	}
> >>  
> >>  	return LD_OPCODE | (rt << 21) | (ra << 16) | (ds << 2);
> >>  }
> >>
> >
> 
>