From df4eb8eee4490606883530f18601cbf60f17d32b Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Fri, 17 May 2024 00:56:54 +0000 Subject: [PATCH 1/7] Minor fix in GCChecker. Args for jl_type_error are unpinned. --- src/clangsa/GCChecker.cpp | 10 +++++++++- src/julia.h | 8 ++++---- src/rtutils.c | 8 ++++---- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp index e3ff6db0fbbb3..c4525145699c6 100644 --- a/src/clangsa/GCChecker.cpp +++ b/src/clangsa/GCChecker.cpp @@ -833,6 +833,7 @@ void GCChecker::checkBeginFunction(CheckerContext &C) const { const auto *FD = dyn_cast(LCtx->getDecl()); if (!FD) return; + logWithDump("checkBeginFunction", FD); ProgramStateRef State = C.getState(); bool Change = false; if (C.inTopFrame()) { @@ -880,6 +881,7 @@ void GCChecker::checkBeginFunction(CheckerContext &C) const { void GCChecker::checkEndFunction(const clang::ReturnStmt *RS, CheckerContext &C) const { + log("checkEndFunction"); ProgramStateRef State = C.getState(); if (RS && gcEnabledHere(C) && RS->getRetValue() && isGCTracked(RS->getRetValue())) { @@ -1153,7 +1155,7 @@ bool GCChecker::processArgumentRooting(const CallEvent &Call, CheckerContext &C, const ValueState *CurrentVState = State->get(RootedSymbol); ValueState NewVState = *OldVState; // If the old state is pinned, the new state is not pinned. - if (OldVState->isPinned() && ((CurrentVState && CurrentVState->isPinnedByAnyway()) || !CurrentVState)) { + if (OldVState->isPinned() && ((CurrentVState && !CurrentVState->isPinnedByAnyway()) || !CurrentVState)) { NewVState = ValueState::getNotPinned(*OldVState); } logWithDump("- Rooted set to", NewVState); @@ -1245,8 +1247,11 @@ bool GCChecker::processAllocationOfResult(const CallEvent &Call, void GCChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { logWithDump("checkPostCall", Call); ProgramStateRef State = C.getState(); + log("- processArgmentRooting"); bool didChange = processArgumentRooting(Call, C, State); + log("- processPotentialsafepoint"); didChange |= processPotentialSafepoint(Call, C, State); + log("- processAllocationOfResult"); didChange |= processAllocationOfResult(Call, C, State); if (didChange) C.addTransition(State); @@ -1255,6 +1260,7 @@ void GCChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { // Implicitly root values that were casted to globally rooted values void GCChecker::checkPostStmt(const CStyleCastExpr *CE, CheckerContext &C) const { + logWithDump("checkpostStmt(CStyleCastExpr)", CE); if (!isGloballyRootedType(CE->getTypeAsWritten())) return; SymbolRef Sym = C.getSVal(CE).getAsSymbol(); @@ -1481,6 +1487,7 @@ void GCChecker::checkPostStmt(const MemberExpr *ME, CheckerContext &C) const { void GCChecker::checkPostStmt(const UnaryOperator *UO, CheckerContext &C) const { + logWithDump("checkPostStmt(UnaryOperator)", UO); if (UO->getOpcode() == UO_Deref) { checkDerivingExpr(UO, UO->getSubExpr(), true, C); } @@ -1994,6 +2001,7 @@ bool GCChecker::rootRegionIfGlobal(const MemRegion *R, ProgramStateRef &State, void GCChecker::checkLocation(SVal SLoc, bool IsLoad, const Stmt *S, CheckerContext &C) const { + logWithDump("checkLocation", SLoc); ProgramStateRef State = C.getState(); bool DidChange = false; const RootState *RS = nullptr; diff --git a/src/julia.h b/src/julia.h index e2ceac942696a..444ef468b95a6 100644 --- a/src/julia.h +++ b/src/julia.h @@ -1871,12 +1871,12 @@ JL_DLLEXPORT void JL_NORETURN jl_exceptionf(jl_datatype_t *ty, JL_DLLEXPORT void JL_NORETURN jl_too_few_args(const char *fname, int min); JL_DLLEXPORT void JL_NORETURN jl_too_many_args(const char *fname, int max); JL_DLLEXPORT void JL_NORETURN jl_type_error(const char *fname, - jl_value_t *expected JL_MAYBE_UNROOTED, - jl_value_t *got JL_MAYBE_UNROOTED); + jl_value_t *expected JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED, + jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED); JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname, const char *context, - jl_value_t *ty JL_MAYBE_UNROOTED, - jl_value_t *got JL_MAYBE_UNROOTED); + jl_value_t *ty JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED, + jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED); JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var); JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_sym_t *type_name, jl_sym_t *var); JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str); diff --git a/src/rtutils.c b/src/rtutils.c index 66a0c9e2bdef1..2f63dc6e271d4 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -109,8 +109,8 @@ JL_DLLEXPORT void JL_NORETURN jl_too_many_args(const char *fname, int max) // with function name / location description, plus extra context JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname, const char *context, - jl_value_t *expected JL_MAYBE_UNROOTED, - jl_value_t *got JL_MAYBE_UNROOTED) + jl_value_t *expected JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED, + jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED) { jl_value_t *ctxt=NULL; JL_GC_PUSH3(&ctxt, &expected, &got); @@ -121,8 +121,8 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname, const char *co // with function name or description only JL_DLLEXPORT void JL_NORETURN jl_type_error(const char *fname, - jl_value_t *expected JL_MAYBE_UNROOTED, - jl_value_t *got JL_MAYBE_UNROOTED) + jl_value_t *expected JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED, + jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED) { jl_type_error_rt(fname, "", expected, got); } From 445f790e7406d0e532d5920b17795d8fcc010c8e Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Fri, 17 May 2024 03:35:35 +0000 Subject: [PATCH 2/7] Fix aotcompile.cpp --- src/aotcompile.cpp | 9 ++++++++- test/clangsa/MissingPinning.c | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/aotcompile.cpp b/src/aotcompile.cpp index 785398fa3ec33..9c56547821d07 100644 --- a/src/aotcompile.cpp +++ b/src/aotcompile.cpp @@ -240,8 +240,13 @@ static void jl_ci_cache_lookup(const jl_cgparams_t &cgparams, jl_method_instance jl_method_t *def = codeinst->def->def.method; if ((jl_value_t*)*src_out == jl_nothing) *src_out = NULL; - if (*src_out && jl_is_method(def)) + if (*src_out && jl_is_method(def)) { + PTR_PIN(def); + PTR_PIN(codeinst); *src_out = jl_uncompress_ir(def, codeinst, (jl_array_t*)*src_out); + PTR_UNPIN(codeinst); + PTR_UNPIN(def); + } } if (*src_out == NULL || !jl_is_code_info(*src_out)) { if (cgparams.lookup != jl_rettype_inferred) { @@ -341,7 +346,9 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm params.tsctx, params.imaging, clone.getModuleUnlocked()->getDataLayout(), Triple(clone.getModuleUnlocked()->getTargetTriple())); + PTR_PIN(codeinst->rettype); jl_llvm_functions_t decls = jl_emit_code(result_m, mi, src, codeinst->rettype, params); + PTR_UNPIN(codeinst->rettype); if (result_m) emitted[codeinst] = {std::move(result_m), std::move(decls)}; } diff --git a/test/clangsa/MissingPinning.c b/test/clangsa/MissingPinning.c index 354612c29e5a4..b119a3c08871a 100644 --- a/test/clangsa/MissingPinning.c +++ b/test/clangsa/MissingPinning.c @@ -6,6 +6,7 @@ #include "julia_internal.h" extern void look_at_value(jl_value_t *v); +extern void process_unrooted(jl_value_t *maybe_unrooted JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED); void unpinned_argument() { jl_svec_t *val = jl_svec1(NULL); // expected-note{{Started tracking value here}} @@ -14,6 +15,11 @@ void unpinned_argument() { // expected-note@-1{{Passing non-pinned value as argument to function that may GC}} } +int allow_unpinned() { + jl_svec_t *val = jl_svec1(NULL); + process_unrooted((jl_value_t*)val); +} + void pinned_argument() { jl_svec_t *val = jl_svec1(NULL); JL_GC_PROMISE_ROOTED(val); From 9145637b0e1102ab000e2d11892d552de442af3a Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Wed, 22 May 2024 05:03:08 +0000 Subject: [PATCH 3/7] Properly inherit pinning state for binding variables, jl_propagate_root and deriving expr. Raise an error if we attempt to pin a moved value. --- src/clangsa/GCChecker.cpp | 77 +++++++++++++++++++++++++++++------ test/clangsa/MissingPinning.c | 63 ++++++++++++++++++++++++++++ test/clangsa/MissingRoots.c | 24 +++++++++-- 3 files changed, 149 insertions(+), 15 deletions(-) diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp index c4525145699c6..8f0e52baef4ac 100644 --- a/src/clangsa/GCChecker.cpp +++ b/src/clangsa/GCChecker.cpp @@ -3,6 +3,10 @@ // Assumptions for pinning: // * args need to be pinned // * JL_ROOTING_ARGUMENT and JL_ROOTED_ARGUMENT will propagate pinning state as well. +// * The checker may not consider alias for derived pointers in some cases. +// * if f(x) returns a derived pointer from x, a = f(x); b = f(x); PTR_PIN(a); The checker will NOT find b as pinned. +// * a = x->y; b = x->y; PTR_PIN(a); The checker will find b as pinned. +// * Need to see if this affects correctness. #include "clang/Frontend/FrontendActions.h" #include "clang/StaticAnalyzer/Checkers/SValExplainer.h" @@ -96,6 +100,7 @@ class GCChecker llvm::dbgs() << ((P == TransitivelyPinned) ? "TransitivelyPinned" : (P == Pinned) ? "Pinned" : (P == NotPinned) ? "NotPinned" + : (P == Moved) ? "Moved" : "Error"); llvm::dbgs() << ","; if (S == Rooted) @@ -325,6 +330,7 @@ class GCChecker SymbolRef getSymbolForResult(const Expr *Result, const ValueState *OldValS, ProgramStateRef &State, CheckerContext &C) const; void validateValue(const GCChecker::ValueState* VS, CheckerContext &C, SymbolRef Sym, const char *message) const; + void validateValueRootnessOnly(const GCChecker::ValueState* VS, CheckerContext &C, SymbolRef Sym, const char *message) const; void validateValue(const GCChecker::ValueState* VS, CheckerContext &C, SymbolRef Sym, const char *message, SourceRange range) const; int validateValueInner(const GCChecker::ValueState* VS) const; GCChecker::ValueState getRootedFromRegion(const MemRegion *Region, const PinState *PS, int Depth) const; @@ -401,6 +407,7 @@ SymbolRef GCChecker::walkToRoot(callback f, const ProgramStateRef &State, const MemRegion *Region) { if (!Region) return nullptr; + logWithDump("- walkToRoot, Region", Region); while (true) { const SymbolicRegion *SR = Region->getSymbolicBase(); if (!SR) { @@ -408,6 +415,8 @@ SymbolRef GCChecker::walkToRoot(callback f, const ProgramStateRef &State, } SymbolRef Sym = SR->getSymbol(); const ValueState *OldVState = State->get(Sym); + logWithDump("- walkToRoot, Sym", Sym); + logWithDump("- walkToRoot, OldVState", OldVState); if (f(Sym, OldVState)) { if (const SymbolRegionValue *SRV = dyn_cast(Sym)) { Region = SRV->getRegion(); @@ -468,6 +477,15 @@ void GCChecker::validateValue(const ValueState* VS, CheckerContext &C, SymbolRef } } +void GCChecker::validateValueRootnessOnly(const ValueState* VS, CheckerContext &C, SymbolRef Sym, const char *message) const { + int v = validateValueInner(VS); + if (v == FREED) { + GCChecker::report_value_error(C, Sym, (std::string(message) + " GCed").c_str()); + } else if (v == MOVED) { + // We don't care if it is moved + } +} + void GCChecker::validateValue(const ValueState* VS, CheckerContext &C, SymbolRef Sym, const char *message) const { int v = validateValueInner(VS); if (v == FREED) { @@ -1106,7 +1124,11 @@ bool GCChecker::processPotentialSafepoint(const CallEvent &Call, if (RetSym == I.getKey()) continue; if (I.getData().isNotPinned()) { - State = State->set(I.getKey(), ValueState::getMoved(I.getData())); + logWithDump("- move unpinned values, Sym", I.getKey()); + logWithDump("- move unpinned values, VS", I.getData()); + auto NewVS = ValueState::getMoved(I.getData()); + State = State->set(I.getKey(), NewVS); + logWithDump("- move unpinned values, NewVS", NewVS); DidChange = true; } } @@ -1178,6 +1200,8 @@ bool GCChecker::processAllocationOfResult(const CallEvent &Call, Call.getOriginExpr(), C.getLocationContext(), QT, C.blockCount()); State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), S); Sym = S.getAsSymbol(); + logWithDump("- conjureSymbolVal, S", S); + logWithDump("- conjureSymbolVal, Sym", Sym); } if (isGloballyRootedType(QT)) State = State->set(Sym, ValueState::getRooted(nullptr, ValueState::pinState(isGloballyTransitivelyPinnedType(QT)), -1)); @@ -1231,8 +1255,11 @@ bool GCChecker::processAllocationOfResult(const CallEvent &Call, const MemRegion *Region = Test.getAsRegion(); const ValueState *OldVState = getValStateForRegion(C.getASTContext(), State, Region); - if (OldVState) - NewVState = *OldVState; + if (OldVState) { + NewVState = ValueState::inheritState(*OldVState); + logWithDump("- jl_propagates_root, OldVState", *OldVState); + logWithDump("- jl_propagates_root, NewVState", NewVState); + } break; } } @@ -1360,6 +1387,7 @@ void GCChecker::checkDerivingExpr(const Expr *Result, const Expr *Parent, SymbolRef OldSym = ParentVal.getAsSymbol(true); const MemRegion *Region = C.getSVal(Parent).getAsRegion(); const ValueState *OldValS = OldSym ? State->get(OldSym) : nullptr; + logWithDump("- Region", Region); logWithDump("- OldSym", OldSym); logWithDump("- OldValS", OldValS); SymbolRef NewSym = getSymbolForResult(Result, OldValS, State, C); @@ -1369,6 +1397,7 @@ void GCChecker::checkDerivingExpr(const Expr *Result, const Expr *Parent, logWithDump("- NewSym", NewSym); // NewSym might already have a better root const ValueState *NewValS = State->get(NewSym); + logWithDump("- NewValS", NewValS); if (Region) { const VarRegion *VR = Region->getAs(); bool inheritedState = false; @@ -1418,8 +1447,9 @@ void GCChecker::checkDerivingExpr(const Expr *Result, const Expr *Parent, validateValue(OldValS, C, OldSym, "Creating derivative of value that may have been"); if (!OldValS->isPotentiallyFreed() && ResultTracked) { logWithDump("- Set as OldValS, Sym", NewSym); - logWithDump("- Set as OldValS, VS", OldValS); - C.addTransition(State->set(NewSym, *OldValS)); + auto InheritVS = ValueState::inheritState(*OldValS); + logWithDump("- Set as OldValS, InheritVS", InheritVS); + C.addTransition(State->set(NewSym, InheritVS)); return; } } @@ -1739,6 +1769,13 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { report_error(C, "Can not understand this pin."); return true; } + + const ValueState *OldVS = C.getState()->get(Sym); + if (OldVS && OldVS->isMoved()) { + report_error(C, "Attempt to PIN a value that is already moved."); + return true; + } + auto MRV = Arg.getAs(); if (!MRV) { report_error(C, "PTR_PIN with something other than a local variable"); @@ -1747,7 +1784,6 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { const MemRegion *Region = MRV->getRegion(); auto State = C.getState()->set(Region, PinState::getPin(CurrentDepth)); logWithDump("- Pin region", Region); - const ValueState *OldVS = State->get(Sym); State = State->set(Sym, ValueState::getPinned(*OldVS)); logWithDump("- Pin value", Sym); C.addTransition(State); @@ -1899,8 +1935,11 @@ void GCChecker::checkBind(SVal LVal, SVal RVal, const clang::Stmt *S, const ValueState *ValSP = nullptr; ValueState ValS; if (rootRegionIfGlobal(R->getBaseRegion(), State, C, &ValS)) { + logWithDump("- rootRegionIfGlobal, base", R->getBaseRegion()); ValSP = &ValS; + logWithDump("- rootRegionIfGlobal ValSP", ValSP); } else { + logWithDump("- getValStateForRegion", R); ValSP = getValStateForRegion(C.getASTContext(), State, R); } if (ValSP && ValSP->isRooted()) { @@ -1910,8 +1949,9 @@ void GCChecker::checkBind(SVal LVal, SVal RVal, const clang::Stmt *S, RValState->RootDepth < ValSP->RootDepth) { logWithDump("- No need to set ValState, current ValState", RValState); } else { - logWithDump("- Set ValState, current ValState", ValSP); - C.addTransition(State->set(Sym, *ValSP)); + auto InheritVS = ValueState::inheritState(*ValSP); + logWithDump("- Set ValState, InheritVS", InheritVS); + C.addTransition(State->set(Sym, InheritVS)); } } } else { @@ -2008,42 +2048,55 @@ void GCChecker::checkLocation(SVal SLoc, bool IsLoad, const Stmt *S, // Loading from a root produces a rooted symbol. TODO: Can we do something // better than this. if (IsLoad && (RS = State->get(SLoc.getAsRegion()))) { + logWithDump("- IsLoad, RS", RS); SymbolRef LoadedSym = State->getSVal(SLoc.getAs().getValue()).getAsSymbol(); if (LoadedSym) { const ValueState *ValS = State->get(LoadedSym); + logWithDump("- IsLoad, LoadedSym", LoadedSym); + logWithDump("- IsLoad, ValS", ValS); if (!ValS || !ValS->isRooted() || ValS->RootDepth > RS->RootedAtDepth) { + auto NewVS = getRootedFromRegion(SLoc.getAsRegion(), State->get(SLoc.getAsRegion()), RS->RootedAtDepth); + logWithDump("- IsLoad, NewVS", NewVS); DidChange = true; - State = State->set( - LoadedSym, - getRootedFromRegion(SLoc.getAsRegion(), State->get(SLoc.getAsRegion()), RS->RootedAtDepth)); + State = State->set(LoadedSym, NewVS); } } } + logWithDump("- getAsRegion()", SLoc.getAsRegion()); // If it's just the symbol by itself, let it be. We allow dead pointer to be // passed around, so long as they're not accessed. However, we do want to // start tracking any globals that may have been accessed. if (rootRegionIfGlobal(SLoc.getAsRegion(), State, C)) { C.addTransition(State); + log("- rootRegionIfGlobal"); return; } SymbolRef SymByItself = SLoc.getAsSymbol(false); + logWithDump("- SymByItself", SymByItself); if (SymByItself) { DidChange &&C.addTransition(State); return; } // This will walk backwards until it finds the base symbol SymbolRef Sym = SLoc.getAsSymbol(true); + logWithDump("- Sym", Sym); if (!Sym) { DidChange &&C.addTransition(State); return; } const ValueState *VState = State->get(Sym); + logWithDump("- VState", VState); if (!VState) { DidChange &&C.addTransition(State); return; } - validateValue(VState, C, Sym, "Trying to access value which may have been"); + // If this is the sym, we verify both rootness and pinning. Otherwise, it may be the parent sym and we only care about the rootness. + if (SymByItself == Sym) { + validateValue(VState, C, Sym, "Trying to access value which may have been"); + } else { + validateValueRootnessOnly(VState, C, Sym, "Trying to access value which may have been"); + } DidChange &&C.addTransition(State); } diff --git a/test/clangsa/MissingPinning.c b/test/clangsa/MissingPinning.c index b119a3c08871a..c4961a5295df3 100644 --- a/test/clangsa/MissingPinning.c +++ b/test/clangsa/MissingPinning.c @@ -36,6 +36,15 @@ void missing_pin_before_safepoint() { // expected-note@-1{{Argument value may have been moved}} } +void pin_after_safepoint() { + jl_svec_t *val = jl_svec1(NULL); + JL_GC_PROMISE_ROOTED(val); + jl_gc_safepoint(); + PTR_PIN(val); // expected-warning{{Attempt to PIN a value that is already moved}} + // expected-note@-1{{Attempt to PIN a value that is already moved}} + look_at_value((jl_value_t*) val); +} + void proper_pin_before_safepoint() { jl_svec_t *val = jl_svec1(NULL); JL_GC_PROMISE_ROOTED(val); @@ -60,3 +69,57 @@ void push_no_tpin_value() { look_at_value((jl_value_t*) val); JL_GC_POP(); } + +void pointer_to_pointer(jl_value_t **v) { + // *v is not pinned. + look_at_value(*v); // expected-warning{{Passing non-pinned value as argument to function that may GC}} + // expected-note@-1{{Passing non-pinned value as argument to function that may GC}} + // expected-note@-2{{Started tracking value here}} +} + +void pointer_to_pointer2(jl_value_t* u, jl_value_t **v) { + *v = u; + look_at_value(*v); // expected-warning{{Passing non-pinned value as argument to function that may GC}} + // expected-note@-1{{Passing non-pinned value as argument to function that may GC}} + // expected-note@+1{{Started tracking value here (root was inherited)}} +} + +extern jl_value_t *first_array_elem(jl_array_t *a JL_PROPAGATES_ROOT); + +void root_propagation(jl_expr_t *expr) { + PTR_PIN(expr->args); + jl_value_t *val = first_array_elem(expr->args); // expected-note{{Started tracking value here}} + PTR_UNPIN(expr->args); + jl_gc_safepoint(); + look_at_value(val); // expected-warning{{Argument value may have been moved}} + // expected-note@-1{{Argument value may have been moved}} +} + +void derive_ptr_alias(jl_method_instance_t *mi) { + jl_value_t* a = mi->specTypes; + jl_value_t* b = mi->specTypes; + PTR_PIN(a); + look_at_value(b); + PTR_UNPIN(a); +} + +void derive_ptr_alias2(jl_method_instance_t *mi) { + PTR_PIN(mi->specTypes); + look_at_value(mi->specTypes); + PTR_UNPIN(mi->specTypes); +} + +// Ignore this case for now. The checker conjures new syms for function return values. +// It pins the first return value, but cannot see the second return value is an alias of the first. +// However, we could rewrite the code so the checker can check it. +// void mtable(jl_value_t *f) { +// PTR_PIN((jl_value_t*)jl_gf_mtable(f)); +// look_at_value((jl_value_t*)jl_gf_mtable(f)); +// } + +void mtable(jl_value_t *f) { + jl_value_t* mtable = (jl_value_t*)jl_gf_mtable(f); + PTR_PIN(mtable); + look_at_value(mtable); + PTR_UNPIN(mtable); +} diff --git a/test/clangsa/MissingRoots.c b/test/clangsa/MissingRoots.c index caca4b10a567c..87d234096bee4 100644 --- a/test/clangsa/MissingRoots.c +++ b/test/clangsa/MissingRoots.c @@ -181,10 +181,15 @@ void globally_rooted() { } extern jl_value_t *first_array_elem(jl_array_t *a JL_PROPAGATES_ROOT); + void root_propagation(jl_expr_t *expr) { + PTR_PIN(expr->args); jl_value_t *val = first_array_elem(expr->args); + PTR_UNPIN(expr->args); + PTR_PIN(val); jl_gc_safepoint(); look_at_value(val); + PTR_UNPIN(val); } void argument_propagation(jl_value_t *a) { @@ -201,7 +206,9 @@ void argument_propagation(jl_value_t *a) { void arg_array(jl_value_t **args) { jl_gc_safepoint(); jl_value_t *val = args[1]; + PTR_PIN(val); look_at_value(val); + PTR_UNPIN(val); jl_value_t *val2 = NULL; JL_GC_PUSH1(&val2); val2 = val; @@ -260,7 +267,9 @@ void nonconst_loads(jl_svec_t *v) size_t i = jl_svec_len(v); jl_method_instance_t **data = (jl_method_instance_t**)jl_svec_data(v); jl_method_instance_t *mi = data[i]; + PTR_PIN(mi->specTypes); look_at_value(mi->specTypes); + PTR_UNPIN(mi->specTypes); } void nonconst_loads2() @@ -278,10 +287,12 @@ static inline void look_at_value2(jl_value_t *v) { look_at_value(v); } void mtable(jl_value_t *f) { - look_at_value2((jl_value_t*)jl_gf_mtable(f)); + jl_value_t* mtable = (jl_value_t*)jl_gf_mtable(f); + PTR_PIN(mtable); + look_at_value2(mtable); jl_value_t *val = NULL; JL_GC_PUSH1(&val); - val = (jl_value_t*)jl_gf_mtable(f); + val = mtable; JL_GC_POP(); } @@ -293,7 +304,10 @@ void mtable2(jl_value_t **v) { } void tparam0(jl_value_t *atype) { - look_at_value(jl_tparam0(atype)); + jl_value_t *param0 = jl_tparam0(atype); + PTR_PIN(param0); + look_at_value(param0); + PTR_UNPIN(param0); } extern jl_value_t *global_atype JL_GLOBALLY_ROOTED JL_GLOBALLY_TPINNED; @@ -330,10 +344,14 @@ void module_member(jl_module_t *m) { for(int i=(int)m->usings.len-1; i >= 0; --i) { jl_module_t *imp = propagation(m); + PTR_PIN(imp); jl_gc_safepoint(); look_at_value((jl_value_t*)imp); jl_module_t *prop = propagation(imp); + PTR_PIN(prop); look_at_value((jl_value_t*)prop); + PTR_UNPIN(prop); + PTR_UNPIN(imp); JL_GC_PUSH1(&imp); jl_gc_safepoint(); look_at_value((jl_value_t*)imp); From 36423d5ae70bcb89275e2fc9d74368201a8aa250 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Thu, 23 May 2024 00:42:45 +0000 Subject: [PATCH 4/7] Add more test cases --- src/clangsa/GCChecker.cpp | 8 ++++++-- test/clangsa/MissingPinning.c | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp index 8f0e52baef4ac..f923e1f7e9add 100644 --- a/src/clangsa/GCChecker.cpp +++ b/src/clangsa/GCChecker.cpp @@ -7,6 +7,9 @@ // * if f(x) returns a derived pointer from x, a = f(x); b = f(x); PTR_PIN(a); The checker will NOT find b as pinned. // * a = x->y; b = x->y; PTR_PIN(a); The checker will find b as pinned. // * Need to see if this affects correctness. +// * The checker may report some vals as moved even if there is a new load for the val after safepoint. +// * f(x->a); jl_safepoint(); f(x->a); x->a is loaded after a safepoint, but the checker may report errors. This seems fine, as the compiler may hoist the load. +// * a = x->a; f(a); jl_safepoint(); f(a); a may be moved in a safepoint, and the checker will report errors. #include "clang/Frontend/FrontendActions.h" #include "clang/StaticAnalyzer/Checkers/SValExplainer.h" @@ -46,7 +49,7 @@ static const Stmt *getStmtForDiagnostics(const ExplodedNode *N) } // Turn on/off the log here -#define DEBUG_LOG 0 +#define DEBUG_LOG 1 class GCChecker : public Checker< @@ -235,7 +238,6 @@ class GCChecker : "Error"); llvm::dbgs() << ",Depth="; llvm::dbgs() << RootedAtDepth; - llvm::dbgs() << "\n"; } }; @@ -1928,6 +1930,7 @@ void GCChecker::checkBind(SVal LVal, SVal RVal, const clang::Stmt *S, log("- No Sym"); return; } + logWithDump("- Sym", Sym); const auto *RootState = State->get(R); logWithDump("- R", R); logWithDump("- RootState for R", RootState); @@ -1941,6 +1944,7 @@ void GCChecker::checkBind(SVal LVal, SVal RVal, const clang::Stmt *S, } else { logWithDump("- getValStateForRegion", R); ValSP = getValStateForRegion(C.getASTContext(), State, R); + logWithDump("- getValStateForRegion", ValSP); } if (ValSP && ValSP->isRooted()) { logWithDump("- Found base region that is rooted", ValSP); diff --git a/test/clangsa/MissingPinning.c b/test/clangsa/MissingPinning.c index c4961a5295df3..0bb94c39e8497 100644 --- a/test/clangsa/MissingPinning.c +++ b/test/clangsa/MissingPinning.c @@ -123,3 +123,24 @@ void mtable(jl_value_t *f) { look_at_value(mtable); PTR_UNPIN(mtable); } + +void pass_arg_to_non_safepoint(jl_tupletype_t *sigt) { + jl_value_t *ati = jl_tparam(sigt, 0); +} + +// Though the code loads the pointer after the safepoint, we don't know if the compiler would hoist the load before the safepoint. +// So it is fine that the checker reports this as an error. +void load_new_pointer_after_safepoint(jl_tupletype_t *t) { + jl_value_t *a0 = jl_svecref(((jl_datatype_t*)(t))->parameters, 0);//expected-note{{Started tracking value here}} + jl_safepoint(); + jl_value_t *a1 = jl_svecref(((jl_datatype_t*)(t))->parameters, 1);//expected-warning{{Argument value may have been moved}} + //expected-note@-1{{Argument value may have been moved}} +} + +void hoist_load_before_safepoint(jl_tupletype_t *t) { + jl_svec_t* params = ((jl_datatype_t*)(t))->parameters; //expected-note{{Started tracking value here}} + jl_value_t *a0 = jl_svecref(params, 0); + jl_safepoint(); + jl_value_t *a1 = jl_svecref(params, 1); //expected-warning{{Argument value may have been moved}} + //expected-note@-1{{Argument value may have been moved}} +} From 0609d9872d5b3ef04ad56c8896081f0332aec87a Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Mon, 27 May 2024 23:08:16 +0000 Subject: [PATCH 5/7] In a few cases, we also check if value state is pinned. When it is not, we try find ways to figure out if it is actually pinned. --- src/clangsa/GCChecker.cpp | 6 +++--- test/clangsa/MissingPinning.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp index f923e1f7e9add..a2cfcb1203a9f 100644 --- a/src/clangsa/GCChecker.cpp +++ b/src/clangsa/GCChecker.cpp @@ -49,7 +49,7 @@ static const Stmt *getStmtForDiagnostics(const ExplodedNode *N) } // Turn on/off the log here -#define DEBUG_LOG 1 +#define DEBUG_LOG 0 class GCChecker : public Checker< @@ -1980,7 +1980,7 @@ void GCChecker::checkBind(SVal LVal, SVal RVal, const clang::Stmt *S, } else { logWithDump("- Found ValState for Sym", RValState); validateValue(RValState, C, Sym, "Trying to root value which may have been"); - if (!RValState->isRooted() || + if (!RValState->isRooted() || !RValState->isPinnedByAnyway() || RValState->RootDepth > RootState->RootedAtDepth) { auto NewVS = getRootedFromRegion(R, State->get(R), RootState->RootedAtDepth); logWithDump("- getRootedFromRegion", NewVS); @@ -2059,7 +2059,7 @@ void GCChecker::checkLocation(SVal SLoc, bool IsLoad, const Stmt *S, const ValueState *ValS = State->get(LoadedSym); logWithDump("- IsLoad, LoadedSym", LoadedSym); logWithDump("- IsLoad, ValS", ValS); - if (!ValS || !ValS->isRooted() || ValS->RootDepth > RS->RootedAtDepth) { + if (!ValS || !ValS->isRooted() || !ValS->isPinnedByAnyway() || ValS->RootDepth > RS->RootedAtDepth) { auto NewVS = getRootedFromRegion(SLoc.getAsRegion(), State->get(SLoc.getAsRegion()), RS->RootedAtDepth); logWithDump("- IsLoad, NewVS", NewVS); DidChange = true; diff --git a/test/clangsa/MissingPinning.c b/test/clangsa/MissingPinning.c index 0bb94c39e8497..927a7bfdc0fa2 100644 --- a/test/clangsa/MissingPinning.c +++ b/test/clangsa/MissingPinning.c @@ -144,3 +144,38 @@ void hoist_load_before_safepoint(jl_tupletype_t *t) { jl_value_t *a1 = jl_svecref(params, 1); //expected-warning{{Argument value may have been moved}} //expected-note@-1{{Argument value may have been moved}} } + +// We tpin a local var, and later rebind a value to the local val. The value should be considered as pinned. +void rebind_tpin(jl_method_instance_t *mi, size_t world) { + jl_code_info_t *src = NULL; + JL_GC_PUSH1(&src); + jl_value_t *ci = jl_rettype_inferred(mi, world, world); + jl_code_instance_t *codeinst = (ci == jl_nothing ? NULL : (jl_code_instance_t*)ci); + if (codeinst) { + PTR_PIN(mi->def.method); + PTR_PIN(codeinst); + src = (jl_code_info_t*)jl_atomic_load_relaxed(&codeinst->inferred); + src = jl_uncompress_ir(mi->def.method, codeinst, (jl_array_t*)src); + PTR_UNPIN(codeinst); + PTR_UNPIN(mi->def.method); + } + JL_GC_POP(); +} + +void rebind_tpin_simple1() { + jl_value_t *t = NULL; + JL_GC_PUSH1(&t); + jl_svec_t *v = jl_svec1(NULL); + t = (jl_value_t*)v; + look_at_value(t); + JL_GC_POP(); +} + +void rebind_tpin_simple2() { + jl_value_t *t = NULL; + JL_GC_PUSH1(&t); + jl_svec_t *v = jl_svec1(NULL); + t = (jl_value_t*)v; + look_at_value(v); + JL_GC_POP(); +} From 38d5d3ab4d1ee566a42e6ab876d8657bacd55981 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 28 May 2024 00:53:15 +0000 Subject: [PATCH 6/7] Fix issues in jitlayers --- src/jitlayers.cpp | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp index 0040d6d52a449..68037063fa461 100644 --- a/src/jitlayers.cpp +++ b/src/jitlayers.cpp @@ -374,7 +374,9 @@ void jl_extern_c_impl(jl_value_t *declrt, jl_tupletype_t *sigt) if (!jl_is_tuple_type(sigt)) jl_type_error("@ccallable", (jl_value_t*)jl_anytuple_type_type, (jl_value_t*)sigt); // check that f is a guaranteed singleton type + PTR_PIN(((jl_datatype_t*)(sigt))->parameters); jl_datatype_t *ft = (jl_datatype_t*)jl_tparam0(sigt); + PTR_PIN(ft); if (!jl_is_datatype(ft) || ft->instance == NULL) jl_error("@ccallable: function object must be a singleton"); @@ -388,12 +390,18 @@ void jl_extern_c_impl(jl_value_t *declrt, jl_tupletype_t *sigt) size_t i, nargs = jl_nparams(sigt); for (i = 1; i < nargs; i++) { jl_value_t *ati = jl_tparam(sigt, i); + PTR_PIN(ati); if (!jl_is_concrete_type(ati) || jl_is_kind(ati) || !jl_type_mappable_to_c(ati)) jl_error("@ccallable: argument types must be concrete"); + PTR_UNPIN(ati); } + PTR_UNPIN(ft); + PTR_UNPIN(((jl_datatype_t*)(sigt))->parameters); // save a record of this so that the alias is generated when we write an object file + PTR_PIN(ft->name->mt); jl_method_t *meth = (jl_method_t*)jl_methtable_lookup(ft->name->mt, (jl_value_t*)sigt, jl_atomic_load_acquire(&jl_world_counter)); + PTR_UNPIN(ft->name->mt); if (!jl_is_method(meth)) jl_error("@ccallable: could not find requested method"); JL_GC_PUSH1(&meth); @@ -420,16 +428,20 @@ jl_code_instance_t *jl_generate_fptr_impl(jl_method_instance_t *mi JL_PROPAGATES compiler_start_time = jl_hrtime(); // if we don't have any decls already, try to generate it now jl_code_info_t *src = NULL; - JL_GC_PUSH1(&src); + jl_code_instance_t *codeinst = NULL; + JL_GC_PUSH2(&src, &codeinst); // There are many places below where we need to pin codeinst, and codeinst is assigned many times. We just T pin &codeinst to make things easier. JL_LOCK(&jl_codegen_lock); // also disables finalizers, to prevent any unexpected recursion jl_value_t *ci = jl_rettype_inferred(mi, world, world); - jl_code_instance_t *codeinst = (ci == jl_nothing ? NULL : (jl_code_instance_t*)ci); + codeinst = (ci == jl_nothing ? NULL : (jl_code_instance_t*)ci); if (codeinst) { src = (jl_code_info_t*)jl_atomic_load_relaxed(&codeinst->inferred); if ((jl_value_t*)src == jl_nothing) src = NULL; - else if (jl_is_method(mi->def.method)) + else if (jl_is_method(mi->def.method)) { + PTR_PIN(mi->def.method); src = jl_uncompress_ir(mi->def.method, codeinst, (jl_array_t*)src); + PTR_UNPIN(mi->def.method); + } } else { // identify whether this is an invalidated method that is being recompiled @@ -493,13 +505,16 @@ void jl_generate_fptr_for_unspecialized_impl(jl_code_instance_t *unspec) jl_code_info_t *src = NULL; JL_GC_PUSH1(&src); jl_method_t *def = unspec->def->def.method; + PTR_PIN(def); if (jl_is_method(def)) { src = (jl_code_info_t*)def->source; if (src == NULL) { // TODO: this is wrong assert(def->generator); // TODO: jl_code_for_staged can throw + PTR_PIN(unspec->def); src = jl_code_for_staged(unspec->def); + PTR_UNPIN(unspec->def); } if (src && (jl_value_t*)src != jl_nothing) src = jl_uncompress_ir(def, NULL, (jl_array_t*)src); @@ -507,6 +522,7 @@ void jl_generate_fptr_for_unspecialized_impl(jl_code_instance_t *unspec) else { src = (jl_code_info_t*)unspec->def->uninferred; } + PTR_UNPIN(def); assert(src && jl_is_code_info(src)); ++UnspecFPtrCount; _jl_compile_codeinst(unspec, src, unspec->min_world, *jl_ExecutionEngine->getContext()); @@ -531,6 +547,7 @@ jl_value_t *jl_dump_method_asm_impl(jl_method_instance_t *mi, size_t world, // printing via disassembly jl_code_instance_t *codeinst = jl_generate_fptr(mi, world); if (codeinst) { + PTR_PIN(codeinst); uintptr_t fptr = (uintptr_t)jl_atomic_load_acquire(&codeinst->invoke); if (getwrapper) return jl_dump_fptr_asm(fptr, raw_mc, asm_variant, debuginfo, binary); @@ -556,8 +573,11 @@ jl_value_t *jl_dump_method_asm_impl(jl_method_instance_t *mi, size_t world, // TODO: jl_code_for_staged can throw src = def->generator ? jl_code_for_staged(mi) : (jl_code_info_t*)def->source; } - if (src && (jl_value_t*)src != jl_nothing) + if (src && (jl_value_t*)src != jl_nothing) { + PTR_PIN(mi->def.method); src = jl_uncompress_ir(mi->def.method, codeinst, (jl_array_t*)src); + PTR_UNPIN(mi->def.method); + } } fptr = (uintptr_t)jl_atomic_load_acquire(&codeinst->invoke); specfptr = (uintptr_t)jl_atomic_load_relaxed(&codeinst->specptr.fptr); @@ -575,6 +595,7 @@ jl_value_t *jl_dump_method_asm_impl(jl_method_instance_t *mi, size_t world, jl_atomic_fetch_add_relaxed(&jl_cumulative_compile_time, end - compiler_start_time); } } + PTR_UNPIN(codeinst); if (specfptr != 0) return jl_dump_fptr_asm(specfptr, raw_mc, asm_variant, debuginfo, binary); } From 2148a9e76d4da97e4bb90878a1bd1a80fe882218 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 28 May 2024 05:29:09 +0000 Subject: [PATCH 7/7] Add note about when a value is moved. Fix aotcompile --- src/aotcompile.cpp | 56 ++++++++++++++++++++++++++++----------- src/clangsa/GCChecker.cpp | 2 ++ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/aotcompile.cpp b/src/aotcompile.cpp index 9c56547821d07..da6208afd3a34 100644 --- a/src/aotcompile.cpp +++ b/src/aotcompile.cpp @@ -243,7 +243,10 @@ static void jl_ci_cache_lookup(const jl_cgparams_t &cgparams, jl_method_instance if (*src_out && jl_is_method(def)) { PTR_PIN(def); PTR_PIN(codeinst); - *src_out = jl_uncompress_ir(def, codeinst, (jl_array_t*)*src_out); + PTR_PIN(*src_out); + auto temp = jl_uncompress_ir(def, codeinst, (jl_array_t*)*src_out); + PTR_UNPIN(*src_out); + *src_out = temp; PTR_UNPIN(codeinst); PTR_UNPIN(def); } @@ -255,7 +258,10 @@ static void jl_ci_cache_lookup(const jl_cgparams_t &cgparams, jl_method_instance else { *src_out = jl_type_infer(mi, world, 0); if (*src_out) { - codeinst = jl_get_method_inferred(mi, (*src_out)->rettype, (*src_out)->min_world, (*src_out)->max_world); + auto rettype = (*src_out)->rettype; + PTR_PIN(rettype); + codeinst = jl_get_method_inferred(mi, rettype, (*src_out)->min_world, (*src_out)->max_world); + PTR_UNPIN(rettype); if ((*src_out)->inferred) { jl_value_t *null = nullptr; jl_atomic_cmpswap_relaxed(&codeinst->inferred, &null, jl_nothing); @@ -326,11 +332,22 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm // to compile, or an svec(rettype, sig) describing a C-callable alias to create. jl_value_t *item = jl_array_ptr_ref(methods, i); if (jl_is_simplevector(item)) { - if (worlds == 1) - jl_compile_extern_c(wrap(&clone), ¶ms, NULL, jl_svecref(item, 0), jl_svecref(item, 1)); + if (worlds == 1) { + // warp is not a safepoint, but it is a function defined in LLVM. We cannot persuade GCChecker that item won't be moved. + PTR_PIN(item); + auto el0 = jl_svecref(item, 0); + auto el1 = jl_svecref(item, 1); + PTR_PIN(el0); + PTR_PIN(el1); + jl_compile_extern_c(wrap(&clone), ¶ms, NULL, el0, el1); + PTR_UNPIN(el1); + PTR_UNPIN(el0); + PTR_UNPIN(item); + } continue; } mi = (jl_method_instance_t*)item; + PTR_PIN(mi); src = NULL; // if this method is generally visible to the current compilation world, // and this is either the primary world, or not applicable in the primary world @@ -342,17 +359,18 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm if (src && !emitted.count(codeinst)) { // now add it to our compilation results JL_GC_PROMISE_ROOTED(codeinst->rettype); + PTR_PIN(codeinst->rettype); orc::ThreadSafeModule result_m = jl_create_ts_module(name_from_method_instance(codeinst->def), params.tsctx, params.imaging, clone.getModuleUnlocked()->getDataLayout(), Triple(clone.getModuleUnlocked()->getTargetTriple())); - PTR_PIN(codeinst->rettype); jl_llvm_functions_t decls = jl_emit_code(result_m, mi, src, codeinst->rettype, params); PTR_UNPIN(codeinst->rettype); if (result_m) emitted[codeinst] = {std::move(result_m), std::move(decls)}; } } + PTR_UNPIN(mi); } // finally, make sure all referenced methods also get compiled or fixed up @@ -1048,8 +1066,11 @@ void jl_add_optimization_passes_impl(LLVMPassManagerRef PM, int opt_level, int l extern "C" JL_DLLEXPORT void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, size_t world, char getwrapper, char optimize, const jl_cgparams_t params) { - if (jl_is_method(mi->def.method) && mi->def.method->source == NULL && - mi->def.method->generator == NULL) { + // Extract this as a new var, otherwise GCChecker won't work correctly. + auto method = mi->def.method; + PTR_PIN(method); + if (jl_is_method(method) && method->source == NULL && + method->generator == NULL) { // not a generic function dump->F = NULL; return; @@ -1059,27 +1080,29 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, siz jl_value_t *jlrettype = (jl_value_t*)jl_any_type; jl_code_info_t *src = NULL; JL_GC_PUSH2(&src, &jlrettype); - if (jl_is_method(mi->def.method) && mi->def.method->source != NULL && jl_ir_flag_inferred((jl_array_t*)mi->def.method->source)) { - src = (jl_code_info_t*)mi->def.method->source; + if (jl_is_method(method) && method->source != NULL && jl_ir_flag_inferred((jl_array_t*)method->source)) { + src = (jl_code_info_t*)method->source; if (src && !jl_is_code_info(src)) - src = jl_uncompress_ir(mi->def.method, NULL, (jl_array_t*)src); + src = jl_uncompress_ir(method, NULL, (jl_array_t*)src); } else { jl_value_t *ci = jl_rettype_inferred(mi, world, world); if (ci != jl_nothing) { jl_code_instance_t *codeinst = (jl_code_instance_t*)ci; + PTR_PIN(codeinst); src = (jl_code_info_t*)jl_atomic_load_relaxed(&codeinst->inferred); - if ((jl_value_t*)src != jl_nothing && !jl_is_code_info(src) && jl_is_method(mi->def.method)) - src = jl_uncompress_ir(mi->def.method, codeinst, (jl_array_t*)src); + if ((jl_value_t*)src != jl_nothing && !jl_is_code_info(src) && jl_is_method(method)) + src = jl_uncompress_ir(method, codeinst, (jl_array_t*)src); jlrettype = codeinst->rettype; + PTR_UNPIN(codeinst); } if (!src || (jl_value_t*)src == jl_nothing) { src = jl_type_infer(mi, world, 0); if (src) jlrettype = src->rettype; - else if (jl_is_method(mi->def.method)) { - src = mi->def.method->generator ? jl_code_for_staged(mi) : (jl_code_info_t*)mi->def.method->source; - if (src && !jl_is_code_info(src) && jl_is_method(mi->def.method)) - src = jl_uncompress_ir(mi->def.method, NULL, (jl_array_t*)src); + else if (jl_is_method(method)) { + src = method->generator ? jl_code_for_staged(mi) : (jl_code_info_t*)method->source; + if (src && !jl_is_code_info(src) && jl_is_method(method)) + src = jl_uncompress_ir(method, NULL, (jl_array_t*)src); } // TODO: use mi->uninferred } @@ -1131,6 +1154,7 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, siz fname = &decls.functionObject; F = cast(m.getModuleUnlocked()->getNamedValue(*fname)); } + PTR_UNPIN(method); JL_GC_POP(); if (measure_compile_time_enabled) { auto end = jl_hrtime(); diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp index a2cfcb1203a9f..c86de709472b7 100644 --- a/src/clangsa/GCChecker.cpp +++ b/src/clangsa/GCChecker.cpp @@ -737,6 +737,8 @@ PDP GCChecker::GCValueBugVisitor::VisitNode(const ExplodedNode *N, return MakePDP(Pos, "Root was released here."); } else if (NewValueState->RootDepth != OldValueState->RootDepth) { return MakePDP(Pos, "Rooting Depth changed here."); + } else if (NewValueState->isMoved() && !OldValueState->isMoved()) { + return MakePDP(Pos, "Value was moved here."); } return nullptr; }