Skip to content

Make @typeInfo return null-terminated strings #18470

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 4 commits into from
Jan 8, 2024
Merged

Make @typeInfo return null-terminated strings #18470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a02bd81
Make `@typeInfo` return null-terminated strings
castholm Jan 7, 2024
c8fa767
Work around stage1 not yet returning null-terminated `@typeInfo` strings
castholm Jan 7, 2024
9245809
Fix failing type reifications
castholm Jan 7, 2024
01b48e9
InternPool: Fix rare buffer overflow when initializing aggregate with…
castholm Jan 7, 2024
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
2 changes: 1 addition & 1 deletion deps/aro/aro/Attribute.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ pub const Arguments = blk: {
var union_fields: [decls.len]ZigType.UnionField = undefined;
for (decls, &union_fields) |decl, *field| {
field.* = .{
.name = decl.name,
.name = decl.name ++ "",
.type = @field(attributes, decl.name),
.alignment = 0,
};
Expand Down
10 changes: 5 additions & 5 deletions lib/std/builtin.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ pub const Type = union(enum) {
/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const StructField = struct {
name: []const u8,
name: [:0]const u8,
type: type,
default_value: ?*const anyopaque,
is_comptime: bool,
Expand Down Expand Up @@ -348,7 +348,7 @@ pub const Type = union(enum) {
/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const Error = struct {
name: []const u8,
name: [:0]const u8,
};

/// This data structure is used by the Zig language code generation and
Expand All @@ -358,7 +358,7 @@ pub const Type = union(enum) {
/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const EnumField = struct {
name: []const u8,
name: [:0]const u8,
value: comptime_int,
};

Expand All @@ -374,7 +374,7 @@ pub const Type = union(enum) {
/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const UnionField = struct {
name: []const u8,
name: [:0]const u8,
type: type,
alignment: comptime_int,
};
Expand Down Expand Up @@ -436,7 +436,7 @@ pub const Type = union(enum) {
/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const Declaration = struct {
name: []const u8,
name: [:0]const u8,
};
};

Expand Down
2 changes: 1 addition & 1 deletion lib/std/enums.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub fn EnumFieldStruct(comptime E: type, comptime Data: type, comptime field_def
var fields: []const StructField = &[_]StructField{};
for (std.meta.fields(E)) |field| {
fields = fields ++ &[_]StructField{.{
.name = field.name,
.name = field.name ++ "",
.type = Data,
.default_value = if (field_default) |d| @as(?*const anyopaque, @ptrCast(&d)) else null,
.is_comptime = false,
Expand Down
2 changes: 1 addition & 1 deletion lib/std/io.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ pub fn PollFiles(comptime StreamEnum: type) type {
var struct_fields: [enum_fields.len]std.builtin.Type.StructField = undefined;
for (&struct_fields, enum_fields) |*struct_field, enum_field| {
struct_field.* = .{
.name = enum_field.name,
.name = enum_field.name ++ "",
.type = fs.File,
.default_value = null,
.is_comptime = false,
Expand Down
6 changes: 3 additions & 3 deletions lib/std/meta.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ pub fn FieldEnum(comptime T: type) type {
var decls = [_]std.builtin.Type.Declaration{};
inline for (field_infos, 0..) |field, i| {
enumFields[i] = .{
.name = field.name,
.name = field.name ++ "",
.value = i,
};
}
Expand Down Expand Up @@ -628,7 +628,7 @@ pub fn DeclEnum(comptime T: type) type {
var enumDecls: [fieldInfos.len]std.builtin.Type.EnumField = undefined;
var decls = [_]std.builtin.Type.Declaration{};
inline for (fieldInfos, 0..) |field, i| {
enumDecls[i] = .{ .name = field.name, .value = i };
enumDecls[i] = .{ .name = field.name ++ "", .value = i };
}
return @Type(.{
.Enum = .{
Expand Down Expand Up @@ -1015,7 +1015,7 @@ fn CreateUniqueTuple(comptime N: comptime_int, comptime types: [N]type) type {
@setEvalBranchQuota(10_000);
var num_buf: [128]u8 = undefined;
tuple_fields[i] = .{
.name = std.fmt.bufPrint(&num_buf, "{d}", .{i}) catch unreachable,
.name = std.fmt.bufPrintZ(&num_buf, "{d}", .{i}) catch unreachable,
.type = T,
.default_value = null,
.is_comptime = false,
Expand Down
2 changes: 1 addition & 1 deletion src/InternPool.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5315,7 +5315,7 @@ pub fn get(ip: *InternPool, gpa: Allocator, key: Key) Allocator.Error!Index {

try ip.extra.ensureUnusedCapacity(
gpa,
@typeInfo(Tag.Aggregate).Struct.fields.len + @as(usize, @intCast(len_including_sentinel)),
@typeInfo(Tag.Aggregate).Struct.fields.len + @as(usize, @intCast(len_including_sentinel + 1)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CI run for Linux ran into a buffer overflow panic when initializing a sentinel-terminated array for reasons seemingly unrelated to the @typeInfo changes.
https://github.com/ziglang/zig/actions/runs/7439194713/job/20238792974

I based this fix on

zig/src/InternPool.zig

Line 5274 in 804cee3

try ip.string_bytes.ensureUnusedCapacity(gpa, @intCast(len_including_sentinel + 1));

I'm not at all familiar with compiler internals so I could be completely off base, but from skimming through code from zirArrayInit to here it appears to me that the problem is that aggregate.storage.elems includes the sentinel

const final_len = try sema.usizeCast(block, src, array_ty.arrayLenIncludingSentinel(mod));
const resolved_args = try gpa.alloc(Air.Inst.Ref, final_len);
// ...
const elem_vals = try sema.arena.alloc(InternPool.Index, resolved_args.len);
// ...
const arr_val = try mod.intern(.{ .aggregate = .{
    .ty = array_ty.toIntern(),
    .storage = .{ .elems = elem_vals },
} });

which makes these two operations require an available capacity of len_including_sentinel + 1

ip.extra.appendSliceAssumeCapacity(@ptrCast(aggregate.storage.elems));
if (sentinel != .none) ip.extra.appendAssumeCapacity(@intFromEnum(sentinel));

Judging by the related (?) fix in 5fa260b it's possible that elems should be sliced to exclude the sentinel

-ip.extra.appendSliceAssumeCapacity(@ptrCast(aggregate.storage.elems));
+ip.extra.appendSliceAssumeCapacity(@ptrCast(aggregate.storage.elems[0..@intCast(len)]));
 if (sentinel != .none) ip.extra.appendAssumeCapacity(@intFromEnum(sentinel));

but I have no idea what the consequences of doing so would be so I decided not to touch it.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the notes

);
ip.items.appendAssumeCapacity(.{
.tag = .aggregate,
Expand Down
56 changes: 31 additions & 25 deletions src/Sema.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17259,28 +17259,29 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
const vals = try sema.arena.alloc(InternPool.Index, names.len);
for (vals, 0..) |*field_val, i| {
// TODO: write something like getCoercedInts to avoid needing to dupe
const name = try sema.arena.dupe(u8, ip.stringToSlice(names.get(ip)[i]));
const name = try sema.arena.dupeZ(u8, ip.stringToSlice(names.get(ip)[i]));
const name_val = v: {
const new_decl_ty = try mod.arrayType(.{
.len = name.len,
.sentinel = .zero_u8,
.child = .u8_type,
});
const new_decl_val = try mod.intern(.{ .aggregate = .{
.ty = new_decl_ty.toIntern(),
.storage = .{ .bytes = name },
} });
break :v try mod.intern(.{ .ptr = .{
.ty = .slice_const_u8_type,
.ty = .slice_const_u8_sentinel_0_type,
.addr = .{ .anon_decl = .{
.val = new_decl_val,
.orig_ty = .slice_const_u8_type,
.orig_ty = .slice_const_u8_sentinel_0_type,
} },
.len = (try mod.intValue(Type.usize, name.len)).toIntern(),
} });
};

const error_field_fields = .{
// name: []const u8,
// name: [:0]const u8,
name_val,
};
field_val.* = try mod.intern(.{ .aggregate = .{
Expand Down Expand Up @@ -17387,28 +17388,29 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
else
(try mod.intValue(Type.comptime_int, i)).toIntern();
// TODO: write something like getCoercedInts to avoid needing to dupe
const name = try sema.arena.dupe(u8, ip.stringToSlice(enum_type.names.get(ip)[i]));
const name = try sema.arena.dupeZ(u8, ip.stringToSlice(enum_type.names.get(ip)[i]));
const name_val = v: {
const new_decl_ty = try mod.arrayType(.{
.len = name.len,
.sentinel = .zero_u8,
.child = .u8_type,
});
const new_decl_val = try mod.intern(.{ .aggregate = .{
.ty = new_decl_ty.toIntern(),
.storage = .{ .bytes = name },
} });
break :v try mod.intern(.{ .ptr = .{
.ty = .slice_const_u8_type,
.ty = .slice_const_u8_sentinel_0_type,
.addr = .{ .anon_decl = .{
.val = new_decl_val,
.orig_ty = .slice_const_u8_type,
.orig_ty = .slice_const_u8_sentinel_0_type,
} },
.len = (try mod.intValue(Type.usize, name.len)).toIntern(),
} });
};

const enum_field_fields = .{
// name: []const u8,
// name: [:0]const u8,
name_val,
// value: comptime_int,
value_val,
Expand Down Expand Up @@ -17512,21 +17514,22 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai

for (union_field_vals, 0..) |*field_val, i| {
// TODO: write something like getCoercedInts to avoid needing to dupe
const name = try sema.arena.dupe(u8, ip.stringToSlice(union_obj.field_names.get(ip)[i]));
const name = try sema.arena.dupeZ(u8, ip.stringToSlice(union_obj.field_names.get(ip)[i]));
const name_val = v: {
const new_decl_ty = try mod.arrayType(.{
.len = name.len,
.sentinel = .zero_u8,
.child = .u8_type,
});
const new_decl_val = try mod.intern(.{ .aggregate = .{
.ty = new_decl_ty.toIntern(),
.storage = .{ .bytes = name },
} });
break :v try mod.intern(.{ .ptr = .{
.ty = .slice_const_u8_type,
.ty = .slice_const_u8_sentinel_0_type,
.addr = .{ .anon_decl = .{
.val = new_decl_val,
.orig_ty = .slice_const_u8_type,
.orig_ty = .slice_const_u8_sentinel_0_type,
} },
.len = (try mod.intValue(Type.usize, name.len)).toIntern(),
} });
Expand All @@ -17539,7 +17542,7 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai

const field_ty = union_obj.field_types.get(ip)[i];
const union_field_fields = .{
// name: []const u8,
// name: [:0]const u8,
name_val,
// type: type,
field_ty,
Expand Down Expand Up @@ -17658,22 +17661,23 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
// TODO: write something like getCoercedInts to avoid needing to dupe
const bytes = if (tuple.names.len != 0)
// https://github.com/ziglang/zig/issues/15709
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can this comment be removed? The issue was closed in May last year.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it can. But maybe it will be nicer if I just merge your PR for now since it's already green.

try sema.arena.dupe(u8, ip.stringToSlice(ip.indexToKey(ty.toIntern()).anon_struct_type.names.get(ip)[i]))
try sema.arena.dupeZ(u8, ip.stringToSlice(ip.indexToKey(ty.toIntern()).anon_struct_type.names.get(ip)[i]))
else
try std.fmt.allocPrint(sema.arena, "{d}", .{i});
try std.fmt.allocPrintZ(sema.arena, "{d}", .{i});
const new_decl_ty = try mod.arrayType(.{
.len = bytes.len,
.sentinel = .zero_u8,
.child = .u8_type,
});
const new_decl_val = try mod.intern(.{ .aggregate = .{
.ty = new_decl_ty.toIntern(),
.storage = .{ .bytes = bytes },
} });
break :v try mod.intern(.{ .ptr = .{
.ty = .slice_const_u8_type,
.ty = .slice_const_u8_sentinel_0_type,
.addr = .{ .anon_decl = .{
.val = new_decl_val,
.orig_ty = .slice_const_u8_type,
.orig_ty = .slice_const_u8_sentinel_0_type,
} },
.len = (try mod.intValue(Type.usize, bytes.len)).toIntern(),
} });
Expand All @@ -17685,7 +17689,7 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
const opt_default_val = if (is_comptime) Value.fromInterned(field_val) else null;
const default_val_ptr = try sema.optRefValue(opt_default_val);
const struct_field_fields = .{
// name: []const u8,
// name: [:0]const u8,
name_val,
// type: type,
field_ty,
Expand Down Expand Up @@ -17713,7 +17717,7 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
for (struct_field_vals, 0..) |*field_val, i| {
// TODO: write something like getCoercedInts to avoid needing to dupe
const name = if (struct_type.fieldName(ip, i).unwrap()) |name_nts|
try sema.arena.dupe(u8, ip.stringToSlice(name_nts))
try sema.arena.dupeZ(u8, ip.stringToSlice(name_nts))
else
try std.fmt.allocPrintZ(sema.arena, "{d}", .{i});
const field_ty = Type.fromInterned(struct_type.field_types.get(ip)[i]);
Expand All @@ -17722,17 +17726,18 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
const name_val = v: {
const new_decl_ty = try mod.arrayType(.{
.len = name.len,
.sentinel = .zero_u8,
.child = .u8_type,
});
const new_decl_val = try mod.intern(.{ .aggregate = .{
.ty = new_decl_ty.toIntern(),
.storage = .{ .bytes = name },
} });
break :v try mod.intern(.{ .ptr = .{
.ty = .slice_const_u8_type,
.ty = .slice_const_u8_sentinel_0_type,
.addr = .{ .anon_decl = .{
.val = new_decl_val,
.orig_ty = .slice_const_u8_type,
.orig_ty = .slice_const_u8_sentinel_0_type,
} },
.len = (try mod.intValue(Type.usize, name.len)).toIntern(),
} });
Expand All @@ -17750,7 +17755,7 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
};

const struct_field_fields = .{
// name: []const u8,
// name: [:0]const u8,
name_val,
// type: type,
field_ty.toIntern(),
Expand Down Expand Up @@ -17957,27 +17962,28 @@ fn typeInfoNamespaceDecls(
if (decl.kind != .named or !decl.is_pub) continue;
const name_val = v: {
// TODO: write something like getCoercedInts to avoid needing to dupe
const name = try sema.arena.dupe(u8, ip.stringToSlice(decl.name));
const name = try sema.arena.dupeZ(u8, ip.stringToSlice(decl.name));
const new_decl_ty = try mod.arrayType(.{
.len = name.len,
.sentinel = .zero_u8,
.child = .u8_type,
});
const new_decl_val = try mod.intern(.{ .aggregate = .{
.ty = new_decl_ty.toIntern(),
.storage = .{ .bytes = name },
} });
break :v try mod.intern(.{ .ptr = .{
.ty = .slice_const_u8_type,
.ty = .slice_const_u8_sentinel_0_type,
.addr = .{ .anon_decl = .{
.orig_ty = .slice_const_u8_type,
.orig_ty = .slice_const_u8_sentinel_0_type,
.val = new_decl_val,
} },
.len = (try mod.intValue(Type.usize, name.len)).toIntern(),
} });
};

const fields = .{
//name: []const u8,
//name: [:0]const u8,
name_val,
};
try decl_vals.append(try mod.intern(.{ .aggregate = .{
Expand Down
2 changes: 1 addition & 1 deletion src/value.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4050,7 +4050,7 @@ pub const Value = struct {
const tags = @typeInfo(Tag).Enum.fields;
var fields: [tags.len]std.builtin.Type.StructField = undefined;
for (&fields, tags) |*field, t| field.* = .{
.name = t.name,
.name = t.name ++ "",
.type = *@field(Tag, t.name).Type(),
.default_value = null,
.is_comptime = false,
Expand Down
4 changes: 2 additions & 2 deletions test/behavior/type.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ test "Type.Fn" {

test "reified struct field name from optional payload" {
comptime {
const m_name: ?[1]u8 = "a".*;
const m_name: ?[1:0]u8 = "a".*;
if (m_name) |*name| {
const T = @Type(.{ .Struct = .{
.layout = .Auto,
Expand Down Expand Up @@ -711,7 +711,7 @@ test "struct field names sliced at comptime from larger string" {
while (it.next()) |name| {
fields = fields ++ &[_]Type.StructField{.{
.alignment = 0,
.name = name,
.name = name ++ "",
.type = usize,
.default_value = null,
.is_comptime = false,
Expand Down