Merge pull request 'fix std.heap.PageAllocator to not intrude on stacks + re-enable LoongArch CI' (#31271) from alexrp/zig:page-allocator-fixes into master

Reviewed-on: https://codeberg.org/ziglang/zig/pulls/31271
Reviewed-by: Andrew Kelley <andrew@ziglang.org>

.forgejo/workflows/ci.yaml

@@ -59,27 +59,26 @@ jobs:
         run: sh ci/aarch64-macos-release.sh
         timeout-minutes: 120
 
-  # https://codeberg.org/ziglang/zig/issues/30800
-  #loongarch64-linux-debug:
-  #  runs-on: [self-hosted, loongarch64-linux]
-  #  steps:
-  #    - name: Checkout
-  #      uses: https://codeberg.org/ziglang/checkout@19af6bac491e2534a4687a50ee84fa7f13258d28
-  #      with:
-  #        fetch-depth: 0
-  #    - name: Build and Test
-  #      run: sh ci/loongarch64-linux-debug.sh
-  #      timeout-minutes: 240
-  #loongarch64-linux-release:
-  #  runs-on: [self-hosted, loongarch64-linux]
-  #  steps:
-  #    - name: Checkout
-  #      uses: https://codeberg.org/ziglang/checkout@19af6bac491e2534a4687a50ee84fa7f13258d28
-  #      with:
-  #        fetch-depth: 0
-  #    - name: Build and Test
-  #      run: sh ci/loongarch64-linux-release.sh
-  #      timeout-minutes: 180
+  loongarch64-linux-debug:
+    runs-on: [self-hosted, loongarch64-linux]
+    steps:
+      - name: Checkout
+        uses: https://codeberg.org/ziglang/checkout@19af6bac491e2534a4687a50ee84fa7f13258d28
+        with:
+          fetch-depth: 0
+      - name: Build and Test
+        run: sh ci/loongarch64-linux-debug.sh
+        timeout-minutes: 240
+  loongarch64-linux-release:
+    runs-on: [self-hosted, loongarch64-linux]
+    steps:
+      - name: Checkout
+        uses: https://codeberg.org/ziglang/checkout@19af6bac491e2534a4687a50ee84fa7f13258d28
+        with:
+          fetch-depth: 0
+      - name: Build and Test
+        run: sh ci/loongarch64-linux-release.sh
+        timeout-minutes: 180
 
   powerpc64le-linux-debug:
     runs-on: [self-hosted, powerpc64le-linux]

lib/std/heap.zig

@@ -41,9 +41,6 @@ pub const MemoryPoolExtra = memory_pool.Extra;
 /// Deprecated; use `memory_pool.Options`.
 pub const MemoryPoolOptions = memory_pool.Options;
 
-/// TODO Utilize this on Windows.
-pub var next_mmap_addr_hint: ?[*]align(page_size_min) u8 = null;
-
 /// comptime-known minimum page size of the target.
 ///
 /// All pointers from `mmap` or `NtAllocateVirtualMemory` are aligned to at least

lib/std/heap/PageAllocator.zig

@@ -19,6 +19,28 @@ pub const vtable: Allocator.VTable = .{
     .free = free,
 };
 
+/// Hhinting is disabled on operating systems that make an effort to not reuse
+/// mappings. For example, OpenBSD aggressively randomizes addresses of mappings
+/// that don't provide a hint (for security reasons, but it serves our needs
+/// too).
+const enable_hints = switch (builtin.target.os.tag) {
+    .openbsd => false,
+    else => true,
+};
+
+/// On operating systems that don't immediately map in the whole stack, we need
+/// to be careful to not hint into the pages after the stack guard gap, which
+/// the stack will expand into. The easiest way to avoid that is to hint in the
+/// same direction as stack growth.
+const stack_direction = builtin.target.stackGrowth();
+
+/// When hinting upwards, this points to the next page that we hope to allocate
+/// at; when hinting downwards, this points to the beginning of the last
+/// successful allocation.
+///
+/// TODO: Utilize this on Windows.
+var addr_hint: ?[*]align(page_size_min) u8 = null;
+
 pub fn map(n: usize, alignment: Alignment) ?[*]u8 {
     const page_size = std.heap.pageSize();
     if (n >= maxInt(usize) - page_size) return null;
@@ -41,7 +63,7 @@ pub fn map(n: usize, alignment: Alignment) ?[*]u8 {
         }
 
         const overalloc_len = n + alignment_bytes - page_size;
-        const aligned_len = mem.alignForward(usize, n, page_size);
+        const page_aligned_len = mem.alignForward(usize, n, page_size);
 
         base_addr = null;
         size = overalloc_len;
@@ -60,7 +82,7 @@ pub fn map(n: usize, alignment: Alignment) ?[*]u8 {
             _ = ntdll.NtFreeVirtualMemory(current_process, @ptrCast(&prefix_base), &prefix_size_param, .{ .RELEASE = true, .PRESERVE_PLACEHOLDER = true });
         }
 
-        const suffix_start = aligned_addr + aligned_len;
+        const suffix_start = aligned_addr + page_aligned_len;
         const suffix_size = (placeholder_addr + overalloc_len) - suffix_start;
         if (suffix_size > 0) {
             var suffix_base = @as(?*anyopaque, @ptrFromInt(suffix_start));
@@ -69,7 +91,7 @@ pub fn map(n: usize, alignment: Alignment) ?[*]u8 {
         }
 
         base_addr = @ptrFromInt(aligned_addr);
-        size = aligned_len;
+        size = page_aligned_len;
 
         status = ntdll.NtAllocateVirtualMemory(current_process, @ptrCast(&base_addr), 0, &size, .{ .COMMIT = true }, .{ .READWRITE = true });
 
@@ -78,20 +100,34 @@ pub fn map(n: usize, alignment: Alignment) ?[*]u8 {
         }
 
         base_addr = @as(?*anyopaque, @ptrFromInt(aligned_addr));
-        size = aligned_len;
+        size = page_aligned_len;
         _ = ntdll.NtFreeVirtualMemory(current_process, @ptrCast(&base_addr), &size, .{ .RELEASE = true });
 
         return null;
     }
 
-    const aligned_len = mem.alignForward(usize, n, page_size);
+    const page_aligned_len = mem.alignForward(usize, n, page_size);
     const max_drop_len = alignment_bytes -| page_size;
-    const overalloc_len = aligned_len + max_drop_len;
-    const maybe_unaligned_hint = @atomicLoad(@TypeOf(std.heap.next_mmap_addr_hint), &std.heap.next_mmap_addr_hint, .unordered);
+    const overalloc_len = page_aligned_len + max_drop_len;
 
-    // Aligning hint does not use mem.alignPointer, because it is slow.
-    // Aligning hint does not use mem.alignForward, because it asserts that there will be no overflow.
-    const hint: ?[*]align(page_size_min) u8 = @ptrFromInt(((@intFromPtr(maybe_unaligned_hint)) +% (alignment_bytes - 1)) & ~(alignment_bytes - 1));
+    const maybe_unaligned_hint, const hint = blk: {
+        if (!enable_hints) break :blk .{ null, null };
+
+        const maybe_unaligned_hint = @atomicLoad(@TypeOf(addr_hint), &addr_hint, .unordered);
+
+        // For the very first mmap, let the kernel pick a good starting address;
+        // we'll begin doing our hinting from there.
+        if (maybe_unaligned_hint == null) break :blk .{ null, null };
+
+        // Aligning hint does not use mem.alignPointer, because it is slow.
+        // Aligning hint does not use mem.alignForward, because it asserts that there will be no overflow.
+        const hint: ?[*]align(page_size_min) u8 = @ptrFromInt(switch (stack_direction) {
+            .down => ((@intFromPtr(maybe_unaligned_hint) -% page_aligned_len) & ~(alignment_bytes - 1)) -% max_drop_len,
+            .up => (@intFromPtr(maybe_unaligned_hint) +% (alignment_bytes - 1)) & ~(alignment_bytes - 1),
+        });
+
+        break :blk .{ maybe_unaligned_hint, hint };
+    };
 
     const slice = posix.mmap(
         hint,
@@ -101,16 +137,24 @@ pub fn map(n: usize, alignment: Alignment) ?[*]u8 {
         -1,
         0,
     ) catch return null;
-    const result_ptr = mem.alignPointer(slice.ptr, alignment_bytes) orelse return null;
+    const result_ptr = mem.alignPointer(slice.ptr, alignment_bytes).?;
+
     // Unmap the extra bytes that were only requested in order to guarantee
     // that the range of memory we were provided had a proper alignment in it
     // somewhere. The extra bytes could be at the beginning, or end, or both.
     const drop_len = result_ptr - slice.ptr;
     if (drop_len != 0) posix.munmap(slice[0..drop_len]);
     const remaining_len = overalloc_len - drop_len;
-    if (remaining_len > aligned_len) posix.munmap(@alignCast(result_ptr[aligned_len..remaining_len]));
-    const new_hint: [*]align(page_size_min) u8 = @alignCast(result_ptr + aligned_len);
-    _ = @cmpxchgStrong(@TypeOf(std.heap.next_mmap_addr_hint), &std.heap.next_mmap_addr_hint, maybe_unaligned_hint, new_hint, .monotonic, .monotonic);
+    if (remaining_len > page_aligned_len) posix.munmap(@alignCast(result_ptr[page_aligned_len..remaining_len]));
+
+    if (enable_hints) {
+        const new_hint: [*]align(page_size_min) u8 = @alignCast(result_ptr + switch (stack_direction) {
+            .up => page_aligned_len,
+            .down => 0,
+        });
+        _ = @cmpxchgStrong(@TypeOf(addr_hint), &addr_hint, maybe_unaligned_hint, new_hint, .monotonic, .monotonic);
+    }
+
     return result_ptr;
 }
 
@@ -181,7 +225,10 @@ pub fn realloc(uncasted_memory: []u8, alignment: Alignment, new_len: usize, may_
     if (new_size_aligned == page_aligned_len)
         return memory.ptr;
 
-    if (posix.MREMAP != void) {
+    // When the stack grows down, only use `mremap` if the allocation may move.
+    // Otherwise, we might grow the allocation and intrude on virtual address
+    // space which we want to keep available to the stack.
+    if (posix.MREMAP != void and (stack_direction == .up or may_move)) {
         // TODO: if the next_mmap_addr_hint is within the remapped range, update it
         const new_memory = posix.mremap(memory.ptr, page_aligned_len, new_size_aligned, .{ .MAYMOVE = may_move }, null) catch return null;
         return new_memory.ptr;

lib/std/heap/debug_allocator.zig

@@ -266,7 +266,7 @@ pub fn DebugAllocator(comptime config: Config) type {
             canary: usize = config.canary,
 
             fn fromPage(page_addr: usize, slot_count: usize) *BucketHeader {
-                const unaligned = page_addr + page_size - bucketSize(slot_count);
+                const unaligned = page_addr +% page_size -% bucketSize(slot_count);
                 return @ptrFromInt(unaligned & ~(@as(usize, @alignOf(BucketHeader)) - 1));
             }