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

Dynamic block nodes should not generate non-null assertions #62328

Closed
SingleAccretion opened this issue Dec 3, 2021 · 2 comments · Fixed by #85103
Closed

Dynamic block nodes should not generate non-null assertions #62328

SingleAccretion opened this issue Dec 3, 2021 · 2 comments · Fixed by #85103
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Dec 3, 2021

Reproduction:

using System.Runtime.CompilerServices;

Problem(ref Unsafe.NullRef<byte>(), ref Unsafe.NullRef<byte>(), 0);

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
private static ref int Problem(ref byte dst, ref byte src, uint count)
{
    Unsafe.CopyBlock(ref dst, ref src, count);

    return ref Unsafe.As<byte, StructWithIndex>(ref src).Value;
}

struct StructWithIndex
{
    public int Index;
    public int Value;
}

Expected result: NRE.
Actual result: the program exits normally.

Cause: we make assertion for the IND struct node which is the source for the dynamic block copy:

GenTreeNode creates assertion:
N002 (  3,  2) [000011] ---X---N----              *  IND       struct <l:$1c1, c:$1c0>
In BB01 New Global Constant Assertion: ($81,$0) Value_Number {InitVal($41)} is not 0, index = #01

We cannot make non-null assertions for addresses of dynamic block nodes because in the degenerate case of zero-sized copies, the indirection will not be realized. There is language in ECMA 335 saying that this is what is to be expected:

III.3.30 cpblk – copy data from memory to memory

Exceptions:
System.NullReferenceException can be thrown if an invalid address is detected. 

This applies to both source and destination addresses.

category:correctness
theme:assertion-prop

@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Dec 3, 2021
@ghost
Copy link

ghost commented Dec 3, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Reproduction:

using System.Runtime.CompilerServices;

Problem(ref Unsafe.NullRef<byte>(), ref Unsafe.NullRef<byte>(), 0);

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
private static ref int Problem(ref byte dst, ref byte src, uint count)
{
    Unsafe.CopyBlock(ref dst, ref src, count);

    return ref Unsafe.As<byte, StructWithIndex>(ref src).Value;
}

struct StructWithIndex
{
    public int Index;
    public int Value;
}

Expected result: NRE.
Actual result: the program exits normally.

Cause: we make assertion for the IND struct node which is the source for the dynamic block copy:

GenTreeNode creates assertion:
N002 (  3,  2) [000011] ---X---N----              *  IND       struct <l:$1c1, c:$1c0>
In BB01 New Global Constant Assertion: ($81,$0) Value_Number {InitVal($41)} is not 0, index = #01

We cannot make non-null assertions for sources of dynamic block nodes because in the degenerate case of zero-sized copies, the indirection will not be realized. There is language in ECMA 335 saying that this is what is to be expected:

III.3.30 cpblk – copy data from memory to memory

Exceptions:
System.NullReferenceException can be thrown if an invalid address is detected. 

This applies to both source and destination addresses.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@SingleAccretion SingleAccretion self-assigned this Dec 3, 2021
@SingleAccretion SingleAccretion removed the untriaged New issue has not been triaged by the area owner label Dec 3, 2021
@SingleAccretion SingleAccretion added this to the Future milestone Dec 3, 2021
@SingleAccretion
Copy link
Contributor Author

Of course, a dynamic block is not the only kind of block that can have a size of 0. Simple BLKs can also be of that size. And they too should not generate non-null assertions (both LHS and RHS).

Note for future readers: #62743 will probably hide this because the RHSs of BLK ASGs are marked with GTF_NO_CSE. There are ways to defeat this:

private static void CpBlkOrder(void** dst, void*[] src, long count)
{
    IL.Emit.Ldarg(0);
    IL.Emit.Ldind_I();
    IL.Emit.Ldarg(1);
    IL.Emit.Ldc_I4(0);
    IL.Emit.Ldelem_I();
    IL.Emit.Ldarg(2);
    IL.Emit.Conv_Ovf_U4();
    IL.Emit.Volatile();
    IL.Emit.Cpblk();
}

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 20, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 23, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant