Commit 7ca86936ca

LemonBoy <thatlemon@gmail.com>
2021-06-06 11:12:41
std: Add helpers to safely align pointers
Add two helpers to ensure people won't ignore some edge cases such as pointers overflowing the address space. Also fix #8924 to some degree, the amount of unchecked alignForward is still scary. Revert "tests: disable i386-linux-gnu -lc target due to CI failures" This reverts commit 97a2f4e7ae9c52c595841347bb0b26572b180dcf.
1 parent 5c765b2
Changed files (3)
lib/std/heap.zig
@@ -247,7 +247,7 @@ const PageAllocator = struct {
             ) catch return error.OutOfMemory;
 
             // If the allocation is sufficiently aligned, use it.
-            if (@ptrToInt(addr) & (alignment - 1) == 0) {
+            if (mem.isAligned(@ptrToInt(addr), alignment)) {
                 return @ptrCast([*]u8, addr)[0..alignPageAllocLen(aligned_len, n, len_align)];
             }
 
@@ -300,13 +300,13 @@ const PageAllocator = struct {
         ) catch return error.OutOfMemory;
         assert(mem.isAligned(@ptrToInt(slice.ptr), mem.page_size));
 
-        const aligned_addr = mem.alignForward(@ptrToInt(slice.ptr), alignment);
-        const result_ptr = @alignCast(mem.page_size, @intToPtr([*]u8, aligned_addr));
+        const result_ptr = mem.alignPointer(slice.ptr, alignment) orelse
+            return error.OutOfMemory;
 
         // 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 = aligned_addr - @ptrToInt(slice.ptr);
+        const drop_len = @ptrToInt(result_ptr) - @ptrToInt(slice.ptr);
         if (drop_len != 0) {
             os.munmap(slice[0..drop_len]);
         }
@@ -372,7 +372,7 @@ const PageAllocator = struct {
             return alignPageAllocLen(new_size_aligned, new_size, len_align);
 
         if (new_size_aligned < buf_aligned_len) {
-            const ptr = @intToPtr([*]align(mem.page_size) u8, @ptrToInt(buf_unaligned.ptr) + new_size_aligned);
+            const ptr = @alignCast(mem.page_size, buf_unaligned.ptr + new_size_aligned);
             // TODO: if the next_mmap_addr_hint is within the unmapped range, update it
             os.munmap(ptr[0 .. buf_aligned_len - new_size_aligned]);
             if (new_size_aligned == 0)
@@ -692,8 +692,9 @@ pub const FixedBufferAllocator = struct {
 
     fn alloc(allocator: *Allocator, n: usize, ptr_align: u29, len_align: u29, ra: usize) ![]u8 {
         const self = @fieldParentPtr(FixedBufferAllocator, "allocator", allocator);
-        const aligned_addr = mem.alignForward(@ptrToInt(self.buffer.ptr) + self.end_index, ptr_align);
-        const adjusted_index = aligned_addr - @ptrToInt(self.buffer.ptr);
+        const adjust_off = mem.alignPointerOffset(self.buffer.ptr + self.end_index, ptr_align) orelse
+            return error.OutOfMemory;
+        const adjusted_index = self.end_index + adjust_off;
         const new_end_index = adjusted_index + n;
         if (new_end_index > self.buffer.len) {
             return error.OutOfMemory;
@@ -765,9 +766,9 @@ pub const ThreadSafeFixedBufferAllocator = blk: {
                 const self = @fieldParentPtr(ThreadSafeFixedBufferAllocator, "allocator", allocator);
                 var end_index = @atomicLoad(usize, &self.end_index, builtin.AtomicOrder.SeqCst);
                 while (true) {
-                    const addr = @ptrToInt(self.buffer.ptr) + end_index;
-                    const adjusted_addr = mem.alignForward(addr, ptr_align);
-                    const adjusted_index = end_index + (adjusted_addr - addr);
+                    const adjust_off = mem.alignPointerOffset(self.buffer.ptr + end_index, ptr_align) orelse
+                        return error.OutOfMemory;
+                    const adjusted_index = end_index + adjust_off;
                     const new_end_index = adjusted_index + n;
                     if (new_end_index > self.buffer.len) {
                         return error.OutOfMemory;
lib/std/mem.zig
@@ -2323,6 +2323,70 @@ pub fn nativeToBig(comptime T: type, x: T) T {
     };
 }
 
+/// Returns the number of elements that, if added to the given pointer, align it
+/// to a multiple of the given quantity, or `null` if one of the following
+/// conditions is met:
+/// - The aligned pointer would not fit the address space,
+/// - The delta required to align the pointer is not a multiple of the pointee's
+///   type.
+pub fn alignPointerOffset(ptr: anytype, align_to: u29) ?usize {
+    assert(align_to != 0 and @popCount(u29, align_to) == 1);
+
+    const T = @TypeOf(ptr);
+    const info = @typeInfo(T);
+    if (info != .Pointer or info.Pointer.size != .Many)
+        @compileError("expected many item pointer, got " ++ @typeName(T));
+
+    // Do nothing if the pointer is already well-aligned.
+    if (align_to <= info.Pointer.alignment)
+        return 0;
+
+    // Calculate the aligned base address with an eye out for overflow.
+    const addr = @ptrToInt(ptr);
+    var new_addr: usize = undefined;
+    if (@addWithOverflow(usize, addr, align_to - 1, &new_addr)) return null;
+    new_addr &= ~@as(usize, align_to - 1);
+
+    // The delta is expressed in terms of bytes, turn it into a number of child
+    // type elements.
+    const delta = new_addr - addr;
+    const pointee_size = @sizeOf(info.Pointer.child);
+    if (delta % pointee_size != 0) return null;
+    return delta / pointee_size;
+}
+
+/// Aligns a given pointer value to a specified alignment factor.
+/// Returns an aligned pointer or null if one of the following conditions is
+/// met:
+/// - The aligned pointer would not fit the address space,
+/// - The delta required to align the pointer is not a multiple of the pointee's
+///   type.
+pub fn alignPointer(ptr: anytype, align_to: u29) ?@TypeOf(ptr) {
+    const adjust_off = alignPointerOffset(ptr, align_to) orelse return null;
+    const T = @TypeOf(ptr);
+    // Avoid the use of intToPtr to avoid losing the pointer provenance info.
+    return @alignCast(@typeInfo(T).Pointer.alignment, ptr + adjust_off);
+}
+
+test "alignPointer" {
+    const S = struct {
+        fn checkAlign(comptime T: type, base: usize, align_to: u29, expected: usize) !void {
+            var ptr = @intToPtr(T, base);
+            var aligned = alignPointer(ptr, align_to);
+            try testing.expectEqual(expected, @ptrToInt(aligned));
+        }
+    };
+
+    try S.checkAlign([*]u8, 0x123, 0x200, 0x200);
+    try S.checkAlign([*]align(4) u8, 0x10, 2, 0x10);
+    try S.checkAlign([*]u32, 0x10, 2, 0x10);
+    try S.checkAlign([*]u32, 0x4, 16, 0x10);
+    // Misaligned.
+    try S.checkAlign([*]align(1) u32, 0x3, 2, 0);
+    // Overflow.
+    try S.checkAlign([*]u32, math.maxInt(usize) - 3, 8, 0);
+}
+
 fn CopyPtrAttrs(comptime source: type, comptime size: std.builtin.TypeInfo.Pointer.Size, comptime child: type) type {
     const info = @typeInfo(source).Pointer;
     return @Type(.{
test/tests.zig
@@ -98,15 +98,14 @@ const test_targets = blk: {
             },
             .link_libc = true,
         },
-        // https://github.com/ziglang/zig/issues/8930
-        //TestTarget{
-        //    .target = .{
-        //        .cpu_arch = .i386,
-        //        .os_tag = .linux,
-        //        .abi = .gnu,
-        //    },
-        //    .link_libc = true,
-        //},
+        TestTarget{
+            .target = .{
+                .cpu_arch = .i386,
+                .os_tag = .linux,
+                .abi = .gnu,
+            },
+            .link_libc = true,
+        },
 
         TestTarget{
             .target = .{