authorgravatar for luuk@degram.devLuuk de Gram <luuk@degram.dev> 2023-05-10 17:03:17+02:00
committergravatar for luuk@degram.devLuuk de Gram <luuk@degram.dev> 2023-05-19 20:19:00+02:00
log43e89026ac90ee6e8c2cb066068eb8ff10352ac1
tree5652c5820a1649275d7b252bdd2b650ca8bd75df
parent8be69f41328ebc0331434fd9d4008985463188c9
signaturelock-open Commit is signed but in an unrecognized format.

wasm: fix double free of locals

A copy was being made of a WValue variable, which meant the call to `free` would insert the local that was being held by said WValue was appended to the free list twice. This led to the same local being reused even though it wasn't free and would lead to it being over- written by a new value.

1 files changed, 17 insertions(+), 10 deletions(-)

src/arch/wasm/CodeGen.zig+17-10
...@@ -127,6 +127,7 @@ const WValue = union(enum) {...@@ -127,6 +127,7 @@ const WValue = union(enum) {
127 .f64 => gen.free_locals_f64.append(gen.gpa, local_value) catch return,127 .f64 => gen.free_locals_f64.append(gen.gpa, local_value) catch return,
128 .v128 => gen.free_locals_v128.append(gen.gpa, local_value) catch return,128 .v128 => gen.free_locals_v128.append(gen.gpa, local_value) catch return,
129 }129 }
130 log.debug("freed local ({d}) of type {}", .{ local_value, valtype });
130 value.* = .dead;131 value.* = .dead;
131 }132 }
132};133};
...@@ -1092,27 +1093,27 @@ fn allocLocal(func: *CodeGen, ty: Type) InnerError!WValue {...@@ -1092,27 +1093,27 @@ fn allocLocal(func: *CodeGen, ty: Type) InnerError!WValue {
1092 const valtype = typeToValtype(ty, func.target);1093 const valtype = typeToValtype(ty, func.target);
1093 switch (valtype) {1094 switch (valtype) {
1094 .i32 => if (func.free_locals_i32.popOrNull()) |index| {1095 .i32 => if (func.free_locals_i32.popOrNull()) |index| {
1095 log.debug("reusing local ({d}) of type {}\n", .{ index, valtype });1096 log.debug("reusing local ({d}) of type {}", .{ index, valtype });
1096 return WValue{ .local = .{ .value = index, .references = 1 } };1097 return WValue{ .local = .{ .value = index, .references = 1 } };
1097 },1098 },
1098 .i64 => if (func.free_locals_i64.popOrNull()) |index| {1099 .i64 => if (func.free_locals_i64.popOrNull()) |index| {
1099 log.debug("reusing local ({d}) of type {}\n", .{ index, valtype });1100 log.debug("reusing local ({d}) of type {}", .{ index, valtype });
1100 return WValue{ .local = .{ .value = index, .references = 1 } };1101 return WValue{ .local = .{ .value = index, .references = 1 } };
1101 },1102 },
1102 .f32 => if (func.free_locals_f32.popOrNull()) |index| {1103 .f32 => if (func.free_locals_f32.popOrNull()) |index| {
1103 log.debug("reusing local ({d}) of type {}\n", .{ index, valtype });1104 log.debug("reusing local ({d}) of type {}", .{ index, valtype });
1104 return WValue{ .local = .{ .value = index, .references = 1 } };1105 return WValue{ .local = .{ .value = index, .references = 1 } };
1105 },1106 },
1106 .f64 => if (func.free_locals_f64.popOrNull()) |index| {1107 .f64 => if (func.free_locals_f64.popOrNull()) |index| {
1107 log.debug("reusing local ({d}) of type {}\n", .{ index, valtype });1108 log.debug("reusing local ({d}) of type {}", .{ index, valtype });
1108 return WValue{ .local = .{ .value = index, .references = 1 } };1109 return WValue{ .local = .{ .value = index, .references = 1 } };
1109 },1110 },
1110 .v128 => if (func.free_locals_v128.popOrNull()) |index| {1111 .v128 => if (func.free_locals_v128.popOrNull()) |index| {
1111 log.debug("reusing local ({d}) of type {}\n", .{ index, valtype });1112 log.debug("reusing local ({d}) of type {}", .{ index, valtype });
1112 return WValue{ .local = .{ .value = index, .references = 1 } };1113 return WValue{ .local = .{ .value = index, .references = 1 } };
1113 },1114 },
1114 }1115 }
1115 log.debug("new local of type {}\n", .{valtype});1116 log.debug("new local of type {}", .{valtype});
1116 // no local was free to be re-used, so allocate a new local instead1117 // no local was free to be re-used, so allocate a new local instead
1117 return func.ensureAllocLocal(ty);1118 return func.ensureAllocLocal(ty);
1118}1119}
...@@ -4948,8 +4949,15 @@ fn airAggregateInit(func: *CodeGen, inst: Air.Inst.Index) InnerError!void {...@@ -4948,8 +4949,15 @@ fn airAggregateInit(func: *CodeGen, inst: Air.Inst.Index) InnerError!void {
4948 else => unreachable,4949 else => unreachable,
4949 }4950 }
4950 };4951 };
4951 // TODO: this is incorrect Liveness handling code4952
4952 func.finishAir(inst, result, &.{});4953 if (elements.len <= Liveness.bpi - 1) {
4954 var buf = [1]Air.Inst.Ref{.none} ** (Liveness.bpi - 1);
4955 @memcpy(buf[0..elements.len], elements);
4956 return func.finishAir(inst, result, &buf);
4957 }
4958 var bt = try func.iterateBigTomb(inst, elements.len);
4959 for (elements) |arg| bt.feed(arg);
4960 return bt.finishAir(result);
4953}4961}
49544962
4955fn airUnionInit(func: *CodeGen, inst: Air.Inst.Index) InnerError!void {4963fn airUnionInit(func: *CodeGen, inst: Air.Inst.Index) InnerError!void {
...@@ -5436,11 +5444,10 @@ fn airAddSubWithOverflow(func: *CodeGen, inst: Air.Inst.Index, op: Op) InnerErro...@@ -5436,11 +5444,10 @@ fn airAddSubWithOverflow(func: *CodeGen, inst: Air.Inst.Index, op: Op) InnerErro
5436 };5444 };
54375445
5438 var bin_op = try (try func.binOp(lhs, rhs, lhs_ty, op)).toLocal(func, lhs_ty);5446 var bin_op = try (try func.binOp(lhs, rhs, lhs_ty, op)).toLocal(func, lhs_ty);
5439 defer bin_op.free(func);
5440 var result = if (wasm_bits != int_info.bits) blk: {5447 var result = if (wasm_bits != int_info.bits) blk: {
5441 break :blk try (try func.wrapOperand(bin_op, lhs_ty)).toLocal(func, lhs_ty);5448 break :blk try (try func.wrapOperand(bin_op, lhs_ty)).toLocal(func, lhs_ty);
5442 } else bin_op;5449 } else bin_op;
5443 defer result.free(func); // no-op when wasm_bits == int_info.bits5450 defer result.free(func);
54445451
5445 const cmp_op: std.math.CompareOperator = if (op == .sub) .gt else .lt;5452 const cmp_op: std.math.CompareOperator = if (op == .sub) .gt else .lt;
5446 const overflow_bit: WValue = if (is_signed) blk: {5453 const overflow_bit: WValue = if (is_signed) blk: {