Deduplicate set_mpp and get_raw_faulting_instr #363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Both instructions performs exactly the same logic in userspace.rs and metal.rs and are not architecture dependent. This commit removes the smell
francois141
force-pushed
the
deduplicate
branch
from
8c5a377 to
e5f963c
Compare
Deduplicating write_pmp is not only a best practice for software engineers but also enhances the coverage of symbolic execution.
francois141
force-pushed
the
deduplicate
branch
from
e5f963c to
f87dc5e
Compare
CharlyCst
approved these changes
Dec 26, 2024
Comment on lines
+524
to
+543
| /// Return the faulting instruction at the provided exception PC. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// The trap info must correspond to a valid trap info, no further checks are performed. | ||
| pub unsafe fn get_raw_faulting_instr(trap_info: &TrapInfo) -> usize { | ||
| if trap_info.mcause == MCause::IllegalInstr as usize { | ||
| // First, try mtval and check if it contains an instruction | ||
| if trap_info.mtval != 0 { | ||
| return trap_info.mtval; | ||
| } | ||
| } | ||
|
|
||
| let instr_ptr = trap_info.mepc as *const u32; | ||
|
|
||
| // With compressed instruction extention ("C") instructions can be misaligned. | ||
| // TODO: add support for 16 bits instructions | ||
| let instr = ptr::read_unaligned(instr_ptr); | ||
| instr as usize | ||
| } |
There was a problem hiding this comment.
This will fault when run in user-space. We probably won't run into those kinds of issues until we verify the instruction fetch, so its fine for now, but lets keep that in mind if it shows up in the future.
There was a problem hiding this comment.
@CharlyCst Good to know. This commit was a simple refactoring and doesn't change the userspace function. Let me check if I find a way to formally verify the decoder
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Both instructions performs exactly the same logic in userspace.rs and metal.rs and are not architecture dependent. This commit removes the smell