Span<byte> from local variable bug in .net 7.x Release #89666

check4game · 2023-07-29T11:18:37Z

Description

When using Span to map a local variable to an array, after a certain number of iterations, changing the local variable does not change the bytes in the Span

    int value = 0;

    Span<byte> span = MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value, 1));

    for (int i = 0; i < numEntries; i++)
    {
        //value = 0; bug fix for .net 7.x Release

        value = random.Next();

        file.Write(span);
    }

Reproduction Steps

using System.Runtime.InteropServices;

#if DEBUG
string config = "Deb";
#else
string config = "Rel";
#endif

string template = "{0}.{1}.dat.{2}";

test(5000, 0);
test(5000, 1);
test(5000, 2);

cmp(0, 1);
cmp(0, 2);
cmp(1, 2);

void cmp(int fIdx, int sIdx)
{
    byte[] f = File.ReadAllBytes(string.Format(template, config, Environment.Version, fIdx));
    byte[] s = File.ReadAllBytes(string.Format(template, config, Environment.Version, sIdx));

    for (int i = 0; i < f.Length; i++)
    {
        if (f[i] != s[i])
        {
            Console.WriteLine("first diff {0} ~ {1}, offset={2}, {3}/{4}", f[i], s[i], i, string.Format(template, config, Environment.Version, fIdx), string.Format(template, config, Environment.Version, sIdx));
            return;
        }
    }

    Console.WriteLine("{0}/{1} is equal", string.Format(template, config, Environment.Version, fIdx), string.Format(template, config, Environment.Version, sIdx));
}

void test(long numEntries, int idx)
{
    Random random = new Random(1);

    using var file = File.OpenWrite(string.Format(template, config, Environment.Version, idx));

    file.SetLength(0);

    int value = 0;

    Span<byte> span = MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value, 1));

    for (int i = 0; i < numEntries; i++)
    {
        //value = 0; bug fix for .net 7.x Release

        value = random.Next();

        file.Write(span);
    }
}

Expected behavior

all output files must be identical

Actual behavior

Results
 
first diff 15 ~ 222, offset=19996, Rel.7.0.9.dat.0/Rel.7.0.9.dat.1
first diff 96 ~ 241, offset=3996, Rel.7.0.9.dat.0/Rel.7.0.9.dat.2
first diff 96 ~ 241, offset=3996, Rel.7.0.9.dat.1/Rel.7.0.9.dat.2

Deb.7.0.9.dat.0/Deb.7.0.9.dat.1 is equal
Deb.7.0.9.dat.0/Deb.7.0.9.dat.2 is equal
Deb.7.0.9.dat.1/Deb.7.0.9.dat.2 is equal

Rel.6.0.20.dat.0/Rel.6.0.20.dat.1 is equal
Rel.6.0.20.dat.0/Rel.6.0.20.dat.2 is equal
Rel.6.0.20.dat.1/Rel.6.0.20.dat.2 is equal

Regression?

No response

Known Workarounds

No response

Configuration

.net 6.0.20
.net 7.0.9
Windows Server 2022

Other information

No response

The text was updated successfully, but these errors were encountered:

ghost · 2023-07-29T11:32:39Z

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

Issue Details

Description

When using Span to map a local variable to an array, after a certain number of iterations, changing the local variable does not change the bytes in the Span

    int value = 0;

    Span<byte> span = MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value, 1));

    for (int i = 0; i < numEntries; i++)
    {
        //value = 0; bug fix for .net 7.x Release

        value = random.Next();

        file.Write(span);
    }

Reproduction Steps

using System.Runtime.InteropServices;

#if DEBUG
string config = "Deb";
#else
string config = "Rel";
#endif

string template = "{0}.{1}.dat.{2}";

test(5000, 0);
test(5000, 1);
test(5000, 2);

cmp(0, 1);
cmp(0, 2);
cmp(1, 2);

void cmp(int fIdx, int sIdx)
{
    byte[] f = File.ReadAllBytes(string.Format(template, config, Environment.Version, fIdx));
    byte[] s = File.ReadAllBytes(string.Format(template, config, Environment.Version, sIdx));

    for (int i = 0; i < f.Length; i++)
    {
        if (f[i] != s[i])
        {
            Console.WriteLine("first diff {0} ~ {1}, offset={2}, {3}/{4}", f[i], s[i], i, string.Format(template, config, Environment.Version, fIdx), string.Format(template, config, Environment.Version, sIdx));
            return;
        }
    }

    Console.WriteLine("{0}/{1} is equal", string.Format(template, config, Environment.Version, fIdx), string.Format(template, config, Environment.Version, sIdx));
}

void test(long numEntries, int idx)
{
    Random random = new Random(1);

    using var file = File.OpenWrite(string.Format(template, config, Environment.Version, idx));

    file.SetLength(0);

    int value = 0;

    Span<byte> span = MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value, 1));

    for (int i = 0; i < numEntries; i++)
    {
        //value = 0; bug fix for .net 7.x Release

        value = random.Next();

        file.Write(span);
    }
}

Expected behavior

all output files must be identical

Actual behavior

Results
 
first diff 15 ~ 222, offset=19996, Rel.7.0.9.dat.0/Rel.7.0.9.dat.1
first diff 96 ~ 241, offset=3996, Rel.7.0.9.dat.0/Rel.7.0.9.dat.2
first diff 96 ~ 241, offset=3996, Rel.7.0.9.dat.1/Rel.7.0.9.dat.2

Deb.7.0.9.dat.0/Deb.7.0.9.dat.1 is equal
Deb.7.0.9.dat.0/Deb.7.0.9.dat.2 is equal
Deb.7.0.9.dat.1/Deb.7.0.9.dat.2 is equal

Rel.6.0.20.dat.0/Rel.6.0.20.dat.1 is equal
Rel.6.0.20.dat.0/Rel.6.0.20.dat.2 is equal
Rel.6.0.20.dat.1/Rel.6.0.20.dat.2 is equal

Regression?

No response

Known Workarounds

No response

Configuration

.net 6.0.20
.net 7.0.9
Windows Server 2022

Other information

No response

Author:	check4game
Assignees:	-
Labels:	`area-CodeGen-coreclr`, `untriaged`, `needs-area-label`
Milestone:	-

MichalPetryka · 2023-07-29T16:00:23Z

Does this reproduce with DOTNET_TieredCompilation=0? Could you check on .Net 8 too?

This looks like an OSR bug, cc @AndyAyersMS

AndyAyersMS · 2023-07-29T17:36:10Z

Does this reproduce with DOTNET_TieredCompilation=0? Could you check on .Net 8 too?

This looks like an OSR bug, cc @AndyAyersMS

Yes, it does.

@check4game thank you for reporting this.

AndyAyersMS · 2023-07-29T17:36:37Z

Also repros in 8.0.

AndyAyersMS · 2023-07-30T05:09:31Z

@dotnet/jit-contrib feel free to poach this one if you want to gain some experience with OSR.

AndyAyersMS · 2023-07-31T21:26:26Z

The bug is in this bit of code. For OSR we don't have accurate ref counts like we do for regular methods, since some of the uses may be in the Tier0 part of the method.

runtime/src/coreclr/jit/liveness.cpp

Lines 1576 to 1599 in e42429e

    
           // We have accurate ref counts when running late liveness so we can eliminate 
        
           // some stores if the lhs local has a ref count of 1. 
        
           if (isDef && compRationalIRForm && (varDsc.lvRefCnt() == 1) && !varDsc.lvPinned) 
        
           { 
        
               if (varDsc.lvIsStructField) 
        
               { 
        
                   if ((lvaGetDesc(varDsc.lvParentLcl)->lvRefCnt() == 1) && 
        
                       (lvaGetParentPromotionType(&varDsc) == PROMOTION_TYPE_DEPENDENT)) 
        
                   { 
        
                       return true; 
        
                   } 
        
               } 
        
               else if (varTypeIsPromotable(varDsc.lvType)) 
        
               { 
        
                   if (lvaGetPromotionType(&varDsc) != PROMOTION_TYPE_INDEPENDENT) 
        
                   { 
        
                       return true; 
        
                   } 
        
               } 
        
               else 
        
               { 
        
                   return true; 
        
               } 
        
           }

Fix is to simply exclude OSR locals here.

The local var ref count for OSR locals can be misleading, since some of the appearances may be in the "tier0" parts of the methods and won't be visible once we trim the OSR method down to just the part we're going to compile. So, we can't rely on ref counts to enable a dead store of an OSR local. Fixes dotnet#89666.

The local var ref count for OSR locals can be misleading, since some of the appearances may be in the "tier0" parts of the methods and won't be visible once we trim the OSR method down to just the part we're going to compile. Fix by giving OSR-exposed locals an implicit ref count. Fixes #89666.

Backport of dotnet#89743 to 7.0. The local var ref count for OSR locals can be misleading, since some of the appearances may be in the "tier0" parts of the methods and won't be visible once we trim the OSR method down to just the part we're going to compile. Fix by giving OSR-exposed locals an implicit ref count. Fixes dotnet#89666.

Backport of #89743 to 7.0. The local var ref count for OSR locals can be misleading, since some of the appearances may be in the "tier0" parts of the methods and won't be visible once we trim the OSR method down to just the part we're going to compile. Fix by giving OSR-exposed locals an implicit ref count. Fixes #89666.

dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 29, 2023

ghost added the untriaged New issue has not been triaged by the area owner label Jul 29, 2023

jkotas added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 29, 2023

AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Jul 29, 2023

AndyAyersMS self-assigned this Jul 29, 2023

AndyAyersMS added this to the 8.0.0 milestone Jul 29, 2023

AndyAyersMS mentioned this issue Jul 31, 2023

JIT: don't dead store untracked OSR locals based on ref count #89743

Merged

ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 31, 2023

AndyAyersMS closed this as completed in #89743 Aug 1, 2023

ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 1, 2023

AndyAyersMS mentioned this issue Aug 2, 2023

JIT: don't dead store OSR exposed locals #89867

Merged

ghost locked as resolved and limited conversation to collaborators Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Span<byte> from local variable bug in .net 7.x Release #89666

Span<byte> from local variable bug in .net 7.x Release #89666

check4game commented Jul 29, 2023

ghost commented Jul 29, 2023

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

MichalPetryka commented Jul 29, 2023

AndyAyersMS commented Jul 29, 2023

AndyAyersMS commented Jul 29, 2023

AndyAyersMS commented Jul 30, 2023

AndyAyersMS commented Jul 31, 2023

Span<byte> from local variable bug in .net 7.x Release #89666

Span<byte> from local variable bug in .net 7.x Release #89666

Comments

check4game commented Jul 29, 2023

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

ghost commented Jul 29, 2023

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

MichalPetryka commented Jul 29, 2023

AndyAyersMS commented Jul 29, 2023

AndyAyersMS commented Jul 29, 2023

AndyAyersMS commented Jul 30, 2023

AndyAyersMS commented Jul 31, 2023