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

[rashmica]

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?
>
> - 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);
>>  }
>>
>