From e134e6c994d6ecd76dc6ae1a24b8de29c4147eab Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Fri, 20 Dec 2019 10:48:03 +0100 Subject: [PATCH 1/4] Pointer arithmetic affects the alignment factor Closes #1528 --- lib/std/start.zig | 4 +-- src/ir.cpp | 43 ++++++++++++++++++++++++------- test/stage1/behavior/pointers.zig | 16 ++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/lib/std/start.zig b/lib/std/start.zig index 7c1353e18b..7a10d87300 100644 --- a/lib/std/start.zig +++ b/lib/std/start.zig @@ -142,14 +142,14 @@ fn posixCallMainAndExit() noreturn { const argc = starting_stack_ptr[0]; const argv = @ptrCast([*][*:0]u8, starting_stack_ptr + 1); - const envp_optional = @ptrCast([*:null]?[*:0]u8, argv + argc + 1); + const envp_optional = @ptrCast([*:null]?[*:0]u8, @alignCast(@alignOf(usize), argv + argc + 1)); var envp_count: usize = 0; while (envp_optional[envp_count]) |_| : (envp_count += 1) {} const envp = @ptrCast([*][*:0]u8, envp_optional)[0..envp_count]; if (builtin.os == .linux) { // Find the beginning of the auxiliary vector - const auxv = @ptrCast([*]std.elf.Auxv, envp.ptr + envp_count + 1); + const auxv = @ptrCast([*]std.elf.Auxv, @alignCast(@alignOf(usize), envp.ptr + envp_count + 1)); std.os.linux.elf_aux_maybe = auxv; // Initialize the TLS area const gnu_stack_phdr = std.os.linux.tls.initTLS() orelse @panic("ELF missing stack size"); diff --git a/src/ir.cpp b/src/ir.cpp index e841da35d0..f7e7aa8902 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -15776,19 +15776,44 @@ static IrInstruction *ir_analyze_bin_op_math(IrAnalyze *ira, IrInstructionBinOp return ir_const_undef(ira, &instruction->base, op1->value->type); } + // NOTE: this variable is meaningful iff op2_val is not null! + uint64_t byte_offset; + if (op2_val != nullptr) { + uint64_t elem_offset; + if (!ir_resolve_usize(ira, casted_op2, &elem_offset)) + return ira->codegen->invalid_instruction; + + ZigType *elem_type = op1->value->type->data.pointer.child_type; + if ((err = type_resolve(ira->codegen, elem_type, ResolveStatusSizeKnown))) + return ira->codegen->invalid_instruction; + byte_offset = type_size(ira->codegen, elem_type) * elem_offset; + } + + // Fast path for cases where the RHS is zero + if (op2_val != nullptr && byte_offset == 0) { + return op1; + } + + ZigType *result_type = op1->value->type; + // The resulting pointer may not be aligned anymore + if (op2_val != nullptr) { + uint32_t align_bytes; + if ((err = resolve_ptr_align(ira, op1->value->type, &align_bytes))) + return ira->codegen->invalid_instruction; + + if (byte_offset != 0 && byte_offset % align_bytes != 0) + result_type = adjust_ptr_align(ira->codegen, result_type, 1); + } else { + // The addend is not a comptime-known value + result_type = adjust_ptr_align(ira->codegen, result_type, 1); + } + if (op2_val != nullptr && op1_val != nullptr && (op1->value->data.x_ptr.special == ConstPtrSpecialHardCodedAddr || op1->value->data.x_ptr.special == ConstPtrSpecialNull)) { uint64_t start_addr = (op1_val->data.x_ptr.special == ConstPtrSpecialNull) ? 0 : op1_val->data.x_ptr.data.hard_coded_addr.addr; - uint64_t elem_offset; - if (!ir_resolve_usize(ira, casted_op2, &elem_offset)) - return ira->codegen->invalid_instruction; - ZigType *elem_type = op1_val->type->data.pointer.child_type; - if ((err = type_resolve(ira->codegen, elem_type, ResolveStatusSizeKnown))) - return ira->codegen->invalid_instruction; - uint64_t byte_offset = type_size(ira->codegen, elem_type) * elem_offset; uint64_t new_addr; if (op_id == IrBinOpAdd) { new_addr = start_addr + byte_offset; @@ -15797,7 +15822,7 @@ static IrInstruction *ir_analyze_bin_op_math(IrAnalyze *ira, IrInstructionBinOp } else { zig_unreachable(); } - IrInstruction *result = ir_const(ira, &instruction->base, op1_val->type); + IrInstruction *result = ir_const(ira, &instruction->base, result_type); result->value->data.x_ptr.special = ConstPtrSpecialHardCodedAddr; result->value->data.x_ptr.mut = ConstPtrMutRuntimeVar; result->value->data.x_ptr.data.hard_coded_addr.addr = new_addr; @@ -15806,7 +15831,7 @@ static IrInstruction *ir_analyze_bin_op_math(IrAnalyze *ira, IrInstructionBinOp IrInstruction *result = ir_build_bin_op(&ira->new_irb, instruction->base.scope, instruction->base.source_node, op_id, op1, casted_op2, true); - result->value->type = op1->value->type; + result->value->type = result_type; return result; } diff --git a/test/stage1/behavior/pointers.zig b/test/stage1/behavior/pointers.zig index e1004243fc..048cdb150f 100644 --- a/test/stage1/behavior/pointers.zig +++ b/test/stage1/behavior/pointers.zig @@ -288,3 +288,19 @@ test "pointer to array at fixed address" { // Silly check just to reference `array` expect(@ptrToInt(&array[0]) == 0x10); } + +test "pointer arithmetic affects the alignment" { + var arr: [10]u8 align(2) = undefined; + var x: usize = 1; + + const ptr = @as([*]u8, &arr); + expect(@typeInfo(@TypeOf(ptr)).Pointer.alignment == 2); + const ptr1 = ptr + 1; + expect(@typeInfo(@TypeOf(ptr1)).Pointer.alignment == 1); + const ptr2 = ptr + 4; + expect(@typeInfo(@TypeOf(ptr2)).Pointer.alignment == 2); + const ptr3 = ptr + 0; + expect(@typeInfo(@TypeOf(ptr3)).Pointer.alignment == 2); + const ptr4 = ptr + x; + expect(@typeInfo(@TypeOf(ptr4)).Pointer.alignment == 1); +} From 7fe13f4a86c04c25f95b237452b90e9ab3103d1f Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Fri, 20 Dec 2019 13:58:19 +0100 Subject: [PATCH 2/4] Pointer alignment fixes for the stdlib --- lib/std/debug.zig | 4 ++-- lib/std/event/fs.zig | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/std/debug.zig b/lib/std/debug.zig index 94f81908da..dfdaca6d3f 100644 --- a/lib/std/debug.zig +++ b/lib/std/debug.zig @@ -1084,7 +1084,7 @@ fn openSelfDebugInfoMacOs(allocator: *mem.Allocator) !DebugInfo { std.macho.LC_SYMTAB => break @ptrCast(*std.macho.symtab_command, ptr), else => {}, } - ptr += lc.cmdsize; // TODO https://github.com/ziglang/zig/issues/1403 + ptr = @alignCast(@alignOf(std.macho.load_command), ptr + lc.cmdsize); } else { return error.MissingDebugInfo; }; @@ -2129,7 +2129,7 @@ fn getLineNumberInfoMacOs(di: *DebugInfo, symbol: MachoSymbol, target_address: u std.macho.LC_SEGMENT_64 => break @ptrCast(*const std.macho.segment_command_64, @alignCast(@alignOf(std.macho.segment_command_64), ptr)), else => {}, } - ptr += lc.cmdsize; // TODO https://github.com/ziglang/zig/issues/1403 + ptr = @alignCast(@alignOf(std.macho.load_command), ptr + lc.cmdsize); } else { return error.MissingDebugInfo; }; diff --git a/lib/std/event/fs.zig b/lib/std/event/fs.zig index 5986f07ad3..ce88ac4dc4 100644 --- a/lib/std/event/fs.zig +++ b/lib/std/event/fs.zig @@ -1263,7 +1263,7 @@ pub fn Watch(comptime V: type) type { var ptr = event_buf[0..].ptr; const end_ptr = ptr + event_buf.len; var ev: *os.linux.inotify_event = undefined; - while (@ptrToInt(ptr) < @ptrToInt(end_ptr)) : (ptr += @sizeOf(os.linux.inotify_event) + ev.len) { + while (@ptrToInt(ptr) < @ptrToInt(end_ptr)) { ev = @ptrCast(*os.linux.inotify_event, ptr); if (ev.mask & os.linux.IN_CLOSE_WRITE == os.linux.IN_CLOSE_WRITE) { const basename_ptr = ptr + @sizeOf(os.linux.inotify_event); @@ -1287,6 +1287,8 @@ pub fn Watch(comptime V: type) type { }); } } + + ptr = @alignCast(@alignOf(os.linux.inotify_event), ptr + @sizeOf(os.linux.inotify_event) + ev.len); } }, os.linux.EINTR => continue, From 7ea7842ed020d5b984d14b824663891aae7549bc Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 8 Jan 2020 21:02:05 +0100 Subject: [PATCH 3/4] Fix calculation of new alignment factor --- src/ir.cpp | 11 +++++++++-- test/stage1/behavior/pointers.zig | 31 +++++++++++++++++++------------ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index f7e7aa8902..2a581e0c03 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -15801,8 +15801,15 @@ static IrInstruction *ir_analyze_bin_op_math(IrAnalyze *ira, IrInstructionBinOp if ((err = resolve_ptr_align(ira, op1->value->type, &align_bytes))) return ira->codegen->invalid_instruction; - if (byte_offset != 0 && byte_offset % align_bytes != 0) - result_type = adjust_ptr_align(ira->codegen, result_type, 1); + if (byte_offset & (align_bytes - 1)) { + // The resulting pointer is aligned to the lcd between the + // offset (an arbitrary number) and the alignment factor (always + // a power of two, non zero) + uint32_t new_align = 1 << ctzll(byte_offset | align_bytes); + // Rough guard to prevent overflows + assert(new_align); + result_type = adjust_ptr_align(ira->codegen, result_type, new_align); + } } else { // The addend is not a comptime-known value result_type = adjust_ptr_align(ira->codegen, result_type, 1); diff --git a/test/stage1/behavior/pointers.zig b/test/stage1/behavior/pointers.zig index 048cdb150f..28b871bd5d 100644 --- a/test/stage1/behavior/pointers.zig +++ b/test/stage1/behavior/pointers.zig @@ -290,17 +290,24 @@ test "pointer to array at fixed address" { } test "pointer arithmetic affects the alignment" { - var arr: [10]u8 align(2) = undefined; - var x: usize = 1; + { + var ptr: [*]align(8) u32 = undefined; + var x: usize = 1; - const ptr = @as([*]u8, &arr); - expect(@typeInfo(@TypeOf(ptr)).Pointer.alignment == 2); - const ptr1 = ptr + 1; - expect(@typeInfo(@TypeOf(ptr1)).Pointer.alignment == 1); - const ptr2 = ptr + 4; - expect(@typeInfo(@TypeOf(ptr2)).Pointer.alignment == 2); - const ptr3 = ptr + 0; - expect(@typeInfo(@TypeOf(ptr3)).Pointer.alignment == 2); - const ptr4 = ptr + x; - expect(@typeInfo(@TypeOf(ptr4)).Pointer.alignment == 1); + expect(@typeInfo(@TypeOf(ptr)).Pointer.alignment == 8); + const ptr1 = ptr + 1; // 1 * 4 = 4 -> lcd(4,8) = 4 + expect(@typeInfo(@TypeOf(ptr1)).Pointer.alignment == 4); + const ptr2 = ptr + 4; // 4 * 4 = 16 -> lcd(16,8) = 8 + expect(@typeInfo(@TypeOf(ptr2)).Pointer.alignment == 8); + const ptr3 = ptr + 0; // no-op + expect(@typeInfo(@TypeOf(ptr3)).Pointer.alignment == 8); + const ptr4 = ptr + x; // runtime-known addend + expect(@typeInfo(@TypeOf(ptr4)).Pointer.alignment == 1); + } + { + var ptr: [*]align(8) [3]u8 = undefined; + + const ptr1 = ptr + 17; // 3 * 17 = 51 + expect(@typeInfo(@TypeOf(ptr1)).Pointer.alignment == 1); + } } From c51b79c56e32594e4fb119fc760ae38b69fb9bbb Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Thu, 9 Jan 2020 11:56:45 +0100 Subject: [PATCH 4/4] Correct alignment calculation for runtime addends --- src/ir.cpp | 34 +++++++++++++++---------------- test/stage1/behavior/pointers.zig | 9 +++++++- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index 2a581e0c03..8b5168e477 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -15776,6 +15776,10 @@ static IrInstruction *ir_analyze_bin_op_math(IrAnalyze *ira, IrInstructionBinOp return ir_const_undef(ira, &instruction->base, op1->value->type); } + ZigType *elem_type = op1->value->type->data.pointer.child_type; + if ((err = type_resolve(ira->codegen, elem_type, ResolveStatusSizeKnown))) + return ira->codegen->invalid_instruction; + // NOTE: this variable is meaningful iff op2_val is not null! uint64_t byte_offset; if (op2_val != nullptr) { @@ -15783,9 +15787,6 @@ static IrInstruction *ir_analyze_bin_op_math(IrAnalyze *ira, IrInstructionBinOp if (!ir_resolve_usize(ira, casted_op2, &elem_offset)) return ira->codegen->invalid_instruction; - ZigType *elem_type = op1->value->type->data.pointer.child_type; - if ((err = type_resolve(ira->codegen, elem_type, ResolveStatusSizeKnown))) - return ira->codegen->invalid_instruction; byte_offset = type_size(ira->codegen, elem_type) * elem_offset; } @@ -15795,24 +15796,23 @@ static IrInstruction *ir_analyze_bin_op_math(IrAnalyze *ira, IrInstructionBinOp } ZigType *result_type = op1->value->type; - // The resulting pointer may not be aligned anymore - if (op2_val != nullptr) { + // Calculate the new alignment of the pointer + { uint32_t align_bytes; if ((err = resolve_ptr_align(ira, op1->value->type, &align_bytes))) return ira->codegen->invalid_instruction; - if (byte_offset & (align_bytes - 1)) { - // The resulting pointer is aligned to the lcd between the - // offset (an arbitrary number) and the alignment factor (always - // a power of two, non zero) - uint32_t new_align = 1 << ctzll(byte_offset | align_bytes); - // Rough guard to prevent overflows - assert(new_align); - result_type = adjust_ptr_align(ira->codegen, result_type, new_align); - } - } else { - // The addend is not a comptime-known value - result_type = adjust_ptr_align(ira->codegen, result_type, 1); + // If the addend is not a comptime-known value we can still count on + // it being a multiple of the type size + uint32_t addend = op2_val ? byte_offset : type_size(ira->codegen, elem_type); + + // The resulting pointer is aligned to the lcd between the + // offset (an arbitrary number) and the alignment factor (always + // a power of two, non zero) + uint32_t new_align = 1 << ctzll(addend | align_bytes); + // Rough guard to prevent overflows + assert(new_align); + result_type = adjust_ptr_align(ira->codegen, result_type, new_align); } if (op2_val != nullptr && op1_val != nullptr && diff --git a/test/stage1/behavior/pointers.zig b/test/stage1/behavior/pointers.zig index 28b871bd5d..fdaa5867d7 100644 --- a/test/stage1/behavior/pointers.zig +++ b/test/stage1/behavior/pointers.zig @@ -302,12 +302,19 @@ test "pointer arithmetic affects the alignment" { const ptr3 = ptr + 0; // no-op expect(@typeInfo(@TypeOf(ptr3)).Pointer.alignment == 8); const ptr4 = ptr + x; // runtime-known addend - expect(@typeInfo(@TypeOf(ptr4)).Pointer.alignment == 1); + expect(@typeInfo(@TypeOf(ptr4)).Pointer.alignment == 4); } { var ptr: [*]align(8) [3]u8 = undefined; + var x: usize = 1; const ptr1 = ptr + 17; // 3 * 17 = 51 expect(@typeInfo(@TypeOf(ptr1)).Pointer.alignment == 1); + const ptr2 = ptr + x; // runtime-known addend + expect(@typeInfo(@TypeOf(ptr2)).Pointer.alignment == 1); + const ptr3 = ptr + 8; // 3 * 8 = 24 -> lcd(8,24) = 8 + expect(@typeInfo(@TypeOf(ptr3)).Pointer.alignment == 8); + const ptr4 = ptr + 4; // 3 * 4 = 12 -> lcd(8,12) = 4 + expect(@typeInfo(@TypeOf(ptr4)).Pointer.alignment == 4); } }