Skip to content

Commit ea6cf2a

Browse files
committed
fix x86_64 crashes for switch_block_err_union
This change only emits the unwrap_errunion_err instruction if the error capture is actually used in a branch.
1 parent 844b9a2 commit ea6cf2a

File tree

4 files changed

+99
-31
lines changed

4 files changed

+99
-31
lines changed

src/AstGen.zig

Lines changed: 66 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7008,8 +7008,7 @@ fn switchExprErrUnion(
70087008
const case_slice = case_scope.instructionsSlice();
70097009
// Since we use the switch_block_err_union instruction itself to refer
70107010
// to the capture, which will not be added to the child block, we need
7011-
// to handle ref_table manually, and the same for the inline tag
7012-
// capture instruction.
7011+
// to handle ref_table manually.
70137012
const refs_len = refs: {
70147013
var n: usize = 0;
70157014
var check_inst = switch_block;
@@ -7045,9 +7044,24 @@ fn switchExprErrUnion(
70457044
break :blk .{ err_name, error_payload };
70467045
};
70477046

7047+
// allocate a shared dummy instruction for the error capture
7048+
const err_inst = err_inst: {
7049+
const inst: Zir.Inst.Index = @enumFromInt(astgen.instructions.len);
7050+
try astgen.instructions.append(astgen.gpa, .{
7051+
.tag = .extended,
7052+
.data = .{ .extended = .{
7053+
.opcode = .value_placeholder,
7054+
.small = undefined,
7055+
.operand = undefined,
7056+
} },
7057+
});
7058+
break :err_inst inst;
7059+
};
7060+
70487061
// In this pass we generate all the item and prong expressions for error cases.
70497062
var multi_case_index: u32 = 0;
70507063
var scalar_case_index: u32 = 0;
7064+
var any_uses_err_capture = false;
70517065
for (case_nodes) |case_node| {
70527066
const case = tree.fullSwitchCase(case_node).?;
70537067

@@ -7057,31 +7071,42 @@ fn switchExprErrUnion(
70577071
var dbg_var_name: ?u32 = null;
70587072
var dbg_var_inst: Zir.Inst.Ref = undefined;
70597073
var err_scope: Scope.LocalVal = undefined;
7060-
var tag_scope: Scope.LocalVal = undefined;
7074+
var capture_scope: Scope.LocalVal = undefined;
70617075

70627076
const sub_scope = blk: {
7063-
const tag_token = case.payload_token orelse break :blk &case_scope.base;
7064-
assert(token_tags[tag_token] == .identifier);
7077+
err_scope = .{
7078+
.parent = &case_scope.base,
7079+
.gen_zir = &case_scope,
7080+
.name = err_name,
7081+
.inst = err_inst.toRef(),
7082+
.token_src = error_payload,
7083+
.id_cat = .capture,
7084+
};
70657085

7066-
const tag_slice = tree.tokenSlice(tag_token);
7067-
if (mem.eql(u8, tag_slice, "_")) {
7068-
return astgen.failTok(tag_token, "discard of error capture; omit it instead", .{});
7086+
const capture_token = case.payload_token orelse break :blk &err_scope.base;
7087+
assert(token_tags[capture_token] == .identifier);
7088+
7089+
const capture_slice = tree.tokenSlice(capture_token);
7090+
if (mem.eql(u8, capture_slice, "_")) {
7091+
return astgen.failTok(capture_token, "discard of error capture; omit it instead", .{});
70697092
}
7070-
const tag_name = try astgen.identAsString(tag_token);
7071-
try astgen.detectLocalShadowing(&case_scope.base, tag_name, tag_token, tag_slice, .capture);
7093+
const tag_name = try astgen.identAsString(capture_token);
7094+
try astgen.detectLocalShadowing(&case_scope.base, tag_name, capture_token, capture_slice, .capture);
70727095

7073-
tag_scope = .{
7096+
capture_scope = .{
70747097
.parent = &case_scope.base,
70757098
.gen_zir = &case_scope,
70767099
.name = tag_name,
70777100
.inst = switch_block.toRef(),
7078-
.token_src = tag_token,
7101+
.token_src = capture_token,
70797102
.id_cat = .capture,
70807103
};
70817104
dbg_var_name = tag_name;
70827105
dbg_var_inst = switch_block.toRef();
70837106

7084-
break :blk &tag_scope.base;
7107+
err_scope.parent = &capture_scope.base;
7108+
7109+
break :blk &err_scope.base;
70857110
};
70867111

70877112
const header_index: u32 = @intCast(payloads.items.len);
@@ -7135,41 +7160,42 @@ fn switchExprErrUnion(
71357160
case_scope.instructions_top = parent_gz.instructions.items.len;
71367161
defer case_scope.unstack();
71377162

7138-
const err_code_inst = try case_scope.addUnNode(.err_union_code, raw_operand, operand_node);
7139-
err_scope = .{
7140-
.parent = sub_scope,
7141-
.gen_zir = &case_scope,
7142-
.name = err_name,
7143-
.inst = err_code_inst,
7144-
.token_src = error_payload,
7145-
.id_cat = .capture,
7146-
};
7147-
71487163
try case_scope.addDbgBlockBegin();
71497164
if (dbg_var_name) |some| {
71507165
try case_scope.addDbgVar(.dbg_var_val, some, dbg_var_inst);
71517166
}
71527167
const target_expr_node = case.ast.target_expr;
7153-
const case_result = try expr(&case_scope, &err_scope.base, block_scope.break_result_info, target_expr_node);
7154-
// check sub_scope, not err_scope to avoid false positive unused error capture
7155-
try checkUsed(parent_gz, &case_scope.base, sub_scope);
7168+
const case_result = try expr(&case_scope, sub_scope, block_scope.break_result_info, target_expr_node);
7169+
// check capture_scope, not err_scope to avoid false positive unused error capture
7170+
try checkUsed(parent_gz, &case_scope.base, err_scope.parent);
7171+
const uses_err = err_scope.used != 0 or err_scope.discarded != 0;
7172+
if (uses_err) {
7173+
try case_scope.addDbgVar(.dbg_var_val, err_name, err_inst.toRef());
7174+
any_uses_err_capture = true;
7175+
}
71567176
try case_scope.addDbgBlockEnd();
71577177
if (!parent_gz.refIsNoReturn(case_result)) {
71587178
_ = try case_scope.addBreakWithSrcNode(.@"break", switch_block, case_result, target_expr_node);
71597179
}
71607180

71617181
const case_slice = case_scope.instructionsSlice();
7162-
// Since we use the switch_block instruction itself to refer to the
7163-
// capture, which will not be added to the child block, we need to
7164-
// handle ref_table manually, and the same for the inline tag
7165-
// capture instruction.
7182+
// Since we use the switch_block_err_union instruction itself to refer
7183+
// to the capture, which will not be added to the child block, we need
7184+
// to handle ref_table manually.
71667185
const refs_len = refs: {
71677186
var n: usize = 0;
71687187
var check_inst = switch_block;
71697188
while (astgen.ref_table.get(check_inst)) |ref_inst| {
71707189
n += 1;
71717190
check_inst = ref_inst;
71727191
}
7192+
if (uses_err) {
7193+
check_inst = err_inst;
7194+
while (astgen.ref_table.get(check_inst)) |ref_inst| {
7195+
n += 1;
7196+
check_inst = ref_inst;
7197+
}
7198+
}
71737199
break :refs n;
71747200
};
71757201
const body_len = refs_len + astgen.countBodyLenAfterFixups(case_slice);
@@ -7183,6 +7209,11 @@ fn switchExprErrUnion(
71837209
if (astgen.ref_table.fetchRemove(switch_block)) |kv| {
71847210
appendPossiblyRefdBodyInst(astgen, payloads, kv.value);
71857211
}
7212+
if (uses_err) {
7213+
if (astgen.ref_table.fetchRemove(err_inst)) |kv| {
7214+
appendPossiblyRefdBodyInst(astgen, payloads, kv.value);
7215+
}
7216+
}
71867217
appendBodyWithFixupsArrayList(astgen, payloads, case_slice);
71877218
}
71887219
}
@@ -7200,13 +7231,18 @@ fn switchExprErrUnion(
72007231
.has_multi_cases = multi_cases_len != 0,
72017232
.has_else = has_else,
72027233
.scalar_cases_len = @intCast(scalar_cases_len),
7234+
.any_uses_err_capture = any_uses_err_capture,
72037235
},
72047236
});
72057237

72067238
if (multi_cases_len != 0) {
72077239
astgen.extra.appendAssumeCapacity(multi_cases_len);
72087240
}
72097241

7242+
if (any_uses_err_capture) {
7243+
astgen.extra.appendAssumeCapacity(@intFromEnum(err_inst));
7244+
}
7245+
72107246
const zir_datas = astgen.instructions.items(.data);
72117247
zir_datas[@intFromEnum(switch_block)].pl_node.payload_index = payload_index;
72127248

src/Sema.zig

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11159,6 +11159,16 @@ fn zirSwitchBlockErrUnion(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Comp
1115911159
break :blk multi_cases_len;
1116011160
} else 0;
1116111161

11162+
const err_capture_inst: Zir.Inst.Index = if (extra.data.bits.any_uses_err_capture) blk: {
11163+
const err_capture_inst: Zir.Inst.Index = @enumFromInt(sema.code.extra[header_extra_index]);
11164+
header_extra_index += 1;
11165+
// SwitchProngAnalysis wants inst_map to have space for the tag capture.
11166+
// Note that the normal capture is referred to via the switch block
11167+
// index, which there is already necessarily space for.
11168+
try sema.inst_map.ensureSpaceForInstructions(gpa, &.{err_capture_inst});
11169+
break :blk err_capture_inst;
11170+
} else undefined;
11171+
1116211172
var case_vals = try std.ArrayListUnmanaged(Air.Inst.Ref).initCapacity(gpa, scalar_cases_len + 2 * multi_cases_len);
1116311173
defer case_vals.deinit(gpa);
1116411174

@@ -11286,6 +11296,12 @@ fn zirSwitchBlockErrUnion(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Comp
1128611296
},
1128711297
}));
1128811298
spa.operand = try sema.analyzeErrUnionCode(block, operand_src, raw_operand_val);
11299+
11300+
if (extra.data.bits.any_uses_err_capture) {
11301+
sema.inst_map.putAssumeCapacity(err_capture_inst, spa.operand);
11302+
}
11303+
defer if (extra.data.bits.any_uses_err_capture) assert(sema.inst_map.remove(err_capture_inst));
11304+
1128911305
return resolveSwitchComptime(
1129011306
sema,
1129111307
spa,
@@ -11336,6 +11352,10 @@ fn zirSwitchBlockErrUnion(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Comp
1133611352
defer gpa.free(true_instructions);
1133711353

1133811354
spa.operand = try sema.analyzeErrUnionCode(&sub_block, operand_src, raw_operand_val);
11355+
if (extra.data.bits.any_uses_err_capture) {
11356+
sema.inst_map.putAssumeCapacity(err_capture_inst, spa.operand);
11357+
}
11358+
defer if (extra.data.bits.any_uses_err_capture) assert(sema.inst_map.remove(err_capture_inst));
1133911359
_ = try sema.analyzeSwitchRuntimeBlock(
1134011360
spa,
1134111361
&sub_block,

src/Zir.zig

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2793,9 +2793,10 @@ pub const Inst = struct {
27932793
has_multi_cases: bool,
27942794
/// If true, there is an else prong. This is mutually exclusive with `has_under`.
27952795
has_else: bool,
2796+
any_uses_err_capture: bool,
27962797
scalar_cases_len: ScalarCasesLen,
27972798

2798-
pub const ScalarCasesLen = u30;
2799+
pub const ScalarCasesLen = u29;
27992800
};
28002801

28012802
pub const MultiProng = struct {

src/print_zir.zig

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2039,8 +2039,19 @@ const Writer = struct {
20392039
break :blk multi_cases_len;
20402040
} else 0;
20412041

2042+
const err_capture_inst: Zir.Inst.Index = if (extra.data.bits.any_uses_err_capture) blk: {
2043+
const tag_capture_inst = self.code.extra[extra_index];
2044+
extra_index += 1;
2045+
break :blk @enumFromInt(tag_capture_inst);
2046+
} else undefined;
2047+
20422048
try self.writeInstRef(stream, extra.data.operand);
20432049

2050+
if (extra.data.bits.any_uses_err_capture) {
2051+
try stream.writeAll(", err_capture=");
2052+
try self.writeInstIndex(stream, err_capture_inst);
2053+
}
2054+
20442055
self.indent += 2;
20452056

20462057
{

0 commit comments

Comments
 (0)