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

Using BinaryFormatter to deserialize an array in a struct in a parent class fails with unfriendly exception [reproducible test case i promise] #8915

Closed
mcclure opened this issue May 28, 2018 · 2 comments · Fixed by #8919

Comments

@mcclure
Copy link

mcclure commented May 28, 2018

Hello everyone, how are you? The following is a reduced-case of code from a commercial Unity product (a friend's).

Steps to Reproduce

Run csc monotest.cs && mono monotest.exe on the following test case. Locally I can reproduce the crash with Unity 2017.4, Homebrew mono 5.0.1.1 and Homebrew mono 5.4.1.6 (all on OS X). Katelyn Gadd says that she tested it on a master-branch build from last week and also saw the problem.

using System;
using System.Runtime.Serialization.Formatters.Binary;
using System.IO;

// NOTE A: Change the "struct" to a "class" on line NOTE A below and crash disappears
// NOTE B: Change the "float[] data" to "float data;" on line NOTE B below and crash disappears
// NOTE C: Change SamCombatant() to PlayerCombatant() on line NOTE C below and crash disappears

class Test {
	[Serializable()]
	public struct SerializableColor { // NOTE A
		public float[] data;          // NOTE B
		public SerializableColor(float a) {
			data = new float[] {a};
		}
	}

	[Serializable()]
	public class PlayerCombatant {
		SerializableColor color;
		public PlayerCombatant() {
			color = new SerializableColor(1);
		}
	}

	[Serializable()]
	public class SamCombatant : PlayerCombatant {
	}

	static void Main() {
		Console.WriteLine("AWAKE");

		// Serialize to file
		{
			FileStream fs = new FileStream("DataFile.dat", FileMode.Create);
			BinaryFormatter formatter = new BinaryFormatter();
			formatter.Serialize(fs, new SamCombatant()); // NOTE C
			fs.Close();
		}

		// Deserialize from file
		{
			FileStream fs = new FileStream("DataFile.dat", FileMode.Open);
			BinaryFormatter formatter = new BinaryFormatter();
			formatter.Deserialize(fs);
			fs.Close();
		}
	}
}

Expected Behavior

The test case is valid C# and should print "AWAKE", serialize the object to a file, and then load it back into memory.

Current Behavior

The program crashes with an extremely user-unfriendly error. The crash is an exception which occurs inside formatter.Deserialize.

System.ArgumentException: 
Parameter name: field
  at (wrapper managed-to-native) System.TypedReference:MakeTypedReferenceInternal (object,System.Reflection.FieldInfo[])
  at System.TypedReference.MakeTypedReference (System.Object target, System.Reflection.FieldInfo[] flds) [0x00123] in <6e9b92f0d119424382ef180639777acb>:0 
  at System.Runtime.Serialization.ObjectManager.DoValueTypeFixup (System.Reflection.FieldInfo memberToFix, System.Runtime.Serialization.ObjectHolder holder, System.Object value) [0x0014d] in <6e9b92f0d119424382ef180639777acb>:0 
  at System.Runtime.Serialization.ObjectManager.CompleteObject (System.Runtime.Serialization.ObjectHolder holder, System.Boolean bObjectFullyComplete) [0x0021a] in <6e9b92f0d119424382ef180639777acb>:0 
[Continues for many lines]

Analysis

MakeTypedReference in referencesource typedreference.cs takes an array of FieldInfos. It iterates the array and runs flds[i] as RuntimeFieldInfo on each one. Apparently a member of the array is null or not a RuntimeFieldInfo.

Although the steps to reproduce the bug are somewhat specific (see the "NOTE A" comments; changing the code even a little causes the bug to not appear) the bug involves only straightforward usage of a non-obscure feature (serialization); a typical user who is not familiar with mono internals could easily encounter this but would not be able to make sense of the error they get.

@mcclure mcclure changed the title Using BinaryFormatter to serialize an array in a struct in a parent class fails with unfriendly exception [reproducible test case i promise] Using BinaryFormatter to deserialize an array in a struct in a parent class fails with unfriendly exception [reproducible test case i promise] May 28, 2018
@mcclure
Copy link
Author

mcclure commented May 28, 2018

For the record, I tested with dotnet 2.1.200 on OS X and this did not reproduce there.

@mcclure
Copy link
Author

mcclure commented May 29, 2018

Can confirm this is fixed in 3c7a203 on my machine, thanks

jonpryor pushed a commit to dotnet/android that referenced this issue Oct 9, 2018
Bumps to mono/llvm:release_60@117a508c
Bumps to xamarin/xamarin-android-api-compatibility:master@7ccb4802

	$ git diff --shortstat e1af6ea..ab3c897d       # mono
        1443 files changed, 66049 insertions(+), 45745 deletions(-)
	$ git diff --shortstat bdb3a116..117a508c      # llvm
	 26794 files changed, 4110589 insertions(+), 754376 deletions(-)
	$ git diff --shortstat c550d1bd..7ccb4802      # xamarin-android-api-compatibility
	 2 files changed, 16260 insertions(+), 12347 deletions(-)

Incomplete summary of easily `grep`able fixes:

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=11199
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=19436
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=23668
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=26983
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=33728
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=46917
fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=60065
Fixes: mono/mono#6173
Fixes: mono/mono#6466
Fixes: mono/mono#6647
Fixes: mono/mono#6834
Fixes: mono/mono#7058
Fixes: mono/mono#7137
Fixes: mono/mono#7260
Fixes: mono/mono#7305
Fixes: mono/mono#7402
Fixes: mono/mono#7525
Fixes: mono/mono#7610
Fixes: mono/mono#7649
Fixes: mono/mono#7655
Fixes: mono/mono#7683
Fixes: mono/mono#7685
Fixes: mono/mono#7716
Fixes: mono/mono#7731
Fixes: mono/mono#7785
Fixes: mono/mono#7828
Fixes: mono/mono#7944
Fixes: mono/mono#7947
Fixes: mono/mono#8036
Fixes: mono/mono#8074
Fixes: mono/mono#8089
Fixes: mono/mono#8112
Fixes: mono/mono#8122
Fixes: mono/mono#8143
Fixes: mono/mono#8149
Fixes: mono/mono#8152
Fixes: mono/mono#8175
Fixes: mono/mono#8177
Fixes: mono/mono#8250
Fixes: mono/mono#8267
Fixes: mono/mono#8273
Fixes: mono/mono#8282
Fixes: mono/mono#8310
Fixes: mono/mono#8311
Fixes: mono/mono#8329
Fixes: mono/mono#8340
Fixes: mono/mono#8372
Fixes: mono/mono#8407
Fixes: mono/mono#8409
Fixes: mono/mono#8422
Fixes: mono/mono#8430
Fixes: mono/mono#8439
fixes: mono/mono#8447
Fixes: mono/mono#8469
Fixes: mono/mono#8504
Fixes: mono/mono#8575
Fixes: mono/mono#8597
Fixes: mono/mono#8623
Fixes: mono/mono#8627
Fixes: mono/mono#8698
Fixes: mono/mono#8701
Fixes: mono/mono#8712
Fixes: mono/mono#8721
Fixes: mono/mono#8726
Fixes: mono/mono#8759
Fixes: mono/mono#8787
Fixes: mono/mono#8820
Fixes: mono/mono#8848
Fixes: mono/mono#8866
Fixes: mono/mono#8897
Fixes: mono/mono#8915
Fixes: mono/mono#8970
Fixes: mono/mono#8979
Fixes: mono/mono#9023
Fixes: mono/mono#9031
Fixes: mono/mono#9033
Fixes: mono/mono#9179
Fixes: mono/mono#9234
Fixes: mono/mono#9262
Fixes: mono/mono#9277
Fixes: mono/mono#9318
Fixes: mono/mono#9542
Fixes: mono/mono#9753
Fixes: mono/mono#9839
Fixes: mono/mono#9869
Fixes: mono/mono#9870
Fixes: mono/mono#9943
Fixes: mono/mono#9996
Fixes: mono/mono#10000
Fixes: mono/mono#10303
Fixes: mono/mono#10447
Fixes: mono/mono#10483
Fixes: mono/mono#10488
Fixes: xamarin/maccore#628
Fixes: xamarin/maccore#673
Fixes: #1561 (comment)
Fixes: #1845
Fixes: xamarin/xamarin-macios#4347
Fixes: xamarin/xamarin-macios#4617
Fixes: xamarin/xamarin-macios#4618
UnityAlex pushed a commit to Unity-Technologies/mono that referenced this issue Feb 21, 2020
spatil-rythmos pushed a commit to Unity-Technologies/mono that referenced this issue Apr 7, 2020
spatil-rythmos pushed a commit to Unity-Technologies/mono that referenced this issue Apr 13, 2020
picenka21 pushed a commit to picenka21/runtime that referenced this issue Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants