diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp index 37521b11dd9f3..be564a80215e9 100644 --- a/src/clangsa/GCChecker.cpp +++ b/src/clangsa/GCChecker.cpp @@ -682,6 +682,16 @@ void GCChecker::checkBeginFunction(CheckerContext &C) const { void GCChecker::checkEndFunction(const clang::ReturnStmt *RS, CheckerContext &C) const { ProgramStateRef State = C.getState(); + + if (RS && gcEnabledHere(C) && RS->getRetValue() && isGCTracked(RS->getRetValue())) { + auto ResultVal = C.getSVal(RS->getRetValue()); + SymbolRef Sym = ResultVal.getAsSymbol(true); + const ValueState *ValS = Sym ? State->get(Sym) : nullptr; + if (ValS && ValS->isPotentiallyFreed()) { + report_value_error(C, Sym, "Return value may have been GCed", RS->getSourceRange()); + } + } + bool Changed = false; if (State->get() == C.getStackFrame()->getIndex()) { State = State->set((unsigned)-1); @@ -695,10 +705,7 @@ void GCChecker::checkEndFunction(const clang::ReturnStmt *RS, C.addTransition(State); if (!C.inTopFrame()) return; - // If `RS` is NULL, the function did not return normally. - // This could be either an abort/assertion failure or an exception throw. - // Do not require the GC frame to match in such case. - if (RS && C.getState()->get() > 0) + if (C.getState()->get() > 0) report_error(C, "Non-popped GC frame present at end of function"); } @@ -867,12 +874,17 @@ bool GCChecker::processPotentialSafepoint(const CallEvent &Call, } } + // Don't free the return value + SymbolRef RetSym = Call.getReturnValue().getAsSymbol(); + // Symbolically free all unrooted values. GCValueMapTy AMap = State->get(); for (auto I = AMap.begin(), E = AMap.end(); I != E; ++I) { if (I.getData().isJustAllocated()) { if (SpeciallyRootedSymbol == I.getKey()) continue; + if (RetSym == I.getKey()) + continue; State = State->set(I.getKey(), ValueState::getFreed()); DidChange = true; } @@ -941,17 +953,7 @@ bool GCChecker::processAllocationOfResult(const CallEvent &Call, State = State->set(Sym, ValueState::getRooted(nullptr, -1)); else { const ValueState *ValS = State->get(Sym); - ValueState NewVState = ValueState::getAllocated(); - if (ValS) { - // If the call was inlined, we may have accidentally killed the return - // value above. Revive it here. - const ValueState *PrevValState = C.getState()->get(Sym); - if (!ValS->isPotentiallyFreed() || - (PrevValState && PrevValState->isPotentiallyFreed())) { - return false; - } - NewVState = *PrevValState; - } + ValueState NewVState = ValS ? *ValS : ValueState::getAllocated(); auto *Decl = Call.getDecl(); const FunctionDecl *FD = Decl ? Decl->getAsFunction() : nullptr; if (FD) { diff --git a/src/disasm.cpp b/src/disasm.cpp index fef1ae0a06f33..086a1deae5a17 100644 --- a/src/disasm.cpp +++ b/src/disasm.cpp @@ -424,35 +424,37 @@ jl_value_t *jl_dump_function_ir(void *f, char strip_ir_metadata, char dump_modul std::string code; raw_string_ostream stream(code); - Function *llvmf = dyn_cast_or_null((Function*)f); - if (!llvmf || (!llvmf->isDeclaration() && !llvmf->getParent())) - jl_error("jl_dump_function_ir: Expected Function* in a temporary Module"); - - JL_LOCK(&codegen_lock); // Might GC - LineNumberAnnotatedWriter AAW{debuginfo}; - if (!llvmf->getParent()) { - // print the function declaration as-is - llvmf->print(stream, &AAW); - delete llvmf; - } - else { - Module *m = llvmf->getParent(); - if (strip_ir_metadata) { - std::string llvmfn(llvmf->getName()); - jl_strip_llvm_addrspaces(m); - jl_strip_llvm_debug(m, true, &AAW); - // rewriting the function type creates a new function, so look it up again - llvmf = m->getFunction(llvmfn); - } - if (dump_module) { - m->print(stream, &AAW); + { + Function *llvmf = dyn_cast_or_null((Function*)f); + if (!llvmf || (!llvmf->isDeclaration() && !llvmf->getParent())) + jl_error("jl_dump_function_ir: Expected Function* in a temporary Module"); + + JL_LOCK(&codegen_lock); // Might GC + LineNumberAnnotatedWriter AAW{debuginfo}; + if (!llvmf->getParent()) { + // print the function declaration as-is + llvmf->print(stream, &AAW); + delete llvmf; } else { - llvmf->print(stream, &AAW); + Module *m = llvmf->getParent(); + if (strip_ir_metadata) { + std::string llvmfn(llvmf->getName()); + jl_strip_llvm_addrspaces(m); + jl_strip_llvm_debug(m, true, &AAW); + // rewriting the function type creates a new function, so look it up again + llvmf = m->getFunction(llvmfn); + } + if (dump_module) { + m->print(stream, &AAW); + } + else { + llvmf->print(stream, &AAW); + } + delete m; } - delete m; + JL_UNLOCK(&codegen_lock); // Might GC } - JL_UNLOCK(&codegen_lock); // Might GC return jl_pchar_to_string(stream.str().data(), stream.str().size()); } diff --git a/src/gf.c b/src/gf.c index 1557ab93d9135..cec1b32910721 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1170,7 +1170,7 @@ static jl_method_instance_t *cache_method( static jl_method_match_t *_gf_invoke_lookup(jl_value_t *types JL_PROPAGATES_ROOT, size_t world, size_t *min_valid, size_t *max_valid); -static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype_t *tt, size_t world) +static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt JL_PROPAGATES_ROOT, jl_datatype_t *tt, size_t world) { // caller must hold the mt->writelock assert(tt->isdispatchtuple || tt->hasfreetypevars); @@ -1811,8 +1811,8 @@ jl_method_instance_t *jl_method_lookup(jl_value_t **args, size_t nargs, size_t w JL_GC_PUSH1(&tt); JL_LOCK(&mt->writelock); jl_method_instance_t *sf = jl_mt_assoc_by_type(mt, tt, world); - JL_GC_POP(); JL_UNLOCK(&mt->writelock); + JL_GC_POP(); return sf; } @@ -2342,8 +2342,8 @@ STATIC_INLINE jl_method_instance_t *jl_lookup_generic_(jl_value_t *F, jl_value_t // cache miss case JL_TIMING(METHOD_LOOKUP_SLOW); mfunc = jl_mt_assoc_by_type(mt, tt, world); - JL_GC_POP(); JL_UNLOCK(&mt->writelock); + JL_GC_POP(); if (jl_options.malloc_log) jl_gc_sync_total_bytes(last_alloc); // discard allocation count from compilation if (mfunc == NULL) { diff --git a/test/clangsa/MissingRoots.c b/test/clangsa/MissingRoots.c index 1f45d5f735864..10c28e6bdbe9e 100644 --- a/test/clangsa/MissingRoots.c +++ b/test/clangsa/MissingRoots.c @@ -239,18 +239,18 @@ void pushargs_as_args() JL_GC_POP(); } -static jl_typemap_entry_t *call_cache[10] JL_GLOBALLY_ROOTED; +static jl_typemap_entry_t *this_call_cache[10] JL_GLOBALLY_ROOTED; void global_array2() { jl_value_t *val = NULL; JL_GC_PUSH1(&val); - val = (jl_value_t*)call_cache[1]->func.linfo; + val = (jl_value_t*)this_call_cache[1]->func.linfo; JL_GC_POP(); } void global_array3() { jl_value_t *val = NULL; jl_typemap_entry_t *tm = NULL; - tm = call_cache[1]; + tm = this_call_cache[1]; val = (jl_value_t*)tm->func.linfo; look_at_value(val); } @@ -416,3 +416,11 @@ void JL_NORETURN throw_internal(jl_value_t *e JL_MAYBE_UNROOTED) jl_gc_unsafe_enter(ptls); look_at_value(e); } + +JL_DLLEXPORT jl_value_t *jl_totally_used_function(int i) +{ + jl_value_t *v = jl_box_int32(i); // expected-note{{Started tracking value here}} + jl_safepoint(); // expected-note{{Value may have been GCed here}} + return v; // expected-warning{{Return value may have been GCed}} + // expected-note@-1{{Return value may have been GCed}} +}