Skip to content

@mem{cpy,set} fixes #15704

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.

Sign up for GitHub

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 3 commits into from
May 16, 2023
Merged

@mem{cpy,set} fixes #15704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab5a72f
Sema: ensure dest ptr of memcpy has length
Vexu May 14, 2023
2703db3
Sema: add more type checks to `@mem{cpy,set}`
Vexu May 14, 2023
0bc5e7b
Sema: use `elemPtrOneLayerOnly` in `zirMemCpy`
Vexu May 14, 2023
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
73 changes: 53 additions & 20 deletions src/Sema.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3416,19 +3416,12 @@ fn indexablePtrLenOrNone(
sema: *Sema,
block: *Block,
src: LazySrcLoc,
object: Air.Inst.Ref,
operand: Air.Inst.Ref,
) CompileError!Air.Inst.Ref {
const object_ty = sema.typeOf(object);
const indexable_ty = t: {
const ptr_size = object_ty.ptrSizeOrNull() orelse break :t object_ty;
break :t switch (ptr_size) {
.Many => return .none,
.One => object_ty.childType(),
else => object_ty,
};
};
try checkIndexable(sema, block, src, indexable_ty);
return sema.fieldVal(block, src, object, "len", src);
const operand_ty = sema.typeOf(operand);
try checkMemOperand(sema, block, src, operand_ty);
if (operand_ty.ptrSize() == .Many) return .none;
return sema.fieldVal(block, src, operand, "len", src);
}

fn zirAllocExtended(
Expand Down Expand Up @@ -22080,19 +22073,25 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
const src_src: LazySrcLoc = .{ .node_offset_builtin_call_arg1 = inst_data.src_node };
const dest_ptr = try sema.resolveInst(extra.lhs);
const src_ptr = try sema.resolveInst(extra.rhs);
const dest_ty = sema.typeOf(dest_ptr);
const src_ty = sema.typeOf(src_ptr);
const dest_len = try indexablePtrLenOrNone(sema, block, dest_src, dest_ptr);
const src_len = try indexablePtrLenOrNone(sema, block, src_src, src_ptr);
const target = sema.mod.getTarget();

if (dest_ty.isConstPtr()) {
return sema.fail(block, dest_src, "cannot memcpy to constant pointer", .{});
}

if (dest_len == .none and src_len == .none) {
const msg = msg: {
const msg = try sema.errMsg(block, src, "unknown @memcpy length", .{});
errdefer msg.destroy(sema.gpa);
try sema.errNote(block, dest_src, msg, "destination type '{}' provides no length", .{
sema.typeOf(dest_ptr).fmt(sema.mod),
dest_ty.fmt(sema.mod),
});
try sema.errNote(block, src_src, msg, "source type '{}' provides no length", .{
sema.typeOf(src_ptr).fmt(sema.mod),
src_ty.fmt(sema.mod),
});
break :msg msg;
};
Expand Down Expand Up @@ -22130,6 +22129,14 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
const ok = try block.addBinOp(.cmp_eq, dest_len, src_len);
try sema.addSafetyCheck(block, ok, .memcpy_len_mismatch);
}
} else if (dest_len != .none) {
if (try sema.resolveDefinedValue(block, dest_src, dest_len)) |dest_len_val| {
len_val = dest_len_val;
}
} else if (src_len != .none) {
if (try sema.resolveDefinedValue(block, src_src, src_len)) |src_len_val| {
len_val = src_len_val;
}
}

const runtime_src = if (try sema.resolveDefinedValue(block, dest_src, dest_ptr)) |dest_ptr_val| rs: {
Expand All @@ -22139,7 +22146,7 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
const len = try sema.usizeCast(block, dest_src, len_u64);
for (0..len) |i| {
const elem_index = try sema.addIntUnsigned(Type.usize, i);
const dest_elem_ptr = try sema.elemPtr(
const dest_elem_ptr = try sema.elemPtrOneLayerOnly(
block,
src,
dest_ptr,
Expand All @@ -22148,7 +22155,7 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
true, // init
false, // oob_safety
);
const src_elem_ptr = try sema.elemPtr(
const src_elem_ptr = try sema.elemPtrOneLayerOnly(
block,
src,
src_ptr,
Expand All @@ -22172,9 +22179,6 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
} else break :rs src_src;
} else dest_src;

const dest_ty = sema.typeOf(dest_ptr);
const src_ty = sema.typeOf(src_ptr);

// If in-memory coercion is not allowed, explode this memcpy call into a
// for loop that copies element-wise.
// Likewise if this is an iterable rather than a pointer, do the same
Expand Down Expand Up @@ -22213,6 +22217,10 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
if (new_src_ptr_ty.isSlice()) {
new_src_ptr = try sema.analyzeSlicePtr(block, src_src, new_src_ptr, new_src_ptr_ty);
}
} else if (dest_len == .none and len_val == null) {
// Change the dest to a slice, since its type must have the length.
const dest_ptr_ptr = try sema.analyzeRef(block, dest_src, new_dest_ptr);
new_dest_ptr = try sema.analyzeSlice(block, dest_src, dest_ptr_ptr, .zero, src_len, .none, .unneeded, dest_src, dest_src, dest_src, false);
}

try sema.requireRuntimeBlock(block, src, runtime_src);
Expand Down Expand Up @@ -22262,7 +22270,11 @@ fn zirMemset(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
const dest_ptr = try sema.resolveInst(extra.lhs);
const uncoerced_elem = try sema.resolveInst(extra.rhs);
const dest_ptr_ty = sema.typeOf(dest_ptr);
try checkIndexable(sema, block, dest_src, dest_ptr_ty);
try checkMemOperand(sema, block, dest_src, dest_ptr_ty);

if (dest_ptr_ty.isConstPtr()) {
return sema.fail(block, dest_src, "cannot memset constant pointer", .{});
}

const dest_elem_ty = dest_ptr_ty.elemType2();
const target = sema.mod.getTarget();
Expand Down Expand Up @@ -31090,6 +31102,27 @@ fn checkIndexable(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) !void {
}
}

fn checkMemOperand(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) !void {
if (ty.zigTypeTag() == .Pointer) {
switch (ty.ptrSize()) {
.Slice, .Many, .C => return,
.One => {
const elem_ty = ty.childType();
if (elem_ty.zigTypeTag() == .Array) return;
// TODO https://github.com/ziglang/zig/issues/15479
// if (elem_ty.isTuple()) return;
},
}
}
const msg = msg: {
const msg = try sema.errMsg(block, src, "type '{}' is not an indexable pointer", .{ty.fmt(sema.mod)});
errdefer msg.destroy(sema.gpa);
try sema.errNote(block, src, msg, "operand must be a slice, a many pointer or a pointer to an array", .{});
break :msg msg;
};
return sema.failWithOwnedErrorMsg(msg);
}

fn resolveUnionLayout(sema: *Sema, ty: Type) CompileError!void {
const resolved_ty = try sema.resolveTypeFields(ty);
const union_obj = resolved_ty.cast(Type.Payload.Union).?.data;
Expand Down
34 changes: 34 additions & 0 deletions test/behavior/memcpy.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,37 @@ fn testMemcpyBothSinglePtrArrayOneIsNullTerminated() !void {
try expect(buf[98] == 'l');
try expect(buf[99] == 'o');
}

test "@memcpy dest many pointer" {
if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;

try testMemcpyDestManyPtr();
try comptime testMemcpyDestManyPtr();
}

fn testMemcpyDestManyPtr() !void {
var str = "hello".*;
var buf: [5]u8 = undefined;
@memcpy(@ptrCast([*]u8, &buf), @ptrCast([*]const u8, &str)[0..5]);
try expect(buf[0] == 'h');
try expect(buf[1] == 'e');
try expect(buf[2] == 'l');
try expect(buf[3] == 'l');
try expect(buf[4] == 'o');
}

comptime {
const S = struct {
buffer: [8]u8 = undefined,
fn set(self: *@This(), items: []const u8) void {
@memcpy(self.buffer[0..items.len], items);
}
};

var s = S{};
s.set("hello");
if (!std.mem.eql(u8, s.buffer[0..5], "hello")) @compileError("bad");
}
25 changes: 21 additions & 4 deletions test/cases/compile_errors/incorrect_type_to_memset_memcpy.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ pub export fn non_matching_lengths() void {
var buf2: [6]u8 = .{ 1, 2, 3, 4, 5, 6 };
@memcpy(&buf2, &buf1);
}
pub export fn memset_const_dest_ptr() void {
const buf: [5]u8 = .{ 1, 2, 3, 4, 5 };
@memset(&buf, 1);
}
pub export fn memcpy_const_dest_ptr() void {
const buf1: [5]u8 = .{ 1, 2, 3, 4, 5 };
var buf2: [5]u8 = .{ 1, 2, 3, 4, 5 };
@memcpy(&buf1, &buf2);
}
pub export fn memset_array() void {
var buf: [5]u8 = .{ 1, 2, 3, 4, 5 };
@memcpy(buf, 1);
}

// error
// backend=stage2
Expand All @@ -27,10 +40,14 @@ pub export fn non_matching_lengths() void {
// :5:5: error: unknown @memcpy length
// :5:18: note: destination type '[*]u8' provides no length
// :5:24: note: source type '[*]align(4) const u8' provides no length
// :10:13: error: type 'u8' does not support indexing
// :10:13: note: operand must be an array, slice, tuple, or vector
// :15:13: error: type '*u8' does not support indexing
// :15:13: note: operand must be an array, slice, tuple, or vector
// :10:13: error: type '*u8' is not an indexable pointer
// :10:13: note: operand must be a slice, a many pointer or a pointer to an array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destination may be a tuple, previous message was more correct

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was that that hasn't been implemented yet #15479?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, it's not implemented yet. It's fine to do as you have it, as long as you're aware 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a link to the issue next to the error message as a reminder.

// :15:13: error: type '*u8' is not an indexable pointer
// :15:13: note: operand must be a slice, a many pointer or a pointer to an array
// :20:5: error: non-matching @memcpy lengths
// :20:13: note: length 6 here
// :20:20: note: length 5 here
// :24:13: error: cannot memset constant pointer
// :29:13: error: cannot memcpy to constant pointer
// :33:13: error: type '[5]u8' is not an indexable pointer
// :33:13: note: operand must be a slice, a many pointer or a pointer to an array