Skip to content

Align jump targets to 4 bytes#210

Merged
romancardenas merged 1 commit into
rust-embedded:masterfrom
hegza:fix/align-jump-targets
May 17, 2024
Merged

Align jump targets to 4 bytes#210
romancardenas merged 1 commit into
rust-embedded:masterfrom
hegza:fix/align-jump-targets

Conversation

@hegza

@hegza hegza commented May 17, 2024

Copy link
Copy Markdown
Contributor

While testing the new v-trap implementation from #200 I noticed that upon trapping a vectored interrupt, the program counter ends up halfway between instructions right before _start_DefaultHandler_trap causing an illegal instruction exception.

The cause seems to be that this location (_start_DefaultHandler_trap) is reached via direct unconditional jump in _continue_interrupt_trap and the jump instruction is taken at 4-byte alignment. Setting these symbols to .align 4 fixes the problem on our (custom) machine.

I think it's a feature of RISC-V unconditional jumps that they can't express addresses below 4-byte alignment but it could be a feature(bug) of our machine as well. I'm a bit confused about this still because the disassembly shows the correct target j 119a <_start_DefaultHandler_trap> while the machine ends up jumping to 1198 and failing.

We use unconditional jumps to reach these locations but I think the
RISC-V unconditional jump cannot express locations that are not aligned
to 4 bytes.
@hegza hegza requested a review from a team as a code owner May 17, 2024 16:13
@romancardenas

Copy link
Copy Markdown
Contributor

Good catch! I wonder if 4 byte alignment is enough or it depends on the bus width?

@romancardenas romancardenas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@romancardenas romancardenas added this pull request to the merge queue May 17, 2024
Merged via the queue into rust-embedded:master with commit f7afa99 May 17, 2024
@hegza hegza deleted the fix/align-jump-targets branch May 29, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants