Skip to content

Conversation

@tormodvolden
Copy link
Contributor

The program header table can describe zero or more segments. However, commit ca16d5f introduced checks in _read_segments() that will fail on a zero number of program header entries.

ELF files created by objcopy based on a binary file may not have any segments / program header table. The number of program header entries e_phnum in the ELF header is zero.

This was fine before commit ca16d5f, but is currently broken and esptool will exit with this error:
A fatal error occurred: No segment header found at offset 0000 in ELF file.

This PR reinstates the possibility of having no such entries by simply returning early and skip these checks in _read_segments() if e_phnum is zero.

The change has no implication if the number of entries is non-zero.

I have tested this change with the following hardware & software combinations:

Tested with ELF files created by objcopy that have no segments. esptool elf2image now successfully creates an image.

I have run the esptool automated integration tests with this change and the above hardware:

NO_TESTING

@github-actions
Copy link

github-actions bot commented Aug 28, 2025

Messages
📖 🎉 Good Job! All checks are passing!

👋 Hello tormodvolden, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against d27ce37

@github-actions github-actions bot changed the title fix(esptool): Allow ELF files without segments again fix(esptool): Allow ELF files without segments again (ESPTOOL-1132) Aug 28, 2025
@Dzarda7
Copy link
Collaborator

Dzarda7 commented Sep 1, 2025

Thanks @tormodvolden for the fix, will try to merge is ASAP.

@Dzarda7 Dzarda7 self-assigned this Sep 1, 2025
@Dzarda7
Copy link
Collaborator

Dzarda7 commented Sep 1, 2025

@tormodvolden can you please change the commit message to something like fix(elf2image): Handle ELF files with zero header counts. Thanks.

@tormodvolden
Copy link
Contributor Author

Sure, done.

@tormodvolden
Copy link
Contributor Author

My original commit summary tried do convey that this fixes some kind of regression, since having zero program header counts was working fine in older versions of esptool. Actually "program header counts" would be better, what do you think?

@Dzarda7
Copy link
Collaborator

Dzarda7 commented Sep 1, 2025

Actually "program header counts" would be better, what do you think?

I am okay with that if you prefer. Will wait for your change. You can mention the regression in the body, but write Allow ELF files without segments again is not the best, especially when this worked like 4 years ago.

@tormodvolden
Copy link
Contributor Author

OK, I edited it to say "program headers".

@Dzarda7
Copy link
Collaborator

Dzarda7 commented Sep 1, 2025

@tormodvolden sorry, we just merged some MR and we do not use merge commits, can you please rebase on master so you have credit in history?

The program header table can describe zero or more segments. However,
commit ca16d5f introduced checks in _read_segments() that will fail
on a zero number of program header entries.

Simply return early and skip these checks in _read_segments() if
e_phnum is zero.

Signed-off-by: Tormod Volden <[email protected]>
@tormodvolden
Copy link
Contributor Author

Rebased to latest master.

@espressif-bot espressif-bot merged commit d27ce37 into espressif:master Sep 1, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants