Improve MMIO base register resolution#220
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ff2dab2ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try: | ||
| offset = int(bracket.group(2), 16) if bracket.group(2) else 0 | ||
| except ValueError: | ||
| return None | ||
| literal_addr = addr + 8 + offset |
There was a problem hiding this comment.
The PC-relative literal resolution assumes literal_addr = addr + 8 + offset, which is only correct for ARM state. In Thumb mode the PC is addr + 4, so this will compute the wrong literal pool address and either miss the literal or resolve to the wrong value if there happens to be data at addr+8+offset. That leads to incorrect target values (or skipped samples) for Thumb disassembly. Consider making the PC advance conditional on the instruction mode (or leaving it unresolved when mode is unknown).
Useful? React with 👍 / 👎.
| resolved_address = _resolve_register_indirect_address(operands, register_bases) | ||
| if resolved_address is not None: | ||
| if op == "READ" and resolved_address in memory_literals: | ||
| target = memory_literals[resolved_address] | ||
| else: |
There was a problem hiding this comment.
register_bases is only populated on literal LDRs and immediate ADDs, but it is never cleared when the same register is overwritten by other instructions (e.g., MOV, SUB, LDR from a non-literal address). That means later [Rn,#imm] loads/stores can be resolved using a stale base and emit incorrect MMIO targets, whereas previously they would be skipped. Consider invalidating mappings on any instruction that writes a tracked register, or restricting the resolution to immediately following instructions.
Useful? React with 👍 / 👎.
…ing-in-mmio.py Improve MMIO base register resolution
1 participant
Motivation
LDR rX, [PC, #imm]) and literal pool data so pointer-table indirections can be followed.ADDinstructions (e.g.ADD rY, rX, #imm) to compute absolute targets.LDR/STR [rX, #imm]when the base register value is known.Description
_parse_data_lineand_DATA_VALUE_RE) and register/operand helpers (_REGISTER_RE,_BRACKET_BASE_RE,_ADD_IMMEDIATE_RE,_normalize_register,_extract_dest_register).memory_literals) and tracked register base values (register_bases) inside_collect_operationsto recordLDR rX, =...,LDR rX, [PC, #imm], and indirect literal-to-pointer mappings.ADD ...,#immsequences to compute derived register addresses and resolved register-indirect addresses (_resolve_register_indirect_address) soREAD/WRITEoperations may include absolutetargetvalues.bridge/tests/unit/test_mmio_heuristics.pyto add coverage for literal base-register indirect accesses and pointer-table literal resolution.Testing
pytest bridge/tests/unit/test_mmio_heuristics.pyand all tests passed (4 passed)._collect_operationslogic with cases for literal pool loads, register-derived addresses, and pointer-table dereferences.samplesinclude resolvedtargetvalues for indirectLDR/STRwhen resolvable.Codex Task