From 9afc4fe0e2114ae0ed53d48d98f5e12b6a269339 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 5 May 2022 14:27:20 -0700 Subject: [PATCH] Sema: solve a false positive "depends on itself" This improves the ABI alignment resolution code. This commit fully enables the MachO linker code in stage3. Note, however, that there are still miscompilations in stage3. --- lib/std/array_hash_map.zig | 4 -- src/Sema.zig | 1 + src/link/MachO.zig | 3 - src/link/MachO/Dylib.zig | 8 +-- src/link/tapi/yaml.zig | 2 +- src/type.zig | 129 ++++++++++++++++++++++++++----------- test/behavior/eval.zig | 111 +++++++++++++++++++++++++++++++ 7 files changed, 208 insertions(+), 50 deletions(-) diff --git a/lib/std/array_hash_map.zig b/lib/std/array_hash_map.zig index 304c98a2a9..d3c3677747 100644 --- a/lib/std/array_hash_map.zig +++ b/lib/std/array_hash_map.zig @@ -82,10 +82,6 @@ pub fn ArrayHashMap( allocator: Allocator, ctx: Context, - comptime { - std.hash_map.verifyContext(Context, K, K, u32, true); - } - /// The ArrayHashMapUnmanaged type using the same settings as this managed map. pub const Unmanaged = ArrayHashMapUnmanaged(K, V, Context, store_hash); diff --git a/src/Sema.zig b/src/Sema.zig index 9b4977c616..76edfbf2cd 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -18073,6 +18073,7 @@ fn elemValSlice( const is_in_bounds = try block.addBinOp(cmp_op, elem_index, len_inst); try sema.addSafetyCheck(block, is_in_bounds, .index_out_of_bounds); } + try sema.queueFullTypeResolution(sema.typeOf(slice)); return block.addBinOp(.slice_elem_val, slice, elem_index); } diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 7caccb1fb8..b6fed43172 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -1322,9 +1322,6 @@ pub fn parseDylib(self: *MachO, path: []const u8, opts: DylibCreateOpts) ParseDy error.EndOfStream, error.NotDylib => { try file.seekTo(0); - // TODO https://github.com/ziglang/zig/issues/11367 - if (@import("builtin").zig_backend != .stage1) return error.Unexpected; - var lib_stub = LibStub.loadFromFile(self.base.allocator, file) catch { dylib.deinit(self.base.allocator); return false; diff --git a/src/link/MachO/Dylib.zig b/src/link/MachO/Dylib.zig index 26bac50144..833c976bf9 100644 --- a/src/link/MachO/Dylib.zig +++ b/src/link/MachO/Dylib.zig @@ -242,20 +242,20 @@ fn addObjCClassSymbol(self: *Dylib, allocator: Allocator, sym_name: []const u8) for (expanded) |sym| { if (self.symbols.contains(sym)) continue; - try self.symbols.putNoClobber(allocator, sym, .{}); + try self.symbols.putNoClobber(allocator, sym, {}); } } fn addObjCIVarSymbol(self: *Dylib, allocator: Allocator, sym_name: []const u8) !void { const expanded = try std.fmt.allocPrint(allocator, "_OBJC_IVAR_$_{s}", .{sym_name}); if (self.symbols.contains(expanded)) return; - try self.symbols.putNoClobber(allocator, expanded, .{}); + try self.symbols.putNoClobber(allocator, expanded, {}); } fn addObjCEhTypeSymbol(self: *Dylib, allocator: Allocator, sym_name: []const u8) !void { const expanded = try std.fmt.allocPrint(allocator, "_OBJC_EHTYPE_$_{s}", .{sym_name}); if (self.symbols.contains(expanded)) return; - try self.symbols.putNoClobber(allocator, expanded, .{}); + try self.symbols.putNoClobber(allocator, expanded, {}); } fn addSymbol(self: *Dylib, allocator: Allocator, sym_name: []const u8) !void { @@ -373,7 +373,7 @@ pub fn parseFromStub( // TODO I thought that we could switch on presence of `parent-umbrella` map; // however, turns out `libsystem_notify.dylib` is fully reexported by `libSystem.dylib` // BUT does not feature a `parent-umbrella` map as the only sublib. Apple's bug perhaps? - try umbrella_libs.put(elem.installName(), .{}); + try umbrella_libs.put(elem.installName(), {}); } switch (elem) { diff --git a/src/link/tapi/yaml.zig b/src/link/tapi/yaml.zig index 7c1997604d..e31aa41985 100644 --- a/src/link/tapi/yaml.zig +++ b/src/link/tapi/yaml.zig @@ -153,7 +153,7 @@ pub const Value = union(ValueType) { if (node.cast(Node.Doc)) |doc| { const inner = doc.value orelse { // empty doc - return Value{ .empty = .{} }; + return Value{ .empty = {} }; }; return Value.fromNode(arena, tree, inner, null); } else if (node.cast(Node.Map)) |map| { diff --git a/src/type.zig b/src/type.zig index ddeec596e1..049f7ec856 100644 --- a/src/type.zig +++ b/src/type.zig @@ -2394,11 +2394,15 @@ pub const Type = extern union { _ = try sk.sema.typeRequiresComptime(sk.block, sk.src, ty); } switch (struct_obj.requires_comptime) { - .wip => unreachable, .yes => return false, - .no => if (struct_obj.known_non_opv) return true, + .wip, .no => if (struct_obj.known_non_opv) return true, .unknown => {}, } + if (struct_obj.status == .field_types_wip) { + // In this case, we guess that hasRuntimeBits() for this type is true, + // and then later if our guess was incorrect, we emit a compile error. + return true; + } if (sema_kit) |sk| { _ = try sk.sema.resolveTypeFields(sk.block, sk.src, ty); } @@ -2735,6 +2739,12 @@ pub const Type = extern union { val: Value, }; + const AbiAlignmentAdvancedStrat = union(enum) { + eager, + lazy: Allocator, + sema_kit: Module.WipAnalysis, + }; + /// If you pass `eager` you will get back `scalar` and assert the type is resolved. /// In this case there will be no error, guaranteed. /// If you pass `lazy` you may get back `scalar` or `val`. @@ -2744,11 +2754,7 @@ pub const Type = extern union { pub fn abiAlignmentAdvanced( ty: Type, target: Target, - strat: union(enum) { - eager, - lazy: Allocator, - sema_kit: Module.WipAnalysis, - }, + strat: AbiAlignmentAdvancedStrat, ) Module.CompileError!AbiAlignmentAdvanced { const sema_kit = switch (strat) { .sema_kit => |sk| sk, @@ -2928,21 +2934,24 @@ pub const Type = extern union { }, .@"struct" => { + const struct_obj = ty.castTag(.@"struct").?.data; if (sema_kit) |sk| { - try sk.sema.resolveTypeLayout(sk.block, sk.src, ty); - } - if (ty.castTag(.@"struct")) |payload| { - const struct_obj = payload.data; - if (!struct_obj.haveLayout()) switch (strat) { - .eager => unreachable, // struct layout not resolved - .sema_kit => unreachable, // handled above - .lazy => |arena| return AbiAlignmentAdvanced{ .val = try Value.Tag.lazy_align.create(arena, ty) }, - }; - if (struct_obj.layout == .Packed) { - var buf: Type.Payload.Bits = undefined; - const int_ty = struct_obj.packedIntegerType(target, &buf); - return AbiAlignmentAdvanced{ .scalar = int_ty.abiAlignment(target) }; + if (struct_obj.status == .field_types_wip) { + // We'll guess "pointer-aligned" and if we guess wrong, emit + // a compile error later. + return AbiAlignmentAdvanced{ .scalar = @divExact(target.cpu.arch.ptrBitWidth(), 8) }; } + _ = try sk.sema.resolveTypeFields(sk.block, sk.src, ty); + } + if (!struct_obj.haveFieldTypes()) switch (strat) { + .eager => unreachable, // struct layout not resolved + .sema_kit => unreachable, // handled above + .lazy => |arena| return AbiAlignmentAdvanced{ .val = try Value.Tag.lazy_align.create(arena, ty) }, + }; + if (struct_obj.layout == .Packed) { + var buf: Type.Payload.Bits = undefined; + const int_ty = struct_obj.packedIntegerType(target, &buf); + return AbiAlignmentAdvanced{ .scalar = int_ty.abiAlignment(target) }; } const fields = ty.structFields(); @@ -2950,7 +2959,16 @@ pub const Type = extern union { for (fields.values()) |field| { if (!(try field.ty.hasRuntimeBitsAdvanced(false, sema_kit))) continue; - const field_align = field.normalAlignment(target); + const field_align = if (field.abi_align != 0) + field.abi_align + else switch (try field.ty.abiAlignmentAdvanced(target, strat)) { + .scalar => |a| a, + .val => switch (strat) { + .eager => unreachable, // struct layout not resolved + .sema_kit => unreachable, // handled above + .lazy => |arena| return AbiAlignmentAdvanced{ .val = try Value.Tag.lazy_align.create(arena, ty) }, + }, + }; big_align = @maximum(big_align, field_align); } return AbiAlignmentAdvanced{ .scalar = big_align }; @@ -2980,24 +2998,14 @@ pub const Type = extern union { const int_tag_ty = ty.intTagType(&buffer); return AbiAlignmentAdvanced{ .scalar = int_tag_ty.abiAlignment(target) }; }, - .@"union" => switch (strat) { - .eager, .sema_kit => { - if (sema_kit) |sk| { - try sk.sema.resolveTypeLayout(sk.block, sk.src, ty); - } - // TODO pass `true` for have_tag when unions have a safety tag - return AbiAlignmentAdvanced{ .scalar = ty.castTag(.@"union").?.data.abiAlignment(target, false) }; - }, - .lazy => |arena| return AbiAlignmentAdvanced{ .val = try Value.Tag.lazy_align.create(arena, ty) }, + .@"union" => { + const union_obj = ty.castTag(.@"union").?.data; + // TODO pass `true` for have_tag when unions have a safety tag + return abiAlignmentAdvancedUnion(ty, target, strat, union_obj, false); }, - .union_tagged => switch (strat) { - .eager, .sema_kit => { - if (sema_kit) |sk| { - try sk.sema.resolveTypeLayout(sk.block, sk.src, ty); - } - return AbiAlignmentAdvanced{ .scalar = ty.castTag(.union_tagged).?.data.abiAlignment(target, true) }; - }, - .lazy => |arena| return AbiAlignmentAdvanced{ .val = try Value.Tag.lazy_align.create(arena, ty) }, + .union_tagged => { + const union_obj = ty.castTag(.union_tagged).?.data; + return abiAlignmentAdvancedUnion(ty, target, strat, union_obj, true); }, .empty_struct, @@ -3023,6 +3031,51 @@ pub const Type = extern union { }; } + pub fn abiAlignmentAdvancedUnion( + ty: Type, + target: Target, + strat: AbiAlignmentAdvancedStrat, + union_obj: *Module.Union, + have_tag: bool, + ) Module.CompileError!AbiAlignmentAdvanced { + const sema_kit = switch (strat) { + .sema_kit => |sk| sk, + else => null, + }; + if (sema_kit) |sk| { + if (union_obj.status == .field_types_wip) { + // We'll guess "pointer-aligned" and if we guess wrong, emit + // a compile error later. + return AbiAlignmentAdvanced{ .scalar = @divExact(target.cpu.arch.ptrBitWidth(), 8) }; + } + _ = try sk.sema.resolveTypeFields(sk.block, sk.src, ty); + } + if (!union_obj.haveFieldTypes()) switch (strat) { + .eager => unreachable, // union layout not resolved + .sema_kit => unreachable, // handled above + .lazy => |arena| return AbiAlignmentAdvanced{ .val = try Value.Tag.lazy_align.create(arena, ty) }, + }; + + var max_align: u32 = 0; + if (have_tag) max_align = union_obj.tag_ty.abiAlignment(target); + for (union_obj.fields.values()) |field| { + if (!(try field.ty.hasRuntimeBitsAdvanced(false, sema_kit))) continue; + + const field_align = if (field.abi_align != 0) + field.abi_align + else switch (try field.ty.abiAlignmentAdvanced(target, strat)) { + .scalar => |a| a, + .val => switch (strat) { + .eager => unreachable, // struct layout not resolved + .sema_kit => unreachable, // handled above + .lazy => |arena| return AbiAlignmentAdvanced{ .val = try Value.Tag.lazy_align.create(arena, ty) }, + }, + }; + max_align = @maximum(max_align, field_align); + } + return AbiAlignmentAdvanced{ .scalar = max_align }; + } + /// Asserts the type has the ABI size already resolved. /// Types that return false for hasRuntimeBits() return 0. pub fn abiSize(self: Type, target: Target) u64 { diff --git a/test/behavior/eval.zig b/test/behavior/eval.zig index 3895b4b7b2..007ea48d87 100644 --- a/test/behavior/eval.zig +++ b/test/behavior/eval.zig @@ -999,3 +999,114 @@ test "comptime break operand passing through runtime switch converted to runtime try S.doTheTest('b'); comptime try S.doTheTest('b'); } + +test "no dependency loop for alignment of self struct" { + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + const S = struct { + fn doTheTest() !void { + var a: namespace.A = undefined; + a.d = .{ .g = &buf }; + a.d.g[3] = 42; + a.d.g[3] += 1; + try expect(a.d.g[3] == 43); + } + + var buf: [10]u8 align(@alignOf([*]u8)) = undefined; + + const namespace = struct { + const B = struct { a: A }; + const A = C(B); + }; + + pub fn C(comptime B: type) type { + return struct { + d: D(F) = .{}, + + const F = struct { b: B }; + }; + } + + pub fn D(comptime F: type) type { + return struct { + g: [*]align(@alignOf(F)) u8 = undefined, + }; + } + }; + try S.doTheTest(); +} + +test "no dependency loop for alignment of self bare union" { + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + const S = struct { + fn doTheTest() !void { + var a: namespace.A = undefined; + a.d = .{ .g = &buf }; + a.d.g[3] = 42; + a.d.g[3] += 1; + try expect(a.d.g[3] == 43); + } + + var buf: [10]u8 align(@alignOf([*]u8)) = undefined; + + const namespace = struct { + const B = union { a: A, b: void }; + const A = C(B); + }; + + pub fn C(comptime B: type) type { + return struct { + d: D(F) = .{}, + + const F = struct { b: B }; + }; + } + + pub fn D(comptime F: type) type { + return struct { + g: [*]align(@alignOf(F)) u8 = undefined, + }; + } + }; + try S.doTheTest(); +} + +test "no dependency loop for alignment of self tagged union" { + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + const S = struct { + fn doTheTest() !void { + var a: namespace.A = undefined; + a.d = .{ .g = &buf }; + a.d.g[3] = 42; + a.d.g[3] += 1; + try expect(a.d.g[3] == 43); + } + + var buf: [10]u8 align(@alignOf([*]u8)) = undefined; + + const namespace = struct { + const B = union(enum) { a: A, b: void }; + const A = C(B); + }; + + pub fn C(comptime B: type) type { + return struct { + d: D(F) = .{}, + + const F = struct { b: B }; + }; + } + + pub fn D(comptime F: type) type { + return struct { + g: [*]align(@alignOf(F)) u8 = undefined, + }; + } + }; + try S.doTheTest(); +}