Merge branch 'sverker/24/erts/fix-factory-undo/OTP-18027' into maint

This merge commit contains a partial revert of
a96f282359cd5f9f035029e99b0a5db360651a94
as we merge an alternative fix to the same problem.

7 files changed, 57 insertions, 60 deletions

diff --git a/erts/emulator/beam/emu/bif_instrs.tab b/erts/emulator/beam/emu/bif_instrs.tab
index eee74c3085..075ab583f2 100644
--- a/erts/emulator/beam/emu/bif_instrs.tab
+++ b/erts/emulator/beam/emu/bif_instrs.tab
@@ -260,7 +260,7 @@ call_light_bif(Bif, Exp) {
     ERTS_VERIFY_UNUSED_TEMP_ALLOC(c_p);
     ERTS_HOLE_CHECK(c_p);
     ERTS_REQ_PROC_MAIN_LOCK(c_p);
-    if (ERTS_IS_GC_DESIRED(c_p)) {
+    if (ERTS_IS_GC_AFTER_BIF_DESIRED(c_p)) {
         Uint arity = GET_EXPORT_ARITY(export);
         result = erts_gc_after_bif_call_lhf(c_p, live_hf_end, result,
                                             reg, arity);
@@ -361,7 +361,7 @@ call_light_bif_only(Bif, Exp) {
     ERTS_VERIFY_UNUSED_TEMP_ALLOC(c_p);
     ERTS_HOLE_CHECK(c_p);
     ERTS_REQ_PROC_MAIN_LOCK(c_p);
-    if (ERTS_IS_GC_DESIRED(c_p)) {
+    if (ERTS_IS_GC_AFTER_BIF_DESIRED(c_p)) {
         Uint arity = GET_EXPORT_ARITY(export);
         result = erts_gc_after_bif_call_lhf(c_p, live_hf_end, result,
                                             reg, arity);
@@ -593,7 +593,7 @@ nif_bif.epilogue() {
     //| -no_next
     ERTS_REQ_PROC_MAIN_LOCK(c_p);
     ERTS_HOLE_CHECK(c_p);
-    if (ERTS_IS_GC_DESIRED(c_p)) {
+    if (ERTS_IS_GC_AFTER_BIF_DESIRED(c_p)) {
         nif_bif_result = erts_gc_after_bif_call_lhf(c_p, live_hf_end,
                                                     nif_bif_result,
                                                     reg, bif_nif_arity);
diff --git a/erts/emulator/beam/emu/msg_instrs.tab b/erts/emulator/beam/emu/msg_instrs.tab
index be3c02493d..440dc45a6c 100644
--- a/erts/emulator/beam/emu/msg_instrs.tab
+++ b/erts/emulator/beam/emu/msg_instrs.tab
@@ -277,7 +277,7 @@ remove_message() {
     erts_save_message_in_proc(c_p, msgp);
     c_p->flags &= ~F_DELAY_GC;
 
-    if (ERTS_IS_GC_DESIRED_INTERNAL(c_p, HTOP, E)) {
+    if (ERTS_IS_GC_DESIRED_INTERNAL(c_p, HTOP, E, 0)) {
         /*
          * We want to GC soon but we leave a few
          * reductions giving the message some time
diff --git a/erts/emulator/beam/erl_gc.c b/erts/emulator/beam/erl_gc.c
index 2fcd0f2ef7..b8dd2e59d1 100644
--- a/erts/emulator/beam/erl_gc.c
+++ b/erts/emulator/beam/erl_gc.c
@@ -412,23 +412,28 @@ erts_gc_after_bif_call_lhf(Process* p, ErlHeapFragment *live_hf_end,
 {
     int cost;
 
-#ifdef DEBUG
-    if (live_hf_end != ERTS_INVALID_HFRAG_PTR) {
-        ErlHeapFragment *it = p->mbuf;
-
-        /* `live_hf_end` MUST be part of the heap fragment list. */
-        while (it != live_hf_end) {
-            ASSERT(it);
-            it = it->next;
-        }
+    if (!p->mbuf) {
+        /* Must have GC:d in BIF call... invalidate live_hf_end */
+        live_hf_end = ERTS_INVALID_HFRAG_PTR;
     }
-#endif
 
-    if (p->flags & F_HIBERNATE_SCHED) {
-	/*
-	 * We just hibernated. We do *not* want to mess
-	 * up the hibernation by an ordinary GC...
-	 */
+    if (p->flags & (F_HIBERNATE_SCHED | F_DISABLE_GC)) {
+
+        if ((p->flags & F_DISABLE_GC)
+            && p->live_hf_end == ERTS_INVALID_HFRAG_PTR
+            && is_non_value(result)
+            && p->freason == TRAP) {
+            /* This is first trap with disabled GC. Save live_hf_end marker. */
+            p->live_hf_end = live_hf_end;
+        }
+        /*else:
+         * a subsequent trap with disabled GC
+         *
+         * OR
+         *
+         * We just hibernated. We do *not* want to mess
+         * up the hibernation by an ordinary GC...
+         */
 	return result;
     }
 
@@ -438,11 +443,6 @@ erts_gc_after_bif_call_lhf(Process* p, ErlHeapFragment *live_hf_end,
 	erts_proc_unlock(p, ERTS_PROC_LOCK_MSGQ);
     }
     
-    if (!p->mbuf) {
-	/* Must have GC:d in BIF call... invalidate live_hf_end */
-	live_hf_end = ERTS_INVALID_HFRAG_PTR;
-    }
-
     if (is_non_value(result)) {
 	if (p->freason == TRAP) {
 	    cost = garbage_collect(p, live_hf_end, 0, regs, p->arity, p->fcalls, 0);
@@ -497,16 +497,6 @@ delay_garbage_collection(Process *p, ErlHeapFragment *live_hf_end, int need, int
 
     ERTS_HOLE_CHECK(p);
 
-    if ((p->flags & F_DISABLE_GC)
-	&& p->live_hf_end == ERTS_INVALID_HFRAG_PTR) {
-	/*
-	 * A BIF yielded with disabled GC. Remember
-	 * heap fragments created by the BIF until we
-	 * do next GC.
-	 */
-	p->live_hf_end = live_hf_end;
-    }
-
     if (need == 0) {
         if (p->flags & (F_DIRTY_MAJOR_GC|F_DIRTY_MINOR_GC)) {
             ASSERT(!ERTS_SCHEDULER_IS_DIRTY(erts_proc_sched_data(p)));
diff --git a/erts/emulator/beam/erl_gc.h b/erts/emulator/beam/erl_gc.h
index d1825d8b2e..65435edf7f 100644
--- a/erts/emulator/beam/erl_gc.h
+++ b/erts/emulator/beam/erl_gc.h
@@ -130,13 +130,19 @@ ERTS_GLB_INLINE Eterm follow_moved(Eterm term, Eterm xptr_tag)
  * Global exported
  */
 
-#define ERTS_IS_GC_DESIRED_INTERNAL(Proc, HTop, STop)			\
+#define ERTS_IS_GC_DESIRED_INTERNAL(Proc, HTop, STop, XtraFlags)	\
     ((((STop) - (HTop) < (Sint)(Proc)->mbuf_sz))                        \
      | ((Proc)->off_heap.overhead > (Proc)->bin_vheap_sz)		\
-     | !!((Proc)->flags & F_FORCE_GC))
+     | !!((Proc)->flags & (F_FORCE_GC|XtraFlags)))
 
 #define ERTS_IS_GC_DESIRED(Proc)					\
-    ERTS_IS_GC_DESIRED_INTERNAL((Proc), (Proc)->htop, (Proc)->stop)
+    ERTS_IS_GC_DESIRED_INTERNAL((Proc), (Proc)->htop, (Proc)->stop, 0)
+
+/* ERTS_IS_GC_AFTER_BIF_DESIRED also triggers for flag F_DISABLE_GC,
+ * not to actually do GC but we need to call erts_gc_after_bif_call_lhf
+ * for some bookkeeping of live_hf_end. */
+#define ERTS_IS_GC_AFTER_BIF_DESIRED(Proc)			        \
+    ERTS_IS_GC_DESIRED_INTERNAL((Proc), (Proc)->htop, (Proc)->stop, F_DISABLE_GC)
 
 #define ERTS_FORCE_GC_INTERNAL(Proc, FCalls)				\
     do {								\
diff --git a/erts/emulator/beam/erl_message.c b/erts/emulator/beam/erl_message.c
index 05d803168a..d183427165 100644
--- a/erts/emulator/beam/erl_message.c
+++ b/erts/emulator/beam/erl_message.c
@@ -1496,32 +1496,33 @@ void erts_factory_undo(ErtsHeapFactory* factory)
         }
 
         if (factory->mode == FACTORY_HALLOC) {
-            /* Reset all the heap fragments we've added. Note that we CANNOT
-             * free them, as someone else might have grabbed a reference to
-             * them (e.g. the callers of `erts_gc_after_bif_call_lhf`).
-             *
-             * The GC will get rid of these later on. Note that we leave
-             * `p->mbuf_sz` untouched to keep the memory pressure of these
-             * fragments. */
-            for (bp = (factory->p)->mbuf;
-                 bp != factory->heap_frags_saved;
-                 bp = bp->next) {
-                ASSERT(bp->off_heap.first == NULL);
-                bp->used_size = 0;
+            /* Free heap frags
+             */
+            bp = factory->p->mbuf;
+            if (bp != factory->heap_frags_saved) {
+                do {
+                    ErlHeapFragment *next_bp = bp->next;
+                    ASSERT(bp->off_heap.first == NULL);
+                    ERTS_HEAP_FREE(ERTS_ALC_T_HEAP_FRAG, (void *) bp,
+                                   ERTS_HEAP_FRAG_SIZE(bp->alloc_size));
+                    bp = next_bp;
+                } while (bp != factory->heap_frags_saved);
+
+                factory->p->mbuf = bp;
             }
 
-            /* Roll back the size of the latest fragment not allocated by us,
-             * as we may have used a part of it. */
-            if (bp != NULL) {
-                ASSERT(bp == factory->heap_frags_saved);
-                bp->used_size = factory->heap_frags_saved_used;
-            }
+            /* Rollback heap top
+	     */
 
-            /* Roll back heap top */
             ASSERT(HEAP_START(factory->p) <= factory->original_htop);
             ASSERT(factory->original_htop <= HEAP_LIMIT(factory->p));
             HEAP_TOP(factory->p) = factory->original_htop;
 
+	    /* Fix last heap frag */
+            if (factory->heap_frags_saved) {
+                ASSERT(factory->heap_frags_saved == factory->p->mbuf);
+                factory->heap_frags_saved->used_size = factory->heap_frags_saved_used;
+            }
             if (factory->message) {
                 ASSERT(factory->message->data.attached != ERTS_MSG_COMBINED_HFRAG);
                 ASSERT(!factory->message->data.heap_frag);
diff --git a/erts/emulator/beam/jit/beam_jit_common.c b/erts/emulator/beam/jit/beam_jit_common.c
index d2d4ad43d6..3e6e989924 100644
--- a/erts/emulator/beam/jit/beam_jit_common.c
+++ b/erts/emulator/beam/jit/beam_jit_common.c
@@ -76,7 +76,7 @@ Eterm beam_jit_call_bif(Process *c_p,
     PROCESS_MAIN_CHK_LOCKS(c_p);
     ERTS_REQ_PROC_MAIN_LOCK(c_p);
 
-    if (ERTS_IS_GC_DESIRED(c_p)) {
+    if (ERTS_IS_GC_AFTER_BIF_DESIRED(c_p)) {
         result = erts_gc_after_bif_call_lhf(c_p,
                                             live_hf_end,
                                             result,
@@ -132,7 +132,7 @@ Eterm beam_jit_call_nif(Process *c_p,
     ERTS_REQ_PROC_MAIN_LOCK(c_p);
     ERTS_HOLE_CHECK(c_p);
 
-    if (ERTS_IS_GC_DESIRED(c_p)) {
+    if (ERTS_IS_GC_AFTER_BIF_DESIRED(c_p)) {
         nif_bif_result = erts_gc_after_bif_call_lhf(c_p,
                                                     live_hf_end,
                                                     nif_bif_result,
@@ -670,7 +670,7 @@ Sint beam_jit_remove_message(Process *c_p,
     erts_save_message_in_proc(c_p, msgp);
     c_p->flags &= ~F_DELAY_GC;
 
-    if (ERTS_IS_GC_DESIRED_INTERNAL(c_p, HTOP, E)) {
+    if (ERTS_IS_GC_DESIRED_INTERNAL(c_p, HTOP, E, 0)) {
         /*
          * We want to GC soon but we leave a few
          * reductions giving the message some time
diff --git a/erts/emulator/beam/jit/x86/instr_bif.cpp b/erts/emulator/beam/jit/x86/instr_bif.cpp
index 41aedd30c7..bc390b0fa8 100644
--- a/erts/emulator/beam/jit/x86/instr_bif.cpp
+++ b/erts/emulator/beam/jit/x86/instr_bif.cpp
@@ -478,7 +478,7 @@ void BeamGlobalAssembler::emit_call_light_bif_shared() {
                   RET);
             a.mov(RETd, x86::dword_ptr(c_p, offsetof(Process, flags)));
             a.seta(x86::cl); /* Clobber ARG1 on windows and ARG4 on Linux */
-            a.and_(RETd, imm(F_FORCE_GC));
+            a.and_(RETd, imm(F_FORCE_GC | F_DISABLE_GC));
             a.or_(x86::cl, RETb);
             a.jne(gc_after_bif_call);