subject:"\[PATCH\] D148573\: Allow \-fsanitize=function on all targets"

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-22 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D148573#4361894 , @sbc100 wrote:

> In D148573#4361509 , @MaskRay wrote:
>
>> In D148573#4361396 , @sbc100 wrote:
>>
>>> This change seems to be causing problems on the emscripten auto-roller:  
>>> https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview
>>>
>>> Failures show up in ubsan tests and look like this:
>>>
>>>   error: symbol '_Z4testi' unsupported subtraction expression used in 
>>> relocation in code section.
>>>   error: symbol '__main_argc_argv' unsupported subtraction expression used 
>>> in relocation in code section.
>>>   fatal error: error in backend: function sections must contain one 
>>> function each
>>>
>>> It seems like enabling this sanitizer perhaps uses features we don't yet 
>>> support?  I will keep investigating but perhaps we can find a way to revert 
>>> he effect on the wasm backend for now?
>>
>> wasm seems to use `-fsanitize=undefined`, which includes 
>> `-fsanitize=function`.
>> wasm doesn't allow data words before the function entry, so we need to 
>> unsupport `-fsanitize=function` for wasm...
>
> That makes sense to me. The wasm specification (and therefore the wasm 
> runtimes) already enforce signature checking for indirect function calls so 
> there should be no need for this sanitizer there anyway.Do you want to 
> make that change or should I?

Thanks for the additional context (and I shall learn about it). Having been 
done in 39ba913d13ab15c76cb6b5aa066fa111ddfe944b 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-22 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 added a comment.

In D148573#4361509 , @MaskRay wrote:

> In D148573#4361396 , @sbc100 wrote:
>
>> This change seems to be causing problems on the emscripten auto-roller:  
>> https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview
>>
>> Failures show up in ubsan tests and look like this:
>>
>>   error: symbol '_Z4testi' unsupported subtraction expression used in 
>> relocation in code section.
>>   error: symbol '__main_argc_argv' unsupported subtraction expression used 
>> in relocation in code section.
>>   fatal error: error in backend: function sections must contain one function 
>> each
>>
>> It seems like enabling this sanitizer perhaps uses features we don't yet 
>> support?  I will keep investigating but perhaps we can find a way to revert 
>> he effect on the wasm backend for now?
>
> wasm seems to use `-fsanitize=undefined`, which includes 
> `-fsanitize=function`.
> wasm doesn't allow data words before the function entry, so we need to 
> unsupport `-fsanitize=function` for wasm...

That makes sense to me. The wasm specification (and therefore the wasm 
runtimes) already enforce signature checking for indirect function calls so 
there should be no need for this sanitizer there anyway.Do you want to make 
that change or should I?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-22 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D148573#4361396 , @sbc100 wrote:

> This change seems to be causing problems on the emscripten auto-roller:  
> https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview
>
> Failures show up in ubsan tests and look like this:
>
>   error: symbol '_Z4testi' unsupported subtraction expression used in 
> relocation in code section.
>   error: symbol '__main_argc_argv' unsupported subtraction expression used in 
> relocation in code section.
>   fatal error: error in backend: function sections must contain one function 
> each
>
> It seems like enabling this sanitizer perhaps uses features we don't yet 
> support?  I will keep investigating but perhaps we can find a way to revert 
> he effect on the wasm backend for now?

wasm seems to use `-fsanitize=undefined`, which includes `-fsanitize=function`.
wasm doesn't allow data words before the function entry, so we need to 
unsupport `-fsanitize=function` for wasm...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-22 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 added a comment.

(I guess this also mean we are lacking some upstream testing for ubsan + wasm + 
mc?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-22 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 added a comment.

This change seems to be causing problems on the emscripten auto-roller:  
https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview

Failures show up in ubsan tests and look like this:

  error: symbol '_Z4testi' unsupported subtraction expression used in 
relocation in code section.
  error: symbol '__main_argc_argv' unsupported subtraction expression used in 
relocation in code section.
  fatal error: error in backend: function sections must contain one function 
each

It seems like enabling this sanitizer perhaps uses features we don't yet 
support?  I will keep investigating but perhaps we can find a way to revert he 
effect on the wasm backend for now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-20 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D148573#4358899 , @vitalybuka 
wrote:

> These bots are broken after the patch
> https://lab.llvm.org/buildbot/#/builders/77/builds/26834
> https://lab.llvm.org/buildbot/#/builders/18/builds/9073

The ppc64le has other unrelated failures.

The aarch64 android bot failed due to being unable to link the executable, 
which seems an infrastructure issue? :) I think we can disable the test by 
modifying `compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py`

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-20 Thread Vitaly Buka via Phabricator via cfe-commits

vitalybuka added a comment.

These bots are broken after the patch
https://lab.llvm.org/buildbot/#/builders/77/builds/26834
https://lab.llvm.org/buildbot/#/builders/18/builds/9073


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-05-19 Thread Fangrui Song via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67cbe1b85972: Allow -fsanitize=function on all targets 
(authored by MaskRay).
Herald added subscribers: pcwang-thead, s.egerton, mstorsjo, simoncook, asb.

Changed prior to commit:
  https://reviews.llvm.org/D148573?vs=515161=523782#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/CodeGen/ubsan-function.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
  llvm/test/CodeGen/AArch64/func-sanitizer.ll
  llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll

Index: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
===
--- llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
+++ llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
@@ -1,5 +1,8 @@
 ; RUN: llc -mtriple=aarch64 %s -o - | FileCheck %s
 
+@_ZTIFvvE = linkonce_odr constant i32 2
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
 define void @f0() "patchable-function-entry"="0" "branch-target-enforcement"="true" {
 ; CHECK-LABEL: f0:
 ; CHECK-NEXT: .Lfunc_begin0:
@@ -85,3 +88,22 @@
   call void asm sideeffect "", ""()
   ret void
 }
+
+;; Test the interaction with -fsanitize=function.
+; CHECK:  .type sanitize_function,@function
+; CHECK-NEXT: .Ltmp{{.*}}:
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   .word   3238382334  // 0xc105cafe
+; CHECK-NEXT:   .word   .L__llvm_rtti_proxy-sanitize_function
+; CHECK-NEXT: sanitize_function:
+; CHECK-NEXT: .Lfunc_begin{{.*}}:
+; CHECK-NEXT:   .cfi_startproc
+; CHECK-NEXT:   // %bb.0:
+; CHECK-NEXT:   hint #34
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   ret
+define void @sanitize_function(ptr noundef %x) "patchable-function-prefix"="1" "patchable-function-entry"="1" "branch-target-enforcement"="true" !func_sanitize !0 {
+  ret void
+}
+
+!0 = !{i32 3238382334, ptr @__llvm_rtti_proxy}
Index: llvm/test/CodeGen/AArch64/func-sanitizer.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/func-sanitizer.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s
+
+; CHECK-LABEL: .type _Z3funv,@function
+; CHECK-NEXT:.word   3238382334  // 0xc105cafe
+; CHECK-NEXT:.word   .L__llvm_rtti_proxy-_Z3funv
+; CHECK-NEXT:  _Z3funv:
+; CHECK-NEXT:  // %bb.0:
+; CHECK-NEXT:ret
+
+; CHECK:   .section .rodata,"a",@progbits
+; CHECK-LABEL: .L__llvm_rtti_proxy:
+; CHECK-NEXT:.xword  _ZTIFvvE
+; CHECK-NEXT:.size   .L__llvm_rtti_proxy, 8
+
+@_ZTIFvvE = linkonce_odr constant i32 1
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
+define dso_local void @_Z3funv() nounwind !func_sanitize !0 {
+  ret void
+}
+
+!0 = !{i32 3238382334, ptr @__llvm_rtti_proxy}
Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
@@ -1,3 +1,2 @@
-# The function type checker is only supported on x86 and x86_64 for now.
-if config.target_arch not in ['x86', 'x86_64']:
+if config.host_os not in ['Darwin', 'FreeBSD', 'Linux', 'NetBSD']:
   config.unsupported = True
Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
@@ -4,9 +4,6 @@
 // RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK $(%run %t-unique UNIQUE)
 // Verify that we can disable symbolization if needed:
 // RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM $(%run %t-unique NOSYM-UNIQUE)
-// XFAIL: target={{.*windows.*}}
-// Unsupported function flag
-// UNSUPPORTED: target={{.*openbsd.*}}
 
 #ifdef DETERMINE_UNIQUE
 
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -24,8 +24,8 @@
 // CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-x86_64)?}}.lib"
 // CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone{{(-x86_64)?}}.a"
 // CHECK-UNDEFINED-WIN-CXX:

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay updated this revision to Diff 515161.
MaskRay retitled this revision from "Port -fsanitize=function to AArch64" to 
"Allow -fsanitize=function on all targets".
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added subscribers: kadircet, ilya-biryukov.

allow all targets


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148573/new/

https://reviews.llvm.org/D148573

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/CodeGen/ubsan-function.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
  llvm/test/CodeGen/AArch64/func-sanitizer.ll
  llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll

Index: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
===
--- llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
+++ llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
@@ -1,5 +1,8 @@
 ; RUN: llc -mtriple=aarch64 %s -o - | FileCheck %s
 
+@_ZTIFvvE = linkonce_odr constant i32 2
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
 define void @f0() "patchable-function-entry"="0" "branch-target-enforcement"="true" {
 ; CHECK-LABEL: f0:
 ; CHECK-NEXT: .Lfunc_begin0:
@@ -85,3 +88,22 @@
   call void asm sideeffect "", ""()
   ret void
 }
+
+;; Test the interaction with -fsanitize=function.
+; CHECK:  .type sanitize_function,@function
+; CHECK-NEXT: .Ltmp{{.*}}:
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   .word   3238382334  // 0xc105cafe
+; CHECK-NEXT:   .word   .L__llvm_rtti_proxy-sanitize_function
+; CHECK-NEXT: sanitize_function:
+; CHECK-NEXT: .Lfunc_begin{{.*}}:
+; CHECK-NEXT:   .cfi_startproc
+; CHECK-NEXT:   // %bb.0:
+; CHECK-NEXT:   hint #34
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   ret
+define void @sanitize_function(ptr noundef %x) "patchable-function-prefix"="1" "patchable-function-entry"="1" "branch-target-enforcement"="true" !func_sanitize !0 {
+  ret void
+}
+
+!0 = !{i32 3238382334, ptr @__llvm_rtti_proxy}
Index: llvm/test/CodeGen/AArch64/func-sanitizer.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/func-sanitizer.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s
+
+; CHECK-LABEL: .type _Z3funv,@function
+; CHECK-NEXT:.word   3238382334  // 0xc105cafe
+; CHECK-NEXT:.word   .L__llvm_rtti_proxy-_Z3funv
+; CHECK-NEXT:  _Z3funv:
+; CHECK-NEXT:  // %bb.0:
+; CHECK-NEXT:ret
+
+; CHECK:   .section .rodata,"a",@progbits
+; CHECK-LABEL: .L__llvm_rtti_proxy:
+; CHECK-NEXT:.xword  _ZTIFvvE
+; CHECK-NEXT:.size   .L__llvm_rtti_proxy, 8
+
+@_ZTIFvvE = linkonce_odr constant i32 1
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
+define dso_local void @_Z3funv() nounwind !func_sanitize !0 {
+  ret void
+}
+
+!0 = !{i32 3238382334, ptr @__llvm_rtti_proxy}
Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
@@ -1,3 +1,3 @@
-# The function type checker is only supported on x86 and x86_64 for now.
-if config.target_arch not in ['x86', 'x86_64']:
+# The function type checker is only supported on aarch64 and x86 for now.
+if config.target_arch not in ['aarch64', 'x86', 'x86_64']:
   config.unsupported = True
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -525,9 +525,10 @@
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-atomics -fno-sanitize-thread-atomics %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ATOMICS-BOTH-OFF
 // CHECK-TSAN-ATOMICS-BOTH-OFF: -cc1{{.*}}tsan-instrument-atomics=0
 
-// RUN: %clang --target=x86_64-apple-darwin10 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSAN-DARWIN
-// RUN: %clang --target=i386-apple-darwin10 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSAN-DARWIN
-// CHECK-FSAN-DARWIN: -cc1{{.*}}"-fsanitize=function" "-fsanitize-recover=function"
+// RUN: %clang --target=x86_64-apple-darwin10 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FUNCTION
+// RUN: %clang --target=i386-apple-darwin10 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FUNCTION
+// RUN: %clang --target=aarch64-unknown-linux-gnu -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FUNCTION
+//

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

[PATCH] D148573: Allow -fsanitize=function on all targets

9 matches

Site Navigation

Mail list logo

Footer information