No longer require arguments to be pinned in GCChecker

src/clangsa/GCChecker.cpp

@@ -1,8 +1,6 @@
 // This file is a part of Julia. License is MIT: https://julialang.org/license
 
 // 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.
@@ -106,8 +104,14 @@ class GCChecker
  : (P == Moved) ? "Moved"
  : "Error");
  llvm::dbgs() << ",";
- if (S == Rooted)
- llvm::dbgs() << "(" << RootDepth << ")";
+ if (S == Rooted) {
+ llvm::dbgs() << "Root(";
+ if (Root) {
+ Root->dump();
+ llvm::dbgs() << ",";
+ }
+ llvm::dbgs() << RootDepth << ")";
+ }
  }
 
  bool operator==(const ValueState &VS) const {
@@ -169,6 +173,9 @@ class GCChecker
  } else if (parent.isPinned()) {
  // If parent is pinned, the child is not pinned.
  return getNotPinned(parent);
+ } else if (parent.isMoved()) {
+ // If parent is moved, the child is not pinned.
+ return getNotPinned(parent);
  } else {
  // For other cases, the children have the same state as the parent.
  return parent;
@@ -194,19 +201,20 @@ class GCChecker
  const ParmVarDecl *PVD) {
  bool isFunctionSafepoint = !isFDAnnotatedNotSafepoint(FD);
  bool maybeUnrooted = declHasAnnotation(PVD, "julia_maybe_unrooted");
- bool maybeUnpinned = declHasAnnotation(PVD, "julia_maybe_unpinned");
- if (!isFunctionSafepoint || maybeUnrooted || maybeUnpinned) {
+ if (!isFunctionSafepoint || maybeUnrooted) {
  ValueState VS = getAllocated();
  VS.PVD = PVD;
  VS.FD = FD;
  return VS;
  }
  bool require_tpin = declHasAnnotation(PVD, "julia_require_tpin");
+ bool require_pin = declHasAnnotation(PVD, "julia_require_pin");
  if (require_tpin) {
  return getRooted(nullptr, ValueState::TransitivelyPinned, -1);
- } else {
- // Assume arguments are pinned
+ } else if (require_pin) {
  return getRooted(nullptr, ValueState::Pinned, -1);
+ } else {
+ return getRooted(nullptr, ValueState::NotPinned, -1);
  }
  }
  };
@@ -339,6 +347,7 @@ class GCChecker
  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;
+ void validateValueRootnessOnly(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;
  template <typename T>
@@ -475,6 +484,15 @@ static const VarRegion *walk_back_to_global_VR(const MemRegion *Region) {
 #define FREED 1
 #define MOVED 2
 
+void GCChecker::validateValueRootnessOnly(const ValueState* VS, CheckerContext &C, SymbolRef Sym, const char *message, SourceRange range) const {
+ int v = validateValueInner(VS);
+ if (v == FREED) {
+ GCChecker::report_value_error(C, Sym, (std::string(message) + " GCed").c_str(), range);
+ } 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, SourceRange range) const {
  int v = validateValueInner(VS);
  if (v == FREED) {
@@ -1133,10 +1151,10 @@ bool GCChecker::processPotentialSafepoint(const CallEvent &Call,
  // Symbolically move all unpinned values.
  GCValueMapTy AMap2 = State->get<GCValueMap>();
  for (auto I = AMap2.begin(), E = AMap2.end(); I != E; ++I) {
+ logWithDump("- check Sym", I.getKey());
  if (RetSym == I.getKey())
  continue;
  if (I.getData().isNotPinned()) {
- logWithDump("- move unpinned values, Sym", I.getKey());
  logWithDump("- move unpinned values, VS", I.getData());
  auto NewVS = ValueState::getMoved(I.getData());
  State = State->set<GCValueMap>(I.getKey(), NewVS);
@@ -1189,9 +1207,9 @@ bool GCChecker::processArgumentRooting(const CallEvent &Call, CheckerContext &C,
  const ValueState *CurrentVState = State->get<GCValueMap>(RootedSymbol);
  ValueState NewVState = *OldVState;
  // If the old state is pinned, the new state is not pinned.
- if (OldVState->isPinned() && ((CurrentVState && !CurrentVState->isPinnedByAnyway()) || !CurrentVState)) {
- NewVState = ValueState::getNotPinned(*OldVState);
- }
+ // if (OldVState->isPinned() && ((CurrentVState && !CurrentVState->isPinnedByAnyway()) || !CurrentVState)) {
+ //  NewVState = ValueState::getNotPinned(*OldVState);
+ // }
  logWithDump("- Rooted set to", NewVState);
  State = State->set<GCValueMap>(RootedSymbol, NewVState);
  return true;
@@ -1627,23 +1645,16 @@ void GCChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
  range);
  }
  }
- if (ValState->isNotPinned()) {
- bool MaybeUnpinned = false;
- if (FD) {
- if (idx < FD->getNumParams()) {
- MaybeUnpinned =
- declHasAnnotation(FD->getParamDecl(idx), "julia_maybe_unpinned");
- }
- }
- if (!MaybeUnpinned && isCalleeSafepoint) {
- report_value_error(C, Sym, "Passing non-pinned value as argument to function that may GC", range);
- }
- }
  if (FD && idx < FD->getNumParams() && declHasAnnotation(FD->getParamDecl(idx), "julia_require_tpin")) {
  if (!ValState->isTransitivelyPinned()) {
  report_value_error(C, Sym, "Passing non-tpinned argument to function that requires a tpin argument.");
  }
  }
+ if (FD && idx < FD->getNumParams() && declHasAnnotation(FD->getParamDecl(idx), "julia_require_pin")) {
+ if (!ValState->isPinnedByAnyway()) {
+ report_value_error(C, Sym, "Passing non-pinned argument to function that requires a pin argument.");
+ }
+ }
  }
 }
 
@@ -1673,12 +1684,20 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
  PoppedRoots.push_back(I.getKey());
  State = State->remove<GCRootMap>(I.getKey());
  State = State->remove<GCPinMap>(I.getKey());
+ logWithDump("- pop root", I.getKey());
  }
  }
+ log("- Iterate value map");
  GCValueMapTy VMap = State->get<GCValueMap>();
  for (const MemRegion *R : PoppedRoots) {
+ logWithDump("-- check popped root", R);
  for (auto I = VMap.begin(), E = VMap.end(); I != E; ++I) {
+ logWithDump("--- check value", I.getKey());
+ logWithDump("--- check state", I.getData());
+ // FIXME: If this is a pop for TPin frame, we should remove TPin as well.
+ // For any region that is reachable from R, its pinning state should be reset.
  if (I.getData().isRootedBy(R)) {
+ logWithDump("--- no longer rooted", ValueState::getAllocated());
  State =
  State->set<GCValueMap>(I.getKey(), ValueState::getAllocated());
  }
@@ -1707,11 +1726,17 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
  return true;
  }
  const MemRegion *Region = MRV->getRegion();
- State = State->set<GCRootMap>(Region, RootState::getRoot(CurrentDepth));
+ RootState RS = RootState::getRoot(CurrentDepth);
+ State = State->set<GCRootMap>(Region, RS);
+ logWithDump("- JL_GC_PUSH, Region", Region);
+ logWithDump("- JL_GC_PUSH, RS", RS);
+ PinState PS = PinState::getNoPin(-1);
  if (tpin)
- State = State->set<GCPinMap>(Region, PinState::getTransitivePin(CurrentDepth));
+ PS = PinState::getTransitivePin(CurrentDepth);
  else
- State = State->set<GCPinMap>(Region, PinState::getPin(CurrentDepth));
+ PS = PinState::getPin(CurrentDepth);
+ State = State->set<GCPinMap>(Region, PS);
+ logWithDump("- JL_GC_PUSH, PS", PS);
  // Now for the value
  SVal Value = State->getSVal(Region);
  SymbolRef Sym = Value.getAsSymbol();
@@ -1730,6 +1755,8 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
  else
  VS = ValueState::getPinned(VS);
  State = State->set<GCValueMap>(Sym, VS);
+ logWithDump("- JL_GC_PUSH, Sym", Sym);
+ logWithDump("- JL_GC_PUSH, VS", VS);
  }
  CurrentDepth += 1;
  State = State->set<GCDepth>(CurrentDepth);
@@ -1788,6 +1815,7 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
  }
 
  const ValueState *OldVS = C.getState()->get<GCValueMap>(Sym);
+ logWithDump("- PTR_PIN OldVS", OldVS);
  if (OldVS && OldVS->isMoved()) {
  report_error(C, "Attempt to PIN a value that is already moved.");
  return true;

src/julia.h

@@ -915,7 +915,6 @@ extern void _JL_GC_PUSHARGS(jl_value_t **, size_t) JL_NOTSAFEPOINT;
 
 extern void JL_GC_POP() JL_NOTSAFEPOINT;
 
-#ifdef MMTK_GC
 extern void JL_GC_PUSH1_NO_TPIN(void *) JL_NOTSAFEPOINT;
 extern void JL_GC_PUSH2_NO_TPIN(void *, void *) JL_NOTSAFEPOINT;
 extern void JL_GC_PUSH3_NO_TPIN(void *, void *, void *) JL_NOTSAFEPOINT;
@@ -931,8 +930,6 @@ extern void _JL_GC_PUSHARGS_NO_TPIN(jl_value_t **, size_t) JL_NOTSAFEPOINT;
  memset(rts_var, 0, sizeof(void*) * (n)); \
  _JL_GC_PUSHARGS_NO_TPIN(rts_var, (n));
 
-#endif
-
 #else
 
 #define JL_GC_PUSH1(arg1) \
@@ -976,9 +973,6 @@ extern void _JL_GC_PUSHARGS_NO_TPIN(jl_value_t **, size_t) JL_NOTSAFEPOINT;
 
 #define JL_GC_POP() (jl_pgcstack = jl_pgcstack->prev)
 
-#endif
-
-#ifdef MMTK_GC
 // these are pinning roots: only the root object needs to be pinned as opposed to
 // the functions above which are transitively pinning
 #define JL_GC_PUSH1_NO_TPIN(arg1) \
@@ -1019,25 +1013,7 @@ extern void _JL_GC_PUSHARGS_NO_TPIN(jl_value_t **, size_t) JL_NOTSAFEPOINT;
  ((void**)rts_var)[-1] = jl_pgcstack; \
  memset((void*)rts_var, 0, (n)*sizeof(jl_value_t*)); \
  jl_pgcstack = (jl_gcframe_t*)&(((void**)rts_var)[-2])
-#else
-// When not using MMTk, default to the stock functions
-#define JL_GC_PUSH1_NO_TPIN(arg1) JL_GC_PUSH1(arg1)
-
-#define JL_GC_PUSH2_NO_TPIN(arg1, arg2) JL_GC_PUSH2(arg1, arg2)
-
-#define JL_GC_PUSH3_NO_TPIN(arg1, arg2, arg3) JL_GC_PUSH3(arg1, arg2, arg3)
-
-#define JL_GC_PUSH4_NO_TPIN(arg1, arg2, arg3, arg4) JL_GC_PUSH4(arg1, arg2, arg3, arg4)
-
-#define JL_GC_PUSH5_NO_TPIN(arg1, arg2, arg3, arg4, arg5) JL_GC_PUSH5(arg1, arg2, arg3, arg4, arg5)
-
-#define JL_GC_PUSH6_NO_TPIN(arg1, arg2, arg3, arg4, arg5, arg6) JL_GC_PUSH6(arg1, arg2, arg3, arg4, arg5, arg6)
-
-#define JL_GC_PUSH7_NO_TPIN(arg1, arg2, arg3, arg4, arg5, arg6, arg7) JL_GC_PUSH7(arg1, arg2, arg3, arg4, arg5, arg6, arg7)
-
-#define JL_GC_PUSH8_NO_TPIN(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8) JL_GC_PUSH8(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8)
 
-#define JL_GC_PUSHARGS_NO_TPIN(rts_var,n) JL_GC_PUSHARGS(rts_var,n)
 #endif
 
 JL_DLLEXPORT int jl_gc_enable(int on);
@@ -1871,12 +1847,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_MAYBE_UNPINNED,
- jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED);
+ jl_value_t *expected JL_MAYBE_UNROOTED,
+ jl_value_t *got JL_MAYBE_UNROOTED);
 JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname,
  const char *context,
- jl_value_t *ty JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED,
- jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED);
+ jl_value_t *ty JL_MAYBE_UNROOTED,
+ jl_value_t *got JL_MAYBE_UNROOTED);
 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);

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_MAYBE_UNPINNED,
- jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED)
+ jl_value_t *expected JL_MAYBE_UNROOTED,
+ jl_value_t *got JL_MAYBE_UNROOTED)
 {
  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_MAYBE_UNPINNED,
- jl_value_t *got JL_MAYBE_UNROOTED JL_MAYBE_UNPINNED)
+ jl_value_t *expected JL_MAYBE_UNROOTED,
+ jl_value_t *got JL_MAYBE_UNROOTED)
 {
  jl_type_error_rt(fname, "", expected, got);
 }

src/support/analyzer_annotations.h

@@ -13,7 +13,6 @@
 #define JL_PROPAGATES_ROOT __attribute__((annotate("julia_propagates_root")))
 #define JL_NOTSAFEPOINT __attribute__((annotate("julia_not_safepoint")))
 #define JL_MAYBE_UNROOTED __attribute__((annotate("julia_maybe_unrooted")))
-#define JL_MAYBE_UNPINNED __attribute__((annotate("julia_maybe_unpinned")))
 #define JL_GLOBALLY_ROOTED __attribute__((annotate("julia_globally_rooted")))
 #define JL_GLOBALLY_PINNED __attribute__((annotate("julia_globally_pinned")))
 #define JL_GLOBALLY_TPINNED __attribute__((annotate("julia_globally_tpinned")))
@@ -24,6 +23,7 @@
 #define JL_ROOTS_TEMPORARILY __attribute__((annotate("julia_temporarily_roots")))
 #define JL_REQUIRE_ROOTED_SLOT __attribute__((annotate("julia_require_rooted_slot")))
 #define JL_REQUIRE_TPIN __attribute__((annotate("julia_require_tpin")))
+#define JL_REQUIRE_PIN __attribute__((annotate("julia_require_pin")))
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -38,7 +38,6 @@ extern "C" {
 #define JL_PROPAGATES_ROOT
 #define JL_NOTSAFEPOINT
 #define JL_MAYBE_UNROOTED
-#define JL_MAYBE_UNPINNED
 // The runtime may mark any object that is reachable from a global root as globally rooted.
 // So JL_GLOBALLY_ROOTED does not need to an actual root. Thus we don't know anything
 // about pining state.
@@ -54,6 +53,7 @@ extern "C" {
 #define JL_ROOTS_TEMPORARILY
 #define JL_REQUIRE_ROOTED_SLOT
 #define JL_REQUIRE_TPIN
+#define JL_REQUIRE_PIN
 #define JL_GC_PROMISE_ROOTED(x) (void)(x)
 #define jl_may_leak(x) (void)(x)