Commit bc4d9f3aa9

Casey Banner <kcbanner@gmail.com>
2023-04-26 16:30:46
translate-c: fix codegen when C source has variables named the same as mangling prefixes
If the C code had variables that were named the same as the prefixes used for name mangling, such as "tmp" or "ref", then the codegen would generate incorrect code in some cases. This was because these aliases were immediately visible to expressions that actually needed to use the original name. I introduced the concept of reserving aliases without enabling them. An alias that isn't enabled isn't visible to expression translation, but is still reserved so that sub-expressions generate aliases that don't overlap. Add test cases to cover the cases that would break before this change. Co-authored-by: Veikka Tuominen <git@vexu.eu>
master
1 parent 7285eed
Changed files (2)
src
translate_c.zig
test
translate_c.zig
src/translate_c.zig
@@ -128,14 +128,29 @@ const Scope = struct {
 
         /// Given the desired name, return a name that does not shadow anything from outer scopes.
         /// Inserts the returned name into the scope.
+        /// The name will not be visible to callers of getAlias.
+        fn reserveMangledName(scope: *Block, c: *Context, name: []const u8) ![]const u8 {
+            return scope.createMangledName(c, name, true);
+        }
+
+        /// Same as reserveMangledName, but enables the alias immediately.
         fn makeMangledName(scope: *Block, c: *Context, name: []const u8) ![]const u8 {
+            return scope.createMangledName(c, name, false);
+        }
+
+        fn createMangledName(scope: *Block, c: *Context, name: []const u8, reservation: bool) ![]const u8 {
             const name_copy = try c.arena.dupe(u8, name);
             var proposed_name = name_copy;
             while (scope.contains(proposed_name)) {
                 scope.mangle_count += 1;
                 proposed_name = try std.fmt.allocPrint(c.arena, "{s}_{d}", .{ name, scope.mangle_count });
             }
-            try scope.variables.append(.{ .name = name_copy, .alias = proposed_name });
+            const new_mangle = try scope.variables.addOne();
+            if (reservation) {
+                new_mangle.* = .{ .name = name_copy, .alias = name_copy };
+            } else {
+                new_mangle.* = .{ .name = name_copy, .alias = proposed_name };
+            }
             return proposed_name;
         }
 
@@ -3806,8 +3821,8 @@ fn transCreatePreCrement(
     // zig: })
     var block_scope = try Scope.Block.init(c, scope, true);
     defer block_scope.deinit();
-    const ref = try block_scope.makeMangledName(c, "ref");
 
+    const ref = try block_scope.reserveMangledName(c, "ref");
     const expr = try transExpr(c, &block_scope.base, op_expr, .used);
     const addr_of = try Tag.address_of.create(c.arena, expr);
     const ref_decl = try Tag.var_simple.create(c.arena, .{ .name = ref, .init = addr_of });
@@ -3853,7 +3868,8 @@ fn transCreatePostCrement(
     // zig: })
     var block_scope = try Scope.Block.init(c, scope, true);
     defer block_scope.deinit();
-    const ref = try block_scope.makeMangledName(c, "ref");
+    const ref = try block_scope.reserveMangledName(c, "ref");
+    const tmp = try block_scope.reserveMangledName(c, "tmp");
 
     const expr = try transExpr(c, &block_scope.base, op_expr, .used);
     const addr_of = try Tag.address_of.create(c.arena, expr);
@@ -3863,7 +3879,6 @@ fn transCreatePostCrement(
     const lhs_node = try Tag.identifier.create(c.arena, ref);
     const ref_node = try Tag.deref.create(c.arena, lhs_node);
 
-    const tmp = try block_scope.makeMangledName(c, "tmp");
     const tmp_decl = try Tag.var_simple.create(c.arena, .{ .name = tmp, .init = ref_node });
     try block_scope.statements.append(tmp_decl);
 
@@ -3968,7 +3983,7 @@ fn transCreateCompoundAssign(
     // zig: })
     var block_scope = try Scope.Block.init(c, scope, true);
     defer block_scope.deinit();
-    const ref = try block_scope.makeMangledName(c, "ref");
+    const ref = try block_scope.reserveMangledName(c, "ref");
 
     const expr = try transExpr(c, &block_scope.base, lhs, .used);
     const addr_of = try Tag.address_of.create(c.arena, expr);
@@ -4098,9 +4113,9 @@ fn transBinaryConditionalOperator(c: *Context, scope: *Scope, stmt: *const clang
     var block_scope = try Scope.Block.init(c, scope, true);
     defer block_scope.deinit();
 
-    const mangled_name = try block_scope.makeMangledName(c, "cond_temp");
+    const cond_temp = try block_scope.reserveMangledName(c, "cond_temp");
     const init_node = try transExpr(c, &block_scope.base, cond_expr, .used);
-    const ref_decl = try Tag.var_simple.create(c.arena, .{ .name = mangled_name, .init = init_node });
+    const ref_decl = try Tag.var_simple.create(c.arena, .{ .name = cond_temp, .init = init_node });
     try block_scope.statements.append(ref_decl);
 
     var cond_scope = Scope.Condition{
@@ -4111,7 +4126,7 @@ fn transBinaryConditionalOperator(c: *Context, scope: *Scope, stmt: *const clang
     };
     defer cond_scope.deinit();
 
-    const cond_ident = try Tag.identifier.create(c.arena, mangled_name);
+    const cond_ident = try Tag.identifier.create(c.arena, cond_temp);
     const ty = getExprQualType(c, cond_expr).getTypePtr();
     const cond_node = try finishBoolExpr(c, &cond_scope.base, cond_expr.getBeginLoc(), ty, cond_ident, .used);
     var then_body = cond_ident;
@@ -4552,11 +4567,12 @@ fn transCreateNodeAssign(
     var block_scope = try Scope.Block.init(c, scope, true);
     defer block_scope.deinit();
 
-    const tmp = try block_scope.makeMangledName(c, "tmp");
+    const tmp = try block_scope.reserveMangledName(c, "tmp");
     var rhs_node = try transExpr(c, &block_scope.base, rhs, .used);
     if (!exprIsBooleanType(lhs) and isBoolRes(rhs_node)) {
         rhs_node = try Tag.bool_to_int.create(c.arena, rhs_node);
     }
+
     const tmp_decl = try Tag.var_simple.create(c.arena, .{ .name = tmp, .init = rhs_node });
     try block_scope.statements.append(tmp_decl);
test/translate_c.zig
@@ -3962,4 +3962,159 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
     , &[_][]const u8{
         \\pub const foo: [3:0]u8 = "bar";
     });
+
+    cases.add("worst-case assign from mangle prefix",
+        \\void foo() {
+        \\    int n, tmp = 1;
+        \\    if (n = tmp) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var n: c_int = undefined;
+        \\    var tmp: c_int = 1;
+        \\    if ((blk: {
+        \\        const tmp_1 = tmp;
+        \\        n = tmp_1;
+        \\        break :blk tmp_1;
+        \\    }) != 0) {}
+        \\}
+    });
+
+    cases.add("worst-case assign to mangle prefix",
+        \\void foo() {
+        \\    int tmp, n = 1;
+        \\    if (tmp = n) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var tmp: c_int = undefined;
+        \\    var n: c_int = 1;
+        \\    if ((blk: {
+        \\        const tmp_1 = n;
+        \\        tmp = tmp_1;
+        \\        break :blk tmp_1;
+        \\    }) != 0) {}
+        \\}
+    });
+
+    cases.add("worst-case precrement mangle prefix",
+        \\void foo() {
+        \\    int n, ref = 1;
+        \\    if (n = ++ref) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var n: c_int = undefined;
+        \\    var ref: c_int = 1;
+        \\    if ((blk: {
+        \\        const tmp = blk_1: {
+        \\            const ref_2 = &ref;
+        \\            ref_2.* += 1;
+        \\            break :blk_1 ref_2.*;
+        \\        };
+        \\        n = tmp;
+        \\        break :blk tmp;
+        \\    }) != 0) {}
+        \\}
+    });
+
+    cases.add("worst-case postcrement mangle prefix",
+        \\void foo() {
+        \\    int n, ref = 1;
+        \\    if (n = ref++) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var n: c_int = undefined;
+        \\    var ref: c_int = 1;
+        \\    if ((blk: {
+        \\        const tmp = blk_1: {
+        \\            const ref_2 = &ref;
+        \\            const tmp_3 = ref_2.*;
+        \\            ref_2.* += 1;
+        \\            break :blk_1 tmp_3;
+        \\        };
+        \\        n = tmp;
+        \\        break :blk tmp;
+        \\    }) != 0) {}
+        \\}
+    });
+
+    cases.add("worst-case compound assign from mangle prefix",
+        \\void foo() {
+        \\    int n, ref = 1;
+        \\    if (n += ref) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var n: c_int = undefined;
+        \\    var ref: c_int = 1;
+        \\    if ((blk: {
+        \\        const ref_1 = &n;
+        \\        ref_1.* += ref;
+        \\        break :blk ref_1.*;
+        \\    }) != 0) {}
+        \\}
+    });
+
+    cases.add("worst-case compound assign to mangle prefix",
+        \\void foo() {
+        \\    int ref, n = 1;
+        \\    if (ref += n) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var ref: c_int = undefined;
+        \\    var n: c_int = 1;
+        \\    if ((blk: {
+        \\        const ref_1 = &ref;
+        \\        ref_1.* += n;
+        \\        break :blk ref_1.*;
+        \\    }) != 0) {}
+        \\}
+    });
+
+    cases.add("binary conditional operator where condition is the mangle prefix",
+        \\void foo() {
+        \\    int f = 1;
+        \\    int n, cond_temp = 1;
+        \\    if (n = (cond_temp)?:(f)) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var f: c_int = 1;
+        \\    var n: c_int = undefined;
+        \\    var cond_temp: c_int = 1;
+        \\    if ((blk: {
+        \\        const tmp = blk_1: {
+        \\            const cond_temp_2 = cond_temp;
+        \\            break :blk_1 if (cond_temp_2 != 0) cond_temp_2 else f;
+        \\        };
+        \\        n = tmp;
+        \\        break :blk tmp;
+        \\    }) != 0) {}
+        \\}
+    });
+
+    cases.add("binary conditional operator where false_expr is the mangle prefix",
+        \\void foo() {
+        \\    int cond_temp = 1;
+        \\    int n, f = 1;
+        \\    if (n = (f)?:(cond_temp)) {}
+        \\}
+    , &[_][]const u8{
+        \\pub export fn foo() void {
+        \\    var cond_temp: c_int = 1;
+        \\    var n: c_int = undefined;
+        \\    var f: c_int = 1;
+        \\    if ((blk: {
+        \\        const tmp = blk_1: {
+        \\            const cond_temp_2 = f;
+        \\            break :blk_1 if (cond_temp_2 != 0) cond_temp_2 else cond_temp;
+        \\        };
+        \\        n = tmp;
+        \\        break :blk tmp;
+        \\    }) != 0) {}
+        \\}
+    });
 }