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

Access violation in clrjit.dll on Unsafe.CopyBlockUnaligned with constant 0 copy length #24846

Closed
RossNordby opened this issue May 30, 2019 · 4 comments

Comments

Projects
None yet
4 participants
@RossNordby
Copy link

commented May 30, 2019

On 3.0.0-preview5-27626-15 and 3.0.0-preview6-27728-04 (Windows 10 1809, x64), attempting to run this:

var destination = new byte[1];
var source = new byte[1];
Unsafe.CopyBlockUnaligned(ref destination[0], ref source[0], 0);

results in an access violation in clrjit.dll.

There is no issue if the parameter is not constant, like so:

[MethodImpl(MethodImplOptions.NoInlining)]
public static void Trub(uint copyCount)
{
    var destination = new byte[1];
    var source = new byte[1];
    Unsafe.CopyBlockUnaligned(ref destination[0], ref source[0], copyCount);
}

public static void Test()
{
    Trub(0);
}

Changing the above MethodImpl to AggressiveInlining reintroduces the access violation.

Disabling optimizations avoids the issue. Does not occur on 2.1.11 or 2.2.3.

@RossNordby

This comment has been minimized.

Copy link
Author

commented May 30, 2019

Also reproduces on 3.0.0-preview-27122-01.

@AndyAyersMS AndyAyersMS added this to the 3.0 milestone May 30, 2019

@AndyAyersMS AndyAyersMS added the bug label May 30, 2019

@AndyAyersMS

This comment has been minimized.

Copy link
Member

commented May 30, 2019

Checked jit asserts:

 Assertion failed 'blkNode->gtOper == GT_DYN_BLK' in 'X:Main()' (IL size 33)

    File: D:\repos\coreclr\src\jit\gentree.cpp Line: 15485
    Image: D:\repos\coreclr\bin\tests\Windows_NT.x64.Checked\Tests\Core_Root\CoreRun.exe

I think we are assuming that a BLK of width 0 is always a DYN_BLK.

cc @CarolEidt

@CarolEidt

This comment has been minimized.

Copy link
Member

commented May 30, 2019

I think we are assuming that a BLK of width 0 is always a DYN_BLK.

Yes, we are. I'll take this.

@CarolEidt CarolEidt self-assigned this May 30, 2019

CarolEidt added a commit to CarolEidt/coreclr that referenced this issue May 30, 2019

CarolEidt added a commit that referenced this issue May 31, 2019

@mikedn

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

FWIW Unsafe.InitBlock(p, 0, 0); asserts in morph:

Assert failure(PID 18128 [0x000046d0], Thread: 4604 [0x11fc]): Assertion failed 'blockSize != 0' in 'GitHub_24846:Test(long,long)' (IL size 9)

    File: d:\projects\coreclr\src\jit\morph.cpp Line: 9489
    Image: D:\Projects\coreclr\bin\Product\Windows_NT.x64.Checked\CoreRun.exe

If the asserts is removed then it seems to work fine, though unexpectedly generating a helper call:

G_M30461_IG02:
       33D2                 xor      edx, edx
       4533C0               xor      r8d, r8d
       E891257B5F           call     CORINFO_HELP_MEMSET
       90                   nop

CarolEidt added a commit to CarolEidt/coreclr that referenced this issue Jun 4, 2019

Cleanup block stores and test for 24846
Fix zero-length assert/bad codegen for initblk.
Remove redundant assertions in codegen and those that don't directly relate to codegen requirements.
Remove zero-length block store in `Lowering`.
Eliminate redundant LEA that was being generated by `genCodeForCpBlk`.
Rename `genCodeFor[Cp|Init]Blk` to `genCodeFor[Cp|Init]BlkHelper` to parallel the other forms.
Fix the test case for dotnet#24846.

CarolEidt added a commit that referenced this issue Jun 5, 2019

Cleanup block stores and test for 24846 (#24950)
* Cleanup block stores and test for 24846

Fix zero-length assert/bad codegen for initblk.
Remove redundant assertions in codegen and those that don't directly relate to codegen requirements.
Eliminate redundant LEA that was being generated by `genCodeForCpBlk`.
Rename `genCodeFor[Cp|Init]Blk` to `genCodeFor[Cp|Init]BlkHelper` to parallel the other forms.
Fix the test case for #24846.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.