correct some bugs

lib/ubsan_rt.zig

@@ -79,12 +79,16 @@ const Value = extern struct {
         };
     }
 
-    fn getFloat(value: Value) c_longdouble {
+    fn getFloat(value: Value) f128 {
         assert(value.td.kind == .float);
         const size = value.td.info.float;
         const max_inline_size = @bitSizeOf(ValueHandle);
         if (size <= max_inline_size) {
-            return @bitCast(@intFromPtr(value.handle));
+            return @as(switch (@bitSizeOf(usize)) {
+                32 => f32,
+                64 => f64,
+                else => @compileError("unsupported target"),
+            }, @bitCast(@intFromPtr(value.handle)));
         }
         return @floatCast(switch (size) {
             64 => @as(*const f64, @alignCast(@ptrCast(value.handle))).*,
@@ -122,6 +126,11 @@ const Value = extern struct {
     ) !void {
         comptime assert(fmt.len == 0);
 
+        if (builtin.zig_backend == .stage2_x86_64 and builtin.os.tag == .windows) {
+            try writer.writeAll("(unknown)");
+            return;
+        }
+
         switch (value.td.kind) {
             .integer => {
                 if (value.td.isSigned()) {
@@ -146,6 +155,14 @@ fn overflowHandler(
     comptime operator: []const u8,
 ) void {
     const S = struct {
+        fn abort(
+            data: *const OverflowData,
+            lhs_handle: ValueHandle,
+            rhs_handle: ValueHandle,
+        ) callconv(.c) noreturn {
+            handler(data, lhs_handle, rhs_handle);
+        }
+
         fn handler(
             data: *const OverflowData,
             lhs_handle: ValueHandle,
@@ -167,7 +184,14 @@ fn overflowHandler(
         }
     };
 
-    exportHandler(&S.handler, sym_name, true);
+    exportHandlerWithAbort(&S.handler, &S.abort, sym_name);
+}
+
+fn negationHandlerAbort(
+    data: *const OverflowData,
+    value_handle: ValueHandle,
+) callconv(.c) noreturn {
+    negationHandler(data, value_handle);
 }
 
 fn negationHandler(
@@ -181,6 +205,14 @@ fn negationHandler(
     );
 }
 
+fn divRemHandlerAbort(
+    data: *const OverflowData,
+    lhs_handle: ValueHandle,
+    rhs_handle: ValueHandle,
+) callconv(.c) noreturn {
+    divRemHandler(data, lhs_handle, rhs_handle);
+}
+
 fn divRemHandler(
     data: *const OverflowData,
     lhs_handle: ValueHandle,
@@ -203,6 +235,20 @@ const AlignmentAssumptionData = extern struct {
     td: *const TypeDescriptor,
 };
 
+fn alignmentAssumptionHandlerAbort(
+    data: *const AlignmentAssumptionData,
+    pointer: ValueHandle,
+    alignment_handle: ValueHandle,
+    maybe_offset: ?ValueHandle,
+) callconv(.c) noreturn {
+    alignmentAssumptionHandler(
+        data,
+        pointer,
+        alignment_handle,
+        maybe_offset,
+    );
+}
+
 fn alignmentAssumptionHandler(
     data: *const AlignmentAssumptionData,
     pointer: ValueHandle,
@@ -248,6 +294,14 @@ const ShiftOobData = extern struct {
     rhs_type: *const TypeDescriptor,
 };
 
+fn shiftOobAbort(
+    data: *const ShiftOobData,
+    lhs_handle: ValueHandle,
+    rhs_handle: ValueHandle,
+) callconv(.c) noreturn {
+    shiftOob(data, lhs_handle, rhs_handle);
+}
+
 fn shiftOob(
     data: *const ShiftOobData,
     lhs_handle: ValueHandle,
@@ -285,7 +339,17 @@ const OutOfBoundsData = extern struct {
     index_type: *const TypeDescriptor,
 };
 
-fn outOfBounds(data: *const OutOfBoundsData, index_handle: ValueHandle) callconv(.c) noreturn {
+fn outOfBoundsAbort(
+    data: *const OutOfBoundsData,
+    index_handle: ValueHandle,
+) callconv(.c) noreturn {
+    outOfBounds(data, index_handle);
+}
+
+fn outOfBounds(
+    data: *const OutOfBoundsData,
+    index_handle: ValueHandle,
+) callconv(.c) noreturn {
     const index: Value = .{ .handle = index_handle, .td = data.index_type };
     logMessage(
         "index {} out of bounds for type {s}",
@@ -297,6 +361,14 @@ const PointerOverflowData = extern struct {
     loc: SourceLocation,
 };
 
+fn pointerOverflowAbort(
+    data: *const PointerOverflowData,
+    base: usize,
+    result: usize,
+) callconv(.c) noreturn {
+    pointerOverflow(data, base, result);
+}
+
 fn pointerOverflow(
     _: *const PointerOverflowData,
     base: usize,
@@ -375,6 +447,13 @@ const TypeMismatchData = extern struct {
     },
 };
 
+fn typeMismatchAbort(
+    data: *const TypeMismatchData,
+    pointer: ?ValueHandle,
+) callconv(.c) noreturn {
+    typeMismatch(data, pointer);
+}
+
 fn typeMismatch(
     data: *const TypeMismatchData,
     pointer: ?ValueHandle,
@@ -416,6 +495,9 @@ const NonNullReturnData = extern struct {
     attribute_loc: SourceLocation,
 };
 
+fn nonNullReturnAbort(data: *const NonNullReturnData) callconv(.c) noreturn {
+    nonNullReturn(data);
+}
 fn nonNullReturn(_: *const NonNullReturnData) callconv(.c) noreturn {
     logMessage("null pointer returned from function declared to never return null", .{});
 }
@@ -426,6 +508,10 @@ const NonNullArgData = extern struct {
     arg_index: i32,
 };
 
+fn nonNullArgAbort(data: *const NonNullArgData) callconv(.c) noreturn {
+    nonNullArg(data);
+}
+
 fn nonNullArg(data: *const NonNullArgData) callconv(.c) noreturn {
     logMessage(
         "null pointer passed as argument {}, which is declared to never be null",
@@ -438,6 +524,13 @@ const InvalidValueData = extern struct {
     td: *const TypeDescriptor,
 };
 
+fn loadInvalidValueAbort(
+    data: *const InvalidValueData,
+    value_handle: ValueHandle,
+) callconv(.c) noreturn {
+    loadInvalidValue(data, value_handle);
+}
+
 fn loadInvalidValue(
     data: *const InvalidValueData,
     value_handle: ValueHandle,
@@ -456,6 +549,9 @@ const InvalidBuiltinData = extern struct {
         clz,
     },
 };
+fn invalidBuiltinAbort(data: *const InvalidBuiltinData) callconv(.c) noreturn {
+    invalidBuiltin(data);
+}
 
 fn invalidBuiltin(data: *const InvalidBuiltinData) callconv(.c) noreturn {
     logMessage(
@@ -469,6 +565,13 @@ const VlaBoundNotPositive = extern struct {
     td: *const TypeDescriptor,
 };
 
+fn vlaBoundNotPositiveAbort(
+    data: *const VlaBoundNotPositive,
+    bound_handle: ValueHandle,
+) callconv(.c) noreturn {
+    vlaBoundNotPositive(data, bound_handle);
+}
+
 fn vlaBoundNotPositive(
     data: *const VlaBoundNotPositive,
     bound_handle: ValueHandle,
@@ -491,6 +594,13 @@ const FloatCastOverflowDataV2 = extern struct {
     to: *const TypeDescriptor,
 };
 
+fn floatCastOverflowAbort(
+    data_handle: *align(8) const anyopaque,
+    from_handle: ValueHandle,
+) callconv(.c) noreturn {
+    floatCastOverflow(data_handle, from_handle);
+}
+
 fn floatCastOverflow(
     data_handle: *align(8) const anyopaque,
     from_handle: ValueHandle,
@@ -514,22 +624,31 @@ fn floatCastOverflow(
 }
 
 inline fn logMessage(comptime fmt: []const u8, args: anytype) noreturn {
-    std.debug.panicExtra(null, @returnAddress(), fmt, args);
+    std.debug.panicExtra(@returnAddress(), fmt, args);
 }
 
 fn exportHandler(
     handler: anytype,
     comptime sym_name: []const u8,
-    comptime abort: bool,
 ) void {
-    const linkage = if (builtin.is_test) .internal else .weak;
+    const linkage = if (builtin.zig_backend == .stage2_x86_64 and builtin.os.tag == .windows) .internal else .weak;
+    const N = "__ubsan_handle_" ++ sym_name;
+    @export(handler, .{ .name = N, .linkage = linkage });
+}
+
+fn exportHandlerWithAbort(
+    handler: anytype,
+    abort_handler: anytype,
+    comptime sym_name: []const u8,
+) void {
+    const linkage = if (builtin.zig_backend == .stage2_x86_64 and builtin.os.tag == .windows) .internal else .weak;
     {
         const N = "__ubsan_handle_" ++ sym_name;
         @export(handler, .{ .name = N, .linkage = linkage });
     }
-    if (abort) {
+    {
         const N = "__ubsan_handle_" ++ sym_name ++ "_abort";
-        @export(handler, .{ .name = N, .linkage = linkage });
+        @export(abort_handler, .{ .name = N, .linkage = linkage });
     }
 }
 
@@ -539,24 +658,29 @@ const can_build_ubsan = switch (builtin.zig_backend) {
 };
 
 comptime {
-    overflowHandler("add_overflow", "+");
+    if (can_build_ubsan) {
-    overflowHandler("mul_overflow", "*");
+        overflowHandler("add_overflow", "+");
-    overflowHandler("sub_overflow", "-");
+        overflowHandler("mul_overflow", "*");
-    exportHandler(&alignmentAssumptionHandler, "alignment_assumption", true);
+        overflowHandler("sub_overflow", "-");
-    exportHandler(&builtinUnreachable, "builtin_unreachable", false);
+        exportHandlerWithAbort(&alignmentAssumptionHandler, &alignmentAssumptionHandlerAbort, "alignment_assumption");
-    exportHandler(&divRemHandler, "divrem_overflow", true);
+
-    exportHandler(&floatCastOverflow, "float_cast_overflow", true);
+        exportHandlerWithAbort(&divRemHandler, &divRemHandlerAbort, "divrem_overflow");
-    exportHandler(&invalidBuiltin, "invalid_builtin", true);
+        exportHandlerWithAbort(&floatCastOverflow, &floatCastOverflowAbort, "float_cast_overflow");
-    exportHandler(&loadInvalidValue, "load_invalid_value", true);
+        exportHandlerWithAbort(&invalidBuiltin, &invalidBuiltinAbort, "invalid_builtin");
-    exportHandler(&missingReturn, "missing_return", false);
+        exportHandlerWithAbort(&loadInvalidValue, &loadInvalidValueAbort, "load_invalid_value");
-    exportHandler(&negationHandler, "negate_overflow", true);
+
-    exportHandler(&nonNullArg, "nonnull_arg", true);
+        exportHandlerWithAbort(&negationHandler, &negationHandlerAbort, "negate_overflow");
-    exportHandler(&nonNullReturn, "nonnull_return_v1", true);
+        exportHandlerWithAbort(&nonNullArg, &nonNullArgAbort, "nonnull_arg");
-    exportHandler(&outOfBounds, "out_of_bounds", true);
+        exportHandlerWithAbort(&nonNullReturn, &nonNullReturnAbort, "nonnull_return_v1");
-    exportHandler(&pointerOverflow, "pointer_overflow", true);
+        exportHandlerWithAbort(&outOfBounds, &outOfBoundsAbort, "out_of_bounds");
-    exportHandler(&shiftOob, "shift_out_of_bounds", true);
+        exportHandlerWithAbort(&pointerOverflow, &pointerOverflowAbort, "pointer_overflow");
-    exportHandler(&typeMismatch, "type_mismatch_v1", true);
+        exportHandlerWithAbort(&shiftOob, &shiftOobAbort, "shift_out_of_bounds");
-    exportHandler(&vlaBoundNotPositive, "vla_bound_not_positive", true);
+        exportHandlerWithAbort(&typeMismatch, &typeMismatchAbort, "type_mismatch_v1");
+        exportHandlerWithAbort(&vlaBoundNotPositive, &vlaBoundNotPositiveAbort, "vla_bound_not_positive");
+
+        exportHandler(&builtinUnreachable, "builtin_unreachable");
+        exportHandler(&missingReturn, "missing_return");
+    }
 
     // these checks are nearly impossible to duplicate in zig, as they rely on nuances
     // in the Itanium C++ ABI.

src/Compilation.zig

@@ -1317,15 +1317,6 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
             break :s .obj;
         };
 
-        const ubsan_rt_strat: RtStrat = s: {
-            const want_ubsan_rt = options.want_ubsan_rt orelse (any_sanitize_c and output_mode != .Obj);
-            if (!want_ubsan_rt) break :s .none;
-            if (options.skip_linker_dependencies) break :s .none;
-            if (have_zcu) break :s .zcu;
-            if (is_exe_or_dyn_lib) break :s .lib;
-            break :s .obj;
-        };
-
         if (compiler_rt_strat == .zcu) {
             // For objects, this mechanism relies on essentially `_ = @import("compiler-rt");`
             // injected into the object.
@@ -1355,6 +1346,15 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
         // unlike compiler_rt, we always want to go through the `_ = @import("ubsan-rt")`
         // approach, since the ubsan runtime uses quite a lot of the standard library
         // and this reduces unnecessary bloat.
+        const ubsan_rt_strat: RtStrat = s: {
+            const want_ubsan_rt = options.want_ubsan_rt orelse (any_sanitize_c and output_mode != .Obj);
+            if (!want_ubsan_rt) break :s .none;
+            if (options.skip_linker_dependencies) break :s .none;
+            if (have_zcu) break :s .zcu;
+            if (is_exe_or_dyn_lib) break :s .lib;
+            break :s .obj;
+        };
+
         if (ubsan_rt_strat == .zcu) {
             const ubsan_rt_mod = try Package.Module.create(arena, .{
                 .global_cache_directory = options.global_cache_directory,
@@ -1362,7 +1362,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
                     .root = .{
                         .root_dir = options.zig_lib_directory,
                     },
-                    .root_src_path = "ubsan.zig",
+                    .root_src_path = "ubsan_rt.zig",
                 },
                 .fully_qualified_name = "ubsan_rt",
                 .cc_argv = &.{},
@@ -1925,25 +1925,8 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
                 comp.remaining_prelink_tasks += 1;
             }
 
-<<<<<<< HEAD
-            if (comp.include_compiler_rt and capable_of_building_compiler_rt) {
-                if (is_exe_or_dyn_lib) {
-=======
-            if (target.isMinGW() and comp.config.any_non_single_threaded) {
-                // LLD might drop some symbols as unused during LTO and GCing, therefore,
-                // we force mark them for resolution here.
-
-                const tls_index_sym = switch (target.cpu.arch) {
-                    .x86 => "__tls_index",
-                    else => "_tls_index",
-                };
-
-                try comp.force_undefined_symbols.put(comp.gpa, tls_index_sym, {});
-            }
-
             if (capable_of_building_compiler_rt) {
                 if (comp.compiler_rt_strat == .lib) {
->>>>>>> 050e3e69ac (Compilation: correct when to include ubsan)
                     log.debug("queuing a job to build compiler_rt_lib", .{});
                     comp.queued_jobs.compiler_rt_lib = true;
                     comp.remaining_prelink_tasks += 1;
@@ -1957,11 +1940,11 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
 
                 if (comp.ubsan_rt_strat == .lib) {
                     log.debug("queuing a job to build ubsan_rt_lib", .{});
-                    comp.job_queued_ubsan_rt_lib = true;
+                    comp.queued_jobs.ubsan_rt_lib = true;
                     comp.remaining_prelink_tasks += 1;
                 } else if (comp.ubsan_rt_strat == .obj) {
                     log.debug("queuing a job to build ubsan_rt_obj", .{});
-                    comp.job_queued_ubsan_rt_obj = true;
+                    comp.queued_jobs.ubsan_rt_obj = true;
                     comp.remaining_prelink_tasks += 1;
                 }
 
@@ -3782,11 +3765,11 @@ fn performAllTheWorkInner(
     }
 
     if (comp.queued_jobs.ubsan_rt_lib and comp.ubsan_rt_lib == null) {
-        comp.link_task_wait_group.spawnManager(buildRt, .{ comp, "ubsan_rt.zig", .libubsan, .Lib, &comp.ubsan_rt_lib, main_progress_node });
+        comp.link_task_wait_group.spawnManager(buildRt, .{ comp, "ubsan_rt.zig", .libubsan, .Lib, false, &comp.ubsan_rt_lib, main_progress_node });
     }
 
     if (comp.queued_jobs.ubsan_rt_obj and comp.ubsan_rt_obj == null) {
-        comp.link_task_wait_group.spawnManager(buildRt, .{ comp, "ubsan_rt.zig", .libubsan, .Obj, &comp.ubsan_rt_obj, main_progress_node });
+        comp.link_task_wait_group.spawnManager(buildRt, .{ comp, "ubsan_rt.zig", .libubsan, .Obj, false, &comp.ubsan_rt_obj, main_progress_node });
     }
 
     if (comp.queued_jobs.glibc_shared_objects) {
@@ -6037,9 +6020,16 @@ pub fn addCCArgs(
                         try argv.append("-fno-sanitize=function");
 
                         // It's recommended to use the minimal runtime in production environments
-                        // due to the security implications of the full runtime.
+                        // due to the security implications of the full runtime. The minimal runtime
+                        // doesn't provide much benefit over simply trapping.
                         if (mod.optimize_mode == .ReleaseSafe) {
-                            try argv.append("-fsanitize-minimal-runtime");
+                            try argv.append("-fsanitize-trap=undefined");
+                        }
+
+                        // This is necessary because, by default, Clang instructs LLVM to embed a COFF link
+                        // dependency on `libclang_rt.ubsan_standalone.a` when the UBSan runtime is used.
+                        if (target.os.tag == .windows) {
+                            try argv.append("-fno-rtlib-defaultlib");
                         }
                     }
                 }

test/link/elf.zig

@@ -2049,6 +2049,8 @@ fn testLargeBss(b: *Build, opts: Options) *Step {
         \\}
     , &.{});
     exe.linkLibC();
+    // Disabled to work around an ELF linker bug.
+    exe.root_module.sanitize_c = false;
 
     const run = addRunArtifact(exe);
     run.expectExitCode(0);
@@ -3552,6 +3554,8 @@ fn testTlsLargeTbss(b: *Build, opts: Options) *Step {
         \\}
     , &.{});
     exe.linkLibC();
+    // Disabled to work around an ELF linker bug.
+    exe.root_module.sanitize_c = false;
 
     const run = addRunArtifact(exe);
     run.expectStdOutEqual("3 0 5 0 0 0\n");

test/link/glibc_compat/build.zig

@@ -22,6 +22,8 @@ pub fn build(b: *std.Build) void {
                 .link_libc = true,
             }),
         });
+        exe.bundle_ubsan_rt = false;
+        exe.root_module.sanitize_c = false;
         exe.root_module.addCSourceFile(.{ .file = b.path("main.c") });
         // TODO: actually test the output
         _ = exe.getEmittedBin();
@@ -62,6 +64,8 @@ pub fn build(b: *std.Build) void {
                 .link_libc = true,
             }),
         });
+        exe.bundle_ubsan_rt = false;
+        exe.root_module.sanitize_c = false;
         exe.root_module.addCSourceFile(.{ .file = b.path("glibc_runtime_check.c") });
 
         // Only try running the test if the host glibc is known to be good enough.  Ideally, the Zig
@@ -161,6 +165,8 @@ pub fn build(b: *std.Build) void {
                 .link_libc = true,
             }),
         });
+        exe.bundle_ubsan_rt = false;
+        exe.root_module.sanitize_c = false;
 
         // Only try running the test if the host glibc is known to be good enough.  Ideally, the Zig
         // test runner would be able to check this, but see https://github.com/ziglang/zig/pull/17702#issuecomment-1831310453

test/link/wasm/export-data/build.zig

@@ -13,6 +13,7 @@ pub fn build(b: *std.Build) void {
         }),
     });
     lib.entry = .disabled;
+    lib.bundle_ubsan_rt = false;
     lib.use_lld = false;
     lib.root_module.export_symbol_names = &.{ "foo", "bar" };
     // Object being linked has neither functions nor globals named "foo" or "bar" and

test/link/wasm/export/build.zig

@@ -19,6 +19,7 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
     no_export.entry = .disabled;
     no_export.use_llvm = false;
     no_export.use_lld = false;
+    no_export.bundle_ubsan_rt = false;
 
     const dynamic_export = b.addExecutable(.{
         .name = "dynamic",
@@ -32,6 +33,8 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
     dynamic_export.rdynamic = true;
     dynamic_export.use_llvm = false;
     dynamic_export.use_lld = false;
+    // don't pull in ubsan, since we're just expecting a minimal executable
+    dynamic_export.bundle_ubsan_rt = false;
 
     const force_export = b.addExecutable(.{
         .name = "force",
@@ -45,6 +48,7 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
     force_export.root_module.export_symbol_names = &.{"foo"};
     force_export.use_llvm = false;
     force_export.use_lld = false;
+    force_export.bundle_ubsan_rt = false;
 
     const check_no_export = no_export.checkObject();
     check_no_export.checkInHeaders();

test/link/wasm/function-table/build.zig

@@ -21,6 +21,7 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
     export_table.use_lld = false;
     export_table.export_table = true;
     export_table.link_gc_sections = false;
+    export_table.bundle_ubsan_rt = false;
 
     const regular_table = b.addExecutable(.{
         .name = "regular_table",
@@ -34,6 +35,7 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
     regular_table.use_llvm = false;
     regular_table.use_lld = false;
     regular_table.link_gc_sections = false; // Ensure function table is not empty
+    regular_table.bundle_ubsan_rt = false;
 
     const check_export = export_table.checkObject();
     const check_regular = regular_table.checkObject();

test/link/wasm/shared-memory/build.zig

@@ -31,6 +31,7 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize_mode: std.builtin.Opt
     exe.shared_memory = true;
     exe.max_memory = 67108864;
     exe.root_module.export_symbol_names = &.{"foo"};
+    exe.bundle_ubsan_rt = false;
 
     const check_exe = exe.checkObject();
 

test/link/wasm/type/build.zig

@@ -21,6 +21,7 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
     exe.use_llvm = false;
     exe.use_lld = false;
     exe.root_module.export_symbol_names = &.{"foo"};
+    exe.bundle_ubsan_rt = false;
     b.installArtifact(exe);
 
     const check_exe = exe.checkObject();

test/src/StackTrace.zig

@@ -81,6 +81,7 @@ fn addExpect(
         }),
         .use_llvm = use_llvm,
     });
+    exe.bundle_ubsan_rt = false;
 
     const run = b.addRunArtifact(exe);
     run.removeEnvironmentVariable("CLICOLOR_FORCE");

tools/incr-check.zig

@@ -108,6 +108,7 @@ pub fn main() !void {
             "build-exe",
             case.root_source_file,
             "-fincremental",
+            "-fno-ubsan-rt",
             "-target",
             target.query,
             "--cache-dir",