Skip to content

stage2: implement stack protectors #12472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 76 additions & 45 deletions src/Compilation.zig
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ astgen_wait_group: WaitGroup = .{},
/// TODO: Remove this when Stage2 becomes the default compiler as it will already have this information.
export_symbol_names: std.ArrayListUnmanaged([]const u8) = .{},

pub const default_stack_protector_buffer_size = 4;
pub const SemaError = Module.SemaError;

pub const CRTFile = struct {
Expand Down Expand Up @@ -837,6 +838,10 @@ pub const InitOptions = struct {
want_pie: ?bool = null,
want_sanitize_c: ?bool = null,
want_stack_check: ?bool = null,
/// null means default.
/// 0 means no stack protector.
/// other number means stack protection with that buffer size.
want_stack_protector: ?u32 = null,
want_red_zone: ?bool = null,
omit_frame_pointer: ?bool = null,
want_valgrind: ?bool = null,
Expand Down Expand Up @@ -1014,6 +1019,15 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
return error.ExportTableAndImportTableConflict;
}

// The `have_llvm` condition is here only because native backends cannot yet build compiler-rt.
// Once they are capable this condition could be removed. When removing this condition,
// also test the use case of `build-obj -fcompiler-rt` with the native backends
// and make sure the compiler-rt symbols are emitted.
const capable_of_building_compiler_rt = build_options.have_llvm;

const capable_of_building_zig_libc = build_options.have_llvm;
const capable_of_building_ssp = build_options.have_llvm;

const comp: *Compilation = comp: {
// For allocations that have the same lifetime as Compilation. This arena is used only during this
// initialization and then is freed in deinit().
Expand Down Expand Up @@ -1289,11 +1303,36 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {

const sanitize_c = options.want_sanitize_c orelse is_safe_mode;

const stack_check: bool = b: {
if (!target_util.supportsStackProbing(options.target))
break :b false;
break :b options.want_stack_check orelse is_safe_mode;
const stack_check: bool = options.want_stack_check orelse b: {
if (!target_util.supportsStackProbing(options.target)) break :b false;
break :b is_safe_mode;
};
if (stack_check and !target_util.supportsStackProbing(options.target))
return error.StackCheckUnsupportedByTarget;

const stack_protector: u32 = options.want_stack_protector orelse b: {
if (!target_util.supportsStackProtector(options.target)) break :b @as(u32, 0);

// This logic is checking for linking libc because otherwise our start code
// which is trying to set up TLS (i.e. the fs/gs registers) but the stack
// protection code depends on fs/gs registers being already set up.
// If we were able to annotate start code, or perhaps the entire std lib,
// as being exempt from stack protection checks, we could change this logic
// to supporting stack protection even when not linking libc.
// TODO file issue about this
if (!link_libc) break :b 0;
if (!capable_of_building_ssp) break :b 0;
if (is_safe_mode) break :b default_stack_protector_buffer_size;
break :b 0;
};
if (stack_protector != 0) {
if (!target_util.supportsStackProtector(options.target))
return error.StackProtectorUnsupportedByTarget;
if (!capable_of_building_ssp)
return error.StackProtectorUnsupportedByBackend;
if (!link_libc)
return error.StackProtectorUnavailableWithoutLibC;
}

const valgrind: bool = b: {
if (!target_util.hasValgrindSupport(options.target))
Expand Down Expand Up @@ -1378,6 +1417,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
cache.hash.add(unwind_tables);
cache.hash.add(tsan);
cache.hash.add(stack_check);
cache.hash.add(stack_protector);
cache.hash.add(red_zone);
cache.hash.add(omit_frame_pointer);
cache.hash.add(link_mode);
Expand Down Expand Up @@ -1741,6 +1781,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
.valgrind = valgrind,
.tsan = tsan,
.stack_check = stack_check,
.stack_protector = stack_protector,
.red_zone = red_zone,
.omit_frame_pointer = omit_frame_pointer,
.single_threaded = single_threaded,
Expand Down Expand Up @@ -1822,6 +1863,8 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
};
errdefer comp.destroy();

const target = comp.getTarget();

// Add a `CObject` for each `c_source_files`.
try comp.c_object_table.ensureTotalCapacity(gpa, options.c_source_files.len);
for (options.c_source_files) |c_source_file| {
Expand All @@ -1837,11 +1880,9 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {

const have_bin_emit = comp.bin_file.options.emit != null or comp.whole_bin_sub_path != null;

if (have_bin_emit and !comp.bin_file.options.skip_linker_dependencies and
options.target.ofmt != .c)
{
if (comp.getTarget().isDarwin()) {
switch (comp.getTarget().abi) {
if (have_bin_emit and !comp.bin_file.options.skip_linker_dependencies and target.ofmt != .c) {
if (target.isDarwin()) {
switch (target.abi) {
.none,
.simulator,
.macabi,
Expand All @@ -1852,9 +1893,9 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
// If we need to build glibc for the target, add work items for it.
// We go through the work queue so that building can be done in parallel.
if (comp.wantBuildGLibCFromSource()) {
if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable;
if (!target_util.canBuildLibC(target)) return error.LibCUnavailable;

if (glibc.needsCrtiCrtn(comp.getTarget())) {
if (glibc.needsCrtiCrtn(target)) {
try comp.work_queue.write(&[_]Job{
.{ .glibc_crt_file = .crti_o },
.{ .glibc_crt_file = .crtn_o },
Expand All @@ -1867,10 +1908,10 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
});
}
if (comp.wantBuildMuslFromSource()) {
if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable;
if (!target_util.canBuildLibC(target)) return error.LibCUnavailable;

try comp.work_queue.ensureUnusedCapacity(6);
if (musl.needsCrtiCrtn(comp.getTarget())) {
if (musl.needsCrtiCrtn(target)) {
comp.work_queue.writeAssumeCapacity(&[_]Job{
.{ .musl_crt_file = .crti_o },
.{ .musl_crt_file = .crtn_o },
Expand All @@ -1887,7 +1928,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
});
}
if (comp.wantBuildWasiLibcFromSource()) {
if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable;
if (!target_util.canBuildLibC(target)) return error.LibCUnavailable;

const wasi_emulated_libs = comp.bin_file.options.wasi_emulated_libs;
try comp.work_queue.ensureUnusedCapacity(wasi_emulated_libs.len + 2); // worst-case we need all components
Expand All @@ -1902,7 +1943,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
});
}
if (comp.wantBuildMinGWFromSource()) {
if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable;
if (!target_util.canBuildLibC(target)) return error.LibCUnavailable;

const static_lib_jobs = [_]Job{
.{ .mingw_crt_file = .mingw32_lib },
Expand All @@ -1921,7 +1962,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
}
}
// Generate Windows import libs.
if (comp.getTarget().os.tag == .windows) {
if (target.os.tag == .windows) {
const count = comp.bin_file.options.system_libs.count();
try comp.work_queue.ensureUnusedCapacity(count);
var i: usize = 0;
Expand All @@ -1940,15 +1981,6 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
try comp.work_queue.writeItem(.libtsan);
}

// The `have_llvm` condition is here only because native backends cannot yet build compiler-rt.
// Once they are capable this condition could be removed. When removing this condition,
// also test the use case of `build-obj -fcompiler-rt` with the native backends
// and make sure the compiler-rt symbols are emitted.
const capable_of_building_compiler_rt = build_options.have_llvm;

const capable_of_building_zig_libc = build_options.have_llvm;
const capable_of_building_ssp = comp.bin_file.options.use_stage1;

if (comp.bin_file.options.include_compiler_rt and capable_of_building_compiler_rt) {
if (is_exe_or_dyn_lib) {
log.debug("queuing a job to build compiler_rt_lib", .{});
Expand All @@ -1962,8 +1994,11 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
}
}
if (needs_c_symbols) {
// MinGW provides no libssp, use our own implementation.
if (comp.getTarget().isMinGW() and capable_of_building_ssp) {
// Related: https://github.com/ziglang/zig/issues/7265.
if (comp.bin_file.options.stack_protector != 0 and
(!comp.bin_file.options.link_libc or
!target_util.libcProvidesStackProtector(target)))
{
try comp.work_queue.writeItem(.{ .libssp = {} });
}

Expand Down Expand Up @@ -4123,6 +4158,17 @@ pub fn addCCArgs(
try argv.append("-fno-omit-frame-pointer");
}

const ssp_buf_size = comp.bin_file.options.stack_protector;
if (ssp_buf_size != 0) {
try argv.appendSlice(&[_][]const u8{
"-fstack-protector-strong",
"--param",
try std.fmt.allocPrint(arena, "ssp-buffer-size={d}", .{ssp_buf_size}),
});
} else {
try argv.append("-fno-stack-protector");
}

switch (comp.bin_file.options.optimize_mode) {
.Debug => {
// windows c runtime requires -D_DEBUG if using debug libraries
Expand All @@ -4131,27 +4177,12 @@ pub fn addCCArgs(
// to -O1. Besides potentially impairing debugging, -O1/-Og significantly
// increases compile times.
try argv.append("-O0");

if (comp.bin_file.options.link_libc and target.os.tag != .wasi) {
try argv.append("-fstack-protector-strong");
try argv.append("--param");
try argv.append("ssp-buffer-size=4");
} else {
try argv.append("-fno-stack-protector");
}
},
.ReleaseSafe => {
// See the comment in the BuildModeFastRelease case for why we pass -O2 rather
// than -O3 here.
try argv.append("-O2");
if (comp.bin_file.options.link_libc and target.os.tag != .wasi) {
try argv.append("-D_FORTIFY_SOURCE=2");
try argv.append("-fstack-protector-strong");
try argv.append("--param");
try argv.append("ssp-buffer-size=4");
} else {
try argv.append("-fno-stack-protector");
}
try argv.append("-D_FORTIFY_SOURCE=2");
},
.ReleaseFast => {
try argv.append("-DNDEBUG");
Expand All @@ -4161,12 +4192,10 @@ pub fn addCCArgs(
// Zig code than it is for C code. Also, C programmers are used to their code
// running in -O2 and thus the -O3 path has been tested less.
try argv.append("-O2");
try argv.append("-fno-stack-protector");
},
.ReleaseSmall => {
try argv.append("-DNDEBUG");
try argv.append("-Os");
try argv.append("-fno-stack-protector");
},
}

Expand Down Expand Up @@ -5031,6 +5060,7 @@ fn buildOutputFromZig(
.use_stage1 = build_options.is_stage1 and comp.bin_file.options.use_stage1,
.want_sanitize_c = false,
.want_stack_check = false,
.want_stack_protector = 0,
.want_red_zone = comp.bin_file.options.red_zone,
.omit_frame_pointer = comp.bin_file.options.omit_frame_pointer,
.want_valgrind = false,
Expand Down Expand Up @@ -5311,6 +5341,7 @@ pub fn build_crt_file(
.optimize_mode = comp.compilerRtOptMode(),
.want_sanitize_c = false,
.want_stack_check = false,
.want_stack_protector = 0,
.want_red_zone = comp.bin_file.options.red_zone,
.omit_frame_pointer = comp.bin_file.options.omit_frame_pointer,
.want_valgrind = false,
Expand Down
15 changes: 8 additions & 7 deletions src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -7567,45 +7567,46 @@ fn handleExternLibName(
) CompileError![:0]u8 {
blk: {
const mod = sema.mod;
const comp = mod.comp;
const target = mod.getTarget();
log.debug("extern fn symbol expected in lib '{s}'", .{lib_name});
if (target_util.is_libc_lib_name(target, lib_name)) {
if (!mod.comp.bin_file.options.link_libc) {
if (!comp.bin_file.options.link_libc and !comp.bin_file.options.parent_compilation_link_libc) {
return sema.fail(
block,
src_loc,
"dependency on libc must be explicitly specified in the build command",
.{},
);
}
mod.comp.bin_file.options.link_libc = true;
comp.bin_file.options.link_libc = true;
break :blk;
}
if (target_util.is_libcpp_lib_name(target, lib_name)) {
if (!mod.comp.bin_file.options.link_libcpp) {
if (!comp.bin_file.options.link_libcpp) {
return sema.fail(
block,
src_loc,
"dependency on libc++ must be explicitly specified in the build command",
.{},
);
}
mod.comp.bin_file.options.link_libcpp = true;
comp.bin_file.options.link_libcpp = true;
break :blk;
}
if (mem.eql(u8, lib_name, "unwind")) {
mod.comp.bin_file.options.link_libunwind = true;
comp.bin_file.options.link_libunwind = true;
break :blk;
}
if (!target.isWasm() and !mod.comp.bin_file.options.pic) {
if (!target.isWasm() and !comp.bin_file.options.pic) {
return sema.fail(
block,
src_loc,
"dependency on dynamic library '{s}' requires enabling Position Independent Code. Fixed by `-l{s}` or `-fPIC`.",
.{ lib_name, lib_name },
);
}
mod.comp.stage1AddLinkLib(lib_name) catch |err| {
comp.stage1AddLinkLib(lib_name) catch |err| {
return sema.fail(block, src_loc, "unable to add link lib '{s}': {s}", .{
lib_name, @errorName(err),
});
Expand Down
45 changes: 40 additions & 5 deletions src/clang_options_data.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3290,7 +3290,14 @@ flagpd1("fno-stack-arrays"),
.psl = false,
},
flagpd1("fno-stack-clash-protection"),
flagpd1("fno-stack-protector"),
.{
.name = "fno-stack-protector",
.syntax = .flag,
.zig_equivalent = .no_stack_protector,
.pd1 = true,
.pd2 = false,
.psl = false,
},
flagpd1("fno-stack-size-section"),
flagpd1("fno-standalone-debug"),
flagpd1("fno-strength-reduce"),
Expand Down Expand Up @@ -3588,9 +3595,30 @@ flagpd1("fstack-arrays"),
.psl = false,
},
flagpd1("fstack-clash-protection"),
flagpd1("fstack-protector"),
flagpd1("fstack-protector-all"),
flagpd1("fstack-protector-strong"),
.{
.name = "fstack-protector",
.syntax = .flag,
.zig_equivalent = .stack_protector,
.pd1 = true,
.pd2 = false,
.psl = false,
},
.{
.name = "fstack-protector-all",
.syntax = .flag,
.zig_equivalent = .stack_protector,
.pd1 = true,
.pd2 = false,
.psl = false,
},
.{
.name = "fstack-protector-strong",
.syntax = .flag,
.zig_equivalent = .stack_protector,
.pd1 = true,
.pd2 = false,
.psl = false,
},
flagpd1("fstack-size-section"),
flagpd1("fstack-usage"),
flagpd1("fstandalone-debug"),
Expand Down Expand Up @@ -4809,7 +4837,14 @@ flagpd1("single_module"),
},
sepd1("split-dwarf-file"),
sepd1("split-dwarf-output"),
sepd1("stack-protector"),
.{
.name = "stack-protector",
.syntax = .separate,
.zig_equivalent = .stack_protector,
.pd1 = true,
.pd2 = false,
.psl = false,
},
sepd1("stack-protector-buffer-size"),
sepd1("stack-usage-file"),
.{
Expand Down
Loading