Commit 7d75c3d3b8

Veikka Tuominen <git@vexu.eu>
2024-01-29 13:33:53
llvm: ensure returned undef is 0xaa bytes when runtime safety is enabled
Closes #13178
1 parent 4dfca01
src/arch/aarch64/CodeGen.zig
@@ -766,6 +766,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
             .not             => try self.airNot(inst),
             .int_from_ptr        => try self.airIntFromPtr(inst),
             .ret             => try self.airRet(inst),
+            .ret_safe        => try self.airRet(inst), // TODO
             .ret_load        => try self.airRetLoad(inst),
             .store           => try self.airStore(inst, false),
             .store_safe      => try self.airStore(inst, true),
src/arch/arm/CodeGen.zig
@@ -752,6 +752,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
             .not             => try self.airNot(inst),
             .int_from_ptr        => try self.airIntFromPtr(inst),
             .ret             => try self.airRet(inst),
+            .ret_safe        => try self.airRet(inst), // TODO
             .ret_load        => try self.airRetLoad(inst),
             .store           => try self.airStore(inst, false),
             .store_safe      => try self.airStore(inst, true),
src/arch/riscv64/CodeGen.zig
@@ -585,6 +585,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
             .not             => try self.airNot(inst),
             .int_from_ptr        => try self.airIntFromPtr(inst),
             .ret             => try self.airRet(inst),
+            .ret_safe        => try self.airRet(inst), // TODO
             .ret_load        => try self.airRetLoad(inst),
             .store           => try self.airStore(inst, false),
             .store_safe      => try self.airStore(inst, true),
src/arch/sparc64/CodeGen.zig
@@ -598,6 +598,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
             .not             => try self.airNot(inst),
             .int_from_ptr        => try self.airIntFromPtr(inst),
             .ret             => try self.airRet(inst),
+            .ret_safe        => try self.airRet(inst), // TODO
             .ret_load        => try self.airRetLoad(inst),
             .store           => try self.airStore(inst, false),
             .store_safe      => try self.airStore(inst, true),
src/arch/wasm/CodeGen.zig
@@ -1957,6 +1957,7 @@ fn genInst(func: *CodeGen, inst: Air.Inst.Index) InnerError!void {
         .ptr_elem_val => func.airPtrElemVal(inst),
         .int_from_ptr => func.airIntFromPtr(inst),
         .ret => func.airRet(inst),
+        .ret_safe => func.airRet(inst), // TODO
         .ret_ptr => func.airRetPtr(inst),
         .ret_load => func.airRetLoad(inst),
         .splat => func.airSplat(inst),
src/arch/x86_64/CodeGen.zig
@@ -2018,6 +2018,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
             .loop            => try self.airLoop(inst),
             .int_from_ptr        => try self.airIntFromPtr(inst),
             .ret             => try self.airRet(inst),
+            .ret_safe        => try self.airRet(inst), // TODO
             .ret_load        => try self.airRetLoad(inst),
             .store           => try self.airStore(inst, false),
             .store_safe      => try self.airStore(inst, true),
src/codegen/c.zig
@@ -3225,6 +3225,7 @@ fn genBodyInner(f: *Function, body: []const Air.Inst.Index) error{ AnalysisFail,
             .int_from_bool      => try airIntFromBool(f, inst),
             .load             => try airLoad(f, inst),
             .ret              => try airRet(f, inst, false),
+            .ret_safe         => try airRet(f, inst, false), // TODO
             .ret_load         => try airRet(f, inst, true),
             .store            => try airStore(f, inst, false),
             .store_safe       => try airStore(f, inst, true),
src/codegen/llvm.zig
@@ -5078,7 +5078,8 @@ pub const FuncGen = struct {
                 .load           => try self.airLoad(body[i..]),
                 .loop           => try self.airLoop(inst),
                 .not            => try self.airNot(inst),
-                .ret            => try self.airRet(inst),
+                .ret            => try self.airRet(inst, false),
+                .ret_safe       => try self.airRet(inst, true),
                 .ret_load       => try self.airRetLoad(inst),
                 .store          => try self.airStore(inst, false),
                 .store_safe     => try self.airStore(inst, true),
@@ -5551,15 +5552,42 @@ pub const FuncGen = struct {
         _ = try fg.wip.@"unreachable"();
     }
 
-    fn airRet(self: *FuncGen, inst: Air.Inst.Index) !Builder.Value {
+    fn airRet(self: *FuncGen, inst: Air.Inst.Index, safety: bool) !Builder.Value {
         const o = self.dg.object;
         const mod = o.module;
         const un_op = self.air.instructions.items(.data)[@intFromEnum(inst)].un_op;
         const ret_ty = self.typeOf(un_op);
+
         if (self.ret_ptr != .none) {
-            const operand = try self.resolveInst(un_op);
             const ptr_ty = try mod.singleMutPtrType(ret_ty);
 
+            const operand = try self.resolveInst(un_op);
+            const val_is_undef = if (try self.air.value(un_op, mod)) |val| val.isUndefDeep(mod) else false;
+            if (val_is_undef and safety) undef: {
+                const ptr_info = ptr_ty.ptrInfo(mod);
+                const needs_bitmask = (ptr_info.packed_offset.host_size != 0);
+                if (needs_bitmask) {
+                    // TODO: only some bits are to be undef, we cannot write with a simple memset.
+                    // meanwhile, ignore the write rather than stomping over valid bits.
+                    // https://github.com/ziglang/zig/issues/15337
+                    break :undef;
+                }
+                const len = try o.builder.intValue(try o.lowerType(Type.usize), ret_ty.abiSize(mod));
+                _ = try self.wip.callMemSet(
+                    self.ret_ptr,
+                    ptr_ty.ptrAlignment(mod).toLlvm(),
+                    try o.builder.intValue(.i8, 0xaa),
+                    len,
+                    if (ptr_ty.isVolatilePtr(mod)) .@"volatile" else .normal,
+                );
+                const owner_mod = self.dg.ownerModule();
+                if (owner_mod.valgrind) {
+                    try self.valgrindMarkUndef(self.ret_ptr, len);
+                }
+                _ = try self.wip.retVoid();
+                return .none;
+            }
+
             const unwrapped_operand = operand.unwrap();
             const unwrapped_ret = self.ret_ptr.unwrap();
 
@@ -5588,8 +5616,28 @@ pub const FuncGen = struct {
 
         const abi_ret_ty = try lowerFnRetTy(o, fn_info);
         const operand = try self.resolveInst(un_op);
+        const val_is_undef = if (try self.air.value(un_op, mod)) |val| val.isUndefDeep(mod) else false;
         const alignment = ret_ty.abiAlignment(mod).toLlvm();
 
+        if (val_is_undef and safety) {
+            const llvm_ret_ty = operand.typeOfWip(&self.wip);
+            const rp = try self.buildAlloca(llvm_ret_ty, alignment);
+            const len = try o.builder.intValue(try o.lowerType(Type.usize), ret_ty.abiSize(mod));
+            _ = try self.wip.callMemSet(
+                rp,
+                alignment,
+                try o.builder.intValue(.i8, 0xaa),
+                len,
+                .normal,
+            );
+            const owner_mod = self.dg.ownerModule();
+            if (owner_mod.valgrind) {
+                try self.valgrindMarkUndef(rp, len);
+            }
+            _ = try self.wip.ret(try self.wip.load(.normal, abi_ret_ty, rp, alignment, ""));
+            return .none;
+        }
+
         if (isByRef(ret_ty, mod)) {
             // operand is a pointer however self.ret_ptr is null so that means
             // we need to return a value.
src/codegen/spirv.zig
@@ -2177,6 +2177,7 @@ const DeclGen = struct {
             .cond_br        => return self.airCondBr(inst),
             .loop           => return self.airLoop(inst),
             .ret            => return self.airRet(inst),
+            .ret_safe       => return self.airRet(inst), // TODO
             .ret_load       => return self.airRetLoad(inst),
             .@"try"         => try self.airTry(inst),
             .switch_br      => return self.airSwitchBr(inst),
src/Liveness/Verify.zig
@@ -151,6 +151,7 @@ fn verifyBody(self: *Verify, body: []const Air.Inst.Index) Error!void {
                 try self.verifyInstOperands(inst, .{ un_op, .none, .none });
             },
             .ret,
+            .ret_safe,
             .ret_load,
             => {
                 const un_op = data[@intFromEnum(inst)].un_op;
src/Air.zig
@@ -516,6 +516,11 @@ pub const Inst = struct {
         /// Uses the `un_op` field.
         /// Triggers `resolveTypeLayout` on the return type.
         ret,
+        /// Same as `ret`, except if the operand is undefined, the
+        /// returned value is 0xaa bytes, and any other safety metadata
+        /// such as Valgrind integrations should be notified of
+        /// this value being undefined.
+        ret_safe,
         /// This instruction communicates that the function's result value is pointed to by
         /// the operand. If the function will pass the result by-ref, the operand is a
         /// `ret_ptr` instruction. Otherwise, this instruction is equivalent to a `load`
@@ -1439,6 +1444,7 @@ pub fn typeOfIndex(air: *const Air, inst: Air.Inst.Index, ip: *const InternPool)
         .cond_br,
         .switch_br,
         .ret,
+        .ret_safe,
         .ret_load,
         .unreach,
         .trap,
@@ -1613,6 +1619,7 @@ pub fn mustLower(air: Air, inst: Air.Inst.Index, ip: *const InternPool) bool {
         .dbg_var_ptr,
         .dbg_var_val,
         .ret,
+        .ret_safe,
         .ret_load,
         .store,
         .store_safe,
src/Liveness.zig
@@ -435,6 +435,7 @@ pub fn categorizeOperand(
         },
 
         .ret,
+        .ret_safe,
         .ret_load,
         => {
             const o = air_datas[@intFromEnum(inst)].un_op;
@@ -1070,6 +1071,7 @@ fn analyzeInst(
         },
 
         .ret,
+        .ret_safe,
         .ret_load,
         => {
             const operand = inst_datas[@intFromEnum(inst)].un_op;
src/print_air.zig
@@ -175,6 +175,7 @@ const Writer = struct {
             .int_from_ptr,
             .int_from_bool,
             .ret,
+            .ret_safe,
             .ret_load,
             .is_named_enum_value,
             .tag_name,
src/Sema.zig
@@ -19437,14 +19437,15 @@ fn analyzeRet(
 
     try sema.resolveTypeLayout(sema.fn_ret_ty);
 
+    const air_tag: Air.Inst.Tag = if (block.wantSafety()) .ret_safe else .ret;
     if (sema.wantErrorReturnTracing(sema.fn_ret_ty)) {
         // Avoid adding a frame to the error return trace in case the value is comptime-known
         // to be not an error.
         const is_non_err = try sema.analyzeIsNonErr(block, src, operand);
-        return sema.retWithErrTracing(block, src, is_non_err, .ret, operand);
+        return sema.retWithErrTracing(block, src, is_non_err, air_tag, operand);
     }
 
-    _ = try block.addUnOp(.ret, operand);
+    _ = try block.addUnOp(air_tag, operand);
 
     return always_noreturn;
 }
test/behavior/undefined.zig
@@ -97,3 +97,25 @@ test "reslice of undefined global var slice" {
     const x = buf[0..1];
     try @import("std").testing.expect(x.len == 1 and x[0] == 0);
 }
+
+test "returned undef is 0xaa bytes when runtime safety is enabled" {
+    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
+    if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
+
+    const Rect = struct {
+        x: f32,
+        fn getUndefStruct() @This() {
+            @setRuntimeSafety(true);
+            return undefined;
+        }
+        fn getUndefInt() u32 {
+            @setRuntimeSafety(true);
+            return undefined;
+        }
+    };
+    try std.testing.expect(@as(u32, @bitCast(Rect.getUndefStruct().x)) == 0xAAAAAAAA);
+    try std.testing.expect(Rect.getUndefInt() == 0xAAAAAAAA);
+}