This introduces three macros, to be used in place of a simple pointer assignment...
author"Andi McClure ext:(%22) <andi.mcclure@xamarin.com>
Tue, 27 Oct 2015 15:41:41 +0000 (11:41 -0400)
committer"Andi McClure ext:(%22) <andi.mcclure@xamarin.com>
Tue, 27 Oct 2015 15:41:41 +0000 (11:41 -0400)
CHECKED_METADATA_WRITE_PTR - Use this to assign a pointer when it points from memory hosted by an image or imageset to another.
CHECKED_METADATA_WRITE_PTR_LOCAL - Use this to assign a pointer when it points from memory noted by an image or imageset to itself.
CHECKED_METADATA_WRITE_PTR_EXEMPT - Use this to assign a pointer in memory hosted by an image or imageset, but which the audit feature cannot correctly check at this time (this is the case when an image must contain a pointer to something that is not located in a mempool, such as a constant string). This is so that at least we know where these exceptions to the audits are.

In non-checked mode, each of these is a simple assignment. In checked mode, they walk the graph of references between images to make sure invariants related to ref_count are not violated.

mono/metadata/Makefile.am
mono/metadata/image-internals.h [new file with mode: 0644]
mono/metadata/image.c
mono/utils/checked-build.c
mono/utils/checked-build.h

index d0e6fc2fa576554f18eb5aa1984da3d40d3f4d17..145ea11f951e1e33eca71409dc8997c90fb6628b 100644 (file)
@@ -133,6 +133,7 @@ common_sources = \
        icall.c                 \
        icall-def.h             \
        image.c                 \
+       image-internals.h       \
        jit-info.c              \
        loader.c                \
        locales.c               \
diff --git a/mono/metadata/image-internals.h b/mono/metadata/image-internals.h
new file mode 100644 (file)
index 0000000..3bfd955
--- /dev/null
@@ -0,0 +1,31 @@
+/* 
+ * Copyright 2015 Xamarin Inc
+ */
+#ifndef __MONO_METADATA_IMAGE_INTERNALS_H__
+#define __MONO_METADATA_IMAGE_INTERNALS_H__
+
+#ifdef CHECKED_BUILD
+
+#include <mono/metadata/image.h>
+#include <mono/metadata/metadata-internals.h>
+
+typedef struct
+{
+       MonoImage *image;
+       MonoImageSet *image_set;
+} MonoMemPoolOwner;
+
+static MonoMemPoolOwner mono_mempool_no_owner = {NULL,NULL};
+
+static gboolean
+check_mempool_owner_eq (MonoMemPoolOwner a, MonoMemPoolOwner b)
+{
+       return a.image == b.image && a.image_set == b.image_set;
+}
+
+MonoMemPoolOwner
+mono_find_mempool_owner (void *ptr);
+
+#endif /* CHECKED_BUILD */
+
+#endif /* __MONO_METADATA_IMAGE_INTERNALS_H__ */
index ab78ab8f23bae36c2c0dda564b8dabff6203bfb0..3013a3e6fe64581702a87cbfba2b37b72316362a 100644 (file)
@@ -38,6 +38,7 @@
 #include <mono/metadata/security-core-clr.h>
 #include <mono/metadata/verify-internals.h>
 #include <mono/metadata/verify.h>
+#include <mono/metadata/image-internals.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #ifdef HAVE_UNISTD_H
@@ -2534,3 +2535,74 @@ mono_image_append_class_to_reflection_info_set (MonoClass *class)
        image->reflection_info_unregister_classes = g_slist_prepend_mempool (image->mempool, image->reflection_info_unregister_classes, class);
        mono_image_unlock (image);
 }
+
+#if CHECKED_BUILD
+
+// These are support for the mempool reference tracking feature in checked-build, but live in image.c due to use of static variables of this file.
+
+// Given an image and a pointer, return the mempool owner if it is either this image or one of its imagesets.
+static MonoMemPoolOwner
+check_for_mempool_owner (void *ptr, MonoImage *image)
+{
+       if (mono_mempool_contains_addr (image->mempool, ptr))
+       {
+               MonoMemPoolOwner owner = {image, NULL};
+               return owner;
+       }
+
+       GSList *l;
+       for (l = image->image_sets; l; l = l->next) {
+               MonoImageSet *set = l->data;
+
+               if (mono_mempool_contains_addr (set->mempool, ptr))
+               {
+                       MonoMemPoolOwner owner = {NULL, set};
+                       return owner;
+               }
+       }
+
+       return mono_mempool_no_owner;
+}
+
+/**
+ * mono_find_mempool_owner:
+ *
+ * Find the image or imageset, if any, which a given pointer is located in the memory of.
+ */
+MonoMemPoolOwner
+mono_find_mempool_owner (void *ptr)
+{
+       mono_images_lock ();
+
+       MonoMemPoolOwner owner = mono_mempool_no_owner;
+       gboolean searching = TRUE;
+
+       // Iterate over both by-path image hashes
+       const int hash_count = 2;
+       const int hash_candidates[hash_count] = {IMAGES_HASH_PATH, IMAGES_HASH_PATH_REFONLY};
+       int hash_idx;
+       for (hash_idx = 0; searching && hash_idx < IMAGES_HASH_COUNT; hash_idx++)
+       {
+               GHashTable *target = loaded_images_hashes [hash_candidates [hash_idx]];
+               GHashTableIter iter;
+               MonoImage *image;
+
+               // Iterate over images within a hash
+               g_hash_table_iter_init (&iter, target);
+               while (searching && g_hash_table_iter_next(&iter, NULL, (gpointer *)&image))
+               {
+                       mono_image_lock (image);
+                       owner = check_for_mempool_owner (ptr, image);
+                       mono_image_unlock (image);
+
+                       // Continue searching if null owner returned
+                       searching = check_mempool_owner_eq (owner, mono_mempool_no_owner);
+               }
+       }
+
+       mono_images_unlock ();
+
+       return owner;
+}
+
+#endif
\ No newline at end of file
index 7dd659918745bfc7419451f9cc3ab93d117fba25..1a07001dc04273ffbe7da00887feeb999954e20b 100644 (file)
 #include <mono/utils/checked-build.h>
 #include <mono/utils/mono-threads.h>
 #include <mono/utils/mono-tls.h>
+#include <mono/metadata/mempool.h>
+#include <mono/metadata/metadata-internals.h>
+#include <mono/metadata/image-internals.h>
+#include <mono/metadata/class-internals.h>
 #include <glib.h>
 
 #define MAX_NATIVE_BT 6
@@ -225,4 +229,269 @@ assert_gc_neutral_mode (void)
        }
 }
 
+// check_metadata_store et al: The goal of these functions is to verify that if there is a pointer from one mempool into
+// another, that the pointed-to memory is protected by the reference count mechanism for MonoImages.
+//
+// Note: The code below catches only some kinds of failures. Failures outside its scope notably incode:
+// * Code below absolutely assumes that no mempool is ever held as "mempool" member by more than one Image or ImageSet at once
+// * Code below assumes reference counts never underflow (ie: if we have a pointer to something, it won't be deallocated while we're looking at it)
+// Locking strategy is a little slapdash overall.
+
+#define check_mempool_assert_message(...) \
+       g_assertion_message("Mempool reference violation: " __VA_ARGS__)
+
+// Say image X "references" image Y if X either contains Y in its modules field, or X’s "references" field contains an
+// assembly whose image is Y.
+// Say image X transitively references image Y if there is any chain of images-referencing-images which leads from X to Y.
+// Once the mempools for two pointers have been looked up, there are four possibilities:
+
+// Case 1. Image FROM points to Image TO: Legal if FROM transitively references TO
+
+// We'll do a simple BFS graph search on images. For each image we visit:
+static void
+check_image_search (GHashTable *visited, GPtrArray *next, MonoImage *candidate, MonoImage *goal, gboolean *success)
+{
+       // Image hasn't even been loaded-- ignore it
+       if (!candidate)
+               return;
+
+       // Image has already been visited-- ignore it
+       if (g_hash_table_lookup_extended (visited, candidate, NULL, NULL))
+               return;
+
+       // Image is the target-- mark success
+       if (candidate == goal)
+       {
+               *success = TRUE;
+               return;
+       }
+
+       // Unvisited image, queue it to have its children visited
+       g_hash_table_insert (visited, candidate, NULL);
+       g_ptr_array_add (next, candidate);
+       return;
+}
+
+static gboolean
+check_image_may_reference_image(MonoImage *from, MonoImage *to)
+{
+       if (to == from) // Shortcut
+               return TRUE;
+
+       // Corlib is never unloaded, and all images implicitly reference it.
+       // Some images avoid explicitly referencing it as an optimization, so special-case it here.
+       if (to == mono_defaults.corlib)
+               return TRUE;
+
+       gboolean success = FALSE;
+
+       // Images to inspect on this pass, images to inspect on the next pass
+       GPtrArray *current = g_ptr_array_sized_new (1), *next = g_ptr_array_new ();
+
+       // Because in practice the image graph contains cycles, we must track which images we've visited
+       GHashTable *visited = g_hash_table_new (NULL, NULL);
+
+       #define CHECK_IMAGE_VISIT(i) check_image_search (visited, next, (i), to, &success)
+
+       CHECK_IMAGE_VISIT (from); // Initially "next" contains only from node
+
+       // For each pass exhaust the "to check" queue while filling up the "check next" queue
+       while (!success && next->len > 0) // Halt on success or when out of nodes to process
+       {
+               // Swap "current" and "next" and clear next
+               GPtrArray *temp = current;
+               current = next;
+               next = temp;
+               g_ptr_array_set_size (next, 0);
+
+               int current_idx;
+               for(current_idx = 0; current_idx < current->len; current_idx++)
+               {
+                       MonoImage *checking = g_ptr_array_index (current, current_idx); // CAST?
+
+                       mono_image_lock (checking);
+
+                       // For each queued image visit all directly referenced images
+                       int inner_idx;
+
+                       for (inner_idx = 0; !success && inner_idx < checking->module_count; inner_idx++)
+                       {
+                               CHECK_IMAGE_VISIT (checking->modules[inner_idx]);
+                       }
+
+                       for (inner_idx = 0; !success && inner_idx < checking->nreferences; inner_idx++)
+                       {
+                               // References are lazy-loaded and thus allowed to be NULL.
+                               // If they are NULL, we don't care about them for this search, because they haven't impacted ref_count yet.
+                               if (checking->references[inner_idx])
+                               {
+                                       CHECK_IMAGE_VISIT (checking->references[inner_idx]->image);
+                               }
+                       }
+
+                       mono_image_unlock (checking);
+               }
+       }
+
+       g_ptr_array_free (current, TRUE); g_ptr_array_free (next, TRUE); g_hash_table_destroy (visited);
+
+       return success;
+}
+
+// Case 2. ImageSet FROM points to Image TO: One of FROM's "images" either is, or transitively references, TO.
+static gboolean
+check_image_set_may_reference_image (MonoImageSet *from, MonoImage *to)
+{
+       // See above-- All images implicitly reference corlib
+       if (to == mono_defaults.corlib)
+               return TRUE;
+
+       int idx;
+       gboolean success = FALSE;
+       mono_image_set_lock (from);
+       for (idx = 0; !success && idx < from->nimages; idx++)
+       {
+               if (!check_image_may_reference_image (from->images[idx], to))
+                       success = TRUE;
+       }
+       mono_image_set_unlock (from);
+
+       return success; // No satisfying image found in from->images
+}
+
+// Case 3. ImageSet FROM points to ImageSet TO: The images in TO are a strict subset of FROM (no transitive relationship is important here)
+static gboolean
+check_image_set_may_reference_image_set (MonoImageSet *from, MonoImageSet *to)
+{
+       if (to == from)
+               return TRUE;
+
+       gboolean valid = TRUE; // Until proven otherwise
+
+       mono_image_set_lock (from); mono_image_set_lock (to);
+
+       int to_idx, from_idx;
+       for (to_idx = 0; valid && to_idx < to->nimages; to_idx++)
+       {
+               gboolean seen = FALSE;
+
+               // For each item in to->images, scan over from->images looking for it.
+               for (from_idx = 0; !seen && from_idx < from->nimages; from_idx++)
+               {
+                       if (to->images[to_idx] == from->images[from_idx])
+                               seen = TRUE;
+               }
+
+               // If the to->images item is not found in from->images, the subset check has failed
+               if (!seen)
+                       valid = FALSE;
+       }
+
+       mono_image_set_unlock (from); mono_image_set_unlock (to);
+
+       return valid; // All items in "to" were found in "from"
+}
+
+// Case 4. Image FROM points to ImageSet TO: FROM transitively references *ALL* of the “images” listed in TO
+static gboolean
+check_image_may_reference_image_set (MonoImage *from, MonoImageSet *to)
+{
+       if (to->nimages == 0) // Malformed image_set
+               return FALSE;
+
+       gboolean valid = TRUE;
+
+       mono_image_set_lock (to);
+       int idx;
+       for (idx = 0; valid && idx < to->nimages; idx++)
+       {
+               if (!check_image_may_reference_image (from, to->images[idx]))
+                       valid = FALSE;
+       }
+       mono_image_set_unlock (to);
+
+       return valid; // All images in to->images checked out
+}
+
+// Small helper-- get a descriptive string for a MonoMemPoolOwner
+static const char *
+check_mempool_owner_name (MonoMemPoolOwner owner)
+{
+       if (owner.image)
+               return owner.image->name;
+       if (owner.image_set) // TODO: Construct a string containing all included images
+               return "(Imageset)";
+       return "(Non-image memory)";
+}
+
+static void
+check_mempool_may_reference_mempool (void *from_ptr, void *to_ptr, gboolean require_local)
+{
+       // Null pointers are OK
+       if (!to_ptr)
+               return;
+
+       MonoMemPoolOwner from = mono_find_mempool_owner (from_ptr), to = mono_find_mempool_owner (to_ptr);
+
+       if (require_local)
+       {
+               if (!check_mempool_owner_eq (from,to))
+                       check_mempool_assert_message ("Pointer in image %s should have been internal, but instead pointed to image %s", check_mempool_owner_name(from), check_mempool_owner_name(to));
+       }
+
+       // Writing into unknown mempool
+       else if (check_mempool_owner_eq (from, mono_mempool_no_owner))
+       {
+               check_mempool_assert_message ("Non-image memory attempting to write pointer to image %s", check_mempool_owner_name(to));
+       }
+
+       // Reading from unknown mempool
+       else if (check_mempool_owner_eq (to, mono_mempool_no_owner))
+       {
+               check_mempool_assert_message ("Attempting to write pointer from image %s to non-image memory", check_mempool_owner_name(from));
+       }
+
+       // Split out the four cases described above:
+       else if (from.image && to.image)
+       {
+               if (!check_image_may_reference_image (from.image, to.image))
+                       check_mempool_assert_message ("Image %s tried to point to image %s, but does not retain a reference", check_mempool_owner_name(from), check_mempool_owner_name(to));
+       }
+
+       else if (from.image && to.image_set)
+       {
+               if (!check_image_may_reference_image_set (from.image, to.image_set))
+                       check_mempool_assert_message ("Image %s tried to point to image set, but does not retain a reference", check_mempool_owner_name(from));
+       }
+
+       else if (from.image_set && to.image_set)
+       {
+               if (!check_image_set_may_reference_image_set (from.image_set, to.image_set))
+                       check_mempool_assert_message ("Image set tried to point to image set, but does not retain a reference");
+       }
+
+       else if (from.image_set && to.image)
+       {
+               if (!check_image_set_may_reference_image (from.image_set, to.image))
+                       check_mempool_assert_message ("Image set tried to point to image %s, but does not retain a reference", check_mempool_owner_name(to));
+       }
+
+       else
+       {
+               check_mempool_assert_message ("Internal logic error: Unreachable code");
+       }
+}
+
+void
+check_metadata_store (void *from, void *to)
+{
+    check_mempool_may_reference_mempool (from, to, FALSE);
+}
+
+void
+check_metadata_store_local (void *from, void *to)
+{
+    check_mempool_may_reference_mempool (from, to, TRUE);
+}
+
 #endif /* CHECKED_BUILD */
index d2ba9ec1b1b9d716a8f7ef147ac35cac1e79534b..17524bdd1b8362c64a2704ba60aba5204b3750c9 100644 (file)
 #define __CHECKED_BUILD_H__
 
 #include <config.h>
+#include <mono/utils/atomic.h>
+
+// This is for metadata writes which we have chosen not to check at the current time.
+// Because in principle this should never happen, we still use a macro so that the exemptions will be easier to find, and remove, later.
+// The current reason why this is needed is for pointers to constant strings, which the checker cannot verify yet.
+#define CHECKED_METADATA_WRITE_PTR_EXEMPT(ptr, val) do { (ptr) = (val); } while (0)
 
 #ifdef CHECKED_BUILD
 
@@ -70,6 +76,18 @@ Functions that can be called from both coop or preept modes.
        assert_gc_neutral_mode ();      \
 } while (0);
 
+// Use when writing a pointer from one image or imageset to another.
+#define CHECKED_METADATA_WRITE_PTR(ptr, val) do {    \
+    check_metadata_store (&(ptr), (val));    \
+    (ptr) = (val);    \
+} while (0);
+
+// Use when writing a pointer from an image or imageset to itself.
+#define CHECKED_METADATA_WRITE_PTR_LOCAL(ptr, val) do {    \
+    check_metadata_store_local (&(ptr), (val));    \
+    (ptr) = (val);    \
+} while (0);
+
 /*
 This can be called by embedders
 */
@@ -93,6 +111,9 @@ void assert_gc_neutral_mode (void);
 void checked_build_init (void);
 void checked_build_thread_transition(const char *transition, void *info, int from_state, int suspend_count, int next_state, int suspend_count_delta);
 
+void check_metadata_store(void *from, void *to);
+void check_metadata_store_local(void *from, void *to);
+
 #else
 
 #define MONO_REQ_GC_SAFE_MODE
@@ -104,6 +125,9 @@ void checked_build_thread_transition(const char *transition, void *info, int fro
 #define CHECKED_MONO_INIT()
 #define CHECKED_BUILD_THREAD_TRANSITION(transition, info, from_state, suspend_count, next_state, suspend_count_delta)
 
+#define CHECKED_METADATA_WRITE_PTR(ptr, val) do { (ptr) = (val); } while (0)
+#define CHECKED_METADATA_WRITE_PTR_LOCAL(ptr, val) do { (ptr) = (val); } while (0)
+
 #endif /* CHECKED_BUILD */
 
 #endif