Skip to content

Commit

Permalink
simplify interpreter-stacktraces code (JuliaLang#34019)
Browse files Browse the repository at this point in the history
The unwinder and debuggers thought that our assembly code here was not
quite correct. Rather than attempt to fix that, let the compiler
generate it so we don't need to maintain it anymore.

This was previously also not particularly optimal for an interpreter to
need a couple extra function calls (by indirect pointer too) to setup
the call frame, so now we avoid that.

This simplifies the design by adding a new flag bit to the existing
pgcstack frames. In the future, we may end up generalizing this support
to handle stack allocation of arbitrary objects, but for now we
implement just enough support for our current needs.

It's unclear why dbghelp StackWalk glitches here a couple times (it is
reporting the stack pointer instead of the instruction pointer as the
return address), but this design is robust against that now (even though
I've manually verified that that particular glitch still happens with
this patch).

fix JuliaLang#33877
  • Loading branch information
vtjnash authored and JeffBezanson committed Dec 12, 2019
1 parent 8bef999 commit 7090b23
Show file tree
Hide file tree
Showing 14 changed files with 218 additions and 610 deletions.
2 changes: 1 addition & 1 deletion src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ $(BUILDDIR)/gc-debug.o $(BUILDDIR)/gc-debug.dbg.obj: $(SRCDIR)/gc.h
$(BUILDDIR)/gc-pages.o $(BUILDDIR)/gc-pages.dbg.obj: $(SRCDIR)/gc.h
$(BUILDDIR)/gc.o $(BUILDDIR)/gc.dbg.obj: $(SRCDIR)/gc.h
$(BUILDDIR)/init.o $(BUILDDIR)/init.dbg.obj: $(SRCDIR)/builtin_proto.h
$(BUILDDIR)/interpreter.o $(BUILDDIR)/interpreter.dbg.obj: $(SRCDIR)/interpreter-stacktrace.c $(SRCDIR)/builtin_proto.h
$(BUILDDIR)/interpreter.o $(BUILDDIR)/interpreter.dbg.obj: $(SRCDIR)/builtin_proto.h
$(BUILDDIR)/jitlayers.o $(BUILDDIR)/jitlayers.dbg.obj: $(SRCDIR)/jitlayers.h $(SRCDIR)/codegen_shared.h
$(BUILDDIR)/jltypes.o $(BUILDDIR)/jltypes.dbg.obj: $(SRCDIR)/builtin_proto.h
$(BUILDDIR)/libllvmcalltest.$(SHLIB_EXT): $(SRCDIR)/codegen_shared.h
Expand Down
4 changes: 3 additions & 1 deletion src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,9 @@ static jl_value_t *do_apply(jl_value_t *F, jl_value_t **args, uint32_t nargs, jl
}
if (arg_heap) {
// optimization: keep only the first root, free the others
((void**)roots)[-2] = (void*)(((size_t)1) << 1);
#ifndef __clang_analyzer__
((void**)roots)[-2] = (void*)JL_GC_ENCODE_PUSHARGS(1);
#endif
}
jl_value_t *result = jl_apply(newargs, n);
JL_GC_POP();
Expand Down
6 changes: 3 additions & 3 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ static void finalize_object(arraylist_t *list, jl_value_t *o,
static void jl_gc_push_arraylist(jl_ptls_t ptls, arraylist_t *list)
{
void **items = list->items;
items[0] = (void*)(((uintptr_t)list->len - 2) << 1);
items[0] = (void*)JL_GC_ENCODE_PUSHARGS(list->len - 2);
items[1] = ptls->pgcstack;
ptls->pgcstack = (jl_gcframe_t*)items;
}
Expand Down Expand Up @@ -2045,7 +2045,7 @@ stack: {
uintptr_t offset = stack->offset;
uintptr_t lb = stack->lb;
uintptr_t ub = stack->ub;
uint32_t nr = nroots >> 1;
uint32_t nr = nroots >> 2;
uintptr_t nptr = 0;
while (1) {
jl_value_t ***rts = (jl_value_t***)(((void**)s) + 2);
Expand Down Expand Up @@ -2087,7 +2087,7 @@ stack: {
uintptr_t new_nroots = gc_read_stack(&s->nroots, offset, lb, ub);
assert(new_nroots <= UINT32_MAX);
nroots = stack->nroots = (uint32_t)new_nroots;
nr = nroots >> 1;
nr = nroots >> 2;
continue;
}
goto pop;
Expand Down
Loading

0 comments on commit 7090b23

Please sign in to comment.