From 8c1ef74bb125bf49d6b1e3f2e80ac235adb27bd8 Mon Sep 17 00:00:00 2001
From: Vlad Brezae <brezaevlad@gmail.com>
Date: Thu, 3 Oct 2019 22:29:55 +0300
Subject: [PATCH] [arm] Fix fetching of method addresses (#17099)

After https://github.com/mono/mono/commit/9ff3b0d65ee4, in an aot image we can emit addresses either as bl offset (which uses 4 bytes) or as ldr pc, =<label>; <addr> (which uses 8 bytes). Before this commit we were dereferencing an instruction pointer, in get_call_table_entry, assuming 8 bytes per entry. This could overflow and crash if we used in fact only 4 bytes per entry. Fix this by including the entry size in the aot image.

Fixes random crashes on arm CI on full-aotmixed suite.
---
 mono/mini/aot-compiler.c |  9 +++++----
 mono/mini/aot-runtime.c  | 12 +++++++-----
 mono/mini/aot-runtime.h  |  4 +++-
 mono/mini/mini-llvm.c    |  5 +++--
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/mono/mini/aot-compiler.c b/mono/mini/aot-compiler.c
index 700eae57c46..012a0cdb16d 100644
--- a/mono/mini/aot-compiler.c
+++ b/mono/mini/aot-compiler.c
@@ -329,6 +329,7 @@ typedef struct MonoAotCompile {
 
 	MonoAotOptions aot_opts;
 	guint32 nmethods;
+	int call_table_entry_size;
 	guint32 nextra_methods;
 	guint32 opts;
 	guint32 simd_opts;
@@ -10096,12 +10097,10 @@ emit_code (MonoAotCompile *acfg)
 
 	for (i = 0; i < acfg->nmethods; ++i) {
 #ifdef MONO_ARCH_AOT_SUPPORTED
-		int call_size;
-
 		if (!ignore_cfg (acfg->cfgs [i])) {
-			arch_emit_label_address (acfg, acfg->cfgs [i]->asm_symbol, FALSE, acfg->thumb_mixed && acfg->cfgs [i]->compile_llvm, NULL, &call_size);
+			arch_emit_label_address (acfg, acfg->cfgs [i]->asm_symbol, FALSE, acfg->thumb_mixed && acfg->cfgs [i]->compile_llvm, NULL, &acfg->call_table_entry_size);
 		} else {
-			arch_emit_label_address (acfg, symbol, FALSE, FALSE, NULL, &call_size);
+			arch_emit_label_address (acfg, symbol, FALSE, FALSE, NULL, &acfg->call_table_entry_size);
 		}
 #endif
 	}
@@ -11117,6 +11116,7 @@ init_aot_file_info (MonoAotCompile *acfg, MonoAotFileInfo *info)
 	info->got_size = acfg->got_offset * sizeof (target_mgreg_t);
 	info->plt_size = acfg->plt_offset;
 	info->nmethods = acfg->nmethods;
+	info->call_table_entry_size = acfg->call_table_entry_size;
 	info->nextra_methods = acfg->nextra_methods;
 	info->flags = acfg->flags;
 	info->opts = acfg->opts;
@@ -11284,6 +11284,7 @@ emit_aot_file_info (MonoAotCompile *acfg, MonoAotFileInfo *info)
 	emit_int32 (acfg, info->card_table_shift_bits);
 	emit_int32 (acfg, info->card_table_mask);
 	emit_int32 (acfg, info->tramp_page_size);
+	emit_int32 (acfg, info->call_table_entry_size);
 	emit_int32 (acfg, info->nshared_got_entries);
 	emit_int32 (acfg, info->datafile_size);
 	emit_int32 (acfg, 0);
diff --git a/mono/mini/aot-runtime.c b/mono/mini/aot-runtime.c
index 9729fd4e38e..d71d3400a8e 100644
--- a/mono/mini/aot-runtime.c
+++ b/mono/mini/aot-runtime.c
@@ -1933,18 +1933,20 @@ check_usable (MonoAssembly *assembly, MonoAotFileInfo *info, guint8 *blob, char
  * TABLE should point to a table of call instructions. Return the address called by the INDEXth entry.
  */
 static void*
-get_call_table_entry (void *table, int index)
+get_call_table_entry (void *table, int index, int entry_size)
 {
 #if defined(TARGET_ARM)
 	guint32 *ins_addr;
 	guint32 ins;
 	gint32 offset;
 
-	ins_addr = (guint32 *)table + (index * 2);
-	if ((guint32) *ins_addr == (guint32 ) 0xe51ff004) { // ldr pc, =<label>
+	if (entry_size == 8) {
+		ins_addr = (guint32 *)table + (index * 2);
+		g_assert ((guint32) *ins_addr == (guint32 ) 0xe51ff004); // ldr pc, =<label>
 		return *((char **) (ins_addr + 1));
 	}
 
+	g_assert (entry_size == 4);
 	ins_addr = (guint32*)table + index;
 	ins = *ins_addr;
 	if ((ins >> ARMCOND_SHIFT) == ARMCOND_NV) {
@@ -2367,7 +2369,7 @@ if (container_assm_name && !container_amodule) {
 
 		/* method_addresses () contains a table of branches, since the ios linker can update those correctly */
 		if (!addr && amodule->info.method_addresses) {
-			addr = get_call_table_entry (amodule->info.method_addresses, i);
+			addr = get_call_table_entry (amodule->info.method_addresses, i, amodule->info.call_table_entry_size);
 			g_assert (addr);
 			if (addr == amodule->info.method_addresses)
 				addr = NULL;
@@ -6030,7 +6032,7 @@ mono_aot_get_unbox_trampoline (MonoMethod *method, gpointer addr)
 		}
 	}
 
-	code = get_call_table_entry (amodule->unbox_trampoline_addresses, entry_index);
+	code = get_call_table_entry (amodule->unbox_trampoline_addresses, entry_index, amodule->info.call_table_entry_size);
 	g_assert (code);
 
 	tinfo = mono_tramp_info_create (NULL, (guint8 *)code, 0, NULL, NULL);
diff --git a/mono/mini/aot-runtime.h b/mono/mini/aot-runtime.h
index 5a01eff9946..31940084c2f 100644
--- a/mono/mini/aot-runtime.h
+++ b/mono/mini/aot-runtime.h
@@ -11,7 +11,7 @@
 #include "mini.h"
 
 /* Version number of the AOT file format */
-#define MONO_AOT_FILE_VERSION 170
+#define MONO_AOT_FILE_VERSION 171
 
 #define MONO_AOT_TRAMP_PAGE_SIZE 16384
 
@@ -200,6 +200,8 @@ typedef struct MonoAotFileInfo
 	guint32 double_align, long_align, generic_tramp_num, card_table_shift_bits, card_table_mask;
 	/* The page size used by trampoline pages */
 	guint32 tramp_page_size;
+	/* Size of each entry stored at method_addresses */
+	guint32 call_table_entry_size;
 	/*
 	 * The number of GOT entries which need to be preinitialized when the
 	 * module is loaded.
diff --git a/mono/mini/mini-llvm.c b/mono/mini/mini-llvm.c
index 0ef1bdc17c3..9b784a55ca0 100644
--- a/mono/mini/mini-llvm.c
+++ b/mono/mini/mini-llvm.c
@@ -9712,7 +9712,7 @@ emit_aot_file_info (MonoLLVMModule *module)
 	info = &module->aot_info;
 
 	/* Create an LLVM type to represent MonoAotFileInfo */
-	nfields = 2 + MONO_AOT_FILE_INFO_NUM_SYMBOLS + 21 + 5;
+	nfields = 2 + MONO_AOT_FILE_INFO_NUM_SYMBOLS + 22 + 5;
 	eltypes = g_new (LLVMTypeRef, nfields);
 	tindex = 0;
 	eltypes [tindex ++] = LLVMInt32Type ();
@@ -9721,7 +9721,7 @@ emit_aot_file_info (MonoLLVMModule *module)
 	for (i = 0; i < MONO_AOT_FILE_INFO_NUM_SYMBOLS; ++i)
 		eltypes [tindex ++] = LLVMPointerType (LLVMInt8Type (), 0);
 	/* Scalars */
-	for (i = 0; i < 20; ++i)
+	for (i = 0; i < 21; ++i)
 		eltypes [tindex ++] = LLVMInt32Type ();
 	/* Arrays */
 	eltypes [tindex ++] = LLVMArrayType (LLVMInt32Type (), MONO_AOT_TABLE_NUM);
@@ -9866,6 +9866,7 @@ emit_aot_file_info (MonoLLVMModule *module)
 	fields [tindex ++] = LLVMConstInt (LLVMInt32Type (), info->card_table_shift_bits, FALSE);
 	fields [tindex ++] = LLVMConstInt (LLVMInt32Type (), info->card_table_mask, FALSE);
 	fields [tindex ++] = LLVMConstInt (LLVMInt32Type (), info->tramp_page_size, FALSE);
+	fields [tindex ++] = LLVMConstInt (LLVMInt32Type (), info->call_table_entry_size, FALSE);
 	fields [tindex ++] = LLVMConstInt (LLVMInt32Type (), info->nshared_got_entries, FALSE);
 	fields [tindex ++] = LLVMConstInt (LLVMInt32Type (), info->datafile_size, FALSE);
 	fields [tindex ++] = LLVMConstInt (LLVMInt32Type (), module->unbox_tramp_num, FALSE);
-- 
2.11.4.GIT