New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kernel: process: omit debug addrs for fixed apps #1930
Conversation
kernel/src/process.rs
Outdated
/// Return Ok(Flash Address, RAM address) if this app has | ||
/// fixed_addresses, Err(()) otherwise. | ||
fn check_for_fixed_address(flash: &'static [u8]) -> Result<(u32, u32), ()> { | ||
// Need to re-parse the TBF header to find if this app has fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a lot of work and non-trivial duplicated logic. Wouldn't it be easier to just store whether the app is PIC in the struct ProcessDebug
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would happen if #1830 (or something similar) was merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oy.. preaching to the choir, but oh what a nightmare cfg
s are.
By the same token though, if we're in the space-optimized side of the cfg
, this whole detail-laden panic handler is gone anyway, right?
If an app is compiled for a fixed address, then we do not need to custom make an LST file based on where it actually executed. The `make lst` command will work just fine.
21c3695
to
6565551
Compare
Ok changed it to save the fixed addresses rather than re-parse them. I don't really know which is better. |
6565551
to
f832848
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bors r+
If an app is compiled for a fixed address, then we do not need to custom make an LST file based on where it actually executed. The
make lst
command will work just fine. This updates the message that is printed on a panic for apps which are compiled with fixed addresses.Testing Strategy
This pull request was tested by verifying the correct message is printed on hail.
TODO or Help Wanted
n/a
Documentation Updated
/docs
, or no updates are required.Our example panic printout is still valid since the PIC app version did not change.
Formatting
make prepush
.