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

replace udis with iced-x86 #41

Merged
merged 2 commits into from
Nov 22, 2023
Merged

Conversation

LunNova
Copy link

@LunNova LunNova commented Oct 20, 2023

#36

Not properly tested/needs more work.

Copy link
Owner

@Hpmason Hpmason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. Any other thoughts about the safety are welcome

src/arch/x86/trampoline/mod.rs Outdated Show resolved Hide resolved
src/arch/x86/trampoline/mod.rs Outdated Show resolved Hide resolved
src/arch/x86/trampoline/mod.rs Outdated Show resolved Hide resolved
src/arch/x86/trampoline/disasm.rs Outdated Show resolved Hide resolved
src/arch/x86/trampoline/mod.rs Outdated Show resolved Hide resolved
@LunNova
Copy link
Author

LunNova commented Nov 21, 2023

I feel like using region as an extra dependency internally might be going too far but can go for that if you want.

I've edited the doc for Builder::build to note that target..target+margin+15 must be valid to read as u8 to avoid UB, and applied the changes you requested.

@LunNova LunNova requested a review from Hpmason November 21, 2023 04:23
@LunNova
Copy link
Author

LunNova commented Nov 21, 2023

CI failure for i686-pc-windows-gnu seems to be unrelated, egor-tensin/setup-mingw@v2 is failing.

@LunNova LunNova marked this pull request as ready for review November 21, 2023 04:27
@Hpmason
Copy link
Owner

Hpmason commented Nov 22, 2023

I feel like using region as an extra dependency internally might be going too far but can go for that if you want.

I've edited the doc for Builder::build to note that target..target+margin+15 must be valid to read as u8 to avoid UB, and applied the changes you requested.

Looking at Detour::new, it already does some region checking so the docs for Builder::build should be enough.

CI failure for i686-pc-windows-gnu seems to be unrelated, egor-tensin/setup-mingw@v2 is failing.

Yeah, it's not the first time it's broken. I need to go in and fix what's breaking it, possibly look into finding a better GH Action to rely on. But it should be fine for this PR.

Thanks again for working on this replacing udis, I really appreciate the help!

@Hpmason Hpmason merged commit 19f2132 into Hpmason:master Nov 22, 2023
4 of 6 checks passed
@Hpmason
Copy link
Owner

Hpmason commented Nov 22, 2023

closes #36

@Hpmason Hpmason linked an issue Nov 22, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider replacing udis with iced-x86
2 participants