Runtime: Memory Corruption using System.Numerics.Vector3

[daeken]

I've discovered a memory corruption bug which seems to affect Vector3s passed to functions. This minimal test case crashes consistently for me on Windows 10 (x64 running on a Ryden CPU) using .NET Core 2.1.301. The same test does not crash for me on MacOS with the same version of .NET Core.

using System.Numerics;
namespace NumericCorruption {
  class Program {
    static void Main(string[] args) {
      var vec0 = new Vector3(0, 0, 0);
      Test(vec0);
    }
    static void Test(Vector3 vec0) {
      //vec0.X = -vec0.X; // Uncommenting this line will cause an access violation on the `new` line
      vec0.X = 0; // Uncommenting this line will cause a null reference exception (?!) on the `new` line
      new Vector3(vec0.X, vec0.Y, vec0.Z);
    }
  }
}

As indicated, some degree of control is possible so this may have security implications in some regard. It also only appears to occur when the Vector3 is passed to another function; if you perform the same operations inside a given function, everything works as expected. Also, this only seems to occur when you assign one of the vector components; accessing components is fine, as is assigning whole vectors.

[saucecontrol]

Looks like a JIT bug. That argument gets morphed to a reference

fgMorphTree BB01, stmt 3 (before)
               [000028] -------N----              /--*  IND       simd12
               [000027] ------------              |  \--*  LEA(b+0)  byref 
               [000025] L-----------              |     \--*  ADDR      byref 
               [000026] ------------              |        \--*  LCL_VAR   simd12 V00 arg0         
               [000031] -A--G-------              *  ASG       simd12 (copy)
               [000030] ----G--N----              \--*  BLK(12)   simd12
               [000021] L-----------                 \--*  ADDR      byref 
               [000020] ------------                    \--*  LCL_VAR   simd12 V02 tmp1         

The assignment [000030] using V02 removes: Constant Assertion: V02 == 0
Replacing address of implicit by ref struct parameter with byref:
               [000026] ------------              *  LCL_VAR   byref  V00 arg0         

Which asserts here

#if defined(FEATURE_SIMD)
    bool lvaMapSimd12ToSimd16(const LclVarDsc* varDsc)
    {
        assert(varDsc->lvType == TYP_SIMD12);
        assert(varDsc->lvExactSize == 12);

because lvType is TYP_BYREF at that point. Not sure why it wouldn't fail the same on MacOS unless you're doing a release build there and debug on Windows. It only fails if the Test method doesn't get inlined.

cc @dotnet/jit-contrib

[daeken]

Thanks for digging into this so quickly! For what it's worth, both debug and release builds seem to fail on Windows for me, in the same ways (access violation if I uncomment out that first line, null ref if I uncomment the second). Neither causes a crash on MacOS. Bit puzzling as to why it's not crashing there as well.

[saucecontrol]

Ah yeah, I guess if you get past the asserts you just end up with an uninitialized local. It works fine for me in release mode on Windows, but I can get it to fail every time by adding [MethodImpl(MethodImplOptions.NoInlining)] to the method. I happened to be debugging the JIT trying to figure out how the magic works with the SIMD types and figured I'd look at it. I'm otherwise in over my head. I'm sure somebody on the JIT team can figure it out.

Oh, and my apologies if it was inappropriate to tag the contrib group. I didn't realize how many people were on that list 😮

[mikedn]

Looks like a JIT bug. That argument gets morphed to a reference

It's not that it is morphed to a reference, it's the other way around :)

               [000003] -----+------                 /--*  CNS_DBL   float  0.00000000000000000
               [000036] ----G+------              /--*  SIMD      simd12 float setX
               [000037] *---G+------              |  \--*  OBJ(12)   simd12
               [000001] -----+------              |     \--*  LCL_VAR   byref  V00 arg0         
               [000005] -A--G+------              *  ASG       float  (copy)
               [000035] D----+-N----              \--*  LCL_VAR   simd12 V00 arg0         

On the RHS of the assignment arg0 was correctly morphed to a reference (structs are passed by ref). On the LHS however, morph missed the LCL_VAR somehow and it's still simd12 instead of byref.

@CarolEidt It looks like fgMorphFieldAssignToSIMDIntrinsicSet needs to call fgMorphImplicitByRefArgs again on the tree. Morph did it before, when the LHS of the assignment was FIELD(ADDR(LCL_VAR)), now the LHS changes to LCL_VAR<simd12> and it will remain like that until lowering. I'm not sure if it's right to call fgMorphImplicitByRefArgs multiple times but it seems that the JIT already does that in a couple of places.

[jakobbotsch]

Not sure why it wouldn't fail the same on MacOS unless you're doing a release build there and debug on Windows.

Like @mikedn said, structs > 8 bytes are passed by ref on Windows (dictated by the calling convention there). OS X follows the SysV ABI which passes such structs by value instead. Considering the analysis by @mikedn above it seems likely to be the reason in this case.

[AndyAyersMS]

cc @dotnet/jit-contrib

[CarolEidt]

my apologies if it was inappropriate to tag the contrib group

No worries - you should feel free to tag. I'm sorry that I haven't had the chance to dig into this - I'll try to have a look next week.