Washi1337 / AsmResolver

A library for creating, reading and editing PE files and .NET modules.
https://docs.washi.dev/asmresolver/
MIT License
848 stars 127 forks source link

[Question] Best Practice for handling 'shortinlinebr' instruction with Out-of-Range Target #563

Closed crackleeeessyp closed 3 months ago

crackleeeessyp commented 3 months ago

Description

When adding instructions within a method, I encounter an error stating:

Branch target at IL_00xx is too far away for a ShortInlineBr instruction.

Is there a more efficient solution to address this issue, rather than iterating through all the instructions in the body and replacing each short 'br' branch instruction with a long one?

ds5678 commented 3 months ago

ExpandMacros Make changes OptimizeMacros

Washi1337 commented 3 months ago

No. This will require a full iteration of the entire method body.

AsmResolver has this expansion and optimization of macros built in via the ExpandMacros and OptimizeMacros methods. A typical workflow for editing method bodies is as follows:

MethodDefinition method = ...;
var instructions = method.CilMethodBody.Instructions;

// Expand all instructions (including branches) in a method body to their long forms.
instructions.ExpandMacros();

/* ... Make your changes  ... */

// Optimize all instructions (including branches) to their short forms where possible.
instructions.OptimizeMacros();

See also: https://docs.washi.dev/asmresolver/guides/dotnet/managed-method-bodies.html#expanding-and-optimizing-macros

crackleeeessyp commented 3 months ago

Thanks all for your help, will have a try~

Washi1337 commented 3 months ago

@crackleeeessyp can I consider this issue solved?

ds5678 commented 3 months ago

How easy would it be for AsmResolver to switch the op code when writing instead of throwing an exception? Is that a solution you would be comfortable with?

Washi1337 commented 3 months ago

@ds5678 I am not a fan of it as it introduces an asymmetry. It would mean short branches would be promoted automatically, but long branches definitely should not be (i.e., it is perfectly valid to have "short long branches" so AsmResolver should be able to write them as well). Besides, I don't think it would be contributing to the comprehensibility/predictability of how the library behaves (i.e., I predict questions such as "why are my instructions/method bodies updating without me explicitly stating so?")

ds5678 commented 3 months ago

All good 👍

crackleeeessyp commented 3 months ago

@crackleeeessyp can I consider this issue solved?

Thanks, I think so, use ExpandMacros solved the problem!