Skip to content

Commit 7090b23

Browse files
vtjnashJeffBezanson
authored andcommitted
simplify interpreter-stacktraces code (#34019)
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 #33877
1 parent 8bef999 commit 7090b23

14 files changed

+218
-610
lines changed

src/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ $(BUILDDIR)/gc-debug.o $(BUILDDIR)/gc-debug.dbg.obj: $(SRCDIR)/gc.h
222222
$(BUILDDIR)/gc-pages.o $(BUILDDIR)/gc-pages.dbg.obj: $(SRCDIR)/gc.h
223223
$(BUILDDIR)/gc.o $(BUILDDIR)/gc.dbg.obj: $(SRCDIR)/gc.h
224224
$(BUILDDIR)/init.o $(BUILDDIR)/init.dbg.obj: $(SRCDIR)/builtin_proto.h
225-
$(BUILDDIR)/interpreter.o $(BUILDDIR)/interpreter.dbg.obj: $(SRCDIR)/interpreter-stacktrace.c $(SRCDIR)/builtin_proto.h
225+
$(BUILDDIR)/interpreter.o $(BUILDDIR)/interpreter.dbg.obj: $(SRCDIR)/builtin_proto.h
226226
$(BUILDDIR)/jitlayers.o $(BUILDDIR)/jitlayers.dbg.obj: $(SRCDIR)/jitlayers.h $(SRCDIR)/codegen_shared.h
227227
$(BUILDDIR)/jltypes.o $(BUILDDIR)/jltypes.dbg.obj: $(SRCDIR)/builtin_proto.h
228228
$(BUILDDIR)/libllvmcalltest.$(SHLIB_EXT): $(SRCDIR)/codegen_shared.h

src/builtins.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,9 @@ static jl_value_t *do_apply(jl_value_t *F, jl_value_t **args, uint32_t nargs, jl
629629
}
630630
if (arg_heap) {
631631
// optimization: keep only the first root, free the others
632-
((void**)roots)[-2] = (void*)(((size_t)1) << 1);
632+
#ifndef __clang_analyzer__
633+
((void**)roots)[-2] = (void*)JL_GC_ENCODE_PUSHARGS(1);
634+
#endif
633635
}
634636
jl_value_t *result = jl_apply(newargs, n);
635637
JL_GC_POP();

src/gc.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ static void finalize_object(arraylist_t *list, jl_value_t *o,
341341
static void jl_gc_push_arraylist(jl_ptls_t ptls, arraylist_t *list)
342342
{
343343
void **items = list->items;
344-
items[0] = (void*)(((uintptr_t)list->len - 2) << 1);
344+
items[0] = (void*)JL_GC_ENCODE_PUSHARGS(list->len - 2);
345345
items[1] = ptls->pgcstack;
346346
ptls->pgcstack = (jl_gcframe_t*)items;
347347
}
@@ -2045,7 +2045,7 @@ stack: {
20452045
uintptr_t offset = stack->offset;
20462046
uintptr_t lb = stack->lb;
20472047
uintptr_t ub = stack->ub;
2048-
uint32_t nr = nroots >> 1;
2048+
uint32_t nr = nroots >> 2;
20492049
uintptr_t nptr = 0;
20502050
while (1) {
20512051
jl_value_t ***rts = (jl_value_t***)(((void**)s) + 2);
@@ -2087,7 +2087,7 @@ stack: {
20872087
uintptr_t new_nroots = gc_read_stack(&s->nroots, offset, lb, ub);
20882088
assert(new_nroots <= UINT32_MAX);
20892089
nroots = stack->nroots = (uint32_t)new_nroots;
2090-
nr = nroots >> 1;
2090+
nr = nroots >> 2;
20912091
continue;
20922092
}
20932093
goto pop;

0 commit comments

Comments
 (0)