| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2023-09-15 01:12:03+01:00 |
| committer |  Andrew Kelley <andrew@ziglang.org> 2023-09-15 01:05:02-07:00 |
| log | cba7e8a4e95aa2a2031d0fbaa8247de37e61fd78 |
| tree | f6e7c7139d90aa41ace1498475d6a56f4c40956f |
| parent | 8592c5cdac41e4e04034e4f9a0fd8cb51e8c4257 |
AstGen: do not forward result pointers through @as
The `coerce_result_ptr` instruction is highly problematic and leads to
unintentional memory reinterpretation in some cases. It is more correct
to simply not forward result pointers through this builtin.
`coerce_result_ptr` is still used for struct and array initializations,
where it can still cause issues. Eliminating this usage will be a future
change.
Resolves: #169913 files changed, 24 insertions(+), 35 deletions(-)
test- behavior
src/AstGen.zig+2-30
| ... | @@ -7526,18 +7526,8 @@ fn as( | ... | @@ -7526,18 +7526,8 @@ fn as( |
| 7526 | rhs: Ast.Node.Index, | 7526 | rhs: Ast.Node.Index, |
| 7527 | ) InnerError!Zir.Inst.Ref { | 7527 | ) InnerError!Zir.Inst.Ref { |
| 7528 | const dest_type = try typeExpr(gz, scope, lhs); | 7528 | const dest_type = try typeExpr(gz, scope, lhs); |
| 7529 | switch (ri.rl) { | 7529 | const result = try reachableExpr(gz, scope, .{ .rl = .{ .ty = dest_type } }, rhs, node); |
| 7530 | .none, .discard, .ref, .ty, .coerced_ty => { | 7530 | return rvalue(gz, ri, result, node); |
| 7531 | const result = try reachableExpr(gz, scope, .{ .rl = .{ .ty = dest_type } }, rhs, node); | ||
| 7532 | return rvalue(gz, ri, result, node); | ||
| 7533 | }, | ||
| 7534 | .ptr => |result_ptr| { | ||
| 7535 | return asRlPtr(gz, scope, ri, node, result_ptr.inst, rhs, dest_type); | ||
| 7536 | }, | ||
| 7537 | .inferred_ptr => |result_ptr| { | ||
| 7538 | return asRlPtr(gz, scope, ri, node, result_ptr, rhs, dest_type); | ||
| 7539 | }, | ||
| 7540 | } | ||
| 7541 | } | 7531 | } |
| 7542 | 7532 | ||
| 7543 | fn unionInit( | 7533 | fn unionInit( |
| ... | @@ -7562,24 +7552,6 @@ fn unionInit( | ... | @@ -7562,24 +7552,6 @@ fn unionInit( |
| 7562 | return rvalue(gz, ri, result, node); | 7552 | return rvalue(gz, ri, result, node); |
| 7563 | } | 7553 | } |
| 7564 | 7554 | ||
| 7565 | fn asRlPtr( | ||
| 7566 | gz: *GenZir, | ||
| 7567 | scope: *Scope, | ||
| 7568 | ri: ResultInfo, | ||
| 7569 | src_node: Ast.Node.Index, | ||
| 7570 | result_ptr: Zir.Inst.Ref, | ||
| 7571 | operand_node: Ast.Node.Index, | ||
| 7572 | dest_type: Zir.Inst.Ref, | ||
| 7573 | ) InnerError!Zir.Inst.Ref { | ||
| 7574 | if (gz.astgen.nodes_need_rl.contains(src_node)) { | ||
| 7575 | const casted_ptr = try gz.addPlNode(.coerce_result_ptr, src_node, Zir.Inst.Bin{ .lhs = dest_type, .rhs = result_ptr }); | ||
| 7576 | return reachableExpr(gz, scope, .{ .rl = .{ .ptr = .{ .inst = casted_ptr } } }, operand_node, src_node); | ||
| 7577 | } else { | ||
| 7578 | const result = try reachableExpr(gz, scope, .{ .rl = .{ .ty = dest_type } }, operand_node, src_node); | ||
| 7579 | return rvalue(gz, ri, result, src_node); | ||
| 7580 | } | ||
| 7581 | } | ||
| 7582 | |||
| 7583 | fn bitCast( | 7555 | fn bitCast( |
| 7584 | gz: *GenZir, | 7556 | gz: *GenZir, |
| 7585 | scope: *Scope, | 7557 | scope: *Scope, |
src/AstRlAnnotate.zig+4-5
| ... | @@ -800,6 +800,8 @@ fn blockExpr(astrl: *AstRlAnnotate, parent_block: ?*Block, ri: ResultInfo, node: | ... | @@ -800,6 +800,8 @@ fn blockExpr(astrl: *AstRlAnnotate, parent_block: ?*Block, ri: ResultInfo, node: |
| 800 | } | 800 | } |
| 801 | 801 | ||
| 802 | fn builtinCall(astrl: *AstRlAnnotate, block: ?*Block, ri: ResultInfo, node: Ast.Node.Index, args: []const Ast.Node.Index) !bool { | 802 | fn builtinCall(astrl: *AstRlAnnotate, block: ?*Block, ri: ResultInfo, node: Ast.Node.Index, args: []const Ast.Node.Index) !bool { |
| 803 | _ = ri; // Currently, no builtin consumes its result location. | ||
| 804 | |||
| 803 | const tree = astrl.tree; | 805 | const tree = astrl.tree; |
| 804 | const main_tokens = tree.nodes.items(.main_token); | 806 | const main_tokens = tree.nodes.items(.main_token); |
| 805 | const builtin_token = main_tokens[node]; | 807 | const builtin_token = main_tokens[node]; |
| ... | @@ -818,11 +820,8 @@ fn builtinCall(astrl: *AstRlAnnotate, block: ?*Block, ri: ResultInfo, node: Ast. | ... | @@ -818,11 +820,8 @@ fn builtinCall(astrl: *AstRlAnnotate, block: ?*Block, ri: ResultInfo, node: Ast. |
| 818 | }, | 820 | }, |
| 819 | .as => { | 821 | .as => { |
| 820 | _ = try astrl.expr(args[0], block, ResultInfo.type_only); | 822 | _ = try astrl.expr(args[0], block, ResultInfo.type_only); |
| 821 | const rhs_consumes_rl = try astrl.expr(args[1], block, ri); | 823 | _ = try astrl.expr(args[1], block, ResultInfo.type_only); |
| 822 | if (rhs_consumes_rl) { | 824 | return false; |
| 823 | try astrl.nodes_need_rl.putNoClobber(astrl.gpa, node, {}); | ||
| 824 | } | ||
| 825 | return rhs_consumes_rl; | ||
| 826 | }, | 825 | }, |
| 827 | .bit_cast => { | 826 | .bit_cast => { |
| 828 | _ = try astrl.expr(args[0], block, ResultInfo.none); | 827 | _ = try astrl.expr(args[0], block, ResultInfo.none); |
test/behavior/cast.zig+18
| ... | @@ -2502,3 +2502,21 @@ test "numeric coercions with undefined" { | ... | @@ -2502,3 +2502,21 @@ test "numeric coercions with undefined" { |
| 2502 | to = 42.0; | 2502 | to = 42.0; |
| 2503 | try expectEqual(@as(f32, 42.0), to); | 2503 | try expectEqual(@as(f32, 42.0), to); |
| 2504 | } | 2504 | } |
| 2505 | |||
| 2506 | test "@as does not corrupt values with incompatible representations" { | ||
| 2507 | if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO | ||
| 2508 | if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO | ||
| 2509 | if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO | ||
| 2510 | if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO | ||
| 2511 | if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO | ||
| 2512 | if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; | ||
| 2513 | |||
| 2514 | const x: f32 = @as(f16, blk: { | ||
| 2515 | if (false) { | ||
| 2516 | // Trick the compiler into trying to use a result pointer if it can! | ||
| 2517 | break :blk .{undefined}; | ||
| 2518 | } | ||
| 2519 | break :blk 1.23; | ||
| 2520 | }); | ||
| 2521 | try std.testing.expectApproxEqAbs(@as(f32, 1.23), x, 0.001); | ||
| 2522 | } |