https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120464

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |16.0
           Priority|P3                          |P1
            Summary|Build broken in ggc-page.cc |[16 Regression] Build
                   |on master                   |broken in ggc-page.cc on
                   |                            |master since r16-852

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the problem is that
1212      /* Remove all free page groups, and release the storage.  */
1213      gp = &G.page_groups;
1214      while ((g = *gp) != NULL)
1215        if (g->in_use == 0)
1216          {
1217            *gp = g->next;
1218            G.bytes_mapped -= g->alloc_size;
1219            n1 += g->alloc_size;
1220            free (g->allocation);
1221          }
1222        else
1223          gp = &g->next;
walks all page groups each time, the first time it frees something, the second
time and onwards doesn't really do anything, but the second and following
freelists might still access those freed groups.

So, I'd go with:
2025-05-28  Jakub Jelinek  <ja...@redhat.com>

        PR bootstrap/120464
        * ggc-page.cc (struct ggc_globals): Fix up comment formatting.
        (find_free_list): Likewise.
        (alloc_page): For defined(USING_MALLOC_PAGE_GROUPS) use
        free_list->free_pages instead of G.free_pages.
        (do_release_pages): Add n1 and n2 arguments, make them used.
        Move defined(USING_MALLOC_PAGE_GROUPS) page group freeing to
        release_pages and dumping of statistics as well.  Formatting fixes.
        (release_pages): Adjust do_release_pages caller, move here
        defined(USING_MALLOC_PAGE_GROUPS) page group freeing and dumping
        of statistics.
        (ggc_handle_finalizers): Fix up comment formatting.

--- gcc/ggc-page.cc.jj  2025-05-27 23:08:37.969726172 +0200
+++ gcc/ggc-page.cc     2025-05-28 22:59:27.206533376 +0200
@@ -426,7 +426,7 @@ static struct ggc_globals
   int dev_zero_fd;
 #endif

-  /* A cache of free system pages. Entry 0 is fallback.  */
+  /* A cache of free system pages.  Entry 0 is fallback.  */
   struct free_list free_lists[num_free_list];

 #ifdef USING_MALLOC_PAGE_GROUPS
@@ -787,7 +787,7 @@ find_free_list (size_t entry_size)
          return &G.free_lists[i];
        }
     }
-  /* Fallback. Does this happen?  */
+  /* Fallback.  Does this happen?  */
   if (GATHER_STATISTICS)
     G.stats.fallback++;
   return &G.free_lists[0];
@@ -955,7 +955,7 @@ alloc_page (unsigned order)
       /* If we allocated multiple pages, put the rest on the free list.  */
       if (multiple_pages)
        {
-         struct page_entry *e, *f = G.free_pages;
+         struct page_entry *e, *f = free_list->free_pages;
          for (a = enda - G.pagesize; a != page; a -= G.pagesize)
            {
              e = XCNEWVAR (struct page_entry, page_entry_size);
@@ -1073,10 +1073,10 @@ free_page (page_entry *entry)
 /* Release the free page cache for FREE_LIST to the system.  */

 static void
-do_release_pages (struct free_list *free_list)
+do_release_pages (struct free_list *free_list, size_t &n1, size_t &n2)
 {
-  size_t n1 = 0;
-  size_t n2 = 0;
+  (void) n1;
+  (void) n2;
 #ifdef USING_MADVISE
   page_entry *p, *start_p;
   char *start;
@@ -1089,7 +1089,7 @@ do_release_pages (struct free_list *free
      This allows other allocators to grab these areas if needed.
      This is only done on larger chunks to avoid fragmentation.
      This does not always work because the free_pages list is only
-     approximately sorted. */
+     approximately sorted.  */

   p = free_list->free_pages;
   prev = NULL;
@@ -1104,7 +1104,7 @@ do_release_pages (struct free_list *free
         {
           len += p->bytes;
          if (!p->discarded)
-             mapped_len += p->bytes;
+           mapped_len += p->bytes;
          newprev = p;
           p = p->next;
         }
@@ -1129,7 +1129,7 @@ do_release_pages (struct free_list *free
    }

   /* Now give back the fragmented pages to the OS, but keep the address
-     space to reuse it next time. */
+     space to reuse it next time.  */

   for (p = free_list->free_pages; p; )
     {
@@ -1149,10 +1149,10 @@ do_release_pages (struct free_list *free
         }
       /* Give the page back to the kernel, but don't free the mapping.
          This avoids fragmentation in the virtual memory map of the
-        process. Next time we can reuse it by just touching it. */
+        process.  Next time we can reuse it by just touching it.  */
       madvise (start, len, MADV_DONTNEED);
       /* Don't count those pages as mapped to not touch the garbage collector
-         unnecessarily. */
+         unnecessarily.  */
       G.bytes_mapped -= len;
       n2 += len;
       while (start_p != p)
@@ -1195,7 +1195,6 @@ do_release_pages (struct free_list *free
 #endif
 #ifdef USING_MALLOC_PAGE_GROUPS
   page_entry **pp, *p;
-  page_group **gp, *g;

   /* Remove all pages from free page groups from the list.  */
   pp = &free_list->free_pages;
@@ -1207,6 +1206,20 @@ do_release_pages (struct free_list *free
       }
     else
       pp = &p->next;
+#endif
+}
+
+/* Release the free page cache to the system.  */
+
+static void
+release_pages ()
+{
+  size_t n1 = 0;
+  size_t n2 = 0;
+  for (int i = 0; i < num_free_list; i++)
+    do_release_pages (&G.free_lists[i], n1, n2);
+#ifdef USING_MALLOC_PAGE_GROUPS
+  page_group **gp, *g;

   /* Remove all free page groups, and release the storage.  */
   gp = &G.page_groups;
@@ -1232,15 +1245,6 @@ do_release_pages (struct free_list *free
     }
 }

-/* Release the free page cache to the system.  */
-
-static void
-release_pages ()
-{
-  for (int i = 0; i < num_free_list; i++)
-    do_release_pages (&G.free_lists[i]);
-}
-
 /* This table provides a fast way to determine ceil(log_2(size)) for
    allocation requests.  The minimum allocation size is eight bytes.  */
 #define NUM_SIZE_LOOKUP 512
@@ -2008,7 +2012,7 @@ clear_marks (void)
     }
 }

-/* Check if any blocks with a registered finalizer have become unmarked. If so
+/* Check if any blocks with a registered finalizer have become unmarked.  If
so
    run the finalizer and unregister it because the block is about to be freed.
    Note that no garantee is made about what order finalizers will run in so
    touching other objects in gc memory is extremely unwise.  */

Reply via email to