Fix: firwmare start address assumptions #69

dragonmux commented 2025年12月17日 10:14:40 +01:00

In this PR we address the dual issues of the ELF container the firmware is read from being thrown away too quickly, and the guessed load address being wrong on parts that have the firmware start at someplace other than 0x08002000.

To address this we introduce a new FirmwareFile type and FirmwareStorage trait that allows bmputil to transparently hold firmware backed by an arbitrary file type and which knows how to determine, when available, the load address a file is intended for. This is then promulgated through to BmpDevice::download() to allow it to more smartly determine load address.

The introduced ELFFirmwareFile type that implemented the FirmwareStorage trait holds onto significantly more information about the ELF that was read in w/o discarding it, thus allowing the actual load address of the firmware to propagate properly. This type also implements proper loading of segments in a manner more conducive to correctly loading the firmware into Flash on the target even in the face of arbitrary section names appearing. It fills gaps between segments with the erased byte to create a contiguous firmware image.

There are still a bunch of assumptions being made that need addressing, however this unblocks us enough on hardware bringup and allows bmputil to be successfully used with other platforms such as Blackpill which also use a different firmware load address.

NB: This change may break ABI, however it does so on functionality that should never have been exposed to the world. Hopefully the new visibilities for the new functionality is set correctly in relation to that.

In this PR we address the dual issues of the ELF container the firmware is read from being thrown away too quickly, and the guessed load address being wrong on parts that have the firmware start at someplace other than 0x08002000. To address this we introduce a new `FirmwareFile` type and `FirmwareStorage` trait that allows `bmputil` to transparently hold firmware backed by an arbitrary file type and which knows how to determine, when available, the load address a file is intended for. This is then promulgated through to `BmpDevice::download()` to allow it to more smartly determine load address. The introduced `ELFFirmwareFile` type that implemented the `FirmwareStorage` trait holds onto significantly more information about the ELF that was read in w/o discarding it, thus allowing the actual load address of the firmware to propagate properly. This type also implements proper loading of segments in a manner more conducive to correctly loading the firmware into Flash on the target even in the face of arbitrary section names appearing. It fills gaps between segments with the erased byte to create a contiguous firmware image. There are still a bunch of assumptions being made that need addressing, however this unblocks us enough on hardware bringup and allows `bmputil` to be successfully used with other platforms such as Blackpill which also use a different firmware load address. NB: This change may break ABI, however it does so on functionality that should never have been exposed to the world. Hopefully the new visibilities for the new functionality is set correctly in relation to that.

semanticdiff-com[bot] commented 2025年12月17日 10:14:44 +01:00 (Migrated from github.com)

Review changes with SemanticDiff

Changed Files

File	Status
src/flasher.rs	10% smaller
src/bmp.rs	3% smaller
src/elf.rs	0% smaller
src/firmware_file/elf.rs	0% smaller
src/firmware_file/ihex.rs	0% smaller
src/firmware_file/mod.rs	0% smaller
src/firmware_file/raw.rs	0% smaller
src/firmware_type.rs	0% smaller
src/lib.rs	0% smaller

Review changes with &nbsp;<a href="https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes"><picture><source media="(prefers-color-scheme: dark)" srcset="https://app.semanticdiff.com/images/github_button.svg?theme=dark&smaller=2"><source media="(prefers-color-scheme: light)" srcset="https://app.semanticdiff.com/images/github_button.svg?theme=light&smaller=2"><img src="https://app.semanticdiff.com/images/github_button.svg?theme=light&smaller=2" align="center" alt="SemanticDiff"></picture></a> <details> <summary>Changed Files</summary> <br/> | File | Status | | :--- | :--- | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/flasher.rs)&nbsp;&nbsp;[src/flasher\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/flasher.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=10">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/flasher.rs) &nbsp;10% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/bmp.rs)&nbsp;&nbsp;[src/bmp\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/bmp.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=3">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/bmp.rs) &nbsp;3% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/elf.rs)&nbsp;&nbsp;[src/elf\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/elf.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=0">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/elf.rs) &nbsp;0% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/elf.rs)&nbsp;&nbsp;[src/firmware\_file/elf\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/elf.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=0">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/elf.rs) &nbsp;0% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/ihex.rs)&nbsp;&nbsp;[src/firmware\_file/ihex\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/ihex.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=0">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/ihex.rs) &nbsp;0% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/mod.rs)&nbsp;&nbsp;[src/firmware\_file/mod\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/mod.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=0">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/mod.rs) &nbsp;0% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/raw.rs)&nbsp;&nbsp;[src/firmware\_file/raw\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/raw.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=0">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_file/raw.rs) &nbsp;0% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_type.rs)&nbsp;&nbsp;[src/firmware\_type\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_type.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=0">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/firmware_type.rs) &nbsp;0% smaller | | [<img src="https://app.semanticdiff.com/static/file-icons/rust.svg" width="20" height="20" align="center">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/lib.rs)&nbsp;&nbsp;[src/lib\.rs](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/lib.rs) | [<img src="https://app.semanticdiff.com/images/progressbar.svg?progress=0">](https://app.semanticdiff.com/gh/blackmagic-debug/bmputil/pull/69/changes#src/lib.rs) &nbsp;0% smaller | </details>

codecov[bot] commented 2025年12月17日 10:17:21 +01:00 (Migrated from github.com)

Codecov Report

❌ Patch coverage is 0% with 201 lines in your changes missing coverage. Please review.
✅ Project coverage is 6.11%. Comparing base (2f500df) to head (a952aba).
⚠️ Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
src/firmware_file/elf.rs	0.00%	66 Missing ⚠️
src/firmware_type.rs	0.00%	61 Missing ⚠️
src/firmware_file/mod.rs	0.00%	25 Missing ⚠️
src/firmware_file/raw.rs	0.00%	18 Missing ⚠️
src/firmware_file/ihex.rs	0.00%	12 Missing ⚠️
src/flasher.rs	0.00%	12 Missing ⚠️
src/bmp.rs	0.00%	7 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@
## main #69 +/- ##
========================================
- Coverage 6.21% 6.11% -0.10% 
========================================
 Files 25 29 +4 
 Lines 3187 3237 +50 
 Branches 3187 3237 +50 
========================================
 Hits 198 198 
- Misses 2969 3019 +50 
 Partials 20 20 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

## [Codecov](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) Report :x: Patch coverage is `0%` with `201 lines` in your changes missing coverage. Please review. :white_check_mark: Project coverage is 6.11%. Comparing base ([`2f500df`](https://app.codecov.io/gh/blackmagic-debug/bmputil/commit/2f500df46d9a4974f83a5ccfd62213f5f6ef72df?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug)) to head ([`a952aba`](https://app.codecov.io/gh/blackmagic-debug/bmputil/commit/a952aba134a512b7cbbeb1bb911251cb71c7e58c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug)). :warning: Report is 17 commits behind head on main. | [Files with missing lines](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | Patch % | Lines | |---|---|---| | [src/firmware\_file/elf.rs](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&filepath=src%2Ffirmware_file%2Felf.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug#diff-c3JjL2Zpcm13YXJlX2ZpbGUvZWxmLnJz) | 0.00% | [66 Missing :warning: ](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | | [src/firmware\_type.rs](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&filepath=src%2Ffirmware_type.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug#diff-c3JjL2Zpcm13YXJlX3R5cGUucnM=) | 0.00% | [61 Missing :warning: ](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | | [src/firmware\_file/mod.rs](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&filepath=src%2Ffirmware_file%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug#diff-c3JjL2Zpcm13YXJlX2ZpbGUvbW9kLnJz) | 0.00% | [25 Missing :warning: ](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | | [src/firmware\_file/raw.rs](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&filepath=src%2Ffirmware_file%2Fraw.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug#diff-c3JjL2Zpcm13YXJlX2ZpbGUvcmF3LnJz) | 0.00% | [18 Missing :warning: ](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | | [src/firmware\_file/ihex.rs](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&filepath=src%2Ffirmware_file%2Fihex.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug#diff-c3JjL2Zpcm13YXJlX2ZpbGUvaWhleC5ycw==) | 0.00% | [12 Missing :warning: ](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | | [src/flasher.rs](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&filepath=src%2Fflasher.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug#diff-c3JjL2ZsYXNoZXIucnM=) | 0.00% | [12 Missing :warning: ](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | | [src/bmp.rs](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&filepath=src%2Fbmp.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug#diff-c3JjL2JtcC5ycw==) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug) | <details><summary>Additional details and impacted files</summary> ```diff @@ Coverage Diff @@ ## main #69 +/- ## ======================================== - Coverage 6.21% 6.11% -0.10% ======================================== Files 25 29 +4 Lines 3187 3237 +50 Branches 3187 3237 +50 ======================================== Hits 198 198 - Misses 2969 3019 +50 Partials 20 20 ``` </details> [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/blackmagic-debug/bmputil/pull/69?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=blackmagic-debug). <details><summary> :rocket: New features to boost your workflow: </summary> - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. </details>