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

* sverker/23/erts/fix-factory-undo/OTP-18027:
  erts: Make erts_factory_undo leave empty heap fragment
  erts: Fix bug in erts_factory_proc_prealloc_init
  erts: Ensure setting p->live_hf_end at first trap with disabled GC

5 files changed, 47 insertions, 38 deletions

diff --git a/erts/emulator/beam/bif_instrs.tab b/erts/emulator/beam/bif_instrs.tab
index 7601b3d346..ab0fdc7489 100644
--- a/erts/emulator/beam/bif_instrs.tab
+++ b/erts/emulator/beam/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/erl_gc.c b/erts/emulator/beam/erl_gc.c
index 54ff1ffbdf..9012f64952 100644
--- a/erts/emulator/beam/erl_gc.c
+++ b/erts/emulator/beam/erl_gc.c
@@ -418,18 +418,35 @@ erts_gc_after_bif_call_lhf(Process* p, ErlHeapFragment *live_hf_end,
 {
     int cost;
 
-    if ((p->flags & F_HIBERNATE_SCHED) ||
+    if (!p->mbuf) {
+        /* Must have GC:d in BIF call... invalidate live_hf_end */
+        live_hf_end = ERTS_INVALID_HFRAG_PTR;
+    }
+
+    if ((p->flags & (F_HIBERNATE_SCHED | F_DISABLE_GC)) ||
         (p->sig_qs.flags & FS_HIPE_RECV_LOCKED)) {
-	/*
-	 * We just hibernated. We do *not* want to mess
-	 * up the hibernation by an ordinary 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...
          *
          * OR
          *
          * We left a receive in HiPE with message
          * queue lock locked, and we do not want to
          * do a GC with message queue locked...
-	 */
+         */
 	return result;
     }
 
@@ -439,11 +456,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) {
 #ifdef HIPE
@@ -501,16 +513,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 b9b1ed728c..868142d060 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) < (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 9cac1e2677..1b58e4b4dd 100644
--- a/erts/emulator/beam/erl_message.c
+++ b/erts/emulator/beam/erl_message.c
@@ -1187,6 +1187,10 @@ void erts_factory_proc_init(ErtsHeapFactory* factory, Process* p)
        heap as that completely destroys the DEBUG emulators
        performance. */
     ErlHeapFragment *bp = p->mbuf;
+
+    factory->heap_frags_saved = bp;
+    factory->heap_frags_saved_used = bp ? bp->used_size : 0;
+
     factory->mode     = FACTORY_HALLOC;
     factory->p        = p;
     factory->hp_start = HEAP_TOP(p);
@@ -1197,8 +1201,6 @@ void erts_factory_proc_init(ErtsHeapFactory* factory, Process* p)
     factory->message  = NULL;
     factory->off_heap_saved.first    = p->off_heap.first;
     factory->off_heap_saved.overhead = p->off_heap.overhead;
-    factory->heap_frags_saved = bp;
-    factory->heap_frags_saved_used = bp ? bp->used_size : 0;
     factory->heap_frags = NULL; /* not used */
     factory->alloc_type = 0; /* not used */
 
@@ -1210,6 +1212,13 @@ void erts_factory_proc_prealloc_init(ErtsHeapFactory* factory,
 				     Sint size)
 {
     ErlHeapFragment *bp = p->mbuf;
+
+    /* `bp->used_size` must be set _BEFORE_ we call `HAlloc`, as that will
+     * update the used size and prevent us from undoing the changes later
+     * on. */
+    factory->heap_frags_saved = bp;
+    factory->heap_frags_saved_used = bp ? bp->used_size : 0;
+
     factory->mode     = FACTORY_HALLOC;
     factory->p        = p;
     factory->original_htop = HEAP_TOP(p);
@@ -1224,8 +1233,6 @@ void erts_factory_proc_prealloc_init(ErtsHeapFactory* factory,
     factory->message  = NULL;
     factory->off_heap_saved.first    = p->off_heap.first;
     factory->off_heap_saved.overhead = p->off_heap.overhead;
-    factory->heap_frags_saved = bp;
-    factory->heap_frags_saved_used = bp ? bp->used_size : 0;
     factory->heap_frags = NULL; /* not used */
     factory->alloc_type = 0; /* not used */
 }
@@ -1629,13 +1636,7 @@ void erts_factory_undo(ErtsHeapFactory* factory)
 	    /* Fix last heap frag */
             if (factory->heap_frags_saved) {
                 ASSERT(factory->heap_frags_saved == factory->p->mbuf);
-                if (factory->hp_start != factory->heap_frags_saved->mem)
-                    factory->heap_frags_saved->used_size = factory->heap_frags_saved_used;
-		else {
-                    factory->p->mbuf = factory->p->mbuf->next;
-                    ERTS_HEAP_FREE(ERTS_ALC_T_HEAP_FRAG, factory->heap_frags_saved,
-                                   ERTS_HEAP_FRAG_SIZE(factory->heap_frags_saved->alloc_size));
-                }
+                factory->heap_frags_saved->used_size = factory->heap_frags_saved_used;
             }
             if (factory->message) {
                 ASSERT(factory->message->data.attached != ERTS_MSG_COMBINED_HFRAG);
diff --git a/erts/emulator/beam/msg_instrs.tab b/erts/emulator/beam/msg_instrs.tab
index ef2a83d71d..d67b1dde64 100644
--- a/erts/emulator/beam/msg_instrs.tab
+++ b/erts/emulator/beam/msg_instrs.tab
@@ -254,7 +254,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