Support deleteing methods for delayed package loading

vchuravy · vchuravy · commit 5e5412664a54 · 2023-10-08T16:30:53.000-04:00
diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp
@@ -1438,7 +1438,8 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
   } else if (name == "JL_GC_PUSH1" || name == "JL_GC_PUSH2" ||
              name == "JL_GC_PUSH3" || name == "JL_GC_PUSH4" ||
              name == "JL_GC_PUSH5" || name == "JL_GC_PUSH6" ||
-             name == "JL_GC_PUSH7" || name == "JL_GC_PUSH8") {
+             name == "JL_GC_PUSH7" || name == "JL_GC_PUSH8" ||
+             name == "JL_GC_PUSH9") {
     ProgramStateRef State = C.getState();
     // Transform slots to roots, transform values to rooted
     unsigned NumArgs = CE->getNumArgs();
diff --git a/src/gf.c b/src/gf.c
@@ -1926,6 +1926,7 @@ JL_DLLEXPORT void jl_method_table_disable_incremental(jl_methtable_t *mt, jl_met
     jl_typemap_entry_t *methodentry = do_typemap_search(mt, method);
     JL_LOCK(&mt->writelock);
     // Narrow the world age on the method to make it uncallable
+    // size_t world = jl_atomic_load_acquire(&jl_world_counter);
     size_t world = jl_atomic_fetch_add(&jl_world_counter, 1);
     jl_method_table_invalidate(mt, methodentry, world, 1);
     JL_UNLOCK(&mt->writelock);
diff --git a/src/julia.h b/src/julia.h
@@ -932,6 +932,7 @@ extern void JL_GC_PUSH4(void *, void *, void *, void *)  JL_NOTSAFEPOINT;
 extern void JL_GC_PUSH5(void *, void *, void *, void *, void *)  JL_NOTSAFEPOINT;
 extern void JL_GC_PUSH7(void *, void *, void *, void *, void *, void *, void *)  JL_NOTSAFEPOINT;
 extern void JL_GC_PUSH8(void *, void *, void *, void *, void *, void *, void *, void *)  JL_NOTSAFEPOINT;
+extern void JL_GC_PUSH9(void *, void *, void *, void *, void *, void *, void *, void *)  JL_NOTSAFEPOINT;
 extern void _JL_GC_PUSHARGS(jl_value_t **, size_t) JL_NOTSAFEPOINT;
 // This is necessary, because otherwise the analyzer considers this undefined
 // behavior and terminates the exploration
@@ -974,6 +975,9 @@ extern void JL_GC_POP() JL_NOTSAFEPOINT;
 #define JL_GC_PUSH8(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8)                                     \
   void *__gc_stkf[] = {(void*)JL_GC_ENCODE_PUSH(8), jl_pgcstack, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8}; \
   jl_pgcstack = (jl_gcframe_t*)__gc_stkf;
+#define JL_GC_PUSH9(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9)                                     \
+  void *__gc_stkf[] = {(void*)JL_GC_ENCODE_PUSH(9), jl_pgcstack, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9}; \
+  jl_pgcstack = (jl_gcframe_t*)__gc_stkf;
 
 
 #define JL_GC_PUSHARGS(rts_var,n)                                                                       \
@@ -1901,6 +1905,7 @@ JL_DLLEXPORT jl_value_t *jl_restore_incremental(const char *fname, jl_array_t *d
 
 JL_DLLEXPORT void jl_set_newly_inferred(jl_value_t *newly_inferred);
 JL_DLLEXPORT void jl_push_newly_inferred(jl_value_t *ci);
+JL_DLLEXPORT void jl_method_table_disable_incremental(jl_methtable_t *mt, jl_method_t *m);
 JL_DLLEXPORT void jl_set_newly_deleted(jl_value_t *newly_deleted);
 JL_DLLEXPORT void jl_push_newly_deleted(jl_value_t *m);
 JL_DLLEXPORT void jl_write_compiler_output(void);
diff --git a/src/staticdata.c b/src/staticdata.c
@@ -2393,7 +2393,7 @@ static void jl_prepare_serialization_data(jl_array_t *mod_array, jl_array_t *new
 static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
                                            jl_array_t *worklist, jl_array_t *extext_methods,
                                            jl_array_t *new_specializations, jl_array_t *method_roots_list,
-                                           jl_array_t *ext_targets, jl_array_t *edges) JL_GC_DISABLED
+                                           jl_array_t *ext_targets, jl_array_t *edges, jl_array_t *newly_deleted) JL_GC_DISABLED
 {
     htable_new(&field_replace, 0);
     // strip metadata and IR when requested
@@ -2514,6 +2514,10 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
             jl_queue_for_serialization(&s, ext_targets);
             jl_queue_for_serialization(&s, edges);
         }
+        if (newly_deleted) {
+            jl_queue_for_serialization(&s, newly_deleted);
+        }
+
         jl_serialize_reachable(&s);
         // step 1.2: ensure all gvars are part of the sysimage too
         record_gvars(&s, &gvars);
@@ -2647,6 +2651,7 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
             jl_write_value(&s, method_roots_list);
             jl_write_value(&s, ext_targets);
             jl_write_value(&s, edges);
+            jl_write_value(&s, newly_deleted);
         }
         write_uint32(f, jl_array_len(s.link_ids_gctags));
         ios_write(f, (char*)jl_array_data(s.link_ids_gctags), jl_array_len(s.link_ids_gctags) * sizeof(uint32_t));
@@ -2725,11 +2730,11 @@ JL_DLLEXPORT void jl_create_system_image(void **_native_data, jl_array_t *workli
     }
 
     jl_array_t *mod_array = NULL, *extext_methods = NULL, *new_specializations = NULL;
-    jl_array_t *method_roots_list = NULL, *ext_targets = NULL, *edges = NULL;
+    jl_array_t *method_roots_list = NULL, *ext_targets = NULL, *edges = NULL, *_newly_deleted = NULL;
     int64_t checksumpos = 0;
     int64_t checksumpos_ff = 0;
     int64_t datastartpos = 0;
-    JL_GC_PUSH6(&mod_array, &extext_methods, &new_specializations, &method_roots_list, &ext_targets, &edges);
+    JL_GC_PUSH7(&mod_array, &extext_methods, &new_specializations, &method_roots_list, &ext_targets, &edges, &_newly_deleted);
 
     if (worklist) {
         mod_array = jl_get_loaded_modules();  // __toplevel__ modules loaded in this session (from Base.loaded_modules_array)
@@ -2776,7 +2781,10 @@ JL_DLLEXPORT void jl_create_system_image(void **_native_data, jl_array_t *workli
     }
     if (_native_data != NULL)
         native_functions = *_native_data;
-    jl_save_system_image_to_stream(ff, mod_array, worklist, extext_methods, new_specializations, method_roots_list, ext_targets, edges);
+    // Otherwise serialization will be confused.
+    if (newly_deleted)
+        _newly_deleted = jl_array_copy(newly_deleted);
+    jl_save_system_image_to_stream(ff, mod_array, worklist, extext_methods, new_specializations, method_roots_list, ext_targets, edges, _newly_deleted);
     if (_native_data != NULL)
         native_functions = NULL;
     // make sure we don't run any Julia code concurrently before this point
@@ -2860,6 +2868,7 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl
                                                  jl_array_t **extext_methods,
                                                  jl_array_t **new_specializations, jl_array_t **method_roots_list,
                                                  jl_array_t **ext_targets, jl_array_t **edges,
+                                                 jl_array_t **newly_deleted,
                                                  char **base, arraylist_t *ccallable_list, pkgcachesizes *cachesizes) JL_GC_DISABLED
 {
     int en = jl_gc_enable(0);
@@ -2921,7 +2930,7 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl
     assert(!ios_eof(f));
     s.s = f;
     uintptr_t offset_restored = 0, offset_init_order = 0, offset_extext_methods = 0, offset_new_specializations = 0, offset_method_roots_list = 0;
-    uintptr_t offset_ext_targets = 0, offset_edges = 0;
+    uintptr_t offset_ext_targets = 0, offset_edges = 0, offset_newly_deleted = 0;
     if (!s.incremental) {
         size_t i;
         for (i = 0; tags[i] != NULL; i++) {
@@ -2955,6 +2964,7 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl
         offset_method_roots_list = jl_read_offset(&s);
         offset_ext_targets = jl_read_offset(&s);
         offset_edges = jl_read_offset(&s);
+        offset_newly_deleted = jl_read_offset(&s);
     }
     s.buildid_depmods_idxs = depmod_to_imageidx(depmods);
     size_t nlinks_gctags = read_uint32(f);
@@ -2988,6 +2998,7 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl
         *method_roots_list = (jl_array_t*)jl_delayed_reloc(&s, offset_method_roots_list);
         *ext_targets = (jl_array_t*)jl_delayed_reloc(&s, offset_ext_targets);
         *edges = (jl_array_t*)jl_delayed_reloc(&s, offset_edges);
+        *newly_deleted = (jl_array_t*)jl_delayed_reloc(&s, offset_newly_deleted);
         if (!*new_specializations)
             *new_specializations = jl_alloc_vec_any(0);
     }
@@ -3175,6 +3186,11 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl
         assert(jl_is_datatype(obj));
         jl_cache_type_((jl_datatype_t*)obj);
     }
+
+    // Delete methods before inserting new ones.
+    if (newly_deleted)
+        jl_delete_methods(*newly_deleted);
+
     // Perform fixups: things like updating world ages, inserting methods & specializations, etc.
     size_t world = jl_atomic_load_acquire(&jl_world_counter);
     for (size_t i = 0; i < s.uniquing_objs.len; i++) {
@@ -3401,11 +3417,11 @@ static jl_value_t *jl_restore_package_image_from_stream(ios_t *f, jl_image_t *im
     assert(datastartpos > 0 && datastartpos < dataendpos);
     needs_permalloc = jl_options.permalloc_pkgimg || needs_permalloc;
     jl_value_t *restored = NULL;
-    jl_array_t *init_order = NULL, *extext_methods = NULL, *new_specializations = NULL, *method_roots_list = NULL, *ext_targets = NULL, *edges = NULL;
+    jl_array_t *init_order = NULL, *extext_methods = NULL, *new_specializations = NULL, *method_roots_list = NULL, *ext_targets = NULL, *edges = NULL, *newly_deleted = NULL;
     jl_svec_t *cachesizes_sv = NULL;
     char *base;
     arraylist_t ccallable_list;
-    JL_GC_PUSH8(&restored, &init_order, &extext_methods, &new_specializations, &method_roots_list, &ext_targets, &edges, &cachesizes_sv);
+    JL_GC_PUSH9(&restored, &init_order, &extext_methods, &new_specializations, &method_roots_list, &ext_targets, &edges, &newly_deleted, &cachesizes_sv);
 
     { // make a permanent in-memory copy of f (excluding the header)
         ios_bufmode(f, bm_none);
@@ -3429,11 +3445,12 @@ static jl_value_t *jl_restore_package_image_from_stream(ios_t *f, jl_image_t *im
                 ios_close(f);
             ios_static_buffer(f, sysimg, len);
             pkgcachesizes cachesizes;
-            jl_restore_system_image_from_stream_(f, image, depmods, checksum, (jl_array_t**)&restored, &init_order, &extext_methods, &new_specializations, &method_roots_list, &ext_targets, &edges, &base, &ccallable_list, &cachesizes);
+            jl_restore_system_image_from_stream_(f, image, depmods, checksum, (jl_array_t**)&restored, &init_order, &extext_methods, &new_specializations, &method_roots_list, &ext_targets, &edges, &newly_deleted, &base, &ccallable_list, &cachesizes);
             JL_SIGATOMIC_END();
 
             // Insert method extensions
             jl_insert_methods(extext_methods);
+
             // No special processing of `new_specializations` is required because recaching handled it
             // Add roots to methods
             jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored));
@@ -3469,7 +3486,7 @@ static jl_value_t *jl_restore_package_image_from_stream(ios_t *f, jl_image_t *im
 static void jl_restore_system_image_from_stream(ios_t *f, jl_image_t *image, uint32_t checksum)
 {
     JL_TIMING(LOAD_IMAGE, LOAD_Sysimg);
-    jl_restore_system_image_from_stream_(f, image, NULL, checksum | ((uint64_t)0xfdfcfbfa << 32), NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL);
+    jl_restore_system_image_from_stream_(f, image, NULL, checksum | ((uint64_t)0xfdfcfbfa << 32), NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL);
 }
 
 JL_DLLEXPORT jl_value_t *jl_restore_incremental_from_buf(const char *buf, jl_image_t *image, size_t sz, jl_array_t *depmods, int completeinfo, const char *pkgname, bool needs_permalloc)
diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c
@@ -838,6 +838,19 @@ static void jl_insert_methods(jl_array_t *list)
     }
 }
 
+static void jl_delete_methods(jl_array_t *list)
+{
+    size_t i, l = jl_array_len(list);
+    for (i = 0; i < l; i++) {
+        jl_method_t *meth = (jl_method_t*)jl_array_ptr_ref(list, i);
+        assert(jl_is_method(meth));
+        assert(!meth->is_for_opaque_closure);
+        jl_methtable_t *mt = jl_method_get_table(meth);
+        assert((jl_value_t*)mt != jl_nothing);
+        jl_method_table_disable_incremental(mt, meth);
+    }
+}
+
 static void jl_copy_roots(jl_array_t *method_roots_list, uint64_t key)
 {
     size_t i, l = jl_array_len(method_roots_list);
diff --git a/stdlib/Random/src/Random.jl b/stdlib/Random/src/Random.jl
@@ -31,28 +31,11 @@ export rand!, randn!,
 
 ## general definitions
 
-# Remove the shim methods
-# TODO: If we could support this as part of package loading
-#       That would be much nicer.
-# for m in Base.methods(rand)
-#     if m.module == Base
-#         Base.delete_method(m)
-#     end
-# end
+Base.Stubs.delete_stubs(Base.Stubs.Random)
 
 function __init__()
     seed!()
     ccall(:jl_gc_init_finalizer_rng_state, Cvoid, ())
-
-    # Remove the shim methods
-    # Is this too coarse? We want to only delete the method
-    # in Base, not any added by the user later on.
-    # We are currently not allowed to run this during any precompilation
-    # so we must guard it until "normal runtime".
-    # TODO: What to do when we are including Random into a sysimg.
-    if !Base.generating_output()
-        Base.Stubs.delete_stubs(Base.Stubs.Random)
-    end
 end
 
 """

Original file line number	Diff line number	Diff line change
`@@ -838,6 +838,19 @@ static void jl_insert_methods(jl_array_t *list)`
`838`	`838`	`}`
`839`	`839`	`}`
`840`	`840`
	`841`	`+static void jl_delete_methods(jl_array_t *list)`
	`842`	`+{`
	`843`	`+ size_t i, l = jl_array_len(list);`
	`844`	`+ for (i = 0; i < l; i++) {`
	`845`	`+ jl_method_t meth = (jl_method_t)jl_array_ptr_ref(list, i);`
	`846`	`+ assert(jl_is_method(meth));`
	`847`	`+ assert(!meth->is_for_opaque_closure);`
	`848`	`+ jl_methtable_t *mt = jl_method_get_table(meth);`
	`849`	`+ assert((jl_value_t*)mt != jl_nothing);`
	`850`	`+ jl_method_table_disable_incremental(mt, meth);`
	`851`	`+ }`
	`852`	`+}`
	`853`	`+`
`841`	`854`	`static void jl_copy_roots(jl_array_t *method_roots_list, uint64_t key)`
`842`	`855`	`{`
`843`	`856`	`size_t i, l = jl_array_len(method_roots_list);`