CosmosOS / Cosmos

Cosmos is an operating system "construction kit". Build your own OS using managed languages such as C#, VB.NET, and more!
https://www.goCosmos.org
BSD 3-Clause "New" or "Revised" License
2.91k stars 549 forks source link

Assigning a sequential struct with pack=1 to a class property corrupts the struct data #2824

Closed GoldenretriverYT closed 8 months ago

GoldenretriverYT commented 9 months ago

Area of Cosmos - What area of Cosmos are we dealing with?

Unsure

Expected Behaviour - What do you think that should happen?

The assignment should work fine and not corrupt data, as shown by the screenshot without the attribute: image This is definitely expected behaviour as this works fine when running that code with the .NET CLR

Actual Behaviour - What unexpectedly happens?

The data gets corrupted when assigning the struct to the class property. This does not happen without the sequential struct layout, not sure if the Pack plays a role. image

Reproduction - How did you get this error to appear?

namespace CosmosReproTest {
    public class Kernel : Sys.Kernel {
        public RandomStruct FieldOfStruct { get; set; }

        protected override void BeforeRun() {
            Console.WriteLine("Cosmos booted successfully. Type a line of text to get it echoed back.");
        }

        protected override void Run() {
            FieldOfStruct = ParseStruct(/* not actual data */);
            Console.WriteLine("FieldOfStruct.a: " + FieldOfStruct.a);
            Console.WriteLine("FieldOfStruct.b: " + FieldOfStruct.b);
            Console.WriteLine("FieldOfStruct.c: " + FieldOfStruct.c);
            Console.WriteLine("FieldOfStruct.d: " + FieldOfStruct.d);
            Console.WriteLine("FieldOfStruct.e: " + FieldOfStruct.e);
            Console.WriteLine("FieldOfStruct.f: " + FieldOfStruct.f);
            Console.ReadLine();
        }

        public RandomStruct ParseStruct(/* not actual data */) {
            var re = new RandomStruct() {
                a = 1,
                b = 2,
                c = 3,
                d = 4,
                e = 5,
                f = 6
            };

            Console.WriteLine("a: " + re.a);
            Console.WriteLine("b: " + re.b);
            Console.WriteLine("c: " + re.c);
            Console.WriteLine("d: " + re.d);
            Console.WriteLine("e: " + re.e);
            Console.WriteLine("f: " + re.f);

            return re;
        }
    }

    [StructLayout(LayoutKind.Sequential, Pack = 1)] // <<< REMOVE THIS LINE AND IT WILL WORK FINE
    public struct RandomStruct {
        public UInt16 a;
        public UInt32 b;
        public UInt64 c;
        public UInt16 d;
        public UInt16 e;
        public UInt64 f;
    }
}

Version - Were you using the User Kit or Dev Kit? And what User Kit version or Dev Kit commit (Cosmos, IL2CPU, X#)?

a78f61d

ascpixi commented 9 months ago

I feel like this is related to IL2CPU, not Cosmos. For reference, the code dealing with handling different kinds of structures can be found here: https://github.com/CosmosOS/IL2CPU/blob/c8a113f51f7621b598ea8a0469e2ece0730c0a67/source/Cosmos.IL2CPU/ILOp.cs#L267

zarlo commented 9 months ago

does it happen when pack is 0?

quajak commented 9 months ago

Thanks for the easily reproducible case, I hope to look into it when I have some time.

zarlo commented 9 months ago

I feel like this is related to IL2CPU, not Cosmos. For reference, the code dealing with handling different kinds of structures can be found here: https://github.com/CosmosOS/IL2CPU/blob/c8a113f51f7621b598ea8a0469e2ece0730c0a67/source/Cosmos.IL2CPU/ILOp.cs#L267

mmm looking at this so when pack is 0 we then set it to (int)SizeOfType(typeof(IntPtr)) i think that is 4 but with all the others we just take the value given in this case 1 could it be that we need to just add (int)SizeOfType(typeof(IntPtr)) to pack

quajak commented 8 months ago

Should be fixed by https://github.com/CosmosOS/IL2CPU/pull/215 and https://github.com/CosmosOS/Cosmos/pull/2866

Thank you for giving such an easy reproduction of the issue!