hipe: Add assertion for gc in bif without wrapper

An easy source of tricky bugs is to start calling the garbage collector
from a built-in function without adding that bif to hipe_bif_list.m4.

With this change we, in the debug build, keep track of whether the
canonical stack and heap pointers are stored in the PCB or in
registers/stack, allowing us to catch this class of mistakes with an
assertion.

7 files changed, 69 insertions, 7 deletions

diff --git a/erts/emulator/hipe/hipe_amd64_asm.m4 b/erts/emulator/hipe/hipe_amd64_asm.m4
index 2c0fbbee2d..409fd0ef89 100644
--- a/erts/emulator/hipe/hipe_amd64_asm.m4
+++ b/erts/emulator/hipe/hipe_amd64_asm.m4
@@ -121,6 +121,22 @@ define(NSP,%rsp)dnl
 
 
 /*
+ * Debugging macros
+ *
+ * Keeps track of whether context has been saved in the debug build, allowing us
+ * to detect when the garbage collector is called when it shouldn't.
+ */
+`#ifdef DEBUG
+#  define SET_GC_UNSAFE			\
+	movq	$1, P_GCUNSAFE(P)
+#  define SET_GC_SAFE			\
+	movq	$0, P_GCUNSAFE(P)
+#else
+#  define SET_GC_UNSAFE
+#  define SET_GC_SAFE
+#endif'
+
+/*
  * Context switching macros.
  */
 `#define SWITCH_C_TO_ERLANG_QUICK	\
@@ -133,12 +149,14 @@ define(NSP,%rsp)dnl
 
 `#define SAVE_CACHED_STATE	\
 	SAVE_HP;		\
-	SAVE_FCALLS'
+	SAVE_FCALLS;		\
+	SET_GC_SAFE'
 
 `#define RESTORE_CACHED_STATE	\
 	RESTORE_HP;		\
 	RESTORE_HEAP_LIMIT;	\
-	RESTORE_FCALLS'
+	RESTORE_FCALLS;		\
+	SET_GC_UNSAFE'
 
 `#define SWITCH_C_TO_ERLANG	\
 	RESTORE_CACHED_STATE;	\
diff --git a/erts/emulator/hipe/hipe_amd64_glue.S b/erts/emulator/hipe/hipe_amd64_glue.S
index b37ed3c68a..f3404888d5 100644
--- a/erts/emulator/hipe/hipe_amd64_glue.S
+++ b/erts/emulator/hipe/hipe_amd64_glue.S
@@ -94,6 +94,7 @@ ASYM(nbif_return):
 .nosave_exit:
 	/* switch to C stack */
 	SWITCH_ERLANG_TO_C_QUICK
+	SET_GC_SAFE
 	/* restore C callee-save registers, drop frame, return */
 	movq	(%rsp), %rbp	# kills P
 	movq	8(%rsp), %rbx
@@ -398,6 +399,7 @@ nbif_4_simple_exception:
 	movl	%eax, P_NARITY(P)	# Note: narity is a 32-bit field
 	/* find and prepare to invoke the handler */
 	SWITCH_ERLANG_TO_C_QUICK	# The cached state is clean and need not be saved.
+	SET_GC_SAFE
 	movq	P, %rdi
 	call	CSYM(hipe_handle_exception)	# Note: hipe_handle_exception() conses
 	SWITCH_C_TO_ERLANG		# %rsp updated by hipe_find_handler()
diff --git a/erts/emulator/hipe/hipe_arm_asm.m4 b/erts/emulator/hipe/hipe_arm_asm.m4
index ae9ec752bb..68a6faa70b 100644
--- a/erts/emulator/hipe/hipe_arm_asm.m4
+++ b/erts/emulator/hipe/hipe_arm_asm.m4
@@ -48,6 +48,24 @@ define(NR_ARG_REGS,3)dnl admissible values are 0 to 6, inclusive
 `#define TEMP_LR	r8'
 
 /*
+ * Debugging macros
+ *
+ * Keeps track of whether context has been saved in the debug build, allowing us
+ * to detect when the garbage collector is called when it shouldn't.
+ */
+`#ifdef DEBUG
+#  define SET_GC_UNSAFE(SCRATCH)	\
+	mov	SCRATCH, #1;		\
+	str	SCRATCH, [P, #P_GCUNSAFE]
+#  define SET_GC_SAFE(SCRATCH)		\
+	mov	SCRATCH, #0;		\
+	str	SCRATCH, [P, #P_GCUNSAFE]
+#else
+#  define SET_GC_UNSAFE(SCRATCH)
+#  define SET_GC_SAFE(SCRATCH)
+#endif'
+
+/*
  * Context switching macros.
  *
  * RESTORE_CONTEXT and RESTORE_CONTEXT_QUICK do not affect
@@ -59,12 +77,14 @@ define(NR_ARG_REGS,3)dnl admissible values are 0 to 6, inclusive
 `#define RESTORE_CONTEXT_QUICK	\
 	mov	lr, TEMP_LR'
 
-`#define SAVE_CACHED_STATE	\
-	str	HP, [P, #P_HP];	\
-	str	NSP, [P, #P_NSP]'
+`#define SAVE_CACHED_STATE		\
+	str	HP, [P, #P_HP];		\
+	str	NSP, [P, #P_NSP];	\
+	SET_GC_SAFE(HP)'
 
-`#define RESTORE_CACHED_STATE	\
-	ldr	HP, [P, #P_HP];	\
+`#define RESTORE_CACHED_STATE		\
+	SET_GC_UNSAFE(HP);		\
+	ldr	HP, [P, #P_HP];		\
 	ldr	NSP, [P, #P_NSP]'
 
 `#define SAVE_CONTEXT_BIF	\
@@ -75,12 +95,14 @@ define(NR_ARG_REGS,3)dnl admissible values are 0 to 6, inclusive
 	ldr	HP, [P, #P_HP]'
 
 `#define SAVE_CONTEXT_GC	\
+	SET_GC_SAFE(TEMP_LR);	\
 	mov	TEMP_LR, lr;	\
 	str	lr, [P, #P_NRA];	\
 	str	NSP, [P, #P_NSP];	\
 	str	HP, [P, #P_HP]'
 
 `#define RESTORE_CONTEXT_GC	\
+	SET_GC_UNSAFE(HP);	\
 	ldr	HP, [P, #P_HP]'
 
 /*
diff --git a/erts/emulator/hipe/hipe_arm_glue.S b/erts/emulator/hipe/hipe_arm_glue.S
index 49ffa8b1d8..5b7f8ad52d 100644
--- a/erts/emulator/hipe/hipe_arm_glue.S
+++ b/erts/emulator/hipe/hipe_arm_glue.S
@@ -342,6 +342,7 @@ nbif_4_gc_after_bif:
 	str	r1, [P, #P_NARITY]
 	str	TEMP_LR, [P, #P_NRA]
 	str	NSP, [P, #P_NSP]
+	SET_GC_SAFE(TEMP_LR)
 	mov	TEMP_LR, lr
 	mov	r3, #0			/* Pass 0 in arity */
 	mov	r2, #0			/* Pass NULL in regs */
@@ -349,6 +350,7 @@ nbif_4_gc_after_bif:
 	mov	r0, P
 	bl	erts_gc_after_bif_call
 	mov	lr, TEMP_LR
+	SET_GC_UNSAFE(TEMP_LR)
 	ldr	TEMP_LR, [P, #P_NRA]
 	mov	r1, #0
 	str	r1, [P, #P_NARITY]
@@ -404,6 +406,7 @@ nbif_4_simple_exception:
 	str	NSP, [P, #P_NSP]
 	str	TEMP_LR, [P, #P_NRA]
 	str	r1, [P, #P_NARITY]
+	SET_GC_SAFE(r0)
 	/* find and prepare to invoke the handler */
 	mov	r0, P
 	bl	hipe_handle_exception	/* Note: hipe_handle_exception() conses */
@@ -423,6 +426,7 @@ nbif_4_simple_exception:
 	str	NSP, [P, #P_NSP]
 	str	r1, [P, #P_NARITY]
 	str	TEMP_LR, [P, #P_NRA]
+	SET_GC_SAFE(NSP)
 	b	.nosave_exit
 
 /*
diff --git a/erts/emulator/hipe/hipe_gc.c b/erts/emulator/hipe/hipe_gc.c
index d0619a0609..566c65882e 100644
--- a/erts/emulator/hipe/hipe_gc.c
+++ b/erts/emulator/hipe/hipe_gc.c
@@ -46,6 +46,8 @@ Eterm *fullsweep_nstack(Process *p, Eterm *n_htop)
     /* arch-specific nstack walk state */
     struct nstack_walk_state walk_state;
 
+    ASSERT(!p->hipe.gc_is_unsafe);
+
     if (!p->hipe.nstack) {
 	ASSERT(!p->hipe.nsp && !p->hipe.nstend);
 	return n_htop;
@@ -136,6 +138,8 @@ void gensweep_nstack(Process *p, Eterm **ptr_old_htop, Eterm **ptr_n_htop)
     char *mature;
     Uint mature_size;
 
+    ASSERT(!p->hipe.gc_is_unsafe);
+
     if (!p->hipe.nstack) {
 	ASSERT(!p->hipe.nsp && !p->hipe.nstend);
 	return;
diff --git a/erts/emulator/hipe/hipe_mkliterals.c b/erts/emulator/hipe/hipe_mkliterals.c
index 0d3493ec6c..b9d4226705 100644
--- a/erts/emulator/hipe/hipe_mkliterals.c
+++ b/erts/emulator/hipe/hipe_mkliterals.c
@@ -525,6 +525,12 @@ static const struct rts_param rts_params[] = {
     { 51, "P_CALLEE_EXP", 1, offsetof(struct process, hipe.u.callee_exp) },
 
     { 52, "THE_NON_VALUE", 1, (int)THE_NON_VALUE },
+
+    { 53, "P_GCUNSAFE",
+#ifdef DEBUG
+      1, offsetof(struct process, hipe.gc_is_unsafe)
+#endif
+    },
 };
 
 #define NR_PARAMS	ARRAY_SIZE(rts_params)
diff --git a/erts/emulator/hipe/hipe_process.h b/erts/emulator/hipe/hipe_process.h
index 21c4239753..a8d5972280 100644
--- a/erts/emulator/hipe/hipe_process.h
+++ b/erts/emulator/hipe/hipe_process.h
@@ -52,6 +52,9 @@ struct hipe_process_state {
 #if defined(ERTS_ENABLE_LOCK_CHECK) && defined(ERTS_SMP)
     void (*bif_callee)(void);   /* When calling BIF's via debug wrapper */
 #endif
+#ifdef DEBUG
+    UWord gc_is_unsafe;         /* Nonzero when GC-required state is on stack */
+#endif
 };
 
 extern void hipe_arch_print_pcb(struct hipe_process_state *p);
@@ -68,6 +71,9 @@ static __inline__ void hipe_init_process(struct hipe_process_state *p)
     p->nra = NULL;
 #endif
     p->narity = 0;
+#ifdef DEBUG
+    p->gc_is_unsafe = 0;
+#endif
 }
 
 static __inline__ void hipe_delete_process(struct hipe_process_state *p)