Let an explicit function map override the JAL-target scan - #206
Open
Sinan-Karakaya wants to merge 1 commit into
Open
Let an explicit function map override the JAL-target scan#206Sinan-Karakaya wants to merge 1 commit into
Sinan-Karakaya wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a function-boundary selection issue when both the internal JAL-target fallback scan and a Ghidra-exported function map are present. It ensures the explicit map’s function bounds take precedence so fallback “carvings” can’t override precise ranges and produce massively inflated outputs.
Changes:
- Parse Ghidra map rows into a temporary vector rather than immediately appending into
m_extraFunctions. - When a map is successfully loaded, remove auto-generated extra functions before appending map-derived functions.
- Preserve map-derived entries by appending them after the purge, then re-sorting/deduping.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1083
to
+1085
| m_extraFunctions.insert(m_extraFunctions.end(), | ||
| std::make_move_iterator(mapFunctions.begin()), | ||
| std::make_move_iterator(mapFunctions.end())); |
Comment on lines
+1066
to
+1074
| // An explicit function map is authoritative over the internal JAL-target | ||
| // scan. Those carvings end at the next JAL target or, for the last one in | ||
| // a region, at the end of the code section - which on single-PROGBITS | ||
| // executables runs straight through interleaved rodata. Because both the | ||
| // carvings ("sub_") and typical map names ("FUN_") count as | ||
| // auto-generated, a carving sharing a start with a map row used to | ||
| // survive this purge and then win the "larger end" tie-break below, | ||
| // replacing precise bounds with runaway ones. Drop every auto-named | ||
| // carving instead, then append the map rows. |
On an ELF with no symbols and no DWARF, parse() carves functions from JAL
targets. Those carvings end at the next JAL target or, for the last one in a
region, at the end of the code section, so on a single-PROGBITS executable
they can run straight through interleaved rodata.
loadGhidraFunctionMap() appended its rows to the same vector and then purged
auto-named entries only where no map row shared the start address. Since both
the carvings ("sub_") and the names Ghidra exports by default ("FUN_") count
as auto-generated, a carving that shared a start with a map row survived the
purge and then won the "larger end" tie-break, so the imprecise bounds
replaced the ones the map had just supplied.
Collect the map rows into a local vector, drop every auto-named carving once
the map has parsed, and append the rows afterwards. Entries named from
symbols or DWARF are unaffected.
On a 3 MB Metrowerks-built PS2 executable with an 11,491-row map, 5,613
functions (48.8%) had been emitted with inflated bounds; the worst grew from
368 bytes to 0x51 KB and produced 22 MB of C++ decoding string data as
instructions. Output for that function is now 19 KB and total output drops
from 235 MB to 180 MB.
Sinan-Karakaya
force-pushed
the
fix/function-map-authority
branch
from
August 17, 2026 16:47
0e5df64 to
6d952a2
Compare
This file contains hidden or 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
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.
On an ELF with no symbols and no DWARF,
parse()carves functions from JAL targets. Those carvings end at the next JAL target or, for the last one in a region, at the end of the code section — so on a single-PROGBITS executable they can run straight through interleaved rodata.loadGhidraFunctionMap()appended its rows to the same vector and then purged auto-named entries only where no map row shared the start address:return IsAutoGeneratedName(func.name) && !mapStarts.contains(func.start);Since both the carvings (
sub_) and the names Ghidra exports by default (FUN_) satisfyIsAutoGeneratedName, a carving that shared a start with a map row survived the purge. It then won the "larger end" tie-break in the sort just below, so the imprecise bounds replaced the ones the map had just supplied.This collects the map rows into a local vector, drops every auto-named carving once the map has parsed, and appends the rows afterwards. Entries named from symbols or DWARF are unaffected, since they are not auto-named.
Impact measured on a 3 MB Metrowerks-built PS2 executable with an 11,491-row map: 5,613 functions (48.8%) were being emitted with inflated bounds. The worst case grew from 368 bytes to 0x51 KB and produced 22 MB of C++ decoding string data as MMI/COP instructions; another went from 484 bytes to 6.9 MB. Overlapped tails were re-emitted inside every function that swallowed them.
After the fix that function emits 19 KB, total output drops from 235 MB to 180 MB, and the run no longer needs a workaround that renamed
FUN_to something non-auto just to win the tie-break. This looks like the cause behind #75 and #99.