Fix multiple hangs, require view selection, and minor improvements to lifting#18
Fix multiple hangs, require view selection, and minor improvements to lifting#18zznop wants to merge 1 commit intofluxchief:masterfrom
Conversation
zznop
commented
May 8, 2025
zznop
commented
- Fixes multiple bugs that cause Binja to hang during analysis
- Forces the user to select the AVR view in open with options
- Some minor improvements to lifting
- Some changes to RAM / ROM sizes for iotn88
* Fixes multiple bugs that cause Binja to hang during analysis * Forces the user to select the AVR view in open with options * Some minor improvements to lifting
| RAM_SIZE = 1024 * 16 | ||
| ROM_SIZE = 32 * 1024 |
|
Thanks for the PR Left a few comments, some context on a few of the changes would be great. |
| def is_valid_for_data(self, data): | ||
| return False # Must be force loaded with "Open with Options" | ||
|
|
||
| @classmethod | ||
| def is_force_loadable(self): |
There was a problem hiding this comment.
self should probably be cls for both of these
fluxchief
left a comment
fluxchief
left a comment
There was a problem hiding this comment.
Turns out I didn't know how to a review "github style" so my comments never showed up.. Anyways, here you go.
| class AVR(binaryninja.Architecture): | ||
| name = 'AVR' | ||
| address_size = 3 | ||
| address_size = 2 |
There was a problem hiding this comment.
Why are you changing this?
Two bytes is not enough to cover the address space of all AVR chips
| return nfo | ||
|
|
||
| nfo.length = ins.length() | ||
| return None |
There was a problem hiding this comment.
I have not tried this change, how does this behave if BN encounters this situation?
IIRC in the past it would just crash and that would not be acceptable
| nfo.add_branch(BranchType.FunctionReturn) | ||
| elif (isinstance(ins, instructions.Instruction_RCALL)): | ||
| v = addr + ins.operands[0].immediate_value | ||
| if v == (addr + 2): |
There was a problem hiding this comment.
I think this optimization is something BN itself should do?
| elif (isinstance(ins, instructions.Instruction_ICALL) or | ||
| isinstance(ins, instructions.Instruction_EICALL) or | ||
| isinstance(ins, instructions.Instruction_IJMP) or | ||
| elif (isinstance(ins, instructions.Instruction_IJMP) or |
| CHIP_ALIASES = ["iotn88", "ATtiny88"] | ||
| RAM_SIZE = 256 * 2 | ||
| ROM_SIZE = 4 * 1024 * 2 | ||
| RAM_SIZE = 1024 * 16 |
There was a problem hiding this comment.
These values are incorrect. Attiny88 has 8K flash and 512b SRAM:
https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/8008S.pdf
| def get_llil(self, il): | ||
| v = self._operands[0].immediate_value + self._addr | ||
| if v == (self._addr + 2): | ||
| il.append(il.push(2, il.const(2, v))) |
There was a problem hiding this comment.
Similar question as above (shouldn't BN figure this out), but also some devices will push 3 bytes instead of two.
3 participants
