From cf4aad4858ac61b4814d8f021c8eae22ee7f63e6 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 30 Mar 2022 23:16:32 -0700 Subject: [PATCH] AstGen: fix referencing unreferencable instructions Sema avoids adding map entries for certain instructions such as `set_eval_branch_quota` and `atomic_store`. This means that result location semantics in AstGen must not emit any instructions that attempt to use the result of any of these instructions. This commit makes AstGen replace such instructions with `Zir.Inst.Ref.void_value` if their result value ends up being referenced. This fixes a compiler crash when running std lib atomic tests. --- src/AstGen.zig | 11 +- src/Zir.zig | 267 ++++++++++++++++++++++++++++++++++++++ test/behavior/atomics.zig | 34 +++++ 3 files changed, 311 insertions(+), 1 deletion(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index 0fe64ec36b2a70826021696c7b01b494d9ebc6f7..0d52a1a2bc084edb45a8b67f807b869e5db46a00 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -8924,9 +8924,18 @@ fn nodeImpliesComptimeOnly(tree: *const Ast, start_node: Ast.Node.Index) bool { fn rvalue( gz: *GenZir, rl: ResultLoc, - result: Zir.Inst.Ref, + raw_result: Zir.Inst.Ref, src_node: Ast.Node.Index, ) InnerError!Zir.Inst.Ref { + const result = r: { + if (refToIndex(raw_result)) |result_index| { + const zir_tags = gz.astgen.instructions.items(.tag); + if (zir_tags[result_index].isAlwaysVoid()) { + break :r Zir.Inst.Ref.void_value; + } + } + break :r raw_result; + }; if (gz.endsWithNoReturn()) return result; switch (rl) { .none, .coerced_ty => return result, diff --git a/src/Zir.zig b/src/Zir.zig index 6c9cd9d778c91e0295aaddf2ae5e27f189bdeb3d..d8e53af4f97dbdcafe15b7f85105c47b0e64f470 100644 --- a/src/Zir.zig +++ b/src/Zir.zig @@ -1262,6 +1262,273 @@ pub const Inst = struct { }; } + /// AstGen uses this to find out if `Ref.void_value` should be used in place + /// of the result of a given instruction. This allows Sema to forego adding + /// the instruction to the map after analysis. + pub fn isAlwaysVoid(tag: Tag) bool { + return switch (tag) { + .breakpoint, + .fence, + .@"break", + .break_inline, + .condbr, + .condbr_inline, + .compile_error, + .ret_node, + .ret_load, + .ret_tok, + .ret_err_value, + .@"unreachable", + .repeat, + .repeat_inline, + .panic, + .dbg_stmt, + .dbg_var_ptr, + .dbg_var_val, + .ensure_result_used, + .ensure_result_non_error, + .ensure_err_payload_void, + .set_eval_branch_quota, + .atomic_store, + .store, + .store_node, + .store_to_block_ptr, + .store_to_inferred_ptr, + .resolve_inferred_alloc, + .validate_array_init_ty, + .validate_struct_init_ty, + .validate_struct_init, + .validate_struct_init_comptime, + .validate_array_init, + .validate_array_init_comptime, + .@"export", + .export_value, + .set_align_stack, + .set_cold, + .set_float_mode, + .set_runtime_safety, + .memcpy, + .memset, + => true, + + .param, + .param_comptime, + .param_anytype, + .param_anytype_comptime, + .add, + .addwrap, + .add_sat, + .alloc, + .alloc_mut, + .alloc_comptime_mut, + .alloc_inferred, + .alloc_inferred_mut, + .alloc_inferred_comptime, + .alloc_inferred_comptime_mut, + .make_ptr_const, + .array_cat, + .array_mul, + .array_type, + .array_type_sentinel, + .vector_type, + .elem_type, + .indexable_ptr_len, + .anyframe_type, + .as, + .as_node, + .bit_and, + .bitcast, + .bit_or, + .block, + .block_inline, + .suspend_block, + .loop, + .bool_br_and, + .bool_br_or, + .bool_not, + .call, + .cmp_lt, + .cmp_lte, + .cmp_eq, + .cmp_gte, + .cmp_gt, + .cmp_neq, + .coerce_result_ptr, + .error_set_decl, + .error_set_decl_anon, + .error_set_decl_func, + .decl_ref, + .decl_val, + .load, + .div, + .elem_ptr, + .elem_val, + .elem_ptr_node, + .elem_ptr_imm, + .elem_val_node, + .field_ptr, + .field_val, + .field_call_bind, + .field_ptr_named, + .field_val_named, + .field_call_bind_named, + .func, + .func_inferred, + .has_decl, + .int, + .int_big, + .float, + .float128, + .int_type, + .is_non_null, + .is_non_null_ptr, + .is_non_err, + .is_non_err_ptr, + .mod_rem, + .mul, + .mulwrap, + .mul_sat, + .param_type, + .ref, + .shl, + .shl_sat, + .shr, + .str, + .sub, + .subwrap, + .sub_sat, + .negate, + .negate_wrap, + .typeof, + .typeof_builtin, + .xor, + .optional_type, + .optional_payload_safe, + .optional_payload_unsafe, + .optional_payload_safe_ptr, + .optional_payload_unsafe_ptr, + .err_union_payload_safe, + .err_union_payload_unsafe, + .err_union_payload_safe_ptr, + .err_union_payload_unsafe_ptr, + .err_union_code, + .err_union_code_ptr, + .error_to_int, + .int_to_error, + .ptr_type, + .ptr_type_simple, + .enum_literal, + .merge_error_sets, + .error_union_type, + .bit_not, + .error_value, + .slice_start, + .slice_end, + .slice_sentinel, + .import, + .typeof_log2_int_type, + .log2_int_type, + .switch_capture, + .switch_capture_ref, + .switch_capture_multi, + .switch_capture_multi_ref, + .switch_block, + .switch_cond, + .switch_cond_ref, + .array_base_ptr, + .field_base_ptr, + .struct_init_empty, + .struct_init, + .struct_init_ref, + .struct_init_anon, + .struct_init_anon_ref, + .array_init, + .array_init_sent, + .array_init_anon, + .array_init_ref, + .array_init_sent_ref, + .array_init_anon_ref, + .union_init, + .field_type, + .field_type_ref, + .int_to_enum, + .enum_to_int, + .type_info, + .size_of, + .bit_size_of, + .ptr_to_int, + .align_of, + .bool_to_int, + .embed_file, + .error_name, + .sqrt, + .sin, + .cos, + .exp, + .exp2, + .log, + .log2, + .log10, + .fabs, + .floor, + .ceil, + .trunc, + .round, + .tag_name, + .reify, + .type_name, + .frame_type, + .frame_size, + .float_to_int, + .int_to_float, + .int_to_ptr, + .float_cast, + .int_cast, + .err_set_cast, + .ptr_cast, + .truncate, + .align_cast, + .has_field, + .clz, + .ctz, + .pop_count, + .byte_swap, + .bit_reverse, + .div_exact, + .div_floor, + .div_trunc, + .mod, + .rem, + .shl_exact, + .shr_exact, + .bit_offset_of, + .offset_of, + .cmpxchg_strong, + .cmpxchg_weak, + .splat, + .reduce, + .shuffle, + .select, + .atomic_load, + .atomic_rmw, + .mul_add, + .builtin_call, + .field_parent_ptr, + .maximum, + .minimum, + .builtin_async_call, + .c_import, + .@"resume", + .@"await", + .await_nosuspend, + .ret_err_value_code, + .extended, + .closure_get, + .closure_capture, + => false, + }; + } + /// Used by debug safety-checking code. pub const data_tags = list: { @setEvalBranchQuota(2000); diff --git a/test/behavior/atomics.zig b/test/behavior/atomics.zig index 123b4b488649886e002b29bafd915af75f930126..71e17d9b4c1a85336dd274cac0d849cfca238fb8 100644 --- a/test/behavior/atomics.zig +++ b/test/behavior/atomics.zig @@ -305,3 +305,37 @@ fn testAtomicsWithType(comptime T: type, a: T, b: T) !void { if (@sizeOf(T) != 0) try expect(@cmpxchgStrong(T, &x, b, a, .SeqCst, .SeqCst).? == a); } + +test "return @atomicStore, using it as a void value" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + const S = struct { + const A = struct { + value: usize, + + pub fn store(self: *A, value: usize) void { + return @atomicStore(usize, &self.value, value, .Unordered); + } + + pub fn store2(self: *A, value: usize) void { + return switch (value) { + else => @atomicStore(usize, &self.value, value, .Unordered), + }; + } + }; + + fn doTheTest() !void { + var x: A = .{ .value = 5 }; + x.store(10); + try expect(x.value == 10); + x.store(100); + try expect(x.value == 100); + } + }; + try S.doTheTest(); + comptime try S.doTheTest(); +} -- 2.54.0