ScratchABit
ScratchABit copied to clipboard
pfalcon
x86_pymsasid plugin doesn't render instruction prefixes, consider Capstone integration
Just found this project, looks very interesting!
Pymsasid is an orphaned disassembler with a lot of issues on quality. You may consider using Capstone instead: https://github.com/aquynh/capstone. If you do, let me know if you need any help.
Cheers.
Thanks. Due to lot of PR, capstone was first thing I tried. But I found few issues (no complete arch-independent semantic information, lack of semantic info for some instructions at all (I remember LEA, maybe it was older capstone version)) which made me look for more maintainable alternative (like Python software, where finding a bug equals fixing it). Capstone is still in the queue of course, but I don't intend to look into it until I'm personally need ARM support (I'm not interested in x86 at all). It would be nice if someone wrote a capstone plugin before that ;-).
Can you provide more details on the problems with LEA (and other instructions)? If those are indeed bugs, fixing them ASAP is the priority.
Regarding "maintainable alternative", bug reports to Capstone are always fixed very quickly, so there is no reason to try to fix yourself.
Btw, if that was from older version, then you should definitely try the latest version since we fixed many issues recently.
Can you provide more details on the problems with LEA (and other instructions)?
In the meantime, if you'd like feedback, let's look at capstone's include/arm.h:
//> Operand type for instruction's operands
typedef enum arm_op_type {
ARM_OP_INVALID = 0, // = CS_OP_INVALID (Uninitialized).
ARM_OP_REG, // = CS_OP_REG (Register operand).
ARM_OP_IMM, // = CS_OP_IMM (Immediate operand).
ARM_OP_MEM, // = CS_OP_MEM (Memory operand).
As can be seen, ARM defines its own types for register, immediate, etc. operands. But every CPU has those types of operands! But in comments it says that ARM_OP_REG equals to CS_OP_REG, i.e. arm-specific value is actually an alias for generic value. So, suggestion, either:
- Get rid of CPU-specific aliases for generic operand types (proper solution) or
- If you think that target audience of you lib can't grok the fact that there're more CPUs than they're currently stuck with, and all CPU are basicly the same, then at least explicitly define aliases as ARM_OP_REG = CS_OP_REG, and provide public API guarantee that generic operand type constants are valid for any capstone-supported CPU. (Of course, a CPU still can define own adhoc operand types, but it would be preferrable to have a function which maps any such type to the closest generic one, e.g. ARM_OP_CIMM and ARM_OP_CIMM are all just CS_OP_IMM in my list).
With the above change, it would be very comfortable to use capstone for a project like scratchabit. Unlike now, where there's one library, but support for each CPU would need to be done separately, which leaves impression that Capstone has inconsistent design and is a random misaligned mix of supported CPUs.
Thanks again for listening.
the operand types are always guaranteed to be the same across all the architectures. see https://github.com/aquynh/capstone/issues/195
the reason i dont just directly put ARM_OP_REG = CS_OP_REG because some external scripts get confused. but value wise, <ARCH>_OP_REG always have the same value. same for other operand types (register, imm, etc)
does this answer your question?
".... which leaves impression that Capstone has inconsistent design and is a random misaligned mix of supported CPUs"
no, that is a wrong impression, and we make sure to not make a mistake here. any new operand types added are always handled with care, so dont worry.
the shared values above were carefully designed, but do not get random at all.
does this answer your question?
To the large part, yes. Thanks for the reference, I could easily submit such a ticket myself. When I get back to it, I'll report any issues/concerns I have to the capstone tracker, knowing that they're welcome.
yes, fixing bugs is always the top priority, certainly higher priority than adding new features. so your bug reports are always appreciated.
Ok, so I restarted work on the Capstone-based plugin, which put on the backburner back in 2015 due to various issues with Capstone.
Well, issues and missing features is still what I'm greeted with. I submitted:
- https://github.com/aquynh/capstone/issues/1069
- https://github.com/aquynh/capstone/issues/1071
- https://github.com/aquynh/capstone/issues/1072
- https://github.com/aquynh/capstone/issues/1075
Anyway, the initial version of the plugin is now in the master: https://github.com/pfalcon/ScratchABit/commit/f6ae905a46416b8987963eaacf51c20421145b1d . First 3 issues are worked around on the plugin side. 4th (#1075) made me however so sad that I decided to breathe more life in my https://github.com/pfalcon/pymsasid3 , as it's clear that Capstone's output can't be trusted and it needs to be verified against other disassemblers.
I will look into these issues, but have you tried with the "next" branch of Capstone yet?
On Jan 11, 2018 06:27, "Paul Sokolovsky" [email protected] wrote:
Ok, so I restarted work on the Capstone-based plugin, which put on the backburner back in 2015 due to various issues with Capstone.
Well, issues and missing features is still what I'm greeted with. I submitted:
- aquynh/capstone#1069 https://github.com/aquynh/capstone/issues/1069
- aquynh/capstone#1071 https://github.com/aquynh/capstone/issues/1071
- aquynh/capstone#1072 https://github.com/aquynh/capstone/issues/1072
- aquynh/capstone#1075 https://github.com/aquynh/capstone/issues/1075
Anyway, the initial version of the plugin is now in the master: f6ae905 https://github.com/pfalcon/ScratchABit/commit/f6ae905a46416b8987963eaacf51c20421145b1d . First 3 issues are worked around on the plugin side. 4th (#1075) made me however so sad that I decided to breathe more life in my https://github.com/pfalcon/pymsasid3 , as it's clear that Capstone's output can't be trusted and it needs to be verified against other disassemblers.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pfalcon/ScratchABit/issues/4#issuecomment-356758461, or mute the thread https://github.com/notifications/unsubscribe-auth/AFsG4EWEkwCWMecN15OrZD4G7Zsoqsjmks5tJTlQgaJpZM4EcFVm .
I will look into these issues, but have you tried with the "next" branch of Capstone yet?
Thanks for the reply! So far, I'm trying to stabilize things on my side of things, and testing with what end users would see, and just record/report what I see. I hope to get to try next branch and/or proposing fixes eventually. Some triaging of/comments to those issues would help.
Here's the latest one: https://github.com/aquynh/capstone/issues/1081
