Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to VS2015, Begin Offset API integration #444

Closed
wants to merge 4 commits into from

Conversation

stevemk14ebr
Copy link
Contributor

No description provided.

@aquynh
Copy link
Collaborator

aquynh commented Aug 8, 2015

i am still not convinced about having these info in the instruction structure, given that it is more towards encoding than decoding. let me think more about this PR.

btw, this should go to the "next" branch if it is merged, rather than the "master" branch.

thanks.

@aquynh aquynh added the question label Aug 8, 2015
@stevemk14ebr
Copy link
Contributor Author

I understand your hesitation but as a library that prides itself as having detailed instruction information i think it is important to include information such as this. Without such it's impossible to do some specific tasks, which would require switching to a different disassembly library, something i find very undesirable.

@aquynh
Copy link
Collaborator

aquynh commented Aug 8, 2015

i am still considering everything. meanwhile, you can always maintain the fork yourself, which is not expensive as this X86 code is quite stable - like what the author of #331 is doing.

thanks.

@ionescu007
Copy link

It looks like this information would be useful if someone wanted the
ability to re-write certain instructions easily, such as in an emulator
analyzing and patching the byte stream.

Best regards,
Alex Ionescu

On Sat, Aug 8, 2015 at 7:37 AM, Nguyen Anh Quynh notifications@github.com
wrote:

i am still considering everything. meanwhile, you can always maintain the
fork yourself, which is not expensive as this X86 code is quite stable -
like the author of #331 #331 is
doing.

thanks.


Reply to this email directly or view it on GitHub
#444 (comment).

@aquynh
Copy link
Collaborator

aquynh commented Aug 8, 2015

On Sat, Aug 8, 2015 at 8:06 AM, ionescu007 notifications@github.com wrote:

It looks like this information would be useful if someone wanted the
ability to re-write certain instructions easily, such as in an emulator
analyzing and patching the byte stream.

yes that is the main reason why this is needed.

thanks,
Q

@stevemk14ebr
Copy link
Contributor Author

I will be maintaining my fork as you suggested and will attempt to expose the suggested relative api in the cleanest manner i can. Thanks for putting Polyhook in the showcase btw!

@obs1dium
Copy link
Contributor

I'd be very interested in having this as well, since I need to reliably determine the position of the ModR/M byte. udis86 for example provides the very handy modrm_offset.

@stevemk14ebr
Copy link
Contributor Author

Check my branch i believe i implemented that
On Aug 26, 2015 9:53 AM, "obs1dium" notifications@github.com wrote:

I'd be very interested in having this as well, since I need to reliably
determine the position of the ModR/M byte. udis86 for example provides the
very handy modrm_offset.


Reply to this email directly or view it on GitHub
#444 (comment).

@obs1dium
Copy link
Contributor

Yes, I was going to quickly implement it myself but then found this PR. Hopefully it'll get merged into the next branch.

@kokole
Copy link

kokole commented Oct 19, 2015

I'm also looking forward to this, would be a great addition. By the way, it would be much better if there was a flag or a bool somewhere to check if certian fields of the structs are actually valid. For example, in cs_x86::disp the value could be 0, but it also means that it's irrelevant, which makes no sense.

@aquynh
Copy link
Collaborator

aquynh commented Feb 15, 2016

this is already handled in https://github.com/aquynh/capstone/tree/encoding. will review to merge this branch into "next" soon, thanks.

@aquynh aquynh closed this Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants