Skip to content

Commit 104c68c

Browse files
authored
Unrolled build for #157669
Rollup merge of #157669 - 1c3t3a:cfi-diag-mode, r=rcvalle cfi: add diag mode support Currently a Rust CFI failure only inserts a ud2. However, for clang we have the option for a helpful diagnostic message that explains the violation and is especially helpful for fixing it. This message works through hooking the UBSan runtime and calling into it with the necessary information for a helpful error message. In clang, this is enabled via `-fno-sanitize-trap=cfi`. This change adds the same behavior to rustc's CFI. Instead of a `no-sanitize-trap` flag, we added `-Z cfi-mode={diag|trap}`, with trap as the default. The diag mode will print the following error message for a violation: ``` tests/ui/sanitizer/cfi/fn-ptr-type-mismatch.rs:1:1: runtime error: control flow integrity check for type fn(i32, i32) -> i32 failed during indirect function call fn_ptr_type_mismatch.ecd806f409c5c1fc-cgu.0: note: fn_ptr_type_mismatch::add_one defined here SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tests/ui/sanitizer/cfi/fn-ptr-type-mismatch.rs:1:1 ``` r? @rcvalle
2 parents 83709ee + 3955b03 commit 104c68c

10 files changed

Lines changed: 229 additions & 20 deletions

File tree

‎compiler/rustc_codegen_llvm/src/builder.rs‎

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use crate::type_of::LayoutLlvmExt;
4444
pub(crate) struct GenericBuilder<'a, 'll, CX: Borrow<SCx<'ll>>> {
4545
pub llbuilder: &'ll mut llvm::Builder<'ll>,
4646
pub cx: &'a GenericCx<'ll, CX>,
47+
pub span: rustc_span::Span,
4748
}
4849

4950
pub(crate) type SBuilder<'a, 'll> = GenericBuilder<'a, 'll, SCx<'ll>>;
@@ -94,7 +95,7 @@ impl<'a, 'll, CX: Borrow<SCx<'ll>>> GenericBuilder<'a, 'll, CX> {
9495
fn with_cx(scx: &'a GenericCx<'ll, CX>) -> Self {
9596
// Create a fresh builder from the simple context.
9697
let llbuilder = unsafe { llvm::LLVMCreateBuilderInContext(scx.deref().borrow().llcx) };
97-
GenericBuilder { llbuilder, cx: scx }
98+
GenericBuilder { llbuilder, cx: scx, span: rustc_span::DUMMY_SP }
9899
}
99100

100101
pub(crate) fn append_block(
@@ -304,7 +305,9 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
304305
unsafe { llvm::LLVMGetInsertBlock(self.llbuilder) }
305306
}
306307

307-
fn set_span(&mut self, _span: Span) {}
308+
fn set_span(&mut self, span: rustc_span::Span) {
309+
self.span = span;
310+
}
308311

309312
fn append_block(cx: &'a CodegenCx<'ll, 'tcx>, llfn: &'ll Value, name: &str) -> &'ll BasicBlock {
310313
unsafe {
@@ -1564,6 +1567,51 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
15641567
pub(crate) fn llfn(&self) -> &'ll Value {
15651568
unsafe { llvm::LLVMGetBasicBlockParent(self.llbb()) }
15661569
}
1570+
1571+
fn generate_ubsan_cfi_diag_data(
1572+
&mut self,
1573+
span: rustc_span::Span,
1574+
expected_ty: String,
1575+
check_kind: u8,
1576+
) -> &'ll Value {
1577+
let cx = self.cx();
1578+
let tcx = cx.tcx;
1579+
1580+
let loc = tcx.sess.source_map().lookup_char_pos(span.lo());
1581+
1582+
let filename_str = format!("{}\0", loc.file.name.prefer_local_unconditionally());
1583+
let filename_val = cx.const_bytes(filename_str.as_bytes());
1584+
let filename_ptr = cx.static_addr_of_impl(filename_val, Align::ONE, None);
1585+
1586+
// SourceLocation UBSan struct: { const char *filename, uint32_t line, uint32_t column }
1587+
let source_location = cx.const_struct(
1588+
&[
1589+
filename_ptr,
1590+
cx.const_u32(loc.line as u32),
1591+
// UBSan columns are 1-based
1592+
cx.const_u32(loc.col.0 as u32 + 1),
1593+
],
1594+
false, // packed = false
1595+
);
1596+
1597+
let ty_name = format!("{}\0", expected_ty);
1598+
let ty_name_val = cx.const_bytes(ty_name.as_bytes());
1599+
1600+
// TypeDescriptor UBSan struct: { uint16_t TypeKind, uint16_t TypeInfo, const char *TypeName }
1601+
let type_descriptor =
1602+
cx.const_struct(&[cx.const_i16(0xffffu16 as i16), cx.const_i16(0), ty_name_val], false);
1603+
1604+
let type_descriptor_ptr =
1605+
cx.static_addr_of_impl(type_descriptor, Align::from_bytes(2).unwrap(), None);
1606+
1607+
// CFICheckFailData UBSan struct: { uint8_t CheckKind, SourceLocation Loc, TypeDescriptor *Type }
1608+
let cfi_check_fail_data = cx
1609+
.const_struct(&[cx.const_u8(check_kind), source_location, type_descriptor_ptr], false);
1610+
let align = tcx.data_layout.aggregate_align;
1611+
1612+
// Returns the final opaque pointer to the struct to be passed to __ubsan_handle_cfi_check_fail
1613+
cx.static_addr_of_mut(cfi_check_fail_data, align, Some("__ubsan_cfi_check_fail_data"))
1614+
}
15671615
}
15681616

15691617
impl<'a, 'll, CX: Borrow<SCx<'ll>>> GenericBuilder<'a, 'll, CX> {
@@ -1983,8 +2031,64 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
19832031
if let Some(dbg_loc) = dbg_loc {
19842032
self.set_dbg_loc(dbg_loc);
19852033
}
1986-
self.abort();
1987-
self.unreachable();
2034+
2035+
let is_diag = self.tcx.sess.opts.unstable_opts.sanitizer_cfi_diag.unwrap_or(false);
2036+
let is_recover =
2037+
self.tcx.sess.opts.unstable_opts.sanitizer_cfi_recover.unwrap_or(false);
2038+
2039+
if is_diag || is_recover {
2040+
let fty = self.cx.type_func(
2041+
&[self.cx.type_ptr(), self.cx.type_isize(), self.cx.type_isize()],
2042+
self.cx.type_void(),
2043+
);
2044+
let ubsan_handler = self.declare_cfn(
2045+
if is_recover {
2046+
"__ubsan_handle_cfi_check_fail"
2047+
} else {
2048+
"__ubsan_handle_cfi_check_fail_abort"
2049+
},
2050+
llvm::UnnamedAddr::Global,
2051+
fty,
2052+
);
2053+
2054+
let mut expected_ty = String::from("fn(");
2055+
for (i, arg) in fn_abi.args.iter().enumerate() {
2056+
if i > 0 {
2057+
expected_ty.push_str(", ");
2058+
}
2059+
use std::fmt::Write;
2060+
write!(&mut expected_ty, "{}", arg.layout.ty).unwrap();
2061+
}
2062+
expected_ty.push(')');
2063+
if !fn_abi.ret.layout.ty.is_unit() {
2064+
use std::fmt::Write;
2065+
write!(&mut expected_ty, " -> {}", fn_abi.ret.layout.ty).unwrap();
2066+
}
2067+
2068+
// 4 for cfi-icall (indirect call)
2069+
let check_kind = 4;
2070+
let diag_data =
2071+
self.generate_ubsan_cfi_diag_data(self.span, expected_ty, check_kind);
2072+
2073+
let function_address = self.ptrtoint(llfn, self.cx.type_isize());
2074+
self.call(
2075+
fty,
2076+
None,
2077+
None,
2078+
ubsan_handler,
2079+
&[diag_data, function_address, self.const_usize(0)],
2080+
None,
2081+
None,
2082+
);
2083+
if is_recover {
2084+
self.br(bb_pass);
2085+
} else {
2086+
self.unreachable();
2087+
}
2088+
} else {
2089+
self.abort();
2090+
self.unreachable();
2091+
}
19882092

19892093
self.switch_to_block(bb_pass);
19902094
if let Some(dbg_loc) = dbg_loc {

‎compiler/rustc_codegen_ssa/src/back/link.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,6 +1600,12 @@ fn add_sanitizer_libraries(
16001600
if sanitizer.contains(SanitizerSet::REALTIME) {
16011601
link_sanitizer_runtime(sess, flavor, linker, "rtsan");
16021602
}
1603+
if sanitizer.contains(SanitizerSet::CFI)
1604+
&& (sess.opts.unstable_opts.sanitizer_cfi_diag.unwrap_or(false)
1605+
|| sess.opts.unstable_opts.sanitizer_cfi_recover.unwrap_or(false))
1606+
{
1607+
link_sanitizer_runtime(sess, flavor, linker, "ubsan");
1608+
}
16031609
}
16041610

16051611
fn link_sanitizer_runtime(

‎compiler/rustc_codegen_ssa/src/back/write.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ pub struct ModuleConfig {
8282
pub instrument_coverage: bool,
8383

8484
pub sanitizer: SanitizerSet,
85+
pub sanitizer_cfi_diag: Option<bool>,
86+
pub sanitizer_cfi_recover: Option<bool>,
8587
pub sanitizer_recover: SanitizerSet,
8688
pub sanitizer_dataflow_abilist: Vec<String>,
8789
pub sanitizer_memory_track_origins: usize,
@@ -182,6 +184,8 @@ impl ModuleConfig {
182184
instrument_coverage: if_regular!(sess.instrument_coverage(), false),
183185

184186
sanitizer: if_regular!(sess.sanitizers(), SanitizerSet::empty()),
187+
sanitizer_cfi_diag: if_regular!(sess.opts.unstable_opts.sanitizer_cfi_diag, None),
188+
sanitizer_cfi_recover: if_regular!(sess.opts.unstable_opts.sanitizer_cfi_recover, None),
185189
sanitizer_dataflow_abilist: if_regular!(
186190
sess.opts.unstable_opts.sanitizer_dataflow_abilist.clone(),
187191
Vec::new()

‎compiler/rustc_session/src/options.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2795,6 +2795,10 @@ written to standard error output)"),
27952795
"enable generalizing pointer types (default: no)"),
27962796
sanitizer_cfi_normalize_integers: Option<bool> = (None, parse_opt_bool, [TRACKED] { TARGET_MODIFIER: SanitizerCfiNormalizeIntegers },
27972797
"enable normalizing integer types (default: no)"),
2798+
sanitizer_cfi_diag: Option<bool> = (None, parse_opt_bool, [TRACKED],
2799+
"enable CFI diagnostics (default: no)"),
2800+
sanitizer_cfi_recover: Option<bool> = (None, parse_opt_bool, [TRACKED],
2801+
"enable CFI recovery (default: no)"),
27982802
sanitizer_dataflow_abilist: Vec<String> = (Vec::new(), parse_comma_list, [TRACKED],
27992803
"additional ABI list files that control how shadow parameters are passed (comma separated)"),
28002804
sanitizer_kcfi_arity: Option<bool> = (None, parse_opt_bool, [TRACKED],

‎src/bootstrap/src/core/build_steps/llvm.rs‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,21 +1525,28 @@ fn supported_sanitizers(
15251525
let darwin_libs = |os: &str, components: &[&str]| -> Vec<SanitizerRuntime> {
15261526
components
15271527
.iter()
1528-
.map(move |c| SanitizerRuntime {
1529-
cmake_target: format!("clang_rt.{c}_{os}_dynamic"),
1530-
path: out_dir.join(format!("build/lib/darwin/libclang_rt.{c}_{os}_dynamic.dylib")),
1531-
name: format!("librustc-{channel}_rt.{c}.dylib"),
1528+
.map(move |c| {
1529+
let cmake_c = if *c == "ubsan" { "ubsan_standalone" } else { *c };
1530+
SanitizerRuntime {
1531+
cmake_target: format!("clang_rt.{cmake_c}_{os}_dynamic"),
1532+
path: out_dir
1533+
.join(format!("build/lib/darwin/libclang_rt.{cmake_c}_{os}_dynamic.dylib")),
1534+
name: format!("librustc-{channel}_rt.{c}.dylib"),
1535+
}
15321536
})
15331537
.collect()
15341538
};
15351539

15361540
let common_libs = |os: &str, arch: &str, components: &[&str]| -> Vec<SanitizerRuntime> {
15371541
components
15381542
.iter()
1539-
.map(move |c| SanitizerRuntime {
1540-
cmake_target: format!("clang_rt.{c}-{arch}"),
1541-
path: out_dir.join(format!("build/lib/{os}/libclang_rt.{c}-{arch}.a")),
1542-
name: format!("librustc-{channel}_rt.{c}.a"),
1543+
.map(move |c| {
1544+
let cmake_c = if *c == "ubsan" { "ubsan_standalone" } else { *c };
1545+
SanitizerRuntime {
1546+
cmake_target: format!("clang_rt.{cmake_c}-{arch}"),
1547+
path: out_dir.join(format!("build/lib/{os}/libclang_rt.{cmake_c}-{arch}.a")),
1548+
name: format!("librustc-{channel}_rt.{c}.a"),
1549+
}
15431550
})
15441551
.collect()
15451552
};
@@ -1550,9 +1557,11 @@ fn supported_sanitizers(
15501557
"aarch64-apple-ios-sim" => darwin_libs("iossim", &["asan", "tsan", "rtsan"]),
15511558
"aarch64-apple-ios-macabi" => darwin_libs("osx", &["asan", "lsan", "tsan"]),
15521559
"aarch64-unknown-fuchsia" => common_libs("fuchsia", "aarch64", &["asan"]),
1553-
"aarch64-unknown-linux-gnu" => {
1554-
common_libs("linux", "aarch64", &["asan", "lsan", "msan", "tsan", "hwasan", "rtsan"])
1555-
}
1560+
"aarch64-unknown-linux-gnu" => common_libs(
1561+
"linux",
1562+
"aarch64",
1563+
&["asan", "lsan", "msan", "tsan", "hwasan", "rtsan", "ubsan"],
1564+
),
15561565
"aarch64-unknown-linux-ohos" => {
15571566
common_libs("linux", "aarch64", &["asan", "lsan", "msan", "tsan", "hwasan"])
15581567
}
@@ -1572,7 +1581,7 @@ fn supported_sanitizers(
15721581
"x86_64-unknown-linux-gnu" => common_libs(
15731582
"linux",
15741583
"x86_64",
1575-
&["asan", "dfsan", "lsan", "msan", "safestack", "tsan", "rtsan"],
1584+
&["asan", "dfsan", "lsan", "msan", "safestack", "tsan", "rtsan", "ubsan"],
15761585
),
15771586
"x86_64-unknown-linux-gnuasan" => common_libs("linux", "x86_64", &["asan"]),
15781587
"x86_64-unknown-linux-gnumsan" => common_libs("linux", "x86_64", &["msan"]),
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Verifies that pointer type membership tests for indirect calls are emitted.
2+
//
3+
//@ needs-sanitizer-cfi
4+
//@ compile-flags: -Clto -Cno-prepopulate-passes -Ctarget-feature=-crt-static -Zsanitizer=cfi -Zsanitizer-cfi-diag=true -Copt-level=0 -C unsafe-allow-abi-mismatch=sanitizer
5+
6+
#![crate_type = "lib"]
7+
8+
pub fn foo(f: fn(i32) -> i32, arg: i32) -> i32 {
9+
// CHECK-LABEL: define{{.*}}foo{{.*}}!type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}}
10+
// CHECK: start:
11+
// CHECK: [[TT:%.+]] = call i1 @llvm.type.test(ptr {{%f|%0}}, metadata !"{{[[:print:]]+}}")
12+
// CHECK-NEXT: br i1 [[TT]], label %type_test.pass, label %type_test.fail
13+
// CHECK: type_test.pass:
14+
// CHECK-NEXT: {{%.+}} = call i32 %f(i32{{.*}} %arg)
15+
// CHECK: type_test.fail:
16+
// CHECK-NEXT: {{%.+}} = ptrtoint ptr {{%f|%0}} to i64
17+
// CHECK-NEXT: call void @__ubsan_handle_cfi_check_fail_abort(
18+
// CHECK-NEXT: unreachable
19+
f(arg)
20+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Verifies that pointer type membership tests for indirect calls are emitted.
2+
//
3+
//@ needs-sanitizer-cfi
4+
//@ compile-flags: -Clto -Cno-prepopulate-passes -Ctarget-feature=-crt-static -Zsanitizer=cfi -Zsanitizer-cfi-recover=true -Copt-level=0 -C unsafe-allow-abi-mismatch=sanitizer
5+
6+
#![crate_type = "lib"]
7+
8+
pub fn foo(f: fn(i32) -> i32, arg: i32) -> i32 {
9+
// CHECK-LABEL: define{{.*}}foo{{.*}}!type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}}
10+
// CHECK: start:
11+
// CHECK: [[TT:%.+]] = call i1 @llvm.type.test(ptr {{%f|%0}}, metadata !"{{[[:print:]]+}}")
12+
// CHECK-NEXT: br i1 [[TT]], label %type_test.pass, label %type_test.fail
13+
// CHECK: type_test.pass:
14+
// CHECK-NEXT: {{%.+}} = call i32 %f(i32{{.*}} %arg)
15+
// CHECK: type_test.fail:
16+
// CHECK-NEXT: {{%.+}} = ptrtoint ptr {{%f|%0}} to i64
17+
// CHECK-NEXT: call void @__ubsan_handle_cfi_check_fail(
18+
// CHECK-NEXT: br label %type_test.pass
19+
f(arg)
20+
}

‎tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Tests that a doc comment will not preclude a field from being considered a diagnostic argument
33
//@ normalize-stderr: "the following other types implement trait `IntoDiagArg`:(?:.*\n){0,9}\s+and \d+ others" -> "normalized in stderr"
44
//@ normalize-stderr: "(COMPILER_DIR/.*\.rs):[0-9]+:[0-9]+" -> "$1:LL:CC"
5+
//@ normalize-stderr: "rustc_errors::Diag::<'a, G>::arg" -> "Diag::<'a, G>::arg"
56

67
// The proc_macro2 crate handles spans differently when on beta/stable release rather than nightly,
78
// changing the output of this test. Since Subdiagnostic is strictly internal to the compiler

‎tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0277]: the trait bound `NotIntoDiagArg: IntoDiagArg` is not satisfied
2-
--> $DIR/diagnostic-derive-doc-comment-field.rs:34:10
2+
--> $DIR/diagnostic-derive-doc-comment-field.rs:35:10
33
|
44
LL | #[derive(Diagnostic)]
55
| ---------- required by a bound introduced by this call
@@ -8,7 +8,7 @@ LL | arg: NotIntoDiagArg,
88
| ^^^^^^^^^^^^^^ unsatisfied trait bound
99
|
1010
help: the nightly-only, unstable trait `IntoDiagArg` is not implemented for `NotIntoDiagArg`
11-
--> $DIR/diagnostic-derive-doc-comment-field.rs:26:1
11+
--> $DIR/diagnostic-derive-doc-comment-field.rs:27:1
1212
|
1313
LL | struct NotIntoDiagArg;
1414
| ^^^^^^^^^^^^^^^^^^^^^
@@ -22,7 +22,7 @@ note: required by a bound in `Diag::<'a, G>::arg`
2222
= note: this error originates in the macro `with_fn` (in Nightly builds, run with -Z macro-backtrace for more info)
2323

2424
error[E0277]: the trait bound `NotIntoDiagArg: IntoDiagArg` is not satisfied
25-
--> $DIR/diagnostic-derive-doc-comment-field.rs:44:10
25+
--> $DIR/diagnostic-derive-doc-comment-field.rs:45:10
2626
|
2727
LL | #[derive(Subdiagnostic)]
2828
| ------------- required by a bound introduced by this call
@@ -31,7 +31,7 @@ LL | arg: NotIntoDiagArg,
3131
| ^^^^^^^^^^^^^^ unsatisfied trait bound
3232
|
3333
help: the nightly-only, unstable trait `IntoDiagArg` is not implemented for `NotIntoDiagArg`
34-
--> $DIR/diagnostic-derive-doc-comment-field.rs:26:1
34+
--> $DIR/diagnostic-derive-doc-comment-field.rs:27:1
3535
|
3636
LL | struct NotIntoDiagArg;
3737
| ^^^^^^^^^^^^^^^^^^^^^
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Verifies that calling a function pointer with a mismatched type triggers a
2+
// CFI violation and causes the process to trap.
3+
4+
//@ revisions: cfi kcfi
5+
// FIXME(#122848) Remove only-linux once OSX CFI binaries work
6+
//@ only-linux
7+
//@ ignore-backends: gcc
8+
//@ [cfi] needs-sanitizer-cfi
9+
//@ [cfi] needs-sanitizer-support
10+
//@ [kcfi] needs-sanitizer-kcfi
11+
//@ compile-flags: -C target-feature=-crt-static
12+
//@ compile-flags: -C unsafe-allow-abi-mismatch=sanitizer
13+
//@ [cfi] compile-flags: -C opt-level=0 -C codegen-units=1 -C lto
14+
//@ [cfi] compile-flags: -C prefer-dynamic=off
15+
//@ [cfi] compile-flags: -Z sanitizer=cfi
16+
//@ [cfi] compile-flags: -Z sanitizer-cfi-diag=true
17+
//@ [kcfi] compile-flags: -Z sanitizer=kcfi
18+
//@ [kcfi] compile-flags: -C panic=abort -C prefer-dynamic=off
19+
//@ run-fail-or-crash
20+
21+
use std::hint::black_box;
22+
use std::mem;
23+
24+
fn add_one(x: i32) -> i32 {
25+
x + 1
26+
}
27+
28+
// Accept a function pointer as a parameter so that the indirect call cannot
29+
// be devirtualized by the compiler.
30+
#[inline(never)]
31+
fn call_with_mismatch(f: fn(i32) -> i32) {
32+
// Transmute fn(i32) -> i32 into fn(i32, i32) -> i32, creating a
33+
// function pointer type mismatch that CFI should catch.
34+
let g: fn(i32, i32) -> i32 = unsafe { mem::transmute(f) };
35+
// This indirect call should fail the CFI type check and trap.
36+
let _result = g(1, 2);
37+
}
38+
39+
fn main() {
40+
call_with_mismatch(black_box(add_one));
41+
}

0 commit comments

Comments
 (0)