From 341f05a75355574e69c70c8ac868e82f9261ce6e Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 12 Dec 2019 05:31:55 -0500 Subject: [PATCH] ccall: report static compile-time load issues correctly (#34062) * ccall: report static compile-time load issues correctly fix #34061 * add a test * Update ccall.jl --- src/ccall.cpp | 63 +++++++++++++---------------------------------- src/jitlayers.cpp | 30 +++++++++++++--------- test/ccall.jl | 7 ++++++ 3 files changed, 42 insertions(+), 58 deletions(-) diff --git a/src/ccall.cpp b/src/ccall.cpp index 03884ddf166b87..f186e069456008 100644 --- a/src/ccall.cpp +++ b/src/ccall.cpp @@ -4,7 +4,7 @@ // Map from symbol name (in a certain library) to its GV in sysimg and the // DL handle address in the current session. -typedef StringMap> SymMapGV; +typedef StringMap SymMapGV; static StringMap> libMapGV; #ifdef _OS_WINDOWS_ static SymMapGV symMapExe; @@ -43,65 +43,48 @@ lazyModule(Func &&func) // Find or create the GVs for the library and symbol lookup. // Return `runtime_lib` (whether the library name is a string) -// Optionally return the symbol address in the current session -// when `symaddr != nullptr`. // The `lib` and `sym` GV returned may not be in the current module. template static bool runtime_sym_gvs(const char *f_lib, const char *f_name, MT &&M, - GlobalVariable *&lib, GlobalVariable *&sym, - void **symaddr=nullptr) + GlobalVariable *&lib, GlobalVariable *&sym) { - void *libsym = NULL; bool runtime_lib = false; GlobalVariable *libptrgv; SymMapGV *symMap; #ifdef _OS_WINDOWS_ if ((intptr_t)f_lib == 1) { libptrgv = jlexe_var; - libsym = jl_exe_handle; symMap = &symMapExe; } else if ((intptr_t)f_lib == 2) { libptrgv = jldll_var; - libsym = jl_dl_handle; symMap = &symMapDl; } else #endif if (f_lib == NULL) { libptrgv = jlRTLD_DEFAULT_var; - libsym = jl_RTLD_DEFAULT_handle; symMap = &symMapDefault; } else { std::string name = "ccalllib_"; name += f_lib; runtime_lib = true; - auto iter = libMapGV.find(f_lib); - if (iter == libMapGV.end()) { + auto &libgv = libMapGV[f_lib]; + if (libgv.first == NULL) { libptrgv = new GlobalVariable(*M, T_pint8, false, GlobalVariable::ExternalLinkage, - NULL, name); - auto &libgv = libMapGV[f_lib]; - libgv = std::make_pair(global_proto(libptrgv), SymMapGV()); - symMap = &libgv.second; - libsym = jl_get_library(f_lib); - assert(libsym != NULL); - *jl_emit_and_add_to_shadow(libptrgv) = libsym; + Constant::getNullValue(T_pint8), name); + libgv.first = global_proto(libptrgv); } else { - libptrgv = iter->second.first; - symMap = &iter->second.second; + libptrgv = libgv.first; } + symMap = &libgv.second; } - if (libsym == NULL) { - libsym = *(void**)jl_get_globalvar(libptrgv); - } - assert(libsym != NULL); - GlobalVariable *llvmgv; - auto sym_iter = symMap->find(f_name); - if (sym_iter == symMap->end()) { + GlobalVariable *&llvmgv = (*symMap)[f_name]; + if (llvmgv == NULL) { // MCJIT forces this to have external linkage eventually, so we would clobber // the symbol of the actual function. std::string name = "ccall_"; @@ -109,19 +92,9 @@ static bool runtime_sym_gvs(const char *f_lib, const char *f_name, MT &&M, name += "_"; name += std::to_string(globalUnique++); llvmgv = new GlobalVariable(*M, T_pvoidfunc, false, - GlobalVariable::ExternalLinkage, NULL, name); + GlobalVariable::ExternalLinkage, + Constant::getNullValue(T_pvoidfunc), name); llvmgv = global_proto(llvmgv); - void *addr; - jl_dlsym(libsym, f_name, &addr, 0); - (*symMap)[f_name] = std::make_pair(llvmgv, addr); - if (symaddr) - *symaddr = addr; - *jl_emit_and_add_to_shadow(llvmgv) = addr; - } - else { - if (symaddr) - *symaddr = sym_iter->second.second; - llvmgv = sym_iter->second.first; } lib = libptrgv; @@ -218,7 +191,7 @@ static GlobalVariable *emit_plt_thunk( const AttributeList &attrs, CallingConv::ID cc, const char *f_lib, const char *f_name, GlobalVariable *libptrgv, GlobalVariable *llvmgv, - void *symaddr, bool runtime_lib) + bool runtime_lib) { PointerType *funcptype = PointerType::get(functype, 0); libptrgv = prepare_global_in(M, libptrgv); @@ -237,8 +210,7 @@ static GlobalVariable *emit_plt_thunk( auto gname = funcName.str(); GlobalVariable *got = new GlobalVariable(*M, T_pvoidfunc, false, GlobalVariable::ExternalLinkage, - nullptr, gname); - *jl_emit_and_add_to_shadow(got) = symaddr; + ConstantExpr::getBitCast(plt, T_pvoidfunc), gname); BasicBlock *b0 = BasicBlock::Create(jl_LLVMContext, "top", plt); IRBuilder<> irbuilder(b0); Value *ptr = runtime_sym_lookup(irbuilder, funcptype, f_lib, f_name, plt, libptrgv, @@ -274,6 +246,7 @@ static GlobalVariable *emit_plt_thunk( } } irbuilder.ClearInsertionPoint(); + got = global_proto(got); // exchange got for the permanent global before jl_finalize_module destroys it jl_finalize_module(M, true); @@ -297,21 +270,19 @@ static Value *emit_plt( assert(!functype->isVarArg()); GlobalVariable *libptrgv; GlobalVariable *llvmgv; - void *symaddr; auto LM = lazyModule([&] { Module *m = new Module(f_name, jl_LLVMContext); jl_setup_module(m); return m; }); - bool runtime_lib = runtime_sym_gvs(f_lib, f_name, LM, - libptrgv, llvmgv, &symaddr); + bool runtime_lib = runtime_sym_gvs(f_lib, f_name, LM, libptrgv, llvmgv); PointerType *funcptype = PointerType::get(functype, 0); auto &pltMap = allPltMap[attrs]; auto key = std::make_tuple(llvmgv, functype, cc); GlobalVariable *&shadowgot = pltMap[key]; if (!shadowgot) { - shadowgot = emit_plt_thunk(LM.get(), functype, attrs, cc, f_lib, f_name, libptrgv, llvmgv, symaddr, runtime_lib); + shadowgot = emit_plt_thunk(LM.get(), functype, attrs, cc, f_lib, f_name, libptrgv, llvmgv, runtime_lib); } else { // `runtime_sym_gvs` shouldn't have created anything in a new module diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp index 0ebdffb69e47b5..666b8afb2b9763 100644 --- a/src/jitlayers.cpp +++ b/src/jitlayers.cpp @@ -460,13 +460,13 @@ void JuliaOJIT::addModule(std::unique_ptr M) { #ifndef JL_NDEBUG // validate the relocations for M - for (Module::iterator I = M->begin(), E = M->end(); I != E; ) { - Function *F = &*I; + for (Module::global_object_iterator I = M->global_object_begin(), E = M->global_object_end(); I != E; ) { + GlobalObject *F = &*I; ++I; if (F->isDeclaration()) { if (F->use_empty()) F->eraseFromParent(); - else if (!(isIntrinsicFunction(F) || + else if (!((isa(F) && isIntrinsicFunction(cast(F))) || findUnmangledSymbol(F->getName()) || SectionMemoryManager::getSymbolAddressInProcess( getMangledName(F->getName())))) { @@ -495,9 +495,10 @@ void JuliaOJIT::addModule(std::unique_ptr M) #endif // Force LLVM to emit the module so that we can register the symbols // in our lookup table. - auto Err = CompileLayer.emitAndFinalize(key); + Error Err = CompileLayer.emitAndFinalize(key); // Check for errors to prevent LLVM from crashing the program. - assert(!Err); + if (Err) + report_fatal_error(std::move(Err)); } void JuliaOJIT::removeModule(ModuleHandleT H) @@ -770,12 +771,15 @@ static void jl_merge_recursive(Module *m, Module *collector) // since the declarations may get destroyed by the jl_merge_module call. // this is also why we copy the Name string, rather than save a StringRef SmallVector to_finalize; - for (Module::iterator I = m->begin(), E = m->end(); I != E; ++I) { - Function *F = &*I; + for (Module::global_object_iterator I = m->global_object_begin(), E = m->global_object_end(); I != E; ++I) { + GlobalObject *F = &*I; if (!F->isDeclaration()) { module_for_fname.erase(F->getName()); } - else if (!isIntrinsicFunction(F)) { + else if (isa(F) && !isIntrinsicFunction(cast(F))) { + to_finalize.push_back(F->getName().str()); + } + else if (isa(F) && module_for_fname.count(F->getName())) { to_finalize.push_back(F->getName().str()); } } @@ -838,11 +842,13 @@ void jl_finalize_module(Module *m, bool shadow) { // record the function names that are part of this Module // so it can be added to the JIT when needed - for (Module::iterator I = m->begin(), E = m->end(); I != E; ++I) { - Function *F = &*I; + for (Module::global_object_iterator I = m->global_object_begin(), E = m->global_object_end(); I != E; ++I) { + GlobalObject *F = &*I; if (!F->isDeclaration()) { - bool known = incomplete_fname.erase(F->getName()); - (void)known; // TODO: assert(known); // llvmcall gets this wrong + if (isa(F)) { + bool known = incomplete_fname.erase(F->getName()); + (void)known; // TODO: assert(known); // llvmcall gets this wrong + } module_for_fname[F->getName()] = m; } } diff --git a/test/ccall.jl b/test/ccall.jl index 6aa61a41590f50..39a8609878adab 100644 --- a/test/ccall.jl +++ b/test/ccall.jl @@ -1535,3 +1535,10 @@ let @test isa(ptr, Ptr{Cvoid}) @test arr[1] == '0' end + +# issue #34061 +o_file = tempname() +run(`$(Base.julia_cmd()) --output-o=$o_file -e 'Base.reinit_stdio(); + f() = ccall((:dne, :does_not_exist), Cvoid, ()); + precompile(f, ())'`) +@test isfile(o_file)