[clang] -fsanitize=function: fix MSVC hashing to sugared type (PR #66816)

Matheus Izvekov via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 19 13:44:01 PDT 2023


https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/66816

Hashing the sugared type instead of the canonical type meant that
a simple example like this would always fail under MSVC:

```
static auto l() {}
int main() {
  auto a = l;
  a();
}
```
`clang --target=x86_64-pc-windows-msvc -fno-exceptions -fsanitize=function -g -O0 -fuse-ld=lld -o test.exe test.cc`

produces:
```
test.cc:4:3: runtime error: call to function l through pointer to incorrect function type 'void (*)()'
```

>From 20a7f3574d7931d908cb016968f488e769cb7007 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Tue, 19 Sep 2023 21:50:39 +0200
Subject: [PATCH 1/3] -fsanitize=function: Add a MSVC test case

---
 clang/test/CodeGen/ubsan-function.cpp | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/clang/test/CodeGen/ubsan-function.cpp b/clang/test/CodeGen/ubsan-function.cpp
index 1c281c8544578fe..bc37a61c98ee3ae 100644
--- a/clang/test/CodeGen/ubsan-function.cpp
+++ b/clang/test/CodeGen/ubsan-function.cpp
@@ -1,12 +1,15 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,64
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,64
-// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,64
-// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,ARM,32
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,GNU,64
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,MSVC,64
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,GNU,64
+// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,GNU,64
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,ARM,GNU,32
 
-// CHECK: define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
+// GNU:  define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
+// MSVC: define{{.*}} void @"?fun@@YAXXZ"() #0 !func_sanitize ![[FUNCSAN:.*]] {
 void fun() {}
 
-// CHECK-LABEL: define{{.*}} void @_Z6callerPFvvE(ptr noundef %f)
+// GNU-LABEL:  define{{.*}} void @_Z6callerPFvvE(ptr noundef %f)
+// MSVC-LABEL: define{{.*}} void @"?caller@@YAXP6AXXZ at Z"(ptr noundef %f)
 // ARM:   ptrtoint ptr {{.*}} to i32, !nosanitize !5
 // ARM:   and i32 {{.*}}, -2, !nosanitize !5
 // ARM:   inttoptr i32 {{.*}} to ptr, !nosanitize !5
@@ -17,7 +20,8 @@ void fun() {}
 // CHECK: [[LABEL1]]:
 // CHECK: getelementptr <{ i32, i32 }>, ptr {{.*}}, i32 -1, i32 1, !nosanitize
 // CHECK: load i32, ptr {{.*}}, align {{.*}}, !nosanitize
-// CHECK: icmp eq i32 {{.*}}, 905068220, !nosanitize
+// GNU:   icmp eq i32 {{.*}}, 905068220, !nosanitize
+// MSVC:  icmp eq i32 {{.*}}, -1600339357, !nosanitize
 // CHECK: br i1 {{.*}}, label %[[LABEL3:.*]], label %[[LABEL2:[^,]*]], {{.*}}!nosanitize
 // CHECK: [[LABEL2]]:
 // 64:    call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], i64 %[[#]]) #[[#]], !nosanitize
@@ -32,4 +36,5 @@ void fun() {}
 // CHECK-NEXT:   ret void
 void caller(void (*f)()) { f(); }
 
-// CHECK: ![[FUNCSAN]] = !{i32 -1056584962, i32 905068220}
+// GNU:  ![[FUNCSAN]] = !{i32 -1056584962, i32 905068220}
+// MSVC: ![[FUNCSAN]] = !{i32 -1056584962, i32 -1600339357}

>From 4fb0135c7c441633cd29e57772fab758d60ca16b Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Tue, 19 Sep 2023 22:15:42 +0200
Subject: [PATCH 2/3] -fsanitize=function: Add a (bugged) test case for a
 sugared function type

---
 clang/test/CodeGen/ubsan-function-sugared.cpp | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 clang/test/CodeGen/ubsan-function-sugared.cpp

diff --git a/clang/test/CodeGen/ubsan-function-sugared.cpp b/clang/test/CodeGen/ubsan-function-sugared.cpp
new file mode 100644
index 000000000000000..238bf31f4aba6d1
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-function-sugared.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,GNU,64
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,MSVC,64
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,GNU,64
+// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,GNU,64
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,ARM,GNU,32
+
+// GNU:  define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
+// MSVC: define{{.*}} void @"?fun@@YA?A?<auto>@@XZ"() #0 !func_sanitize ![[FUNCSAN:.*]] {
+auto fun() {}
+
+// GNU-LABEL:  define{{.*}} void @_Z6callerv()
+// MSVC-LABEL: define{{.*}} void @"?caller@@YAXXZ"()
+// ARM:   ptrtoint ptr {{.*}} to i32, !nosanitize !4
+// ARM:   and i32 {{.*}}, -2, !nosanitize !4
+// ARM:   inttoptr i32 {{.*}} to ptr, !nosanitize !4
+// CHECK: getelementptr <{ i32, i32 }>, ptr {{.*}}, i32 -1, i32 0, !nosanitize
+// CHECK: load i32, ptr {{.*}}, align {{.*}}, !nosanitize
+// CHECK: icmp eq i32 {{.*}}, -1056584962, !nosanitize
+// CHECK: br i1 {{.*}}, label %[[LABEL1:.*]], label %[[LABEL4:.*]], !nosanitize
+// CHECK: [[LABEL1]]:
+// CHECK: getelementptr <{ i32, i32 }>, ptr {{.*}}, i32 -1, i32 1, !nosanitize
+// CHECK: load i32, ptr {{.*}}, align {{.*}}, !nosanitize
+// GNU:   icmp eq i32 {{.*}}, 905068220, !nosanitize
+// MSVC:  icmp eq i32 {{.*}}, -1600339357, !nosanitize
+// CHECK: br i1 {{.*}}, label %[[LABEL3:.*]], label %[[LABEL2:[^,]*]], {{.*}}!nosanitize
+// CHECK: [[LABEL2]]:
+// 64:    call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], i64 %[[#]]) #[[#]], !nosanitize
+// 32:    call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], i32 %[[#]]) #[[#]], !nosanitize
+// CHECK-NEXT: unreachable, !nosanitize
+// CHECK-EMPTY:
+// CHECK-NEXT: [[LABEL3]]:
+// CHECK: br label %[[LABEL4]], !nosanitize
+// CHECK-EMPTY:
+// CHECK-NEXT: [[LABEL4]]:
+// CHECK-NEXT:   call void
+// CHECK-NEXT:   ret void
+void caller() {
+  auto a = fun;
+  a();
+}
+
+// GNU:  ![[FUNCSAN]] = !{i32 -1056584962, i32 905068220}
+// FIXME: Wrong hash
+// MSVC: ![[FUNCSAN]] = !{i32 -1056584962, i32 165986058}

>From 0c34156207f658daa8229c4e01552460b85c1251 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Tue, 19 Sep 2023 22:21:25 +0200
Subject: [PATCH 3/3] -fsanitize=function: fix MSVC hashing to sugared type

Hashing the sugared type instead of the canonical type meant that
a simple example like this would always fail under MSVC:

```
static auto l() {}
int main() {
  auto a = l;
  a();
}
```
`clang --target=x86_64-pc-windows-msvc -fno-exceptions -fsanitize=function -g -O0 -fuse-ld=lld -o test.exe test.cc`

produces:
```
test.cc:4:3: runtime error: call to function l through pointer to incorrect function type 'void (*)()'
```
---
 clang/lib/CodeGen/CodeGenFunction.cpp         | 1 +
 clang/test/CodeGen/ubsan-function-sugared.cpp | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index bf83171e2c68146..78b3faa970be96b 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -570,6 +570,7 @@ bool CodeGenFunction::AlwaysEmitXRayTypedEvents() const {
 
 llvm::ConstantInt *
 CodeGenFunction::getUBSanFunctionTypeHash(QualType Ty) const {
+  Ty = Ty.getCanonicalType();
   // Remove any (C++17) exception specifications, to allow calling e.g. a
   // noexcept function through a non-noexcept pointer.
   if (!Ty->isFunctionNoProtoType())
diff --git a/clang/test/CodeGen/ubsan-function-sugared.cpp b/clang/test/CodeGen/ubsan-function-sugared.cpp
index 238bf31f4aba6d1..fb2487c024ba903 100644
--- a/clang/test/CodeGen/ubsan-function-sugared.cpp
+++ b/clang/test/CodeGen/ubsan-function-sugared.cpp
@@ -40,5 +40,4 @@ void caller() {
 }
 
 // GNU:  ![[FUNCSAN]] = !{i32 -1056584962, i32 905068220}
-// FIXME: Wrong hash
-// MSVC: ![[FUNCSAN]] = !{i32 -1056584962, i32 165986058}
+// MSVC: ![[FUNCSAN]] = !{i32 -1056584962, i32 -1600339357}



More information about the cfe-commits mailing list