Skip to content

Commit d4f3176

Browse files
committed
Make a missing GC root slightly more obvious to the checker
Finding the missing GC root is really hard for the checker. The correct error message is on the store to newargs, complaining that we are rooting a value that may have been GC'ed. However, `newargs` gets moved around and re-assigned all over that place in such a way that the analyzer has a hard time tracking it (I've seen it be successful if that function is cut down a bit, so it might also be a heuristics problem). By introducing a fake use that is easier for the analyzer to reason about, hopefully we can catch regressions to this particular (very hairy) part of the code in the future.
1 parent de324af commit d4f3176

File tree

3 files changed

+13
-0
lines changed

3 files changed

+13
-0
lines changed

src/builtins.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,10 +607,12 @@ JL_CALLABLE(jl_f__apply)
607607
jl_value_t *state = jl_fieldref(next, 1);
608608
roots[stackalloc] = state;
609609
_grow_to(&roots[0], &newargs, &arg_heap, &n_alloc, n + precount + 1, extra);
610+
JL_GC_ASSERT_LIVE(value);
610611
newargs[n++] = value;
611612
if (arg_heap)
612613
jl_gc_wb(arg_heap, value);
613614
roots[stackalloc + 1] = NULL;
615+
JL_GC_ASSERT_LIVE(state);
614616
args[1] = state;
615617
next = jl_apply_generic(jl_iterate_func, args, 2);
616618
}

src/julia_internal.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,16 @@ extern arraylist_t partial_inst;
10621062
#define JL_GCC_IGNORE_STOP
10631063
#endif // _COMPILER_GCC_
10641064

1065+
#ifdef __clang_analyzer__
1066+
// Not a safepoint (so it dosn't free other values), but an artificial use.
1067+
// Usually this is unnecessary because the analyzer can see all real uses,
1068+
// but sometimes real uses are harder for the analyzer to see, or it may
1069+
// give up before it sees it, so this can be helpful to be explicit.
1070+
void JL_GC_ASSERT_LIVE(jl_value_t *v) JL_NOTSAFEPOINT;
1071+
#else
1072+
#define JL_GC_ASSERT_LIVE(x) (void)(x)
1073+
#endif
1074+
10651075
#ifdef __cplusplus
10661076
}
10671077
#endif

src/support/analyzer_annotations.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,6 @@ extern "C" {
4545
#define JL_ROOTED_VALUE_COLLECTION
4646
#define JL_GC_PROMISE_ROOTED(x) (void)(x)
4747
#define jl_may_leak(x) (void)(x)
48+
#define JL_GC_ASSERT_LIVE(x)
4849

4950
#endif

0 commit comments

Comments
 (0)