Conversation
|
Thanks! It seems to be a different tracer compared with the existing trace extension made by @elicn |
No problem, figured I'd share the code I had to make anyways. Yeah that one is for viewing the straight disassembly trace out of the box. This one is loaded into IDA's Tenet so you can see how it ran through the decompiled binary, what basic blocks it took, etc.. |
|
Nice work! Welcome to Qiling! |
elicn
left a comment
elicn
left a comment
There was a problem hiding this comment.
Thanks for the great contribution!
I haven't got the chance to review it on time, but here are my belated comments anyway - for your reference. Some are petty, but some are not (:
| def __init__(self): | ||
| super().__init__() | ||
|
|
||
| @property |
There was a problem hiding this comment.
Aggregating property on top of staticmethod doesn't work as a static property anymore.
See: https://stackoverflow.com/questions/58871322/property-with-staticmethod-in-python-3 (see references on top)
| universally usable by different tracers | ||
| """ | ||
|
|
||
| arm_registers = { |
There was a problem hiding this comment.
These registers mappings are [somewhat] a duplication of the arch classes registers mappings.
Can we re-use the existing ones to create these and avoid code duplication? (with some necessary modifications, of course)
| } | ||
|
|
||
| def __init__(self, arch): | ||
| if isinstance(arch, arm.QlArchARM): |
There was a problem hiding this comment.
Wouldn't a dictionaty-based selection [by arch.type] look better then multiple elifs..?
| @@ -0,0 +1,103 @@ | |||
| # This code structure is copied and modified from the coverage extension | |||
|
|
|||
| import qiling | |||
There was a problem hiding this comment.
Better do from qiling import Qiling.
If this is only for type annotations, then it's would be best to put it under if TYPE_CHECKING to reduce unnecesary runtime depedencies.
|
|
||
| def _add_delta(self): | ||
| # Cover glitch cases where nothing changed | ||
| if self.current_delta != []: |
There was a problem hiding this comment.
petty: Could omit the != [] part to make it look cleaner.
|
|
||
|
|
||
| @staticmethod | ||
| def mem_access_callback(ql, access, address, size, value, self): |
There was a problem hiding this comment.
Type annotations (where possible) could be great to keep the code readable and maintainable.
| # Since we are reading memory, we just read it ourselves | ||
| access_type = "mr" | ||
| value = ql.mem.read(address, size) | ||
| delta = f"{access_type}={hex(address)}:{value.hex()}" |
There was a problem hiding this comment.
petty: Use f'{address:#x}' intead of hex(address)
| elif access == UC_MEM_WRITE: | ||
| # Hook is before it's written, so we have to use the "value" | ||
| access_type = "mw" | ||
| if ql.arch.endian == QL_ENDIAN.EL: |
There was a problem hiding this comment.
petty: Use ternary operation when an if-else construct asigns a value to the same variable.
| endian = 'little' | ||
| else: | ||
| endian = 'big' | ||
| if value < 0: |
There was a problem hiding this comment.
petty: Could be just sign = (value < 0)
| def dump_trace(self, trace_file: str): | ||
| with open(trace_file, "w") as trace: | ||
| # Write out each delta on a separate line | ||
| for delta in self.deltas: |
There was a problem hiding this comment.
petty: Alternatively, trace.write('\n'.join(self.deltas) + '\n') could be faster and cleaner
5 participants
Checklist
Which kind of PR do you create?
This pull request adds a tracer for IDAPro's Tenet plugin in the extensions. The code structure is based completely off of the coverage extension using the context manager, allowing other tracing formats to be easily added in the future.
Tenet trace format
Currently supports:
Coding convention?
Extra tests?
Changelog?
Target branch?
One last thing