Reboxing byte values changed since .NET 4.5 #1323

Closed
sbradno opened this Issue Jul 31, 2015 · 7 comments

Projects

None yet

7 participants

@sbradno
sbradno commented Jul 31, 2015

Here's my solved stack overflow thread on this issue
http://stackoverflow.com/questions/31752991/net-4-6-breaks-xor-cipher-pattern

@mikedn
Contributor
mikedn commented Jul 31, 2015

That's a bug in the JIT compiler. The following C# code

class Program {
    static ushort SkillLevel;

    static int Main() {
        SkillLevel = 0x2121;
        SkillLevel = (ushort)((byte)SkillLevel ^ 0x21);
        return SkillLevel;
    }
}

is compiled to

 mov         word ptr [7FFA61024742h],2121h  
 xor         word ptr [7FFA61024742h],21h  ; missing byte cast
 movzx       eax,word ptr [7FFA61024742h]  
 ret

Basically the cast to byte has disappeared, it xors directly with SkillLevel.

@ayende
ayende commented Jul 31, 2015

Another ryujit issue?
On Aug 1, 2015 00:16, "mikedn" notifications@github.com wrote:

That's a bug in the JIT compiler. The following C# code

class Program {
static ushort SkillLevel;

static int Main() {
    SkillLevel = 0x2121;
    SkillLevel = (ushort)((byte)SkillLevel ^ 0x21);
    return SkillLevel;
}

}

is compiled to

mov word ptr [7FFA61024742h],2121h
xor word ptr [7FFA61024742h],21h ; missing byte cast
movzx eax,word ptr [7FFA61024742h]
ret

Basically the cast to byte has disappeared, it xors directly with
SkillLevel.


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

@mikedn
Contributor
mikedn commented Aug 1, 2015

@ayende Yes, it's a RyuJIT bug.

It appears that Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

Looks like this can be easily fixed by adding the following to IndirsAreEquivalent

    if (candidate->gtType != storeInd->gtType)
        return false;
@kangaroo
Contributor
kangaroo commented Aug 1, 2015
@ayende
ayende commented Aug 2, 2015

This is the 3rd - 4th? RyuJIT by now?

*Hibernating Rhinos Ltd *

Oren Eini* l CEO l *Mobile: + 972-52-548-6969

Office: +972-4-622-7811 *l *Fax: +972-153-4-622-7811

On Sat, Aug 1, 2015 at 9:00 AM, mikedn notifications@github.com wrote:

@ayende https://github.com/ayende Yes, it's a RyuJIT but.

It appears that Lowering::IndirsAreEquivalent doesn't check if the 2
indirections have the same type and because of that a RMW style instruction
is generated instead of the expected

movzx rax, byte ptr [7FFA61024742h]
xor eax, 33
mov word ptr [7FFA61024742h], ax

Looks like this can be easily fixed by adding the following to
IndirsAreEquivalent

if (candidate->gtType != storeInd->gtType)
    return false;


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

@mikedn
Contributor
mikedn commented Aug 2, 2015

@ayende Maybe, I don't keep count. I've seen more optimization bugs in VC++ over the years, RyuJIT does quite well 😄.

@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 3, 2015
@AndreyAkinshin AndreyAkinshin Fix #1323 (Reboxing byte values changed since .NET 4.5)
Implementation of the mikedn's approach: dotnet#1323 (comment)
a2e96c0
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 3, 2015
@AndreyAkinshin AndreyAkinshin Fix #1323 (xor and disappeared cast to byte)
Implementation of the mikedn's approach: dotnet#1323 (comment)
9c0ad41
@jkotas jkotas added bug CodeGen labels Aug 3, 2015
@cmckinsey
Contributor

@sbradno - thanks. @mikedn thanks your triage is super helpful and looks correct. I'll have someone grab this and run through testing on the desktop side.

@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 3, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behaviour was fixed by adding an additional check to IndirsAreEquivalent (Implementation of the mikedn's approach: dotnet#1323 (comment)).

Fix #1323.
f334878
@mmitche mmitche was assigned by cmckinsey Aug 3, 2015
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 3, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behaviour was fixed by adding an additional check to IndirsAreEquivalent (implementation of the mikedn's approach: dotnet#1323 (comment)).

Fix #1323.
f706bbe
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 3, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behaviour was fixed by adding an additional check to IndirsAreEquivalent (implementation of the mikedn's approach: dotnet#1323 (comment)).

Fix #1323.
9680a9f
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 3, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behavior was fixed by adding an additional check to IndirsAreEquivalent (implementation of the mikedn's approach: dotnet#1323 (comment)).

Fix #1323.
ffe3832
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 4, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behaviour was fixed by adding an additional check to IndirsAreEquivalent (implementation of the mikedn's approach: dotnet#1323 (comment)).

Fix #1323.
f333ca9
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 12, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behaviour was fixed by adding an additional check to IndirsAreEquivalent (implementation of the mikedn's approach: https://github.com/dotnet/coreclr/pull/1329/files#r36397171).

Fix #1323.
dd0c070
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 12, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behaviour was fixed by adding an additional check to IndirsAreEquivalent (implementation of the mikedn's approach: dotnet#1329 (comment)).

Fix #1323.
2ca313a
@AndreyAkinshin AndreyAkinshin added a commit to AndreyAkinshin/coreclr that referenced this issue Aug 12, 2015
@AndreyAkinshin AndreyAkinshin Fix a bug with disappeared cast to byte in numeric binary expressions
Lowering::IndirsAreEquivalent doesn't check if the 2 indirections have the same type and because of that a RMW style instruction is generated instead of the expected

movzx    rax, byte  ptr [7FFA61024742h]
xor      eax, 33
mov      word  ptr [7FFA61024742h], ax

This behavior was fixed by adding an additional check to IndirsAreEquivalent (implementation of the mikedn's approach: dotnet#1329 (comment)).

Fix #1323.
3bfbb9b
@mmitche mmitche closed this in #1329 Aug 12, 2015
@mmitche mmitche added a commit to mmitche/coreclr that referenced this issue Oct 14, 2015
@mmitche mmitche Remove assert found in internal testing
This assert was introduced with the fix for #1323 as a sanity check that truly incompatible types couldn't enter this function.  However, there are at least some cases where the types could be different.  In the case found, REF and LONG were the types and the operand was a GT_LEA, which is handled later in the function.
This assert was introduced with the fix for #1323 as a sanity check that truly incompatible types couldn't enter this function.  However, there are at least some cases where the types could be different.  In the case found, REF and LONG were the types and the operand was a GT_LEA, which is handled later in the function.
9006df0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment