Skip to content

Commit

Permalink
Compilation: correct when to include ubsan
Browse files Browse the repository at this point in the history
  • Loading branch information
Rexicon226 committed Jan 27, 2025
1 parent 89da0d5 commit 921e87c
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 69 deletions.
48 changes: 7 additions & 41 deletions lib/ubsan.zig → lib/ubsan_rt.zig
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const Value = extern struct {
return switch (size) {
64 => @as(*const u64, @alignCast(@ptrCast(value.handle))).*,
128 => @as(*const u128, @alignCast(@ptrCast(value.handle))).*,
else => unreachable,
else => @trap(),
};
}

Expand Down Expand Up @@ -186,8 +186,8 @@ fn divRemHandler(
lhs_handle: ValueHandle,
rhs_handle: ValueHandle,
) callconv(.c) noreturn {
const lhs: Value = .{ .handle = lhs_handle, .td = data.lhs_type };
const rhs: Value = .{ .handle = rhs_handle, .td = data.rhs_type };
const lhs: Value = .{ .handle = lhs_handle, .td = data.td };
const rhs: Value = .{ .handle = rhs_handle, .td = data.td };

if (rhs.isMinusOne()) {
logMessage(
Expand Down Expand Up @@ -533,26 +533,10 @@ fn exportHandler(
}
}

fn exportMinimal(
comptime err_name: []const u8,
comptime sym_name: []const u8,
comptime abort: bool,
) void {
const S = struct {
fn handler() callconv(.c) noreturn {
logMessage("{s}", .{err_name});
}
};
const linkage = if (builtin.is_test) .internal else .weak;
{
const N = "__ubsan_handle_" ++ sym_name ++ "_minimal";
@export(&S.handler, .{ .name = N, .linkage = linkage });
}
if (abort) {
const N = "__ubsan_handle_" ++ sym_name ++ "_minimal_abort";
@export(&S.handler, .{ .name = N, .linkage = linkage });
}
}
const can_build_ubsan = switch (builtin.zig_backend) {
.stage2_riscv64 => false,
else => true,
};

comptime {
overflowHandler("add_overflow", "+");
Expand All @@ -574,24 +558,6 @@ comptime {
exportHandler(&typeMismatch, "type_mismatch_v1", true);
exportHandler(&vlaBoundNotPositive, "vla_bound_not_positive", true);

exportMinimal("add-overflow", "add_overflow", true);
exportMinimal("sub-overflow", "sub_overflow", true);
exportMinimal("mul-overflow", "mul_overflow", true);
exportMinimal("alignment-assumption-handler", "alignment_assumption", true);
exportMinimal("builtin-unreachable", "builtin_unreachable", false);
exportMinimal("divrem-handler", "divrem_overflow", true);
exportMinimal("float-cast-overflow", "float_cast_overflow", true);
exportMinimal("invalid-builtin", "invalid_builtin", true);
exportMinimal("load-invalid-value", "load_invalid_value", true);
exportMinimal("missing-return", "missing_return", true);
exportMinimal("negation-handler", "negate_overflow", true);
exportMinimal("nonnull-arg", "nonnull_arg", true);
exportMinimal("out-of-bounds", "out_of_bounds", true);
exportMinimal("pointer-overflow", "pointer_overflow", true);
exportMinimal("shift-oob", "shift_out_of_bounds", true);
exportMinimal("type-mismatch", "type_mismatch", true);
exportMinimal("vla-bound-not-positive", "vla_bound_not_positive", true);

// these checks are nearly impossible to duplicate in zig, as they rely on nuances
// in the Itanium C++ ABI.
// exportHelper("dynamic_type_cache_miss", "dynamic-type-cache-miss", true);
Expand Down
81 changes: 57 additions & 24 deletions src/Compilation.zig
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ implib_emit: ?Path,
/// This is non-null when `-femit-docs` is provided.
docs_emit: ?Path,
root_name: [:0]const u8,
include_compiler_rt: bool,
include_ubsan_rt: bool,
compiler_rt_strat: RtStrat,
ubsan_rt_strat: RtStrat,
/// Resolved into known paths, any GNU ld scripts already resolved.
link_inputs: []const link.Input,
/// Needed only for passing -F args to clang.
Expand Down Expand Up @@ -1253,6 +1253,8 @@ fn addModuleTableToCacheHash(
}
}

const RtStrat = enum { none, lib, obj, zcu };

pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compilation {
const output_mode = options.config.output_mode;
const is_dyn_lib = switch (output_mode) {
Expand Down Expand Up @@ -1284,6 +1286,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
const any_unwind_tables = options.config.any_unwind_tables or options.root_mod.unwind_tables != .none;
const any_non_single_threaded = options.config.any_non_single_threaded or !options.root_mod.single_threaded;
const any_sanitize_thread = options.config.any_sanitize_thread or options.root_mod.sanitize_thread;
const any_sanitize_c = options.config.any_sanitize_c or options.root_mod.sanitize_c;
const any_fuzz = options.config.any_fuzz or options.root_mod.fuzz;

const link_eh_frame_hdr = options.link_eh_frame_hdr or any_unwind_tables;
Expand All @@ -1302,13 +1305,25 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil

const sysroot = options.sysroot orelse libc_dirs.sysroot;

const include_compiler_rt = options.want_compiler_rt orelse
(!options.skip_linker_dependencies and is_exe_or_dyn_lib);
const compiler_rt_strat: RtStrat = s: {
if (options.skip_linker_dependencies) break :s .none;
const want = options.want_compiler_rt orelse is_exe_or_dyn_lib;
if (!want) break :s .none;
if (have_zcu and output_mode == .Obj) break :s .zcu;
if (is_exe_or_dyn_lib) break :s .lib;
break :s .obj;
};

const include_ubsan_rt = options.want_ubsan_rt orelse
(!options.skip_linker_dependencies and is_exe_or_dyn_lib and !have_zcu);
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 (include_compiler_rt and output_mode == .Obj) {
if (compiler_rt_strat == .zcu) {
// For objects, this mechanism relies on essentially `_ = @import("compiler-rt");`
// injected into the object.
const compiler_rt_mod = try Package.Module.create(arena, .{
Expand Down Expand Up @@ -1337,7 +1352,7 @@ 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.
if (!options.skip_linker_dependencies and have_zcu) {
if (ubsan_rt_strat == .zcu) {
const ubsan_rt_mod = try Package.Module.create(arena, .{
.global_cache_directory = options.global_cache_directory,
.paths = .{
Expand Down Expand Up @@ -1533,8 +1548,8 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
.windows_libs = windows_libs,
.version = options.version,
.libc_installation = libc_dirs.libc_installation,
.include_compiler_rt = include_compiler_rt,
.include_ubsan_rt = include_ubsan_rt,
.compiler_rt_strat = compiler_rt_strat,
.ubsan_rt_strat = ubsan_rt_strat,
.link_inputs = options.link_inputs,
.framework_dirs = options.framework_dirs,
.llvm_opt_bisect_limit = options.llvm_opt_bisect_limit,
Expand All @@ -1560,6 +1575,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
comp.config.any_unwind_tables = any_unwind_tables;
comp.config.any_non_single_threaded = any_non_single_threaded;
comp.config.any_sanitize_thread = any_sanitize_thread;
comp.config.any_sanitize_c = any_sanitize_c;
comp.config.any_fuzz = any_fuzz;

const lf_open_opts: link.File.OpenOptions = .{
Expand Down Expand Up @@ -1914,34 +1930,51 @@ 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;
} else if (output_mode != .Obj) {
} else if (comp.compiler_rt_strat == .obj) {
log.debug("queuing a job to build compiler_rt_obj", .{});
// In this case we are making a static library, so we ask
// for a compiler-rt object to put in it.
comp.queued_jobs.compiler_rt_obj = true;
comp.remaining_prelink_tasks += 1;
}
}

if (comp.include_ubsan_rt and capable_of_building_compiler_rt) {
if (is_exe_or_dyn_lib) {
if (comp.ubsan_rt_strat == .lib) {
log.debug("queuing a job to build ubsan_rt_lib", .{});
comp.job_queued_ubsan_rt_lib = true;
} else if (output_mode != .Obj) {
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.remaining_prelink_tasks += 1;
}
}

if (is_exe_or_dyn_lib and comp.config.any_fuzz and capable_of_building_compiler_rt) {
log.debug("queuing a job to build libfuzzer", .{});
comp.queued_jobs.fuzzer_lib = true;
comp.remaining_prelink_tasks += 1;
if (is_exe_or_dyn_lib and comp.config.any_fuzz) {
log.debug("queuing a job to build libfuzzer", .{});
comp.queued_jobs.fuzzer_lib = true;
comp.remaining_prelink_tasks += 1;
}
}
}

Expand Down Expand Up @@ -2659,8 +2692,8 @@ fn addNonIncrementalStuffToCacheManifest(
man.hash.addOptional(comp.version);
man.hash.add(comp.link_eh_frame_hdr);
man.hash.add(comp.skip_linker_dependencies);
man.hash.add(comp.include_compiler_rt);
man.hash.add(comp.include_ubsan_rt);
man.hash.add(comp.compiler_rt_strat);
man.hash.add(comp.ubsan_rt_strat);
man.hash.add(comp.rc_includes);
man.hash.addListOfBytes(comp.force_undefined_symbols.keys());
man.hash.addListOfBytes(comp.framework_dirs);
Expand Down Expand Up @@ -3750,11 +3783,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.zig", .libubsan, .Lib, &comp.ubsan_rt_lib, main_progress_node });
comp.link_task_wait_group.spawnManager(buildRt, .{ comp, "ubsan_rt.zig", .libubsan, .Lib, &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.zig", .libubsan, .Obj, &comp.ubsan_rt_obj, main_progress_node });
comp.link_task_wait_group.spawnManager(buildRt, .{ comp, "ubsan_rt.zig", .libubsan, .Obj, &comp.ubsan_rt_obj, main_progress_node });
}

if (comp.queued_jobs.glibc_shared_objects) {
Expand Down
3 changes: 3 additions & 0 deletions src/Compilation/Config.zig
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ any_non_single_threaded: bool,
/// per-Module setting.
any_error_tracing: bool,
any_sanitize_thread: bool,
any_sanitize_c: bool,
any_fuzz: bool,
pie: bool,
/// If this is true then linker code is responsible for making an LLVM IR
Expand Down Expand Up @@ -87,6 +88,7 @@ pub const Options = struct {
ensure_libcpp_on_non_freestanding: bool = false,
any_non_single_threaded: bool = false,
any_sanitize_thread: bool = false,
any_sanitize_c: bool = false,
any_fuzz: bool = false,
any_unwind_tables: bool = false,
any_dyn_libs: bool = false,
Expand Down Expand Up @@ -476,6 +478,7 @@ pub fn resolve(options: Options) ResolveError!Config {
.any_non_single_threaded = options.any_non_single_threaded,
.any_error_tracing = any_error_tracing,
.any_sanitize_thread = options.any_sanitize_thread,
.any_sanitize_c = options.any_sanitize_c,
.any_fuzz = options.any_fuzz,
.san_cov_trace_pc_guard = options.san_cov_trace_pc_guard,
.root_error_tracing = root_error_tracing,
Expand Down
4 changes: 2 additions & 2 deletions src/link.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1101,12 +1101,12 @@ pub const File = struct {

log.debug("zcu_obj_path={s}", .{if (zcu_obj_path) |s| s else "(null)"});

const compiler_rt_path: ?Path = if (comp.include_compiler_rt)
const compiler_rt_path: ?Path = if (comp.compiler_rt_strat == .obj)
comp.compiler_rt_obj.?.full_object_path
else
null;

const ubsan_rt_path: ?Path = if (comp.include_ubsan_rt)
const ubsan_rt_path: ?Path = if (comp.ubsan_rt_strat == .obj)
comp.ubsan_rt_obj.?.full_object_path
else
null;
Expand Down
4 changes: 2 additions & 2 deletions src/link/MachO/relocatable.zig
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ pub fn flushStaticLib(macho_file: *MachO, comp: *Compilation, module_obj_path: ?

if (module_obj_path) |path| try positionals.append(try link.openObjectInput(diags, path));

if (comp.include_compiler_rt) {
if (comp.compiler_rt_strat == .obj) {
try positionals.append(try link.openObjectInput(diags, comp.compiler_rt_obj.?.full_object_path));
}

if (comp.include_ubsan_rt) {
if (comp.ubsan_rt_strat == .obj) {
try positionals.append(try link.openObjectInput(diags, comp.ubsan_rt_obj.?.full_object_path));
}

Expand Down

0 comments on commit 921e87c

Please sign in to comment.