Skip to content
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

Update ConvSym to 2.12.1 (symbol heap overflow fix) #381

Merged
merged 1 commit into from
Dec 15, 2024

Conversation

vladikcomper
Copy link
Contributor

This PR updates ConvSym from 2.12 to 2.12.1. A minor patch fixes an edge-case bug in symbol generation where if one block is too large and symbol heap exceeds 64 kb, all further blocks are skipped. It also improves ConvSym README and adds some formatting improvements.

Fix background

If symbol generation pushed to absolute limit, where combined symbol texts for a small 64kb slice of ROM exceed 64kb even in compressed form, ConvSym gracefully displays an error like this:

ERROR: Symbols heap for block 01 exceeded 64kb limit, no more symbols can be stored in this block.

Block 01 in the example covers offsets 0x010000..0x01FFFF (second 64kb of ROM). The error means symbol table container ran out of space to store symbols across this small 64kb slice/block at the beginning of the ROM.

This error isn't critical: some symbols will be lost, but symbol table itself is perfectly valid.

However, there was an overlooked bug, where this error caused all further blocks to be ignored, meaning symbol table will be cut after the first heap overflow.

Fixed an edge-case bug in symbol generation where if one block is too large and symbol heap exceeds 64 kb, all further blocks are skipped.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Windows build of ConvSym is significantly smaller now. It appears the previous was either built -Og (it's now -O2) or I didn't strip debug information. It's still bigger than Linux build (~200 kb), because this one is statically linked.

Copy link
Owner

@Stephane-D Stephane-D left a comment

Choose a reason for hiding this comment

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

Thanks for the very reactive / quick fix 👍

@Stephane-D Stephane-D merged commit 1fcfb6f into Stephane-D:master Dec 15, 2024
1 of 2 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.

2 participants