[mono] CodeQL fix set #1 (dotnet#99637)

CodeQL flagged various places where we're dereferencing pointers that could be NULL, this PR systematically cleans some of them up via g_assert.
* g_assert result of g_build_path calls
* Allocation failure handling
* mono_class_inflate_generic_class_checked can return NULL

Author: kg

src/mono/mono/eglib/gfile-posix.c

@@ -142,6 +142,7 @@ g_file_open_tmp (const gchar *tmpl, gchar **name_used, GError **gerror)
 	}
 
 	t = g_build_filename (g_get_tmp_dir (), tmpl, (const char*)NULL);
+	g_assert (t);
 
 	#ifdef HOST_WASI
 	g_critical ("g_file_open_tmp is not implemented for WASI");

src/mono/mono/metadata/appdomain.c

@@ -612,6 +612,7 @@ try_load_from (MonoAssembly **assembly,
 
 	*assembly = NULL;
 	fullpath = g_build_filename (path1, path2, path3, path4, (const char*)NULL);
+	g_assert (fullpath);
 
 	found = g_file_test (fullpath, G_FILE_TEST_IS_REGULAR);
 

src/mono/mono/metadata/assembly.c

@@ -336,6 +336,7 @@ load_in_path (const char *basename, const char** search_path, const MonoAssembly
 
 	for (i = 0; search_path [i]; ++i) {
 		fullpath = g_build_filename (search_path [i], basename, (const char*)NULL);
+		g_assert (fullpath);
 		result = mono_assembly_request_open (fullpath, req, status);
 		g_free (fullpath);
 		if (result)
@@ -1407,6 +1408,7 @@ absolute_dir (const gchar *filename)
 
 	cwd = g_get_current_dir ();
 	mixed = g_build_filename (cwd, filename, (const char*)NULL);
+	g_assert (mixed);
 	parts = g_strsplit (mixed, G_DIR_SEPARATOR_S, 0);
 	g_free (mixed);
 	g_free (cwd);

src/mono/mono/metadata/class.c

@@ -6785,10 +6785,13 @@ mono_method_get_base_method (MonoMethod *method, gboolean definition, MonoError
 			if (mono_class_is_open_constructed_type (m_class_get_byval_arg (parent))) {
 				parent = mono_class_inflate_generic_class_checked (parent, generic_inst, error);
 				return_val_if_nok  (error, NULL);
+				g_assert (parent);
 			}
+
 			if (mono_class_is_ginst (parent)) {
 				parent_inst = mono_class_get_context (parent);
 				parent = mono_class_get_generic_class (parent)->container_class;
+				g_assert (parent);
 			}
 
 			mono_class_setup_vtable (parent);
@@ -6808,6 +6811,7 @@ mono_method_get_base_method (MonoMethod *method, gboolean definition, MonoError
 		if (mono_class_is_open_constructed_type (m_class_get_byval_arg (klass))) {
 			klass = mono_class_inflate_generic_class_checked (klass, generic_inst, error);
 			return_val_if_nok (error, NULL);
+			g_assert (klass);
 
 			generic_inst = NULL;
 		}
@@ -6821,6 +6825,7 @@ mono_method_get_base_method (MonoMethod *method, gboolean definition, MonoError
 	if (generic_inst) {
 		klass = mono_class_inflate_generic_class_checked (klass, generic_inst, error);
 		return_val_if_nok (error, NULL);
+		g_assert (klass);
 		generic_inst = NULL;
 	}
 
@@ -6909,7 +6914,7 @@ mono_class_has_default_constructor (MonoClass *klass, gboolean public_only)
  * \param klass class in which the failure was detected
  * \param fmt \c printf -style error message string.
  *
- * Sets a deferred failure in the class and prints a warning message. 
+ * Sets a deferred failure in the class and prints a warning message.
  * The deferred failure allows the runtime to attempt setting up the class layout at runtime.
  *
  * LOCKING: Acquires the loader lock.

src/mono/mono/metadata/icall.c

@@ -4564,6 +4564,7 @@ ves_icall_System_Reflection_RuntimeAssembly_GetInfo (MonoQCallAssemblyHandle ass
 		else
 			absolute = g_build_filename (assembly->basedir, filename, (const char*)NULL);
 
+		g_assert (absolute);
 		mono_icall_make_platform_path (absolute);
 
 		const gchar *prepend = mono_icall_get_file_path_prefix (absolute);

src/mono/mono/metadata/image.c

@@ -2511,6 +2511,7 @@ mono_image_load_file_for_image_checked (MonoImage *image, uint32_t fileidx, Mono
 	fname = mono_metadata_string_heap (image, fname_id);
 	base_dir = g_path_get_dirname (image->name);
 	name = g_build_filename (base_dir, fname, (const char*)NULL);
+	g_assert (name);
 	res = mono_image_open (name, NULL);
 	if (!res)
 		goto done;

src/mono/mono/metadata/object.c

@@ -4243,6 +4243,8 @@ prepare_run_main (MonoMethod *method, int argc, char *argv[])
 						    basename,
 						    (const char*)NULL);
 
+		g_assert (fullpath);
+
 		utf8_fullpath = utf8_from_external (fullpath);
 		if(utf8_fullpath == NULL) {
 			/* Printing the arg text will cause glib to
@@ -5355,7 +5357,7 @@ MonoObjectHandle
 mono_object_new_handle (MonoClass *klass, MonoError *error)
 {
 	MONO_REQ_GC_UNSAFE_MODE;
-	
+
 	if (MONO_CLASS_IS_IMPORT(klass)) {
 		mono_error_set_not_supported (error, "Built-in COM interop is not supported on Mono.");
 		return MONO_HANDLE_NEW (MonoObject, NULL);

src/mono/mono/metadata/sgen-tarjan-bridge.c

@@ -819,8 +819,10 @@ create_scc (ScanData *data)
 			g_error ("Invalid state when building SCC %d", other->state);
 		}
 
-		if (other->is_bridge)
+		if (other->is_bridge) {
+			g_assert (color_data);
 			dyn_array_ptr_add (&color_data->bridges, other->obj);
+		}
 
 		// Maybe we should make sure we are not adding duplicates here. It is not really a problem
 		// since we will get rid of duplicates before submitting the SCCs to the client in gather_xrefs

src/mono/mono/mini/aot-compiler.c

@@ -5508,10 +5508,10 @@ MONO_RESTORE_WARNING
 					if (decoded_args->named_args_info [j].field && !strcmp (decoded_args->named_args_info [j].field->name, "EntryPoint")) {
 						named = (const char *)decoded_args->named_args[j]->value.primitive;
 						slen = mono_metadata_decode_value (named, &named);
-						
+
 						int prefix_len = (int)strlen (acfg->user_symbol_prefix);
 						g_assert (prefix_len < 2);
-						
+
 						export_name = (char *)g_malloc (prefix_len + slen + 1);
 						if (prefix_len == 1)
 							export_name[0] = *acfg->user_symbol_prefix;
@@ -5851,12 +5851,14 @@ add_generic_class_with_depth (MonoAotCompile *acfg, MonoClass *klass, int depth,
 
 		icomparable_inst = mono_class_inflate_generic_class_checked (icomparable, &ctx, error);
 		mono_error_assert_ok (error); /* FIXME don't swallow the error */
+		g_assert (icomparable_inst);
 
 		if (mono_class_is_assignable_from_internal (icomparable_inst, tclass)) {
 			MonoClass *gcomparer_inst;
 			gcomparer = mono_class_load_from_name (mono_defaults.corlib, "System.Collections.Generic", "GenericComparer`1");
 			gcomparer_inst = mono_class_inflate_generic_class_checked (gcomparer, &ctx, error);
 			mono_error_assert_ok (error); /* FIXME don't swallow the error */
+			g_assert (gcomparer_inst);
 
 			add_generic_class (acfg, gcomparer_inst, FALSE, "Comparer<T>");
 		}
@@ -5878,13 +5880,15 @@ add_generic_class_with_depth (MonoAotCompile *acfg, MonoClass *klass, int depth,
 
 		iface_inst = mono_class_inflate_generic_class_checked (iface, &ctx, error);
 		mono_error_assert_ok (error); /* FIXME don't swallow the error */
+		g_assert (iface_inst);
 
 		if (mono_class_is_assignable_from_internal (iface_inst, tclass)) {
 			MonoClass *gcomparer_inst;
 
 			gcomparer = mono_class_load_from_name (mono_defaults.corlib, "System.Collections.Generic", "GenericEqualityComparer`1");
 			gcomparer_inst = mono_class_inflate_generic_class_checked (gcomparer, &ctx, error);
 			mono_error_assert_ok (error); /* FIXME don't swallow the error */
+			g_assert (gcomparer_inst);
 			add_generic_class (acfg, gcomparer_inst, FALSE, "EqualityComparer<T>");
 		}
 	}
@@ -5906,6 +5910,7 @@ add_generic_class_with_depth (MonoAotCompile *acfg, MonoClass *klass, int depth,
 			enum_comparer = mono_class_load_from_name (mono_defaults.corlib, "System.Collections.Generic", "EnumEqualityComparer`1");
 			enum_comparer_inst = mono_class_inflate_generic_class_checked (enum_comparer, &ctx, error);
 			mono_error_assert_ok (error); /* FIXME don't swallow the error */
+			g_assert (enum_comparer_inst);
 			add_generic_class (acfg, enum_comparer_inst, FALSE, "EqualityComparer<T>");
 		}
 	}
@@ -5927,6 +5932,7 @@ add_generic_class_with_depth (MonoAotCompile *acfg, MonoClass *klass, int depth,
 			comparer = mono_class_load_from_name (mono_defaults.corlib, "System.Collections.Generic", "ObjectComparer`1");
 			comparer_inst = mono_class_inflate_generic_class_checked (comparer, &ctx, error);
 			mono_error_assert_ok (error); /* FIXME don't swallow the error */
+			g_assert (comparer_inst);
 			add_generic_class (acfg, comparer_inst, FALSE, "Comparer<T>");
 		}
 	}
@@ -5950,6 +5956,7 @@ add_instances_of (MonoAotCompile *acfg, MonoClass *klass, MonoType **insts, int
 		ctx.class_inst = mono_metadata_get_generic_inst (1, args);
 		generic_inst = mono_class_inflate_generic_class_checked (klass, &ctx, error);
 		mono_error_assert_ok (error); /* FIXME don't swallow the error */
+		g_assert (generic_inst);
 		add_generic_class (acfg, generic_inst, force, "");
 	}
 }
@@ -11566,6 +11573,9 @@ emit_exception_info (MonoAotCompile *acfg)
 		char *aot_file = g_strdup_printf("%s%s", image_basename, SEQ_POINT_AOT_EXT);
 		char *aot_file_path = g_build_filename (dir, aot_file, (const char*)NULL);
 
+		g_assert (dir);
+		g_assert (aot_file_path);
+
 		if (g_ensure_directory_exists (aot_file_path) == FALSE) {
 			fprintf (stderr, "AOT : failed to create msym directory: %s\n", aot_file_path);
 			exit (1);
@@ -15345,6 +15355,8 @@ set_paths (MonoAotCompile *acfg)
 			}
 
 			acfg->tmpbasename = g_build_filename (temp_path, "temp", (const char*)NULL);
+			g_assert (acfg->tmpbasename);
+
 			acfg->asm_fname = g_strdup_printf ("%s.s", acfg->tmpbasename);
 			acfg->llvm_sfile = g_strdup_printf ("%s-llvm.s", acfg->tmpbasename);
 
@@ -15379,6 +15391,8 @@ set_paths (MonoAotCompile *acfg)
 			/* Done later */
 		} else {
 			acfg->tmpbasename = g_build_filename (acfg->aot_opts.temp_path, "temp", (const char*)NULL);
+			g_assert (acfg->tmpbasename);
+
 			acfg->asm_fname = g_strdup_printf ("%s.s", acfg->tmpbasename);
 		}
 	}
@@ -15624,6 +15638,7 @@ compile_assemblies_in_child (MonoAotOptions *aot_opts, MonoAssembly **assemblies
 
 #ifdef HOST_WIN32
 	response_fname = g_build_filename (aot_opts->temp_path, "temp.rsp", (const char*)NULL);
+	g_assert (response_fname);
 	response = fopen (response_fname, "w");
 	g_assert (response);
 #endif

src/mono/mono/mini/mini-generic-sharing.c

@@ -1312,6 +1312,7 @@ get_wrapper_shared_vtype (MonoType *t)
 
 	MonoClass *tuple_inst = mono_class_inflate_generic_class_checked (tuple_class, &ctx, error);
 	mono_error_assert_ok (error);
+	g_assert (tuple_inst);
 
 	//printf ("T: %s\n", mono_class_full_name (tuple_inst));
 
@@ -1411,6 +1412,7 @@ get_wrapper_shared_type_full (MonoType *t, gboolean is_field)
 		}
 		klass = mono_class_inflate_generic_class_checked (mono_class_get_generic_class (klass)->container_class, &ctx, error);
 		mono_error_assert_ok (error); /* FIXME don't swallow the error */
+		g_assert (klass);
 
 		t = m_class_get_byval_arg (klass);
 		MonoType *shared_type = get_wrapper_shared_vtype (t);
@@ -4349,6 +4351,7 @@ get_shared_type (MonoType *t, MonoType *type)
 
 		k = mono_class_inflate_generic_class_checked (gclass->container_class, &context, error);
 		mono_error_assert_ok (error); /* FIXME don't swallow the error */
+		g_assert (k);
 
 		return mini_get_shared_gparam (t, m_class_get_byval_arg (k));
 	} else if (MONO_TYPE_ISSTRUCT (type)) {

src/mono/mono/mini/mini-runtime.c

@@ -404,14 +404,17 @@ void *(mono_global_codeman_reserve) (int size)
 			global_codeman = mono_code_manager_new ();
 		else
 			global_codeman = mono_code_manager_new_aot ();
-		return mono_code_manager_reserve (global_codeman, size);
+		ptr = mono_code_manager_reserve (global_codeman, size);
 	}
 	else {
 		mono_jit_lock ();
 		ptr = mono_code_manager_reserve (global_codeman, size);
 		mono_jit_unlock ();
-		return ptr;
 	}
+
+	/* Virtually all call sites for this API assume it can't return NULL. */
+	g_assert (ptr);
+	return ptr;
 }
 
 /* The callback shouldn't take any locks */
@@ -2167,7 +2170,7 @@ mono_emit_jit_dump (MonoJitInfo *jinfo, gpointer code)
 	int i;
 
 	memset (&rec, 0, sizeof (rec));
-	
+
 	// populating info relating debug methods
 	dmji = mono_debug_find_method (jinfo->d.method, NULL);
 
@@ -4540,20 +4543,20 @@ mini_llvm_init (void)
 }
 
 #ifdef ENSURE_PRIMARY_STACK_SIZE
-/*++ 
- Function: 
-   EnsureStackSize 
-  
- Abstract: 
-   This fixes a problem on MUSL where the initial stack size reported by the 
-   pthread_attr_getstack is about 128kB, but this limit is not fixed and 
-   the stack can grow dynamically. The problem is that it makes the 
-   functions ReflectionInvocation::[Try]EnsureSufficientExecutionStack 
-   to fail for real life scenarios like e.g. compilation of corefx. 
-   Since there is no real fixed limit for the stack, the code below 
-   ensures moving the stack limit to a value that makes reasonable 
-   real life scenarios work. 
-  
+/*++
+ Function:
+   EnsureStackSize
+
+ Abstract:
+   This fixes a problem on MUSL where the initial stack size reported by the
+   pthread_attr_getstack is about 128kB, but this limit is not fixed and
+   the stack can grow dynamically. The problem is that it makes the
+   functions ReflectionInvocation::[Try]EnsureSufficientExecutionStack
+   to fail for real life scenarios like e.g. compilation of corefx.
+   Since there is no real fixed limit for the stack, the code below
+   ensures moving the stack limit to a value that makes reasonable
+   real life scenarios work.
+
  --*/
 static MONO_NO_OPTIMIZATION MONO_NEVER_INLINE void
 ensure_stack_size (size_t size)
@@ -4737,7 +4740,7 @@ mini_init (const char *filename)
 	mono_w32handle_init ();
 #endif
 
-#ifdef ENSURE_PRIMARY_STACK_SIZE 
+#ifdef ENSURE_PRIMARY_STACK_SIZE
 	ensure_stack_size (5 * 1024 * 1024);
 #endif // ENSURE_PRIMARY_STACK_SIZE
 

src/mono/mono/mini/mini.c

@@ -4322,6 +4322,7 @@ mini_handle_call_res_devirt (MonoMethod *cmethod)
 
 		inst = mono_class_inflate_generic_class_checked (mono_class_get_iequatable_class (), &ctx, error);
 		mono_error_assert_ok (error);
+		g_assert (inst);
 
 		// EqualityComparer<T>.Default returns specific types depending on T
 		// FIXME: Special case more types: byte, string, nullable, enum ?