std.heap.PageAllocator: hint mmaps in the same direction as stack growth

The old logic was fine for targets where the stack grows up (so, literally just
hppa), but problematic on targets where it grows down, because we could hint
that we wanted an allocation to happen in an area of the address space that the
kernel expects to be able to expand the stack into. The kernel is happy to
satisfy such a hint despite the obvious problems this leads to later down the
road.

Co-authored-by: rpkak <rpkak@noreply.codeberg.org>

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;
 }