Protoflux incorrect casting of byte

[gameboycjp]

Describe the bug?

Byte incorrectly casts to other types when the value is above 127, it sets bits 9-32 to true.

To Reproduce

Cast a byte to any type other than decimal, and set the byte to any number above 127.

Expected behavior

It shouldn't set bits 9-32 to true when the value is above 127.

Screenshots

All casts from top to bottom, in the order listed in the protoflux browser.

127 or below, correct behavior.

128 or above, incorrect behaviour.

Resonite Version Number

Beta 2024.6.5.1084

What Platforms does this occur on?

Windows

What headset if any do you use?

Desktop

Log Files

DESKTOP-3EE2DAL - 2024.6.5.1084 - 2024-06-07 21_54_25.log

Additional Context

Casting a byte to a ulong on a C# project targetting .NET Framework 4.6.2 and 4.7.1 works fine.

Reporters

@gamethecupdog 
@Lexevo
@yosh

[ProbablePrime]

This is really weird, our cast nodes just do a simple cast. For example this is an abridged version of Byte -> Float conversion. return unchecked((float)Input)

https://dotnetfiddle.net/PxOiPN shows basically what its doing working.

[mpmxyz]

I injected the following into FrooxEngine:

          byte[] bytes = new byte[] { 125, 126, 127, 128, 129, 130, 131, 132, 133, 134 };
          for (int index = 0; index < bytes.Length; index++)
          {
              byte valueByte = Unsafe.ReadUnaligned<byte>(ref bytes[index]);
              int valueIntDirect = (int)Unsafe.ReadUnaligned<byte>(ref bytes[index]);
              int valueIntIndirect = (int)valueByte;
              UniLog.Log($"Byte:         {valueByte}");
              UniLog.Log($"Int Direct:   {valueIntDirect}");
              UniLog.Log($"Int Indirect: {valueIntIndirect}");
          }

It resulted in the following output:

21:00:10.413 ( -1 FPS)  Byte:         125
21:00:10.413 ( -1 FPS)  Int Direct:   125
21:00:10.413 ( -1 FPS)  Int Indirect: 125
21:00:10.413 ( -1 FPS)  Byte:         126
21:00:10.413 ( -1 FPS)  Int Direct:   126
21:00:10.413 ( -1 FPS)  Int Indirect: 126
21:00:10.413 ( -1 FPS)  Byte:         127
21:00:10.413 ( -1 FPS)  Int Direct:   127
21:00:10.413 ( -1 FPS)  Int Indirect: 127
21:00:10.413 ( -1 FPS)  Byte:         128
21:00:10.413 ( -1 FPS)  Int Direct:   -128
21:00:10.413 ( -1 FPS)  Int Indirect: 128
21:00:10.413 ( -1 FPS)  Byte:         129
21:00:10.413 ( -1 FPS)  Int Direct:   -127
21:00:10.413 ( -1 FPS)  Int Indirect: 129
21:00:10.413 ( -1 FPS)  Byte:         130
21:00:10.413 ( -1 FPS)  Int Direct:   -126
21:00:10.413 ( -1 FPS)  Int Indirect: 130
21:00:10.413 ( -1 FPS)  Byte:         131
21:00:10.413 ( -1 FPS)  Int Direct:   -125
21:00:10.413 ( -1 FPS)  Int Indirect: 131
21:00:10.413 ( -1 FPS)  Byte:         132
21:00:10.413 ( -1 FPS)  Int Direct:   -124
21:00:10.413 ( -1 FPS)  Int Indirect: 132
21:00:10.413 ( -1 FPS)  Byte:         133
21:00:10.413 ( -1 FPS)  Int Direct:   -123
21:00:10.413 ( -1 FPS)  Int Indirect: 133
21:00:10.413 ( -1 FPS)  Byte:         134
21:00:10.413 ( -1 FPS)  Int Direct:   -122
21:00:10.413 ( -1 FPS)  Int Indirect: 134

Interestingly the injected bytecode does not contain any mention of a type cast:

.maxstack 3
.locals init (
  [0] unsigned int8[] 'bytes [Range(Instruction(IL_0056 stloc.0)-Instruction(IL_00c0 ldloc.0))]',
  [1] int32 'index [Range(Instruction(IL_0058 stloc.1)-Instruction(IL_00bf ldloc.1))]',
  [2] unsigned int8 'valueByte [Range(Instruction(IL_0067 stloc.2)-Instruction(IL_007d ldloc.2))]',
  [3] int32 'valueIntDirect [Range(Instruction(IL_0074 stloc.3)-Instruction(IL_0093 ldloc.3))]',
  [4] int32 'valueIntIndirect [Range(Instruction(IL_0076 stloc.s)-Instruction(IL_00a9 ldloc.s))]'
)
[...] //skipped parts, code within loop follows:
IL_005b: ldloc.0      // 'bytes [Range(Instruction(IL_0056 stloc.0)-Instruction(IL_00c0 ldloc.0))]'
IL_005c: ldloc.1      // 'index [Range(Instruction(IL_0058 stloc.1)-Instruction(IL_00bf ldloc.1))]'
IL_005d: ldelema      [mscorlib]System.Byte
IL_0062: call         !!0/*unsigned int8*/ [System.Runtime.CompilerServices.Unsafe]System.Runtime.CompilerServices.Unsafe::ReadUnaligned<unsigned int8>(unsigned int8&)
IL_0067: stloc.2      // 'valueByte [Range(Instruction(IL_0067 stloc.2)-Instruction(IL_007d ldloc.2))]'
IL_0068: ldloc.0      // 'bytes [Range(Instruction(IL_0056 stloc.0)-Instruction(IL_00c0 ldloc.0))]'
IL_0069: ldloc.1      // 'index [Range(Instruction(IL_0058 stloc.1)-Instruction(IL_00bf ldloc.1))]'
IL_006a: ldelema      [mscorlib]System.Byte
IL_006f: call         !!0/*unsigned int8*/ [System.Runtime.CompilerServices.Unsafe]System.Runtime.CompilerServices.Unsafe::ReadUnaligned<unsigned int8>(unsigned int8&)
IL_0074: stloc.3      // 'valueIntDirect [Range(Instruction(IL_0074 stloc.3)-Instruction(IL_0093 ldloc.3))]'
IL_0075: ldloc.2      // 'valueByte [Range(Instruction(IL_0067 stloc.2)-Instruction(IL_007d ldloc.2))]'
IL_0076: stloc.s      'valueIntIndirect [Range(Instruction(IL_0076 stloc.s)-Instruction(IL_00a9 ldloc.s))]'
IL_0078: ldstr        "Byte:         {0}"
IL_007d: ldloc.2      // 'valueByte [Range(Instruction(IL_0067 stloc.2)-Instruction(IL_007d ldloc.2))]'
IL_007e: box          [mscorlib]System.Byte
IL_0083: call         string [mscorlib]System.String::Format(string, object)
IL_0088: ldc.i4.0
IL_0089: call         void [Elements.Core]Elements.Core.UniLog::Log(string, bool)
IL_008e: ldstr        "Int Direct:   {0}"
IL_0093: ldloc.3      // 'valueIntDirect [Range(Instruction(IL_0074 stloc.3)-Instruction(IL_0093 ldloc.3))]'
IL_0094: box          [mscorlib]System.Int32
IL_0099: call         string [mscorlib]System.String::Format(string, object)
IL_009e: ldc.i4.0
IL_009f: call         void [Elements.Core]Elements.Core.UniLog::Log(string, bool)
IL_00a4: ldstr        "Int Indirect: {0}"
IL_00a9: ldloc.s      'valueIntIndirect [Range(Instruction(IL_0076 stloc.s)-Instruction(IL_00a9 ldloc.s))]'
IL_00ab: box          [mscorlib]System.Int32
IL_00b0: call         string [mscorlib]System.String::Format(string, object)
IL_00b5: ldc.i4.0
IL_00b6: call         void [Elements.Core]Elements.Core.UniLog::Log(string, bool)
IL_00bb: ldloc.1      // 'index [Range(Instruction(IL_0058 stloc.1)-Instruction(IL_00bf ldloc.1))]'
IL_00bc: ldc.i4.1
IL_00bd: add
IL_00be: stloc.1      // 'index [Range(Instruction(IL_0058 stloc.1)-Instruction(IL_00bf ldloc.1))]'
IL_00bf: ldloc.1      // 'index [Range(Instruction(IL_0058 stloc.1)-Instruction(IL_00bf ldloc.1))]'
IL_00c0: ldloc.0      // 'bytes [Range(Instruction(IL_0056 stloc.0)-Instruction(IL_00c0 ldloc.0))]'
IL_00c1: ldlen
IL_00c2: conv.i4
IL_00c3: blt.s        IL_005b
// end of loop

Is it the responsibility of the runtime to get the types in order? It looks like the Mono version used by Unity screws that up here. I don't know what to search for so I couldn't find confirmation on that.

[mpmxyz]

An addendum on what I assume happens here: The runtime knows the that the return value of System.Runtime.CompilerServices.Unsafe::ReadUnaligned is !!0 - generic argument 0 - and does not apply proper handling if the specific type is not what it is expected to be. (int32)

[gameboycjp]

Actually, yeah, I kind of guessed it was a runtime issue. Is there a way we could test across a couple more to verify? If it is indeed a runtime issue, I think it'll probably be blocked by #706.

[Banane9]

Actually, yeah, I kind of guessed it was a runtime issue. Is there a way we could test across a couple more to verify? If it is indeed a runtime issue, I think it'll probably be blocked by #706.

well, as @mpmxyz showed, it would seem to be possible to circumvent the issue at least, when first assigning the value to a variable and then casting that.

[ProbablePrime]

If it is mono it should be relatively easy to re-produce this in a sample Unity scene. Could anyone take that on? Sorry and thank you!

[gameboycjp]

Unless I'm doing this wrong, it's not reproducing in my sample scene. I could be doing something wrong though, as I have no idea what I'm doing, I've never referenced libraries in a Unity project before today. There could also be more to this at play, like Unity project settings, that I'm missing.

[BlueCyro]

If it is mono it should be relatively easy to re-produce this in a sample Unity scene. Could anyone take that on? Sorry and thank you!

I can tell you it's probably the version of mono used by unity in specific. Testing this on a headless server running under mono 6.12.0.206 or .NET 8 yield the correct casting result of 128 for a ulong.

[ProbablePrime]

We're on: 2019.4.19f1 of unity. So whatever Mono version that uses.

PS C:\Program Files\Unity\Hub\Editor\2019.4.19f1\Editor\Data\MonoBleedingEdge\bin> .\mono.exe --version
Mono JIT compiler version 5.11.0 (Visual Studio built mono)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
        TLS:           normal
        SIGSEGV:       normal
        Notification:  Thread + polling
        Architecture:  x86
        Disabled:      none
        Misc:          softdebug
        Interpreter:   yes
        LLVM:          supported, not enabled.
        GC:            sgen (concurrent by default)

So pretty easy to diagnose, get that version of Mono and write some sample code. I'll do that after a dog walk.

[ProbablePrime]

That didn't work out,

oh well.

[Banane9]

That didn't work out

@mpmxyz tried it with the Unsafe Read, so that might make a difference 🤔

[gameboycjp]

Now that I know Mono can be run from the command line, I've got some more testing I want to do.

[gameboycjp]

So, remember how I didn't find a recreation case? I found it. Running in the Unity editor does not reproduce the problem, but building and running does reproduce the issue! Using the same code that mpm provided, but swapping the unilog logging for unity logging.

[gameboycjp]

Also happens on 2019.4.40f1. Also only on built Unity applications here too.

[shiftyscales]

It was planned in #585 to try to upgrade to 2019.4.40- but it sounds like that won't resolve this issue either- so instead, we'd probably need to wait until #706 is done so we can move onto .NET and away from Unity's Mono.

With that said- it doesn't seem likely we will be able to address this issue directly, and it should implicitly be resolved by other issues which allow us to switch away from Unity's Mono. Thoughts, @ProbablePrime?

This should probably be closed as not planned.

[ProbablePrime]

I have some difficulties with closing as not-planned. Its important that we track this issue as we upgrade items because after all the planned upgrades we still need to verify if it works. Otherwise we're going to forget to check.

[gameboycjp]

Probably sounds like blocked for the time being?

[Frooxius]

We could fix this by adding manual code to work around the issue.

Question is if it's worth putting time into it. Does this come up often enough?

[gameboycjp]

It depends how often bytes with values above 127 are used in protoflux, and casted. Is there any way to use them without converting them to other types anyways? If not, then bytes are useless unless the value is guaranteed under 128, or the broken behavior is being relied on, the latter of which sounds like a really bad idea. I use them every now and then, no idea about other peoples usage. I vaguely recall at least one or two using them, but I can't name one and can't track down the other to verify it uses them, or at all for that matter. I'm personally fine with shelving the projects I'm using bytes in until 706 is ready if it's not worth fixing.

[Frooxius]

I'd certainly advise against people relying on this kind of behavior - that will break at some point.

If this comes up a lot, we can implement a workaround, but we'd like some feedback from people running into this.

[Banane9]

If this comes up a lot, we can implement a workaround, but we'd like some feedback from people running into this.

Not sure how often it really comes up, but considering how easy the workaround found by @mpmxyz is (just introducing a temporary variable before casting), it would seem like the engineering overhead would be minimal.

          byte[] bytes = new byte[] { 125, 126, 127, 128, 129, 130, 131, 132, 133, 134 };
          for (int index = 0; index < bytes.Length; index++)
          {
              byte valueByte = Unsafe.ReadUnaligned<byte>(ref bytes[index]);
              int valueIntDirect = (int)Unsafe.ReadUnaligned<byte>(ref bytes[index]);
              int valueIntIndirect = (int)valueByte;
              UniLog.Log($"Byte:         {valueByte}");
              UniLog.Log($"Int Direct:   {valueIntDirect}");
              UniLog.Log($"Int Indirect: {valueIntIndirect}");
          }