Corrected issue with byte order expectations for target registers.

This was the cause for that issue with GDB using the incorrect memory addresses for local variables, after the PC and SP had been changed.
Also renamed SP start address parameter to make the byte order of the register clear.
This commit is contained in:
Nav
2021-07-02 01:34:17 +01:00
parent 9861c3652a
commit aa6395a002
5 changed files with 30 additions and 17 deletions

View File

@@ -44,7 +44,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
public ?int $ocdDataRegister = null;
public ?int $statusRegisterStartAddress = null;
public ?int $statusRegisterSize = null;
public ?int $stackPointerRegisterStartAddress = null;
public ?int $stackPointerRegisterLowAddress = null;
public ?int $stackPointerRegisterSize = null;
public ?int $spmcRegisterStartAddress = null;
public ?int $osccalAddress = null;
@@ -223,13 +223,13 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
if (isset($cpuRegisterGroup->registersMappedByName['sp'])) {
$stackPointerRegister = $cpuRegisterGroup->registersMappedByName['sp'];
$this->stackPointerRegisterSize = $stackPointerRegister->size;
$this->stackPointerRegisterStartAddress = $stackPointerRegister->offset;
$this->stackPointerRegisterLowAddress = $stackPointerRegister->offset;
} else {
if (isset($cpuRegisterGroup->registersMappedByName['spl'])) {
$stackPointerLowRegister = $cpuRegisterGroup->registersMappedByName['spl'];
$this->stackPointerRegisterSize = $stackPointerLowRegister->size;
$this->stackPointerRegisterStartAddress = $stackPointerLowRegister->offset;
$this->stackPointerRegisterLowAddress = $stackPointerLowRegister->offset;
if (isset($cpuRegisterGroup->registersMappedByName['sph'])) {
$stackPointerHighRegister = $cpuRegisterGroup->registersMappedByName['sph'];
@@ -451,7 +451,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
$failures[] = 'Unknown AVR8 family';
}
if (is_null($this->stackPointerRegisterStartAddress)) {
if (is_null($this->stackPointerRegisterLowAddress)) {
$failures[] = 'Missing stack pointer register start address.';
}

View File

@@ -312,10 +312,12 @@ void GdbRspDebugServer::handleGdbPacket(CommandPackets::ReadGeneralRegisters& pa
);
/*
* Finally, implode the register values, convert to hexadecimal form and send to the GDB client.
* Finally, reverse the register values (as they're all currently in MSB, but GDB expects them in LSB), implode
* the register values, convert to hexadecimal form and send to the GDB client.
*/
auto registers = std::vector<unsigned char>();
for (const auto& reg : registerSet) {
for (auto& reg : registerSet) {
std::reverse(reg.value.begin(), reg.value.end());
registers.insert(registers.end(), reg.value.begin(), reg.value.end());
}

View File

@@ -508,7 +508,7 @@ void EdbgAvr8Interface::configure(const TargetConfig& targetConfig) {
void EdbgAvr8Interface::setTargetParameters(const Avr8Bit::TargetParameters& config) {
this->targetParameters = config;
if (!config.stackPointerRegisterStartAddress.has_value()) {
if (!config.stackPointerRegisterLowAddress.has_value()) {
throw Exception("Failed to find stack pointer register start address");
}
@@ -816,11 +816,19 @@ std::uint32_t EdbgAvr8Interface::getProgramCounter() {
}
TargetRegister EdbgAvr8Interface::getStackPointerRegister() {
return TargetRegister(TargetRegisterDescriptor(TargetRegisterType::STACK_POINTER), this->readMemory(
auto stackPointerValue = this->readMemory(
Avr8MemoryType::SRAM,
this->targetParameters.stackPointerRegisterStartAddress.value(),
this->targetParameters.stackPointerRegisterLowAddress.value(),
this->targetParameters.stackPointerRegisterSize.value()
));
);
/*
* The SP low byte comes before the high byte, so the SP is stored in LSB form. We use std::reverse() here to
* convert it to MSB.
*/
std::reverse(stackPointerValue.begin(), stackPointerValue.end());
return TargetRegister(TargetRegisterDescriptor(TargetRegisterType::STACK_POINTER), stackPointerValue);
}
TargetRegister EdbgAvr8Interface::getStatusRegister() {
@@ -843,9 +851,12 @@ void EdbgAvr8Interface::setStackPointerRegister(const TargetRegister& stackPoint
registerValue.insert(registerValue.begin(), maximumStackPointerRegisterSize - registerValue.size(), 0x00);
}
// Convert SP to LSB
std::reverse(registerValue.begin(), registerValue.end());
this->writeMemory(
Avr8MemoryType::SRAM,
this->targetParameters.stackPointerRegisterStartAddress.value(),
this->targetParameters.stackPointerRegisterLowAddress.value(),
registerValue
);
}

View File

@@ -106,7 +106,7 @@ void Avr8::loadTargetParameters() {
auto stackPointerRegister = this->targetDescriptionFile->getStackPointerRegister();
if (stackPointerRegister.has_value()) {
this->targetParameters->stackPointerRegisterStartAddress = cpuRegistersOffset + stackPointerRegister->offset;
this->targetParameters->stackPointerRegisterLowAddress = cpuRegistersOffset + stackPointerRegister->offset;
this->targetParameters->stackPointerRegisterSize = stackPointerRegister->size;
} else {
@@ -115,7 +115,7 @@ void Avr8::loadTargetParameters() {
auto stackPointerHighRegister = this->targetDescriptionFile->getStackPointerHighRegister();
if (stackPointerLowRegister.has_value()) {
this->targetParameters->stackPointerRegisterStartAddress = cpuRegistersOffset
this->targetParameters->stackPointerRegisterLowAddress = cpuRegistersOffset
+ stackPointerLowRegister->offset;
this->targetParameters->stackPointerRegisterSize = stackPointerLowRegister->size;
}
@@ -760,10 +760,10 @@ TargetRegister Avr8::getProgramCounterRegister() {
auto programCounter = this->getProgramCounter();
return TargetRegister(TargetRegisterDescriptor(TargetRegisterType::PROGRAM_COUNTER), {
static_cast<unsigned char>(programCounter),
static_cast<unsigned char>(programCounter >> 8),
static_cast<unsigned char>(programCounter >> 16),
static_cast<unsigned char>(programCounter >> 24),
static_cast<unsigned char>(programCounter >> 16),
static_cast<unsigned char>(programCounter >> 8),
static_cast<unsigned char>(programCounter),
});
}

View File

@@ -29,7 +29,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
std::optional<std::uint8_t> ocdDataRegister;
std::optional<std::uint16_t> statusRegisterStartAddress;
std::optional<std::uint16_t> statusRegisterSize;
std::optional<std::uint16_t> stackPointerRegisterStartAddress;
std::optional<std::uint16_t> stackPointerRegisterLowAddress;
std::optional<std::uint16_t> stackPointerRegisterSize;
std::optional<std::uint8_t> spmcRegisterStartAddress;
std::optional<std::uint8_t> osccalAddress;