Check for conflicting @ccallable name before JIT registration

This turns the existing segfault into a warning, as on 1.9: ```julia julia> using Foo, Bar WARNING: @ccallable was already defined for this method name ``` Resolves JuliaLang#54878
topolarity · Sep 19, 2024 · 450fc9d · 450fc9d
1 parent a73ba3b
commit 450fc9d
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 14 deletions.
diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -7584,13 +7584,14 @@ static jl_cgval_t emit_cfunction(jl_codectx_t &ctx, jl_value_t *output_type, con
 
 // do codegen to create a C-callable alias/wrapper, or if sysimg_handle is set,
 // restore one from a loaded system image.
-const char *jl_generate_ccallable(LLVMOrcThreadSafeModuleRef llvmmod, void *sysimg_handle, jl_value_t *declrt, jl_value_t *sigt, jl_codegen_params_t &params)
+bool jl_generate_ccallable(LLVMOrcThreadSafeModuleRef llvmmod, void *sysimg_handle, jl_value_t *declrt, jl_value_t *sigt, jl_codegen_params_t &params)
 {
     ++GeneratedCCallables;
     jl_datatype_t *ft = (jl_datatype_t*)jl_tparam0(sigt);
     jl_value_t *ff = ft->instance;
     assert(ff);
     const char *name = jl_symbol_name(ft->name->mt->name);
+    bool name_conflict = (jl_ExecutionEngine->getGlobalValueAddress(StringRef(name)) != 0);
     jl_value_t *crt = declrt;
     if (jl_is_abstract_ref_type(declrt)) {
         declrt = jl_tparam0(declrt);
@@ -7619,20 +7620,20 @@ const char *jl_generate_ccallable(LLVMOrcThreadSafeModuleRef llvmmod, void *sysi
                 // restore a ccallable from the system image
                 void *addr;
                 int found = jl_dlsym(sysimg_handle, name, &addr, 0);
-                if (found)
-                    add_named_global(name, addr);
-                else {
-                    err = jl_get_exceptionf(jl_errorexception_type, "%s not found in sysimg", name);
+                if (!found) {
+                    err = jl_get_exceptionf(jl_errorexception_type, "@ccallable: symbol '%s' not found in sysimg", name);
                     jl_throw(err);
                 }
+                if (!name_conflict)
+                    add_named_global(name, addr);
             }
             else {
                 jl_method_instance_t *lam = jl_get_specialization1((jl_tupletype_t*)sigt, world, &min_valid, &max_valid, 0);
                 //Safe b/c params holds context lock
                 gen_cfun_wrapper(unwrap(llvmmod)->getModuleUnlocked(), params, sig, ff, name, declrt, lam, NULL, NULL, NULL, min_valid, max_valid);
             }
             JL_GC_POP();
-            return name;
+            return !name_conflict;
         }
         err = jl_get_exceptionf(jl_errorexception_type, "%s", sig.err_msg.c_str());
     }

diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp
@@ -362,7 +362,7 @@ static jl_callptr_t _jl_compile_codeinst(
     return fptr;
 }
 
-const char *jl_generate_ccallable(LLVMOrcThreadSafeModuleRef llvmmod, void *sysimg_handle, jl_value_t *declrt, jl_value_t *sigt, jl_codegen_params_t &params);
+bool jl_generate_ccallable(LLVMOrcThreadSafeModuleRef llvmmod, void *sysimg_handle, jl_value_t *declrt, jl_value_t *sigt, jl_codegen_params_t &params);
 
 // compile a C-callable alias
 extern "C" JL_DLLEXPORT_CODEGEN
@@ -396,13 +396,9 @@ int jl_compile_extern_c_impl(LLVMOrcThreadSafeModuleRef llvmmod, void *p, void *
     if (pparams == NULL)
         pparams = &params;
     assert(pparams->tsctx.getContext() == into->getContext().getContext());
-    const char *name = jl_generate_ccallable(wrap(into), sysimg, declrt, sigt, *pparams);
-    bool success = true;
-    if (!sysimg) {
+    bool success = jl_generate_ccallable(wrap(into), sysimg, declrt, sigt, *pparams);
+    if (success && !sysimg) {
         JL_LOCK(&jl_ExecutionEngine->jitlock);
-        if (jl_ExecutionEngine->getGlobalValueAddress(name)) {
-            success = false;
-        }
         if (success && p == NULL) {
             jl_jit_globals(params.global_targets);
             assert(params.workqueue.empty());

diff --git a/src/staticdata.c b/src/staticdata.c
@@ -2349,7 +2349,6 @@ static void jl_compile_extern(jl_method_t *m, void *sysimg_handle) JL_GC_DISABLE
     int success = jl_compile_extern_c(NULL, NULL, sysimg_handle, jl_svecref(sv, 0), jl_svecref(sv, 1));
     if (!success)
         jl_safe_printf("WARNING: @ccallable was already defined for this method name\n"); // enjoy a very bad time
-    assert(success || !sysimg_handle);
 }