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

Fixed correcting branch sizes #35

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixed correcting branch sizes #35

wants to merge 2 commits into from

Conversation

svick
Copy link

@svick svick commented Jan 13, 2021

When using Unbreakable on some async code (see #1 (comment)), I got InvalidProgramException. This happens when, after AssemblyGuard's rewrite, there is a br.s instruction that jumps just over the limit of 127 bytes. Because Unbreakable computes IL offsets incorrectly, it decides the instruction does not need to be modified to br, even when it has to be.

The added test gets into the same situation by inserting over 100 nop instructions using empty statements. Without this fix, the test fails for me with:

    System.InvalidProgramException : Common Language Runtime detected an invalid program.
  Stack Trace: 
    Program.M()

@svick svick changed the title Fixed computing of IL offsets when correcting branch sizes Fixed correcting branch sizes Jan 15, 2021
@svick
Copy link
Author

svick commented Jan 15, 2021

I missed a case when one expansion of short jump forces a previous short jump to be expanded too, that's not fixed too.

@ashmind
Copy link
Owner

ashmind commented Feb 4, 2021

Thanks! I'll take a look now.

var operandValue = ((Instruction)instruction.Operand).Offset - (instruction.Offset + instruction.GetSize());
if (operandValue >= sbyte.MinValue && operandValue <= sbyte.MaxValue)
continue;
// expanding one short branch can force a previous short branch to be expanded, so we have to do this repeatedly
Copy link
Owner

Choose a reason for hiding this comment

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

Does this mean that instead of doing that repeatedly we can go once but in reverse?

@ashmind ashmind changed the base branch from master to main March 29, 2021 10:05
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.

2 participants