[release/6.0] [Mono] Fix uninitialized vtable bug (dotnet#67759)

* Add functional test

* Fix vtable setup

* Add suggested code changes

* Improve arguments ordering

Co-authored-by: Simon Rozsival <[email protected]>

src/mono/mono/metadata/icall.c

@@ -3227,44 +3227,47 @@ init_io_stream_slots (void)
 	io_stream_slots_set = TRUE;
 }
 
-MonoBoolean
-ves_icall_System_IO_Stream_HasOverriddenBeginEndRead (MonoObjectHandle stream, MonoError *error)
+
+static MonoBoolean
+stream_has_overriden_begin_or_end_method (MonoObjectHandle stream, int begin_slot, int end_slot, MonoError *error)
 {
 	MonoClass* curr_klass = MONO_HANDLE_GET_CLASS (stream);
 	MonoClass* base_klass = mono_class_try_get_stream_class ();
 
-	if (!io_stream_slots_set)
-		init_io_stream_slots ();
+	mono_class_setup_vtable (curr_klass);
+	if (mono_class_has_failure (curr_klass)) {
+		mono_error_set_for_class_failure (error, curr_klass);
+		return_val_if_nok (error, FALSE);
+	}
 
 	// slots can still be -1 and it means Linker removed the methods from the base class (Stream)
 	// in this case we can safely assume the methods are not overridden
 	// otherwise - check vtable
 	MonoMethod **curr_klass_vtable = m_class_get_vtable (curr_klass);
-	gboolean begin_read_is_overriden = io_stream_begin_read_slot != -1 && curr_klass_vtable [io_stream_begin_read_slot]->klass != base_klass;
-	gboolean end_read_is_overriden = io_stream_end_read_slot != -1 && curr_klass_vtable [io_stream_end_read_slot]->klass != base_klass;
+	gboolean begin_is_overriden = begin_slot != -1 && curr_klass_vtable [begin_slot] != NULL && curr_klass_vtable [begin_slot]->klass != base_klass;
+	gboolean end_is_overriden = end_slot != -1 && curr_klass_vtable [end_slot] != NULL && curr_klass_vtable [end_slot]->klass != base_klass;
+
+	return begin_is_overriden || end_is_overriden;
+}
+
+MonoBoolean
+ves_icall_System_IO_Stream_HasOverriddenBeginEndRead (MonoObjectHandle stream, MonoError *error)
+{
+	if (!io_stream_slots_set)
+		init_io_stream_slots ();
 
 	// return true if BeginRead or EndRead were overriden
-	return begin_read_is_overriden || end_read_is_overriden;
+	return stream_has_overriden_begin_or_end_method (stream, io_stream_begin_read_slot, io_stream_end_read_slot, error);
 }
 
 MonoBoolean
 ves_icall_System_IO_Stream_HasOverriddenBeginEndWrite (MonoObjectHandle stream, MonoError *error)
 {
-	MonoClass* curr_klass = MONO_HANDLE_GETVAL (stream, vtable)->klass;
-	MonoClass* base_klass = mono_class_try_get_stream_class ();
-
 	if (!io_stream_slots_set)
 		init_io_stream_slots ();
 
-	// slots can still be -1 and it means Linker removed the methods from the base class (Stream)
-	// in this case we can safely assume the methods are not overridden
-	// otherwise - check vtable
-	MonoMethod **curr_klass_vtable = m_class_get_vtable (curr_klass);
-	gboolean begin_write_is_overriden = io_stream_begin_write_slot != -1 && curr_klass_vtable [io_stream_begin_write_slot]->klass != base_klass;
-	gboolean end_write_is_overriden = io_stream_end_write_slot != -1 && curr_klass_vtable [io_stream_end_write_slot]->klass != base_klass;
-
 	// return true if BeginWrite or EndWrite were overriden
-	return begin_write_is_overriden || end_write_is_overriden;
+	return stream_has_overriden_begin_or_end_method (stream, io_stream_begin_write_slot, io_stream_end_write_slot, error);
 }
 
 MonoBoolean

src/tests/FunctionalTests/Android/Device_Emulator/AOT_System.IO.Stream/Android.Device_Emulator.Aot_System.IO.Stream.Test.csproj

@@ -0,0 +1,16 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <RunAOTCompilation>true</RunAOTCompilation>
+    <MonoForceInterpreter>false</MonoForceInterpreter>
+    <TestRuntime>true</TestRuntime>
+    <TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
+    <MainLibraryFileName>Android.Device_Emulator.Aot_System.IO.Stream.Test.dll</MainLibraryFileName>
+    <ExpectedExitCode>42</ExpectedExitCode>
+    <EnableAggressiveTrimming>true</EnableAggressiveTrimming>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Compile Include="Program.cs" />
+  </ItemGroup>
+</Project>

src/tests/FunctionalTests/Android/Device_Emulator/AOT_System.IO.Stream/Program.cs

@@ -0,0 +1,48 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+#nullable enable
+
+using System;
+using System.IO;
+
+// https://github.com/dotnet/runtime/issues/67402
+
+public static class Program
+{
+    public static int Main(string[] args)
+    {
+        var stream = new DummyStream();
+        var buffer = new byte[stream.Length];
+        int read = stream.ReadAsync(buffer, 0, buffer.Length).GetAwaiter().GetResult();
+        return read + buffer[0];
+    }
+
+    private sealed class DummyStream : System.IO.Stream
+    {
+        protected override void Dispose (bool disposing) => throw new NotImplementedException ();
+
+        public override int Read (byte[] buffer, int offset, int count)
+        {
+            buffer[0] = 41;
+            return 1;
+        }
+
+        public override long Seek (long offset, SeekOrigin origin) => 0;
+        public override void SetLength (long value) {}
+        public override void Write (byte[] buffer, int offset, int count) {}
+        public override void Flush () {}
+
+        public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state)
+        {
+            Console.WriteLine("BeginRead");
+            return base.BeginRead(buffer, offset, count, callback, state);
+        }
+
+        public override bool CanRead => true;
+        public override bool CanSeek => false;
+        public override bool CanWrite => false;
+
+        public override long Length => 1;
+        public override long Position { get; set; } = 0;
+    }
+}