From 26f3c2d0614f4fb37752b1931cb0b43aed2696d2 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 18 Dec 2019 16:43:56 -0500 Subject: [PATCH] fix std.mem.addNullByte and implement sentinel slicing see #3770 --- lib/std/buffer.zig | 4 +- lib/std/cstr.zig | 12 +++++- lib/std/fs.zig | 9 ++--- lib/std/mem.zig | 5 ++- lib/std/os.zig | 25 +++++-------- src/all_types.hpp | 2 + src/ir.cpp | 82 +++++++++++++++++++++++++++++++++++------ src/parser.cpp | 8 +++- test/compile_errors.zig | 11 ++++++ 9 files changed, 119 insertions(+), 39 deletions(-) diff --git a/lib/std/buffer.zig b/lib/std/buffer.zig index 55a5a890d6..6313d693b7 100644 --- a/lib/std/buffer.zig +++ b/lib/std/buffer.zig @@ -82,11 +82,11 @@ pub const Buffer = struct { } pub fn toSlice(self: Buffer) [:0]u8 { - return self.list.toSlice()[0..self.len()]; + return self.list.toSlice()[0..self.len() :0]; } pub fn toSliceConst(self: Buffer) [:0]const u8 { - return self.list.toSliceConst()[0..self.len()]; + return self.list.toSliceConst()[0..self.len() :0]; } pub fn shrink(self: *Buffer, new_len: usize) void { diff --git a/lib/std/cstr.zig b/lib/std/cstr.zig index 6dbae8342e..5194dc1a13 100644 --- a/lib/std/cstr.zig +++ b/lib/std/cstr.zig @@ -31,13 +31,21 @@ fn testCStrFnsImpl() void { testing.expect(mem.len(u8, "123456789") == 9); } -/// Returns a mutable slice with 1 more byte of length which is a null byte. +/// Returns a mutable, null-terminated slice with the same length as `slice`. /// Caller owns the returned memory. pub fn addNullByte(allocator: *mem.Allocator, slice: []const u8) ![:0]u8 { const result = try allocator.alloc(u8, slice.len + 1); mem.copy(u8, result, slice); result[slice.len] = 0; - return result; + return result[0..slice.len :0]; +} + +test "addNullByte" { + var buf: [30]u8 = undefined; + const allocator = &std.heap.FixedBufferAllocator.init(&buf).allocator; + const slice = try addNullByte(allocator, "hello"[0..4]); + testing.expect(slice.len == 4); + testing.expect(slice[4] == 0); } pub const NullTerminated2DArray = struct { diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 9e7daf0a9a..ea62086f59 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -221,16 +221,16 @@ pub const AtomicFile = struct { } tmp_path_buf[tmp_path_len] = 0; + const tmp_path_slice = tmp_path_buf[0..tmp_path_len :0]; const my_cwd = cwd(); while (true) { try crypto.randomBytes(rand_buf[0..]); - b64_fs_encoder.encode(tmp_path_buf[dirname_component_len..tmp_path_len], &rand_buf); + b64_fs_encoder.encode(tmp_path_slice[dirname_component_len..tmp_path_len], &rand_buf); - // TODO https://github.com/ziglang/zig/issues/3770 to clean up this @ptrCast const file = my_cwd.createFileC( - @ptrCast([*:0]u8, &tmp_path_buf), + tmp_path_slice, .{ .mode = mode, .exclusive = true }, ) catch |err| switch (err) { error.PathAlreadyExists => continue, @@ -1488,8 +1488,7 @@ pub fn openSelfExe() OpenSelfExeError!File { var buf: [MAX_PATH_BYTES]u8 = undefined; const self_exe_path = try selfExePath(&buf); buf[self_exe_path.len] = 0; - // TODO avoid @ptrCast here using slice syntax with https://github.com/ziglang/zig/issues/3770 - return openFileAbsoluteC(@ptrCast([*:0]u8, self_exe_path.ptr), .{}); + return openFileAbsoluteC(self_exe_path[0..self_exe_path.len :0].ptr, .{}); } test "openSelfExe" { diff --git a/lib/std/mem.zig b/lib/std/mem.zig index 8082d31063..824ab2f556 100644 --- a/lib/std/mem.zig +++ b/lib/std/mem.zig @@ -231,9 +231,10 @@ pub const Allocator = struct { pub fn free(self: *Allocator, memory: var) void { const Slice = @typeInfo(@TypeOf(memory)).Pointer; const bytes = @sliceToBytes(memory); - if (bytes.len == 0) return; + const bytes_len = bytes.len + @boolToInt(Slice.sentinel != null); + if (bytes_len == 0) return; const non_const_ptr = @intToPtr([*]u8, @ptrToInt(bytes.ptr)); - const shrink_result = self.shrinkFn(self, non_const_ptr[0..bytes.len], Slice.alignment, 0, 1); + const shrink_result = self.shrinkFn(self, non_const_ptr[0..bytes_len], Slice.alignment, 0, 1); assert(shrink_result.len == 0); } }; diff --git a/lib/std/os.zig b/lib/std/os.zig index 6d49bcd38e..3cc43357bd 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -805,9 +805,9 @@ pub fn execvpeC(file: [*:0]const u8, child_argv: [*:null]const ?[*:0]const u8, e mem.copy(u8, &path_buf, search_path); path_buf[search_path.len] = '/'; mem.copy(u8, path_buf[search_path.len + 1 ..], file_slice); - path_buf[search_path.len + file_slice.len + 1] = 0; - // TODO avoid @ptrCast here using slice syntax with https://github.com/ziglang/zig/issues/3770 - err = execveC(@ptrCast([*:0]u8, &path_buf), child_argv, envp); + const path_len = search_path.len + file_slice.len + 1; + path_buf[path_len] = 0; + err = execveC(path_buf[0..path_len :0].ptr, child_argv, envp); switch (err) { error.AccessDenied => seen_eacces = true, error.FileNotFound, error.NotDir => {}, @@ -841,18 +841,14 @@ pub fn execvpe( const arg_buf = try allocator.alloc(u8, arg.len + 1); @memcpy(arg_buf.ptr, arg.ptr, arg.len); arg_buf[arg.len] = 0; - - // TODO avoid @ptrCast using slice syntax with https://github.com/ziglang/zig/issues/3770 - argv_buf[i] = @ptrCast([*:0]u8, arg_buf.ptr); + argv_buf[i] = arg_buf[0..arg.len :0].ptr; } argv_buf[argv_slice.len] = null; + const argv_ptr = argv_buf[0..argv_slice.len :null].ptr; const envp_buf = try createNullDelimitedEnvMap(allocator, env_map); defer freeNullDelimitedEnvMap(allocator, envp_buf); - // TODO avoid @ptrCast here using slice syntax with https://github.com/ziglang/zig/issues/3770 - const argv_ptr = @ptrCast([*:null]?[*:0]u8, argv_buf.ptr); - return execvpeC(argv_buf.ptr[0].?, argv_ptr, envp_buf.ptr); } @@ -869,16 +865,13 @@ pub fn createNullDelimitedEnvMap(allocator: *mem.Allocator, env_map: *const std. @memcpy(env_buf.ptr, pair.key.ptr, pair.key.len); env_buf[pair.key.len] = '='; @memcpy(env_buf.ptr + pair.key.len + 1, pair.value.ptr, pair.value.len); - env_buf[env_buf.len - 1] = 0; - - // TODO avoid @ptrCast here using slice syntax with https://github.com/ziglang/zig/issues/3770 - envp_buf[i] = @ptrCast([*:0]u8, env_buf.ptr); + const len = env_buf.len - 1; + env_buf[len] = 0; + envp_buf[i] = env_buf[0..len :0].ptr; } assert(i == envp_count); } - // TODO avoid @ptrCast here using slice syntax with https://github.com/ziglang/zig/issues/3770 - assert(envp_buf[envp_count] == null); - return @ptrCast([*:null]?[*:0]u8, envp_buf.ptr)[0..envp_count]; + return envp_buf[0..envp_count :null]; } pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) void { diff --git a/src/all_types.hpp b/src/all_types.hpp index bcef45f718..144cdd4fc5 100644 --- a/src/all_types.hpp +++ b/src/all_types.hpp @@ -810,6 +810,7 @@ struct AstNodeSliceExpr { AstNode *array_ref_expr; AstNode *start; AstNode *end; + AstNode *sentinel; // can be null }; struct AstNodeFieldAccessExpr { @@ -3388,6 +3389,7 @@ struct IrInstructionSliceSrc { IrInstruction *ptr; IrInstruction *start; IrInstruction *end; + IrInstruction *sentinel; ResultLoc *result_loc; }; diff --git a/src/ir.cpp b/src/ir.cpp index 3000269ad3..e172b69c96 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -2967,18 +2967,21 @@ static IrInstruction *ir_build_memcpy(IrBuilder *irb, Scope *scope, AstNode *sou } static IrInstruction *ir_build_slice_src(IrBuilder *irb, Scope *scope, AstNode *source_node, - IrInstruction *ptr, IrInstruction *start, IrInstruction *end, bool safety_check_on, ResultLoc *result_loc) + IrInstruction *ptr, IrInstruction *start, IrInstruction *end, IrInstruction *sentinel, + bool safety_check_on, ResultLoc *result_loc) { IrInstructionSliceSrc *instruction = ir_build_instruction(irb, scope, source_node); instruction->ptr = ptr; instruction->start = start; instruction->end = end; + instruction->sentinel = sentinel; instruction->safety_check_on = safety_check_on; instruction->result_loc = result_loc; ir_ref_instruction(ptr, irb->current_basic_block); ir_ref_instruction(start, irb->current_basic_block); if (end) ir_ref_instruction(end, irb->current_basic_block); + if (sentinel) ir_ref_instruction(sentinel, irb->current_basic_block); return &instruction->base; } @@ -8483,6 +8486,7 @@ static IrInstruction *ir_gen_slice(IrBuilder *irb, Scope *scope, AstNode *node, AstNode *array_node = slice_expr->array_ref_expr; AstNode *start_node = slice_expr->start; AstNode *end_node = slice_expr->end; + AstNode *sentinel_node = slice_expr->sentinel; IrInstruction *ptr_value = ir_gen_node_extra(irb, array_node, scope, LValPtr, nullptr); if (ptr_value == irb->codegen->invalid_instruction) @@ -8501,7 +8505,17 @@ static IrInstruction *ir_gen_slice(IrBuilder *irb, Scope *scope, AstNode *node, end_value = nullptr; } - IrInstruction *slice = ir_build_slice_src(irb, scope, node, ptr_value, start_value, end_value, true, result_loc); + IrInstruction *sentinel_value; + if (sentinel_node) { + sentinel_value = ir_gen_node(irb, sentinel_node, scope); + if (sentinel_value == irb->codegen->invalid_instruction) + return irb->codegen->invalid_instruction; + } else { + sentinel_value = nullptr; + } + + IrInstruction *slice = ir_build_slice_src(irb, scope, node, ptr_value, start_value, end_value, + sentinel_value, true, result_loc); return ir_lval_wrap(irb, scope, slice, lval, result_loc); } @@ -10533,6 +10547,18 @@ static ConstCastOnly types_match_const_cast_only(IrAnalyze *ira, ZigType *wanted result.id = ConstCastResultIdInvalid; return result; } + bool ok_sentinels = + wanted_ptr_type->data.pointer.sentinel == nullptr || + (actual_ptr_type->data.pointer.sentinel != nullptr && + const_values_equal(ira->codegen, wanted_ptr_type->data.pointer.sentinel, + actual_ptr_type->data.pointer.sentinel)); + if (!ok_sentinels) { + result.id = ConstCastResultIdPtrSentinel; + result.data.bad_ptr_sentinel = allocate_nonzero(1); + result.data.bad_ptr_sentinel->wanted_type = wanted_ptr_type; + result.data.bad_ptr_sentinel->actual_type = actual_ptr_type; + return result; + } if ((!actual_ptr_type->data.pointer.is_const || wanted_ptr_type->data.pointer.is_const) && (!actual_ptr_type->data.pointer.is_volatile || wanted_ptr_type->data.pointer.is_volatile) && actual_ptr_type->data.pointer.bit_offset_in_host == wanted_ptr_type->data.pointer.bit_offset_in_host && @@ -19313,6 +19339,20 @@ static ZigType *adjust_ptr_align(CodeGen *g, ZigType *ptr_type, uint32_t new_ali ptr_type->data.pointer.sentinel); } +static ZigType *adjust_ptr_sentinel(CodeGen *g, ZigType *ptr_type, ZigValue *new_sentinel) { + assert(ptr_type->id == ZigTypeIdPointer); + return get_pointer_to_type_extra2(g, + ptr_type->data.pointer.child_type, + ptr_type->data.pointer.is_const, ptr_type->data.pointer.is_volatile, + ptr_type->data.pointer.ptr_len, + ptr_type->data.pointer.explicit_alignment, + ptr_type->data.pointer.bit_offset_in_host, ptr_type->data.pointer.host_int_bytes, + ptr_type->data.pointer.allow_zero, + ptr_type->data.pointer.vector_index, + ptr_type->data.pointer.inferred_struct_field, + new_sentinel); +} + static ZigType *adjust_slice_align(CodeGen *g, ZigType *slice_type, uint32_t new_align) { assert(is_slice(slice_type)); ZigType *ptr_type = adjust_ptr_align(g, slice_type->data.structure.fields[slice_ptr_index]->type_entry, @@ -25051,50 +25091,70 @@ static IrInstruction *ir_analyze_instruction_slice(IrAnalyze *ira, IrInstruction end = nullptr; } - ZigType *return_type; + ZigType *non_sentinel_slice_ptr_type; + ZigType *elem_type; if (array_type->id == ZigTypeIdArray) { + elem_type = array_type->data.array.child_type; bool is_comptime_const = ptr_ptr->value->special == ConstValSpecialStatic && ptr_ptr->value->data.x_ptr.mut == ConstPtrMutComptimeConst; - ZigType *slice_ptr_type = get_pointer_to_type_extra(ira->codegen, array_type->data.array.child_type, + non_sentinel_slice_ptr_type = get_pointer_to_type_extra(ira->codegen, elem_type, ptr_ptr_type->data.pointer.is_const || is_comptime_const, ptr_ptr_type->data.pointer.is_volatile, PtrLenUnknown, ptr_ptr_type->data.pointer.explicit_alignment, 0, 0, false); - return_type = get_slice_type(ira->codegen, slice_ptr_type); } else if (array_type->id == ZigTypeIdPointer) { if (array_type->data.pointer.ptr_len == PtrLenSingle) { ZigType *main_type = array_type->data.pointer.child_type; if (main_type->id == ZigTypeIdArray) { - ZigType *slice_ptr_type = get_pointer_to_type_extra(ira->codegen, - main_type->data.pointer.child_type, + elem_type = main_type->data.pointer.child_type; + non_sentinel_slice_ptr_type = get_pointer_to_type_extra(ira->codegen, + elem_type, array_type->data.pointer.is_const, array_type->data.pointer.is_volatile, PtrLenUnknown, array_type->data.pointer.explicit_alignment, 0, 0, false); - return_type = get_slice_type(ira->codegen, slice_ptr_type); } else { ir_add_error(ira, &instruction->base, buf_sprintf("slice of single-item pointer")); return ira->codegen->invalid_instruction; } } else { + elem_type = array_type->data.pointer.child_type; if (array_type->data.pointer.ptr_len == PtrLenC) { array_type = adjust_ptr_len(ira->codegen, array_type, PtrLenUnknown); } - return_type = get_slice_type(ira->codegen, array_type); + non_sentinel_slice_ptr_type = array_type; if (!end) { ir_add_error(ira, &instruction->base, buf_sprintf("slice of pointer must include end value")); return ira->codegen->invalid_instruction; } } } else if (is_slice(array_type)) { - ZigType *ptr_type = array_type->data.structure.fields[slice_ptr_index]->type_entry; - return_type = get_slice_type(ira->codegen, ptr_type); + non_sentinel_slice_ptr_type = array_type->data.structure.fields[slice_ptr_index]->type_entry; + elem_type = non_sentinel_slice_ptr_type->data.pointer.child_type; } else { ir_add_error(ira, &instruction->base, buf_sprintf("slice of non-array type '%s'", buf_ptr(&array_type->name))); return ira->codegen->invalid_instruction; } + ZigType *return_type; + ZigValue *sentinel_val = nullptr; + if (instruction->sentinel) { + IrInstruction *uncasted_sentinel = instruction->sentinel->child; + if (type_is_invalid(uncasted_sentinel->value->type)) + return ira->codegen->invalid_instruction; + IrInstruction *sentinel = ir_implicit_cast(ira, uncasted_sentinel, elem_type); + if (type_is_invalid(sentinel->value->type)) + return ira->codegen->invalid_instruction; + sentinel_val = ir_resolve_const(ira, sentinel, UndefBad); + if (sentinel_val == nullptr) + return ira->codegen->invalid_instruction; + ZigType *slice_ptr_type = adjust_ptr_sentinel(ira->codegen, non_sentinel_slice_ptr_type, sentinel_val); + return_type = get_slice_type(ira->codegen, slice_ptr_type); + } else { + return_type = get_slice_type(ira->codegen, non_sentinel_slice_ptr_type); + } + if (instr_is_comptime(ptr_ptr) && value_is_comptime(casted_start->value) && (!end || value_is_comptime(end->value))) diff --git a/src/parser.cpp b/src/parser.cpp index 4b5e0e3ebb..a0c5cf794b 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -2723,7 +2723,7 @@ static AstNode *ast_parse_prefix_type_op(ParseContext *pc) { } // SuffixOp -// <- LBRACKET Expr (DOT2 Expr?)? RBRACKET +// <- LBRACKET Expr (DOT2 (Expr (COLON Expr)?)?)? RBRACKET // / DOT IDENTIFIER // / DOTASTERISK // / DOTQUESTIONMARK @@ -2733,12 +2733,17 @@ static AstNode *ast_parse_suffix_op(ParseContext *pc) { AstNode *start = ast_expect(pc, ast_parse_expr); AstNode *end = nullptr; if (eat_token_if(pc, TokenIdEllipsis2) != nullptr) { + AstNode *sentinel = nullptr; end = ast_parse_expr(pc); + if (eat_token_if(pc, TokenIdColon) != nullptr) { + sentinel = ast_parse_expr(pc); + } expect_token(pc, TokenIdRBracket); AstNode *res = ast_create_node(pc, NodeTypeSliceExpr, lbracket); res->data.slice_expr.start = start; res->data.slice_expr.end = end; + res->data.slice_expr.sentinel = sentinel; return res; } @@ -3041,6 +3046,7 @@ void ast_visit_node_children(AstNode *node, void (*visit)(AstNode **, void *cont visit_field(&node->data.slice_expr.array_ref_expr, visit, context); visit_field(&node->data.slice_expr.start, visit, context); visit_field(&node->data.slice_expr.end, visit, context); + visit_field(&node->data.slice_expr.sentinel, visit, context); break; case NodeTypeFieldAccessExpr: visit_field(&node->data.field_access_expr.struct_expr, visit, context); diff --git a/test/compile_errors.zig b/test/compile_errors.zig index c826d45cea..450f91b9be 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -2,6 +2,17 @@ const tests = @import("tests.zig"); const builtin = @import("builtin"); pub fn addCases(cases: *tests.CompileErrorContext) void { + cases.add("slice sentinel mismatch", + \\fn foo() [:0]u8 { + \\ var x: []u8 = undefined; + \\ return x; + \\} + \\comptime { _ = foo; } + , &[_][]const u8{ + "tmp.zig:3:12: error: expected type '[:0]u8', found '[]u8'", + "tmp.zig:3:12: note: destination pointer requires a terminating '0' sentinel", + }); + cases.add("intToPtr with misaligned address", \\pub fn main() void { \\ var y = @intToPtr([*]align(4) u8, 5);