Skip to content

fix std.mem.addNullByte and implement sentinel slicing #3940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/std/buffer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions lib/std/cstr.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
34 changes: 19 additions & 15 deletions lib/std/debug.zig
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ pub fn panic(comptime format: []const u8, args: var) noreturn {
}

/// TODO multithreaded awareness
var panicking: u8 = 0; // TODO make this a bool
var panicking: u8 = 0;

pub fn panicExtra(trace: ?*const builtin.StackTrace, first_trace_addr: ?usize, comptime format: []const u8, args: var) noreturn {
@setCold(true);
Expand All @@ -230,21 +230,25 @@ pub fn panicExtra(trace: ?*const builtin.StackTrace, first_trace_addr: ?usize, c
resetSegfaultHandler();
}

if (@atomicRmw(u8, &panicking, builtin.AtomicRmwOp.Xchg, 1, builtin.AtomicOrder.SeqCst) == 1) {
// Panicked during a panic.

// TODO detect if a different thread caused the panic, because in that case
// we would want to return here instead of calling abort, so that the thread
// which first called panic can finish printing a stack trace.
os.abort();
}
const stderr = getStderrStream();
stderr.print(format ++ "\n", args) catch os.abort();
if (trace) |t| {
dumpStackTrace(t.*);
switch (@atomicRmw(u8, &panicking, .Add, 1, .SeqCst)) {
0 => {
const stderr = getStderrStream();
stderr.print(format ++ "\n", args) catch os.abort();
if (trace) |t| {
dumpStackTrace(t.*);
}
dumpCurrentStackTrace(first_trace_addr);
},
1 => {
// TODO detect if a different thread caused the panic, because in that case
// we would want to return here instead of calling abort, so that the thread
// which first called panic can finish printing a stack trace.
warn("Panicked during a panic. Aborting.\n", .{});
},
else => {
// Panicked while printing "Panicked during a panic."
},
}
dumpCurrentStackTrace(first_trace_addr);

os.abort();
}

Expand Down
9 changes: 4 additions & 5 deletions lib/std/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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" {
Expand Down
9 changes: 5 additions & 4 deletions lib/std/mem.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};
Expand Down Expand Up @@ -363,11 +364,11 @@ pub fn len(comptime T: type, ptr: [*:0]const T) usize {
}

pub fn toSliceConst(comptime T: type, ptr: [*:0]const T) [:0]const T {
return ptr[0..len(T, ptr)];
return ptr[0..len(T, ptr) :0];
}

pub fn toSlice(comptime T: type, ptr: [*:0]T) [:0]T {
return ptr[0..len(T, ptr)];
return ptr[0..len(T, ptr) :0];
}

/// Returns true if all elements in a slice are equal to the scalar value provided
Expand Down
25 changes: 9 additions & 16 deletions lib/std/os.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {},
Expand Down Expand Up @@ -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);
}

Expand All @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions lib/std/zig/ast.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,7 @@ pub const Node = struct {
pub const Slice = struct {
start: *Node,
end: ?*Node,
sentinel: ?*Node,
};
};

Expand Down Expand Up @@ -1732,6 +1733,10 @@ pub const Node = struct {
if (i < 1) return end;
i -= 1;
}
if (range.sentinel) |sentinel| {
if (i < 1) return sentinel;
i -= 1;
}
},
.ArrayInitializer => |*exprs| {
if (i < exprs.len) return exprs.at(i).*;
Expand Down
7 changes: 6 additions & 1 deletion lib/std/zig/parse.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2331,7 +2331,7 @@ fn parsePrefixTypeOp(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node
}

/// SuffixOp
/// <- LBRACKET Expr (DOT2 Expr?)? RBRACKET
/// <- LBRACKET Expr (DOT2 (Expr (COLON Expr)?)?)? RBRACKET
/// / DOT IDENTIFIER
/// / DOTASTERISK
/// / DOTQUESTIONMARK
Expand All @@ -2349,11 +2349,16 @@ fn parseSuffixOp(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {

if (eatToken(it, .Ellipsis2) != null) {
const end_expr = try parseExpr(arena, it, tree);
const sentinel: ?*ast.Node = if (eatToken(it, .Colon) != null)
try parseExpr(arena, it, tree)
else
null;
break :blk OpAndToken{
.op = Op{
.Slice = Op.Slice{
.start = index_expr,
.end = end_expr,
.sentinel = sentinel,
},
},
.token = try expectToken(it, tree, .RBracket),
Expand Down
3 changes: 3 additions & 0 deletions lib/std/zig/parser_test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,13 @@ test "zig fmt: pointer of unknown length" {
test "zig fmt: spaces around slice operator" {
try testCanonical(
\\var a = b[c..d];
\\var a = b[c..d :0];
\\var a = b[c + 1 .. d];
\\var a = b[c + 1 ..];
\\var a = b[c .. d + 1];
\\var a = b[c .. d + 1 :0];
\\var a = b[c.a..d.e];
\\var a = b[c.a..d.e :0];
\\
);
}
Expand Down
8 changes: 7 additions & 1 deletion lib/std/zig/render.zig
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,13 @@ fn renderExpression(
try renderExpression(allocator, stream, tree, indent, start_col, range.start, after_start_space);
try renderToken(tree, stream, dotdot, indent, start_col, after_op_space); // ..
if (range.end) |end| {
try renderExpression(allocator, stream, tree, indent, start_col, end, Space.None);
const after_end_space = if (range.sentinel != null) Space.Space else Space.None;
try renderExpression(allocator, stream, tree, indent, start_col, end, after_end_space);
}
if (range.sentinel) |sentinel| {
const colon = tree.prevToken(sentinel.firstToken());
try renderToken(tree, stream, colon, indent, start_col, Space.None); // :
try renderExpression(allocator, stream, tree, indent, start_col, sentinel, Space.None);
}
return renderToken(tree, stream, suffix_op.rtoken, indent, start_col, space); // ]
},
Expand Down
3 changes: 3 additions & 0 deletions src/all_types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,7 @@ struct AstNodeSliceExpr {
AstNode *array_ref_expr;
AstNode *start;
AstNode *end;
AstNode *sentinel; // can be null
};

struct AstNodeFieldAccessExpr {
Expand Down Expand Up @@ -1778,6 +1779,7 @@ enum PanicMsgId {
PanicMsgIdResumedFnPendingAwait,
PanicMsgIdBadNoAsyncCall,
PanicMsgIdResumeNotSuspendedFn,
PanicMsgIdBadSentinel,

PanicMsgIdCount,
};
Expand Down Expand Up @@ -3388,6 +3390,7 @@ struct IrInstructionSliceSrc {
IrInstruction *ptr;
IrInstruction *start;
IrInstruction *end;
IrInstruction *sentinel;
ResultLoc *result_loc;
};

Expand Down
51 changes: 48 additions & 3 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,8 @@ static Buf *panic_msg_buf(PanicMsgId msg_id) {
return buf_create_from_str("async function called with noasync suspended");
case PanicMsgIdResumeNotSuspendedFn:
return buf_create_from_str("resumed a non-suspended function");
case PanicMsgIdBadSentinel:
return buf_create_from_str("sentinel mismatch");
}
zig_unreachable();
}
Expand Down Expand Up @@ -1419,6 +1421,27 @@ static void add_bounds_check(CodeGen *g, LLVMValueRef target_val,
LLVMPositionBuilderAtEnd(g->builder, ok_block);
}

static void add_sentinel_check(CodeGen *g, LLVMValueRef sentinel_elem_ptr, ZigValue *sentinel) {
LLVMValueRef expected_sentinel = gen_const_val(g, sentinel, "");

LLVMValueRef actual_sentinel = gen_load_untyped(g, sentinel_elem_ptr, 0, false, "");
LLVMValueRef ok_bit;
if (sentinel->type->id == ZigTypeIdFloat) {
ok_bit = LLVMBuildFCmp(g->builder, LLVMRealOEQ, actual_sentinel, expected_sentinel, "");
} else {
ok_bit = LLVMBuildICmp(g->builder, LLVMIntEQ, actual_sentinel, expected_sentinel, "");
}

LLVMBasicBlockRef fail_block = LLVMAppendBasicBlock(g->cur_fn_val, "SentinelFail");
LLVMBasicBlockRef ok_block = LLVMAppendBasicBlock(g->cur_fn_val, "SentinelOk");
LLVMBuildCondBr(g->builder, ok_bit, ok_block, fail_block);

LLVMPositionBuilderAtEnd(g->builder, fail_block);
gen_safety_crash(g, PanicMsgIdBadSentinel);

LLVMPositionBuilderAtEnd(g->builder, ok_block);
}

static LLVMValueRef gen_assert_zero(CodeGen *g, LLVMValueRef expr_val, ZigType *int_type) {
LLVMValueRef zero = LLVMConstNull(get_llvm_type(g, int_type));
LLVMValueRef ok_bit = LLVMBuildICmp(g->builder, LLVMIntEQ, expr_val, zero, "");
Expand Down Expand Up @@ -5244,6 +5267,9 @@ static LLVMValueRef ir_render_slice(CodeGen *g, IrExecutable *executable, IrInst

bool want_runtime_safety = instruction->safety_check_on && ir_want_runtime_safety(g, &instruction->base);

ZigType *res_slice_ptr_type = instruction->base.value->type->data.structure.fields[slice_ptr_index]->type_entry;
ZigValue *sentinel = res_slice_ptr_type->data.pointer.sentinel;

if (array_type->id == ZigTypeIdArray ||
(array_type->id == ZigTypeIdPointer && array_type->data.pointer.ptr_len == PtrLenSingle))
{
Expand All @@ -5265,6 +5291,15 @@ static LLVMValueRef ir_render_slice(CodeGen *g, IrExecutable *executable, IrInst
LLVMValueRef array_end = LLVMConstInt(g->builtin_types.entry_usize->llvm_type,
array_type->data.array.len, false);
add_bounds_check(g, end_val, LLVMIntEQ, nullptr, LLVMIntULE, array_end);

if (sentinel != nullptr) {
LLVMValueRef indices[] = {
LLVMConstNull(g->builtin_types.entry_usize->llvm_type),
end_val,
};
LLVMValueRef sentinel_elem_ptr = LLVMBuildInBoundsGEP(g->builder, array_ptr, indices, 2, "");
add_sentinel_check(g, sentinel_elem_ptr, sentinel);
}
}
}
if (!type_has_bits(array_type)) {
Expand Down Expand Up @@ -5297,6 +5332,10 @@ static LLVMValueRef ir_render_slice(CodeGen *g, IrExecutable *executable, IrInst

if (want_runtime_safety) {
add_bounds_check(g, start_val, LLVMIntEQ, nullptr, LLVMIntULE, end_val);
if (sentinel != nullptr) {
LLVMValueRef sentinel_elem_ptr = LLVMBuildInBoundsGEP(g->builder, array_ptr, &end_val, 1, "");
add_sentinel_check(g, sentinel_elem_ptr, sentinel);
}
}

if (type_has_bits(array_type)) {
Expand Down Expand Up @@ -5337,18 +5376,24 @@ static LLVMValueRef ir_render_slice(CodeGen *g, IrExecutable *executable, IrInst
end_val = prev_end;
}

LLVMValueRef src_ptr_ptr = LLVMBuildStructGEP(g->builder, array_ptr, (unsigned)ptr_index, "");
LLVMValueRef src_ptr = gen_load_untyped(g, src_ptr_ptr, 0, false, "");

if (want_runtime_safety) {
assert(prev_end);
add_bounds_check(g, start_val, LLVMIntEQ, nullptr, LLVMIntULE, end_val);
if (instruction->end) {
add_bounds_check(g, end_val, LLVMIntEQ, nullptr, LLVMIntULE, prev_end);

if (sentinel != nullptr) {
LLVMValueRef sentinel_elem_ptr = LLVMBuildInBoundsGEP(g->builder, src_ptr, &end_val, 1, "");
add_sentinel_check(g, sentinel_elem_ptr, sentinel);
}
}
}

LLVMValueRef src_ptr_ptr = LLVMBuildStructGEP(g->builder, array_ptr, (unsigned)ptr_index, "");
LLVMValueRef src_ptr = gen_load_untyped(g, src_ptr_ptr, 0, false, "");
LLVMValueRef ptr_field_ptr = LLVMBuildStructGEP(g->builder, tmp_struct_ptr, (unsigned)ptr_index, "");
LLVMValueRef slice_start_ptr = LLVMBuildInBoundsGEP(g->builder, src_ptr, &start_val, (unsigned)len_index, "");
LLVMValueRef slice_start_ptr = LLVMBuildInBoundsGEP(g->builder, src_ptr, &start_val, 1, "");
gen_store_untyped(g, slice_start_ptr, ptr_field_ptr, 0, false);

LLVMValueRef len_field_ptr = LLVMBuildStructGEP(g->builder, tmp_struct_ptr, (unsigned)len_index, "");
Expand Down
Loading