From 34a7d7ebebc93bf9c4f166b0b523ceab844c7d91 Mon Sep 17 00:00:00 2001 From: stijn Date: Tue, 30 Apr 2019 11:05:57 +0200 Subject: [PATCH] unix/gcollect: Make sure stack/regs get captured properly for GC. When building with link time optimization enabled it is possible both gc_collect() and gc_collect_regs_and_stack() get inlined into gc_alloc() which can result in the regs variable being pushed on the stack earlier than some of the registers. Depending on the calling convention, those registers might however contain pointers to blocks which have just been allocated in the caller of gc_alloc(). Then those pointers end up higher on the stack than regs, aren't marked by gc_collect_root() and hence get sweeped, even though they're still in use. As reported in #4652 this happened for in 32-bit msvc release builds: mp_lexer_new() does two consecutive allocations and the latter triggered a gc_collect() which would sweep the memory of the first allocation again. --- ports/unix/gccollect.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ports/unix/gccollect.c b/ports/unix/gccollect.c index 02f6fc91a..ddc2d92c3 100644 --- a/ports/unix/gccollect.c +++ b/ports/unix/gccollect.c @@ -149,9 +149,14 @@ STATIC void gc_helper_get_regs(regs_t arr) { #endif // MICROPY_GCREGS_SETJMP // this function is used by mpthreadport.c -void gc_collect_regs_and_stack(void); +MP_NOINLINE void gc_collect_regs_and_stack(void); -void gc_collect_regs_and_stack(void) { +// Explicitly mark this as noinline to make sure the regs variable +// is effectively at the top of the stack: otherwise, in builds where +// LTO is enabled and a lot of inlining takes place we risk a stack +// layout where regs is lower on the stack than pointers which have +// just been allocated but not yet marked, and get incorrectly sweeped. +MP_NOINLINE void gc_collect_regs_and_stack(void) { regs_t regs; gc_helper_get_regs(regs); // GC stack (and regs because we captured them)