[clang] 69935d8 - [Clang][Sanitizers] Expect test failure on {arm, thumb}v7

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-05-28T11:33:32+02:00
New Revision: 69935d86aed1b691c5f33a2141f15cb3aaee1af6

URL: 
https://github.com/llvm/llvm-project/commit/69935d86aed1b691c5f33a2141f15cb3aaee1af6
DIFF: 
https://github.com/llvm/llvm-project/commit/69935d86aed1b691c5f33a2141f15cb3aaee1af6.diff

LOG: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

Summary:
Versions of LLVM built on {arm,thumb}v7 appear to have differently
configured pass managers, which causes restrictions on which sanitizers
we may use.

As such, expect failure of the recently added "sanitize-coverage.c" test
on these architectures until we can investigate armv7's restrictions.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=46117

Reviewers: vitalybuka, glider

Reviewed By: glider

Subscribers: glider, kristof.beyls, danielkiss, cfe-commits, vvereschaka

Tags: #clang

Differential Revision: https://reviews.llvm.org/D80668

Added: 


Modified: 
clang/test/CodeGen/sanitize-coverage.c

Removed: 




diff  --git a/clang/test/CodeGen/sanitize-coverage.c 
b/clang/test/CodeGen/sanitize-coverage.c
index 6fc8e39354d4..ea4ac9296b48 100644
--- a/clang/test/CodeGen/sanitize-coverage.c
+++ b/clang/test/CodeGen/sanitize-coverage.c
@@ -4,6 +4,9 @@
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=memory -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,MSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=thread -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,TSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=undefined  -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,UBSAN
+//
+// Host armv7 is currently unsupported: 
https://bugs.llvm.org/show_bug.cgi?id=46117
+// XFAIL: armv7, thumbv7
 
 int x[10];
 



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

[clang] 866ee23 - [KernelAddressSanitizer] Make globals constructors compatible with kernel

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-05T20:20:46+02:00
New Revision: 866ee2353f7d0224644799d0d1faed53c7f3a06d

URL: 
https://github.com/llvm/llvm-project/commit/866ee2353f7d0224644799d0d1faed53c7f3a06d
DIFF: 
https://github.com/llvm/llvm-project/commit/866ee2353f7d0224644799d0d1faed53c7f3a06d.diff

LOG: [KernelAddressSanitizer] Make globals constructors compatible with kernel

Summary:
This makes -fsanitize=kernel-address emit the correct globals
constructors for the kernel. We had to do the following:

- Disable generation of constructors that rely on linker features such
  as dead-global elimination.

- Only emit constructors for globals *not* in explicit sections. The
  kernel uses sections for special globals, which we should not touch.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203493

Tested:
1. With 'clang/test/CodeGen/asan-globals.cpp'.
2. With test_kasan.ko, we can see:

BUG: KASAN: global-out-of-bounds in kasan_global_oob+0xb3/0xba 
[test_kasan]

Reviewers: glider, andreyknvl

Reviewed By: glider

Subscribers: cfe-commits, nickdesaulniers, hiraditya, llvm-commits

Tags: #llvm, #clang

Differential Revision: https://reviews.llvm.org/D80805

Added: 


Modified: 
clang/test/CodeGen/asan-globals.cpp
llvm/include/llvm/Transforms/Utils/ModuleUtils.h
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/ModuleUtils.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index 93abb0023cfa..2feb305ebecd 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -1,40 +1,59 @@
 // RUN: echo "int extra_global;" > %t.extra-source.cpp
 // RUN: echo "global:*blacklisted_global*" > %t.blacklist
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,ASAN
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,KASAN
 // The blacklist file uses regexps, so Windows path backslashes.
 // RUN: echo "src:%s" | sed -e 's/\\//g' > %t.blacklist-src
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
 
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
 int blacklisted_global;
+int __attribute__((section(".foo.bar"))) sectioned_global;
 
 void func() {
   static int static_var = 0;
   const char *literal = "Hello, world!";
 }
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK-LABEL: define internal void @asan.module_ctor
+// ASAN-NEXT: call void @__asan_init
+// ASAN-NEXT: call void @__asan_version_mismatch_check
+// KASAN-NOT: call void @__asan_init
+// KASAN-NOT: call void @__asan_version_mismatch_check
+// ASAN-NEXT: call void @__asan_register_globals(i64 ptrtoint ({{.*}}, i64 6)
+// KASAN-NEXT: call void @__asan_register_globals(i64 ptrtoint ({{.*}}, i64 5)
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @asan.module_dtor
+// CHECK-NEXT: call void @__asan_unregister_globals
+// CHECK-NEXT: ret void
+
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], 
![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], 
!"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, 
i1 false}
-// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 8, i32 5}
+// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], 
!"dyn_init_global", i1 true, i1 false}
-// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 9, i32 5}
+// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
 // CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![

[clang] 2dd83a9 - [ASan][Test] Fix globals test for Mach-O

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-05T23:08:11+02:00
New Revision: 2dd83a923046a5cd9585dbf9f90daeab6c37265c

URL: 
https://github.com/llvm/llvm-project/commit/2dd83a923046a5cd9585dbf9f90daeab6c37265c
DIFF: 
https://github.com/llvm/llvm-project/commit/2dd83a923046a5cd9585dbf9f90daeab6c37265c.diff

LOG: [ASan][Test] Fix globals test for Mach-O

Summary: Use a portable section name, as for the test's purpose any name will 
do.

Reviewers: nickdesaulniers, thakis

Reviewed By: thakis

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D81306

Added: 


Modified: 
clang/test/CodeGen/asan-globals.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index 2feb305ebecd..f02bd173d31b 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -11,7 +11,7 @@ int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
 int blacklisted_global;
-int __attribute__((section(".foo.bar"))) sectioned_global;
+int __attribute__ ((section("__DATA, __common"))) sectioned_global;
 
 void func() {
   static int static_var = 0;
@@ -41,7 +41,7 @@ void func() {
 // CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![[SECTIONED_GLOBAL_LOC:[0-9]+]], 
!"sectioned_global", i1 false, i1 false}
-// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 
42}
+// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 
51}
 // CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 
false, i1 false}
 // CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 14}
 // CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"", i1 false, i1 false}



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

[clang] 97a6709 - [ASan][Test] Fix globals test on 32-bit architectures

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-06T11:23:16+02:00
New Revision: 97a670958c240d469c6baf2d3c601d4dea286069

URL: 
https://github.com/llvm/llvm-project/commit/97a670958c240d469c6baf2d3c601d4dea286069
DIFF: 
https://github.com/llvm/llvm-project/commit/97a670958c240d469c6baf2d3c601d4dea286069.diff

LOG: [ASan][Test] Fix globals test on 32-bit architectures

Buildbot reports failures on e.g. armv7 and thumbv7. Fix the test by
expecting either i32 or i64 for the size-argument.

Added: 


Modified: 
clang/test/CodeGen/asan-globals.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index f02bd173d31b..a5374caae5c4 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -23,8 +23,8 @@ void func() {
 // ASAN-NEXT: call void @__asan_version_mismatch_check
 // KASAN-NOT: call void @__asan_init
 // KASAN-NOT: call void @__asan_version_mismatch_check
-// ASAN-NEXT: call void @__asan_register_globals(i64 ptrtoint ({{.*}}, i64 6)
-// KASAN-NEXT: call void @__asan_register_globals(i64 ptrtoint ({{.*}}, i64 5)
+// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 6)
+// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
 // CHECK-NEXT: ret void
 
 // CHECK-LABEL: define internal void @asan.module_dtor



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

[clang] c6ec352 - Revert "[KernelAddressSanitizer] Make globals constructors compatible with kernel"

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-08T10:34:03+02:00
New Revision: c6ec352a6bde1995794c523adc2ebab802ccdf0a

URL: 
https://github.com/llvm/llvm-project/commit/c6ec352a6bde1995794c523adc2ebab802ccdf0a
DIFF: 
https://github.com/llvm/llvm-project/commit/c6ec352a6bde1995794c523adc2ebab802ccdf0a.diff

LOG: Revert "[KernelAddressSanitizer] Make globals constructors compatible with 
kernel"

This reverts commit 866ee2353f7d0224644799d0d1faed53c7f3a06d.

Building the kernel results in modpost failures due to modpost relying
on debug info and inspecting kernel modules' globals:
https://github.com/ClangBuiltLinux/linux/issues/1045#issuecomment-640381783

Added: 


Modified: 
clang/test/CodeGen/asan-globals.cpp
llvm/include/llvm/Transforms/Utils/ModuleUtils.h
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/ModuleUtils.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index a5374caae5c4..93abb0023cfa 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -1,59 +1,40 @@
 // RUN: echo "int extra_global;" > %t.extra-source.cpp
 // RUN: echo "global:*blacklisted_global*" > %t.blacklist
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,ASAN
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,KASAN
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s
 // The blacklist file uses regexps, so Windows path backslashes.
 // RUN: echo "src:%s" | sed -e 's/\\//g' > %t.blacklist-src
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
 
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
 int blacklisted_global;
-int __attribute__ ((section("__DATA, __common"))) sectioned_global;
 
 void func() {
   static int static_var = 0;
   const char *literal = "Hello, world!";
 }
 
-// CHECK-LABEL: define internal void @asan.module_ctor
-// ASAN-NEXT: call void @__asan_init
-// ASAN-NEXT: call void @__asan_version_mismatch_check
-// KASAN-NOT: call void @__asan_init
-// KASAN-NOT: call void @__asan_version_mismatch_check
-// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 6)
-// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
-// CHECK-NEXT: ret void
-
-// CHECK-LABEL: define internal void @asan.module_dtor
-// CHECK-NEXT: call void @__asan_unregister_globals
-// CHECK-NEXT: ret void
-
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], 
![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], 
!"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, 
i1 false}
-// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
+// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 8, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], 
!"dyn_init_global", i1 true, i1 false}
-// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
+// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 9, i32 5}
 // CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![[SECTIONED_GLOBAL_LOC:[0-9]+]], 
!"sectioned_global", i1 false, i1 false}
-// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 
51}
 // CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 
false, i1 false}
-// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 14}
+// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 14}
 // CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"", i1 false, i1 false}
-// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-glo

[clang] d3f8931 - [KernelAddressSanitizer] Make globals constructors compatible with kernel [v2]

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-10T15:08:42+02:00
New Revision: d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7

URL: 
https://github.com/llvm/llvm-project/commit/d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7
DIFF: 
https://github.com/llvm/llvm-project/commit/d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7.diff

LOG: [KernelAddressSanitizer] Make globals constructors compatible with kernel 
[v2]

[ v1 was reverted by c6ec352a6bde1995794c523adc2ebab802ccdf0a due to
  modpost failing; v2 fixes this. More info:
  https://github.com/ClangBuiltLinux/linux/issues/1045#issuecomment-640381783 ]

This makes -fsanitize=kernel-address emit the correct globals
constructors for the kernel. We had to do the following:

* Disable generation of constructors that rely on linker features such
  as dead-global elimination.

* Only instrument globals *not* in explicit sections. The kernel uses
  sections for special globals, which we should not touch.

* Do not instrument globals that are prefixed with "__" nor that are
  aliased by a symbol that is prefixed with "__". For example, modpost
  relies on specially named aliases to find globals and checks their
  contents. Unfortunately modpost relies on size stored as ELF debug info
  and any padding of globals currently causes the debug info to cause size
  reported to be *with* redzone which throws modpost off.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203493

Tested:
* With 'clang/test/CodeGen/asan-globals.cpp'.

* With test_kasan.ko, we can see:

BUG: KASAN: global-out-of-bounds in kasan_global_oob+0xb3/0xba 
[test_kasan]

* allyesconfig, allmodconfig (x86_64)

Reviewed By: glider

Tags: #clang, #llvm

Differential Revision: https://reviews.llvm.org/D81390

Added: 


Modified: 
clang/test/CodeGen/asan-globals.cpp
llvm/include/llvm/Transforms/Utils/ModuleUtils.h
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/ModuleUtils.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index 93abb0023cfa..cf07710d31da 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -1,40 +1,78 @@
 // RUN: echo "int extra_global;" > %t.extra-source.cpp
 // RUN: echo "global:*blacklisted_global*" > %t.blacklist
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,ASAN
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,KASAN
 // The blacklist file uses regexps, so Windows path backslashes.
 // RUN: echo "src:%s" | sed -e 's/\\//g' > %t.blacklist-src
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
 
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
 int blacklisted_global;
 
+int __attribute__((section("__DATA, __common"))) sectioned_global; // [KASAN] 
ignore globals in a section
+extern "C" {
+int aliased_global; // [KASAN] 
ignore globals prefixed by aliases with __-prefix [below]
+extern int __attribute__((alias("aliased_global"))) __global_alias; // [KASAN] 
aliased_global ignored
+int __special_global;   // [KASAN] 
ignore globals with __-prefix
+}
+
 void func() {
   static int static_var = 0;
   const char *literal = "Hello, world!";
 }
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// ASAN: sectioned_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
+// KASAN: sectioned_global{{.*}}i32
+// ASAN: @aliased_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
+// KASAN: @aliased_global{{.*}}i32
+// ASAN: @__special_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
+// KASAN: @__special_global{{.*}}i32
+
+// CHECK-LABEL: define internal void @asan.module_ctor
+// ASAN-NEXT: call void @__asan_init
+// ASAN-NEXT: call void @__asan_version_mismatch_check
+// KASAN-NOT: call void @__asan_init
+// KASAN-NOT: call void @__asan_version_mismatch_check
+// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 8)
+// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
+// CHECK-NEXT: ret void
+
+/

[clang] 0f04f10 - [ASan][Test] Split out global alias test

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-10T19:58:50+02:00
New Revision: 0f04f104537bf037d07933cc306d0a0c0772e78f

URL: 
https://github.com/llvm/llvm-project/commit/0f04f104537bf037d07933cc306d0a0c0772e78f
DIFF: 
https://github.com/llvm/llvm-project/commit/0f04f104537bf037d07933cc306d0a0c0772e78f.diff

LOG: [ASan][Test] Split out global alias test

Some platforms do not support aliases. Split the test, and pass explicit
triple to avoid test failure.

Reviewed By: thakis

Tags: #clang

Differential Revision: https://reviews.llvm.org/D81591

Added: 
clang/test/CodeGen/asan-globals-alias.cpp

Modified: 
clang/test/CodeGen/asan-globals.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals-alias.cpp 
b/clang/test/CodeGen/asan-globals-alias.cpp
new file mode 100644
index ..1b6d4183261c
--- /dev/null
+++ b/clang/test/CodeGen/asan-globals-alias.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-linux -fsanitize=address -emit-llvm -o - %s 
| FileCheck %s --check-prefixes=CHECK,ASAN
+// RUN: %clang_cc1 -triple x86_64-linux -fsanitize=kernel-address -emit-llvm 
-o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
+//
+// Not all platforms support aliases - test for Linux only.
+
+int global; // to 
generate ctor for at least 1 global
+int aliased_global; // KASAN - 
ignore globals prefixed by aliases with __-prefix (below)
+extern int __attribute__((alias("aliased_global"))) __global_alias; // KASAN - 
aliased_global ignored
+
+// ASAN: @aliased_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
+// KASAN: @aliased_global{{.*}}i32
+
+// CHECK-LABEL: define internal void @asan.module_ctor
+// ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 2)
+// KASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 1)
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @asan.module_dtor
+// CHECK-NEXT: call void @__asan_unregister_globals
+// CHECK-NEXT: ret void

diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index cf07710d31da..76373f59e424 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -12,11 +12,9 @@ int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
 int blacklisted_global;
 
-int __attribute__((section("__DATA, __common"))) sectioned_global; // [KASAN] 
ignore globals in a section
+int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - 
ignore globals in a section
 extern "C" {
-int aliased_global; // [KASAN] 
ignore globals prefixed by aliases with __-prefix [below]
-extern int __attribute__((alias("aliased_global"))) __global_alias; // [KASAN] 
aliased_global ignored
-int __special_global;   // [KASAN] 
ignore globals with __-prefix
+int __special_global; // KASAN - ignore globals with __-prefix
 }
 
 void func() {
@@ -26,8 +24,6 @@ void func() {
 
 // ASAN: sectioned_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
 // KASAN: sectioned_global{{.*}}i32
-// ASAN: @aliased_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
-// KASAN: @aliased_global{{.*}}i32
 // ASAN: @__special_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
 // KASAN: @__special_global{{.*}}i32
 
@@ -36,7 +32,7 @@ void func() {
 // ASAN-NEXT: call void @__asan_version_mismatch_check
 // KASAN-NOT: call void @__asan_init
 // KASAN-NOT: call void @__asan_version_mismatch_check
-// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 8)
+// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 7)
 // KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
 // CHECK-NEXT: ret void
 
@@ -44,7 +40,7 @@ void func() {
 // CHECK-NEXT: call void @__asan_unregister_globals
 // CHECK-NEXT: ret void
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], 
![[ALIASED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], 
![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], 
![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], 
!"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, 
i1 false}
@@ -55,16 +51,14 @@ void func() {
 // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[SECTIONED_G

[clang] 52cae05 - [ASan][Test] Fix expected strings for globals test

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-10T20:26:24+02:00
New Revision: 52cae05e087b3d4fd02849fc37c387c720055ffb

URL: 
https://github.com/llvm/llvm-project/commit/52cae05e087b3d4fd02849fc37c387c720055ffb
DIFF: 
https://github.com/llvm/llvm-project/commit/52cae05e087b3d4fd02849fc37c387c720055ffb.diff

LOG: [ASan][Test] Fix expected strings for globals test

The expected strings would previously not catch bugs when redzones were
added when they were not actually expected. Fix by adding "global "
before the type.

Added: 


Modified: 
clang/test/CodeGen/asan-globals-alias.cpp
clang/test/CodeGen/asan-globals.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals-alias.cpp 
b/clang/test/CodeGen/asan-globals-alias.cpp
index 1b6d4183261c..faf160ac79c9 100644
--- a/clang/test/CodeGen/asan-globals-alias.cpp
+++ b/clang/test/CodeGen/asan-globals-alias.cpp
@@ -7,8 +7,8 @@ int global; 
// to genera
 int aliased_global; // KASAN - 
ignore globals prefixed by aliases with __-prefix (below)
 extern int __attribute__((alias("aliased_global"))) __global_alias; // KASAN - 
aliased_global ignored
 
-// ASAN: @aliased_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
-// KASAN: @aliased_global{{.*}}i32
+// ASAN: @aliased_global{{.*}} global { i32, [60 x i8] }{{.*}}, align 32
+// KASAN: @aliased_global{{.*}} global i32
 
 // CHECK-LABEL: define internal void @asan.module_ctor
 // ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 2)

diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index 76373f59e424..cb6f2174c6a5 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -22,10 +22,10 @@ void func() {
   const char *literal = "Hello, world!";
 }
 
-// ASAN: sectioned_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
-// KASAN: sectioned_global{{.*}}i32
-// ASAN: @__special_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
-// KASAN: @__special_global{{.*}}i32
+// ASAN: sectioned_global{{.*}} global { i32, [60 x i8] }{{.*}}, align 32
+// KASAN: sectioned_global{{.*}} global i32
+// ASAN: @__special_global{{.*}} global { i32, [60 x i8] }{{.*}}, align 32
+// KASAN: @__special_global{{.*}} global i32
 
 // CHECK-LABEL: define internal void @asan.module_ctor
 // ASAN-NEXT: call void @__asan_init



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

[clang] c4842bb - [Clang] Introduce -fexperimental-sanitize-metadata=

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2022-09-07T21:25:40+02:00
New Revision: c4842bb2e98e2f1ee23bb3bc753752816927b7b3

URL: 
https://github.com/llvm/llvm-project/commit/c4842bb2e98e2f1ee23bb3bc753752816927b7b3
DIFF: 
https://github.com/llvm/llvm-project/commit/c4842bb2e98e2f1ee23bb3bc753752816927b7b3.diff

LOG: [Clang] Introduce -fexperimental-sanitize-metadata=

Introduces the frontend flag -fexperimental-sanitize-metadata=, which
enables SanitizerBinaryMetadata instrumentation.

The first intended user of the binary metadata emitted will be a variant
of GWP-TSan [1]. The plan is to open source a stable and production
quality version of GWP-TSan. The development of which, however, requires
upstream compiler support.

[1] https://llvm.org/devmtg/2020-09/slides/Morehouse-GWP-Tsan.pdf

Until the tool has been open sourced, we mark this kind of
instrumentation as "experimental", and reserve the option to change
binary format, remove features, and similar.

Reviewed By: vitalybuka, MaskRay

Differential Revision: https://reviews.llvm.org/D130888

Added: 
clang/test/CodeGen/sanitize-metadata.c
clang/test/Driver/fsanitize-metadata.c

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/SanitizerArgs.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 55c6940fb34c1..ca2aa0d09aaf9 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -286,6 +286,8 @@ CODEGENOPT(SanitizeCoverageNoPrune, 1, 0) ///< Disable 
coverage pruning.
 CODEGENOPT(SanitizeCoverageStackDepth, 1, 0) ///< Enable max stack depth 
tracing
 CODEGENOPT(SanitizeCoverageTraceLoads, 1, 0) ///< Enable tracing of loads.
 CODEGENOPT(SanitizeCoverageTraceStores, 1, 0) ///< Enable tracing of stores.
+CODEGENOPT(SanitizeBinaryMetadataCovered, 1, 0) ///< Emit PCs for covered 
functions.
+CODEGENOPT(SanitizeBinaryMetadataAtomics, 1, 0) ///< Emit PCs for atomic 
operations.
 CODEGENOPT(SanitizeStats , 1, 0) ///< Collect statistics for sanitizers.
 CODEGENOPT(SimplifyLibCalls  , 1, 1) ///< Set when -fbuiltin is enabled.
 CODEGENOPT(SoftFloat , 1, 0) ///< -soft-float.

diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 7fffb38338889..0504012badb04 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -485,6 +485,11 @@ class CodeGenOptions : public CodeGenOptionsBase {
SanitizeCoverageTraceCmp || SanitizeCoverageTraceLoads ||
SanitizeCoverageTraceStores;
   }
+
+  // Check if any one of SanitizeBinaryMetadata* is enabled.
+  bool hasSanitizeBinaryMetadata() const {
+return SanitizeBinaryMetadataCovered || SanitizeBinaryMetadataAtomics;
+  }
 };
 
 }  // end namespace clang

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index f9dfb8104395e..cfc0994d69d4e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1673,6 +1673,12 @@ def fsanitize_coverage_ignorelist : Joined<["-"], 
"fsanitize-coverage-ignorelist
 HelpText<"Disable sanitizer coverage instrumentation for modules and 
functions "
  "that match the provided special case list, even the allowed 
ones">,
 
MarshallingInfoStringVector>;
+def fexperimental_sanitize_metadata_EQ : CommaJoined<["-"], 
"fexperimental-sanitize-metadata=">,
+  Group,
+  HelpText<"Specify the type of metadata to emit for binary analysis 
sanitizers">;
+def fno_experimental_sanitize_metadata_EQ : CommaJoined<["-"], 
"fno-experimental-sanitize-metadata=">,
+  Group, Flags<[CoreOption]>,
+  HelpText<"Disable emitting metadata for binary analysis sanitizers">;
 def fsanitize_memory_track_origins_EQ : Joined<["-"], 
"fsanitize-memory-track-origins=">,
 Group,
 HelpText<"Enable origins tracking in 
MemorySanitizer">,
@@ -5497,6 +5503,14 @@ def fsanitize_coverage_trace_stores
 : Flag<["-"], "fsanitize-coverage-trace-stores">,
   HelpText<"Enable tracing of stores">,
   MarshallingInfoFlag>;
+def fexperimental_sanitize_metadata_EQ_covered
+: Flag<["-"], "fexperimental-sanitize-metadata=covered">,
+  HelpText<"Emit PCs for code covered with binary analysis sanitizers">,
+  MarshallingInfoFlag>;
+def fexperimental_sanitize_metadata_EQ_atomics
+: Flag<["-"], "fexperimental-sanitize-metadata=atomics">,
+  HelpText<"Emit PCs for atomic operations used by binary analysis 
sanitizers">,
+  MarshallingInfoFlag>;
 def fpatchable_function_entry_offset_EQ
 : Joined<

[clang] c28b18a - [KernelAddressSanitizer] Fix globals exclusion for indirect aliases

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-12-11T12:20:40+01:00
New Revision: c28b18af19621e6b5cca257ef7139ba93833df0c

URL: 
https://github.com/llvm/llvm-project/commit/c28b18af19621e6b5cca257ef7139ba93833df0c
DIFF: 
https://github.com/llvm/llvm-project/commit/c28b18af19621e6b5cca257ef7139ba93833df0c.diff

LOG: [KernelAddressSanitizer] Fix globals exclusion for indirect aliases

GlobalAlias::getAliasee() may not always point directly to a
GlobalVariable. In such cases, try to find the canonical GlobalVariable
that the alias refers to.

Link: https://github.com/ClangBuiltLinux/linux/issues/1208

Reviewed By: dvyukov, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D92846

Added: 


Modified: 
clang/test/CodeGen/asan-globals-alias.cpp
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals-alias.cpp 
b/clang/test/CodeGen/asan-globals-alias.cpp
index faf160ac79c9..c859d2f2b44a 100644
--- a/clang/test/CodeGen/asan-globals-alias.cpp
+++ b/clang/test/CodeGen/asan-globals-alias.cpp
@@ -1,17 +1,42 @@
 // RUN: %clang_cc1 -triple x86_64-linux -fsanitize=address -emit-llvm -o - %s 
| FileCheck %s --check-prefixes=CHECK,ASAN
+// RUN: %clang_cc1 -triple x86_64-linux -O2 -fsanitize=address -emit-llvm -o - 
%s | FileCheck %s --check-prefixes=CHECK,ASAN
 // RUN: %clang_cc1 -triple x86_64-linux -fsanitize=kernel-address -emit-llvm 
-o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
+// RUN: %clang_cc1 -triple x86_64-linux -O2 -fsanitize=kernel-address 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
 //
 // Not all platforms support aliases - test for Linux only.
 
-int global; // to 
generate ctor for at least 1 global
-int aliased_global; // KASAN - 
ignore globals prefixed by aliases with __-prefix (below)
-extern int __attribute__((alias("aliased_global"))) __global_alias; // KASAN - 
aliased_global ignored
+int global; // generate ctor for at least 1 global
+int aliased_global; // KASAN ignored
+extern int __attribute__((alias("aliased_global"))) __global_alias;
+
+// Recursive alias:
+int aliased_global_2; // KASAN ignored
+extern int __attribute__((alias("aliased_global_2"))) global_alias_2;
+extern int __attribute__((alias("global_alias_2"))) __global_alias_2_alias;
+
+// Potential indirect alias:
+struct input_device_id {
+  unsigned long keybit[24];
+  unsigned long driver_info;
+};
+struct input_device_id joydev_ids[] = { { {1}, 1234 } }; // KASAN ignored
+extern struct input_device_id __attribute__((alias("joydev_ids"))) 
__mod_joydev_ids_device_table;
 
 // ASAN: @aliased_global{{.*}} global { i32, [60 x i8] }{{.*}}, align 32
+// ASAN: @aliased_global_2{{.*}} global { i32, [60 x i8] }{{.*}}, align 32
+// ASAN: @joydev_ids{{.*}} global { {{.*}}[56 x i8] zeroinitializer }, align 32
 // KASAN: @aliased_global{{.*}} global i32
+// KASAN: @aliased_global_2{{.*}} global i32
+// KASAN: @joydev_ids{{.*}} global [1 x {{.*}}i64 1234 }], align 16
+
+// Check the aliases exist:
+// CHECK: @__global_alias = alias
+// CHECK: @global_alias_2 = alias
+// CHECK: @__global_alias_2_alias = alias
+// CHECK: @__mod_joydev_ids_device_table = alias
 
 // CHECK-LABEL: define internal void @asan.module_ctor
-// ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 2)
+// ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 4)
 // KASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 1)
 // CHECK-NEXT: ret void
 

diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 7990bea90dc2..36d3942ffe14 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -792,7 +792,7 @@ class ModuleAddressSanitizer {
   StringRef InternalSuffix);
   Instruction *CreateAsanModuleDtor(Module &M);
 
-  bool canInstrumentAliasedGlobal(const GlobalAlias &GA) const;
+  const GlobalVariable *getExcludedAliasedGlobal(const GlobalAlias &GA) const;
   bool shouldInstrumentGlobal(GlobalVariable *G) const;
   bool ShouldUseMachOGlobalsSection() const;
   StringRef getGlobalMetadataSection() const;
@@ -1787,20 +1787,22 @@ void 
ModuleAddressSanitizer::createInitializerPoisonCalls(
   }
 }
 
-bool ModuleAddressSanitizer::canInstrumentAliasedGlobal(
-const GlobalAlias &GA) const {
+const GlobalVariable *
+ModuleAddressSanitizer::getExcludedAliasedGlobal(const GlobalAlias &GA) const {
   // In case this function should be expanded to include rules that do not just
   // apply when CompileKernel is true, either guard all existing rules with an
   // 'if (CompileKernel) { ... }' or be absolutely sure that all these rules
   // should also apply to user space.
   assert(CompileKern

[clang] ca6df73 - [NFC][CodeGenOptions] Refactor checking SanitizeCoverage options

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2021-05-25T12:57:14+02:00
New Revision: ca6df734069ae590d1632e920ceba03bea317549

URL: 
https://github.com/llvm/llvm-project/commit/ca6df734069ae590d1632e920ceba03bea317549
DIFF: 
https://github.com/llvm/llvm-project/commit/ca6df734069ae590d1632e920ceba03bea317549.diff

LOG: [NFC][CodeGenOptions] Refactor checking SanitizeCoverage options

Refactor checking SanitizeCoverage options into
CodeGenOptions::hasSanitizeCoverage().

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D102927

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/lib/CodeGen/BackendUtil.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index c77c34a8a57d6..617c255641efd 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -452,6 +452,12 @@ class CodeGenOptions : public CodeGenOptionsBase {
   bool hasMaybeUnusedDebugInfo() const {
 return getDebugInfo() >= codegenoptions::UnusedTypeInfo;
   }
+
+  // Check if any one of SanitizeCoverage* is enabled.
+  bool hasSanitizeCoverage() const {
+return SanitizeCoverageType || SanitizeCoverageIndirectCalls ||
+   SanitizeCoverageTraceCmp;
+  }
 };
 
 }  // end namespace clang

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index a2d219e92d6b0..ca1067dbb79f5 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -727,9 +727,7 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager 
&MPM,
addBoundsCheckingPass);
   }
 
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
+  if (CodeGenOpts.hasSanitizeCoverage()) {
 PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
addSanitizerCoveragePass);
 PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
@@ -1099,9 +1097,7 @@ static void addSanitizers(const Triple &TargetTriple,
   const LangOptions &LangOpts, PassBuilder &PB) {
   PB.registerOptimizerLastEPCallback([&](ModulePassManager &MPM,
  PassBuilder::OptimizationLevel Level) 
{
-if (CodeGenOpts.SanitizeCoverageType ||
-CodeGenOpts.SanitizeCoverageIndirectCalls ||
-CodeGenOpts.SanitizeCoverageTraceCmp) {
+if (CodeGenOpts.hasSanitizeCoverage()) {
   auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
   MPM.addPass(ModuleSanitizerCoveragePass(
   SancovOpts, CodeGenOpts.SanitizeCoverageAllowlistFiles,



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

[clang] 85feebf - [NFC][SanitizeCoverage] Test always_inline functions work

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2021-05-25T12:57:14+02:00
New Revision: 85feebf5a3401eab4c71288e2dc089faf547ab4c

URL: 
https://github.com/llvm/llvm-project/commit/85feebf5a3401eab4c71288e2dc089faf547ab4c
DIFF: 
https://github.com/llvm/llvm-project/commit/85feebf5a3401eab4c71288e2dc089faf547ab4c.diff

LOG: [NFC][SanitizeCoverage] Test always_inline functions work

Test that always_inline functions are instrumented as expected.

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D102929

Added: 


Modified: 
clang/test/CodeGen/sanitize-coverage.c

Removed: 




diff  --git a/clang/test/CodeGen/sanitize-coverage.c 
b/clang/test/CodeGen/sanitize-coverage.c
index 6fc8e39354d4f..b3e44e28cbce9 100644
--- a/clang/test/CodeGen/sanitize-coverage.c
+++ b/clang/test/CodeGen/sanitize-coverage.c
@@ -19,4 +19,16 @@ void foo(int n) {
   if (n)
 x[n] = 42;
 }
+
+static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) 
{
+  if (n)
+x[n] = 42;
+}
+// CHECK-LABEL: define dso_local void @test_always_inline(
+void test_always_inline(int n) {
+  // CHECK-DAG: call void @__sanitizer_cov_trace_pc
+  // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
+  always_inlined_fn(n);
+}
+
 // CHECK-LABEL: declare void



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

[clang] 2803330 - [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2021-05-25T12:57:14+02:00
New Revision: 280333021e9550d80f5c1152a34e33e81df1e178

URL: 
https://github.com/llvm/llvm-project/commit/280333021e9550d80f5c1152a34e33e81df1e178
DIFF: 
https://github.com/llvm/llvm-project/commit/280333021e9550d80f5c1152a34e33e81df1e178.diff

LOG: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

We really ought to support no_sanitize("coverage") in line with other
sanitizers. This came up again in discussions on the Linux-kernel
mailing lists, because we currently do workarounds using objtool to
remove coverage instrumentation. Since that support is only on x86, to
continue support coverage instrumentation on other architectures, we
must support selectively disabling coverage instrumentation via function
attributes.

Unfortunately, for SanitizeCoverage, it has not been implemented as a
sanitizer via fsanitize= and associated options in Sanitizers.def, but
rolls its own option fsanitize-coverage. This meant that we never got
"automatic" no_sanitize attribute support.

Implement no_sanitize attribute support by special-casing the string
"coverage" in the NoSanitizeAttr implementation. To keep the feature as
unintrusive to existing IR generation as possible, define a new negative
function attribute NoSanitizeCoverage to propagate the information
through to the instrumentation pass.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=49035

Reviewed By: vitalybuka, morehouse

Differential Revision: https://reviews.llvm.org/D102772

Added: 


Modified: 
clang/docs/SanitizerCoverage.rst
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/CodeGen/sanitize-coverage.c
llvm/bindings/go/llvm/ir_test.go
llvm/docs/BitCodeFormat.rst
llvm/docs/LangRef.rst
llvm/include/llvm/AsmParser/LLToken.h
llvm/include/llvm/Bitcode/LLVMBitCodes.h
llvm/include/llvm/IR/Attributes.td
llvm/lib/AsmParser/LLLexer.cpp
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/Attributes.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/test/Bitcode/attributes.ll
llvm/test/Bitcode/compatibility.ll
llvm/utils/emacs/llvm-mode.el
llvm/utils/llvm.grm
llvm/utils/vim/syntax/llvm.vim
llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Removed: 




diff  --git a/clang/docs/SanitizerCoverage.rst 
b/clang/docs/SanitizerCoverage.rst
index 485a73d273aed..b0bb9823eb043 100644
--- a/clang/docs/SanitizerCoverage.rst
+++ b/clang/docs/SanitizerCoverage.rst
@@ -312,11 +312,17 @@ will not be instrumented.
   // for every non-constant array index.
   void __sanitizer_cov_trace_gep(uintptr_t Idx);
 
-Partially disabling instrumentation
-===
+Disabling instrumentation with ``__attribute__((no_sanitize("coverage")))``
+===
+
+It is possible to disable coverage instrumentation for select functions via the
+function attribute ``__attribute__((no_sanitize("coverage")))``.
+
+Disabling instrumentation without source modification
+=
 
 It is sometimes useful to tell SanitizerCoverage to instrument only a subset 
of the
-functions in your target.
+functions in your target without modifying source files.
 With ``-fsanitize-coverage-allowlist=allowlist.txt``
 and ``-fsanitize-coverage-blocklist=blocklist.txt``,
 you can specify such a subset through the combination of an allowlist and a 
blocklist.

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 5e04f32187cd2..6a39453153930 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2897,6 +2897,10 @@ def NoSanitize : InheritableAttr {
   }
   return Mask;
 }
+
+bool hasCoverage() const {
+  return llvm::is_contained(sanitizers(), "coverage");
+}
   }];
 }
 

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index cae9cec804ce9..e9ee45d91dc50 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2564,12 +2564,17 @@ def NoSanitizeDocs : Documentation {
   let Content = [{
 Use the ``no_sanitize`` attribute on a function or a global variable
 declaration to specify that a particular instrumentation or set of
-instrumentations should not be applied. The attribute takes a list of
-string literals, which have the same meaning as values accepted by the
-``-fno-sanitize=`` flag. For example,
-``__attribute__((no_sanitize("address"

[clang] 4fbc66c - [Clang] Enable __has_feature(coverage_sanitizer)

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2021-05-27T18:24:21+02:00
New Revision: 4fbc66cd6d90d8d5169c43fcc1b1e26e8a98d3a9

URL: 
https://github.com/llvm/llvm-project/commit/4fbc66cd6d90d8d5169c43fcc1b1e26e8a98d3a9
DIFF: 
https://github.com/llvm/llvm-project/commit/4fbc66cd6d90d8d5169c43fcc1b1e26e8a98d3a9.diff

LOG: [Clang] Enable __has_feature(coverage_sanitizer)

Like other sanitizers, enable __has_feature(coverage_sanitizer) if clang
has enabled at least one SanitizerCoverage instrumentation type.

Because coverage instrumentation selection is not handled via normal
-fsanitize= (and thus not in SanitizeSet), passing this information
through to LangOptions required propagating the already parsed
-fsanitize-coverage= options from CodeGenOptions through to LangOptions
in FixupInvocation().

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D103159

Added: 
clang/test/Lexer/has_feature_coverage_sanitizer.cpp

Modified: 
clang/docs/SanitizerCoverage.rst
clang/include/clang/Basic/Features.def
clang/include/clang/Basic/LangOptions.h
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/docs/SanitizerCoverage.rst 
b/clang/docs/SanitizerCoverage.rst
index b0bb9823eb043..ebd5d72127aaa 100644
--- a/clang/docs/SanitizerCoverage.rst
+++ b/clang/docs/SanitizerCoverage.rst
@@ -316,7 +316,9 @@ Disabling instrumentation with 
``__attribute__((no_sanitize("coverage")))``
 ===
 
 It is possible to disable coverage instrumentation for select functions via the
-function attribute ``__attribute__((no_sanitize("coverage")))``.
+function attribute ``__attribute__((no_sanitize("coverage")))``. Because this
+attribute may not be supported by other compilers, it is recommended to use it
+together with ``__has_feature(coverage_sanitizer)``.
 
 Disabling instrumentation without source modification
 =

diff  --git a/clang/include/clang/Basic/Features.def 
b/clang/include/clang/Basic/Features.def
index 4f7e2db7683d0..a7a5e06a937e0 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -49,6 +49,7 @@ FEATURE(memtag_sanitizer, 
LangOpts.Sanitize.has(SanitizerKind::MemTag))
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(undefined_behavior_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
+FEATURE(coverage_sanitizer, LangOpts.SanitizeCoverage)
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index f5975d89a299e..d618daf3d23c2 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -280,6 +280,8 @@ class LangOptions : public LangOptionsBase {
 
   /// Set of enabled sanitizers.
   SanitizerSet Sanitize;
+  /// Is at least one coverage instrumentation type enabled.
+  bool SanitizeCoverage = false;
 
   /// Paths to files specifying which objects
   /// (files, functions, variables) should not be instrumented.

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 809492a36d3fa..28cd8391e32e2 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -453,7 +453,7 @@ static bool FixupInvocation(CompilerInvocation &Invocation,
   CodeGenOpts.XRayAlwaysEmitTypedEvents = LangOpts.XRayAlwaysEmitTypedEvents;
   CodeGenOpts.DisableFree = FrontendOpts.DisableFree;
   FrontendOpts.GenerateGlobalModuleIndex = FrontendOpts.UseGlobalModuleIndex;
-
+  LangOpts.SanitizeCoverage = CodeGenOpts.hasSanitizeCoverage();
   LangOpts.ForceEmitVTables = CodeGenOpts.ForceEmitVTables;
   LangOpts.SpeculativeLoadHardening = CodeGenOpts.SpeculativeLoadHardening;
   LangOpts.CurrentModule = LangOpts.ModuleName;

diff  --git a/clang/test/Lexer/has_feature_coverage_sanitizer.cpp 
b/clang/test/Lexer/has_feature_coverage_sanitizer.cpp
new file mode 100644
index 0..dfbe3973be043
--- /dev/null
+++ b/clang/test/Lexer/has_feature_coverage_sanitizer.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang -E -fsanitize-coverage=indirect-calls %s -o - | FileCheck 
--check-prefix=CHECK-SANCOV %s
+// RUN: %clang -E -fsanitize-coverage=inline-8bit-counters %s -o - | FileCheck 
--check-prefix=CHECK-SANCOV %s
+// RUN: %clang -E -fsanitize-coverage=trace-cmp %s -o - | FileCheck 
--check-prefix=CHECK-SANCOV %s
+// RUN: %clang -E -fsanitize-coverage=trace-pc %s -o - | FileCheck 
--check-prefix=CHECK-SANCOV %s
+// RUN: %clang -E -fsanitize-coverage=trace-pc-guard %s -o - | FileCheck 
--check-prefix=CHECK-SANCOV %s
+// RUN: %clang -E  %s -o - | FileCheck --check-prefix=CHECK-NO-SANCOV %s
+
+#if __has_feature(coverage_sanitizer)
+int SancovEnabled();
+#e

[clang] b95646f - Revert "Use-after-return sanitizer binary metadata"

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2022-11-30T23:35:50+01:00
New Revision: b95646fe7058471cc0ecda1efa25003009af0317

URL: 
https://github.com/llvm/llvm-project/commit/b95646fe7058471cc0ecda1efa25003009af0317
DIFF: 
https://github.com/llvm/llvm-project/commit/b95646fe7058471cc0ecda1efa25003009af0317.diff

LOG: Revert "Use-after-return sanitizer binary metadata"

This reverts commit d3c851d3fc8b69dda70bf5f999c5b39dc314dd73.

Some bots broke:

- 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8796062278266465473/overview
- https://lab.llvm.org/buildbot/#/builders/124/builds/5759/steps/7/logs/stdio

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/SanitizerArgs.cpp
llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
llvm/include/llvm/CodeGen/MachinePassRegistry.def
llvm/include/llvm/CodeGen/Passes.h
llvm/include/llvm/InitializePasses.h
llvm/include/llvm/Transforms/Instrumentation.h
llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
llvm/lib/CodeGen/CMakeLists.txt
llvm/lib/CodeGen/CodeGen.cpp
llvm/lib/CodeGen/TargetPassConfig.cpp
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
llvm/test/CodeGen/AArch64/O0-pipeline.ll
llvm/test/CodeGen/AArch64/O3-pipeline.ll
llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
llvm/test/CodeGen/ARM/O3-pipeline.ll
llvm/test/CodeGen/M68k/pipeline.ll
llvm/test/CodeGen/PowerPC/O3-pipeline.ll
llvm/test/CodeGen/RISCV/O0-pipeline.ll
llvm/test/CodeGen/RISCV/O3-pipeline.ll
llvm/test/CodeGen/X86/O0-pipeline.ll
llvm/test/CodeGen/X86/opt-pipeline.ll

Removed: 
clang/test/Instrumentation/SanitizerBinaryMetadata/common.h
clang/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
clang/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp



diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 81d5ccd4856d4..43521b76652db 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -288,8 +288,6 @@ CODEGENOPT(SanitizeCoverageTraceLoads, 1, 0) ///< Enable 
tracing of loads.
 CODEGENOPT(SanitizeCoverageTraceStores, 1, 0) ///< Enable tracing of stores.
 CODEGENOPT(SanitizeBinaryMetadataCovered, 1, 0) ///< Emit PCs for covered 
functions.
 CODEGENOPT(SanitizeBinaryMetadataAtomics, 1, 0) ///< Emit PCs for atomic 
operations.
-CODEGENOPT(SanitizeBinaryMetadataUAR, 1, 0) ///< Emit PCs for start of 
functions
-///< that are subject for 
use-after-return checking.
 CODEGENOPT(SanitizeStats , 1, 0) ///< Collect statistics for sanitizers.
 CODEGENOPT(SimplifyLibCalls  , 1, 1) ///< Set when -fbuiltin is enabled.
 CODEGENOPT(SoftFloat , 1, 0) ///< -soft-float.

diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index a1a20995f211d..13794035c9076 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -497,8 +497,7 @@ class CodeGenOptions : public CodeGenOptionsBase {
 
   // Check if any one of SanitizeBinaryMetadata* is enabled.
   bool hasSanitizeBinaryMetadata() const {
-return SanitizeBinaryMetadataCovered || SanitizeBinaryMetadataAtomics ||
-   SanitizeBinaryMetadataUAR;
+return SanitizeBinaryMetadataCovered || SanitizeBinaryMetadataAtomics;
   }
 };
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 1d577ab70788e..8da5e25bd38d0 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5575,10 +5575,6 @@ def fexperimental_sanitize_metadata_EQ_atomics
 : Flag<["-"], "fexperimental-sanitize-metadata=atomics">,
   HelpText<"Emit PCs for atomic operations used by binary analysis 
sanitizers">,
   MarshallingInfoFlag>;
-def fexperimental_sanitize_metadata_EQ_uar
-: Flag<["-"], "fexperimental-sanitize-metadata=uar">,
-  HelpText<"Emit PCs for start of functions that are subject for 
use-after-return checking.">,
-  MarshallingInfoFlag>;
 def fpatchable_function_entry_offset_EQ
 : Joined<["-"], "fpatchable-function-entry-offset=">, MetaVarName<"">,
   HelpText<"Generate M NOPs before function entry">,

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 717ec33441232..2b241967f5e7b 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -234,7 +234,6 @@ getSanitizerBinaryMetadataOptions(const CodeGenOptions 
&CGOpts) {
   SanitizerBinaryMetadataOptions Opts;
   Opts.Covered = CGOpts.SanitizeBinaryMe

[clang] 61ed649 - [SanitizerBinaryMetadata] Do not add to GPU code

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2023-03-09T10:15:28+01:00
New Revision: 61ed64954b979df0d5bfdfbe54a7c27e20be9001

URL: 
https://github.com/llvm/llvm-project/commit/61ed64954b979df0d5bfdfbe54a7c27e20be9001
DIFF: 
https://github.com/llvm/llvm-project/commit/61ed64954b979df0d5bfdfbe54a7c27e20be9001.diff

LOG: [SanitizerBinaryMetadata] Do not add to GPU code

SanitizerBinaryMetadata should only apply to to host code, and not GPU
code. Recently AMD GPU target code has experimental sanitizer support.

If we're compiling a mixed host/device source file, only add sanitizer
metadata to host code.

Differential Revision: https://reviews.llvm.org/D145519

Added: 


Modified: 
clang/lib/Driver/SanitizerArgs.cpp
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Removed: 




diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index 390faef0dbdf1..299d4adfa 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1100,13 +1100,16 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const 
llvm::opt::ArgList &Args,
   // NVPTX doesn't currently support sanitizers.  Bailing out here means
   // that e.g. -fsanitize=address applies only to host code, which is what we
   // want for now.
-  //
-  // AMDGPU sanitizer support is experimental and controlled by -fgpu-sanitize.
-  if (TC.getTriple().isNVPTX() ||
-  (TC.getTriple().isAMDGPU() &&
-   !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
- true)))
+  if (TC.getTriple().isNVPTX())
 return;
+  // AMDGPU sanitizer support is experimental and controlled by -fgpu-sanitize.
+  bool GPUSanitize = false;
+  if (TC.getTriple().isAMDGPU()) {
+if (!Args.hasFlag(options::OPT_fgpu_sanitize, 
options::OPT_fno_gpu_sanitize,
+  true))
+  return;
+GPUSanitize = true;
+  }
 
   // Translate available CoverageFeatures to corresponding clang-cc1 flags.
   // Do it even if Sanitizers.empty() since some forms of coverage don't 
require
@@ -1143,20 +1146,22 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const 
llvm::opt::ArgList &Args,
   addSpecialCaseListOpt(Args, CmdArgs, "-fsanitize-coverage-ignorelist=",
 CoverageIgnorelistFiles);
 
-  // Translate available BinaryMetadataFeatures to corresponding clang-cc1
-  // flags. Does not depend on any other sanitizers.
-  const std::pair BinaryMetadataFlags[] = {
-  std::make_pair(BinaryMetadataCovered, "covered"),
-  std::make_pair(BinaryMetadataAtomics, "atomics"),
-  std::make_pair(BinaryMetadataUAR, "uar")};
-  for (const auto &F : BinaryMetadataFlags) {
-if (BinaryMetadataFeatures & F.first)
-  CmdArgs.push_back(
-  Args.MakeArgString("-fexperimental-sanitize-metadata=" + F.second));
+  if (!GPUSanitize) {
+// Translate available BinaryMetadataFeatures to corresponding clang-cc1
+// flags. Does not depend on any other sanitizers. Unsupported on GPUs.
+const std::pair BinaryMetadataFlags[] = {
+std::make_pair(BinaryMetadataCovered, "covered"),
+std::make_pair(BinaryMetadataAtomics, "atomics"),
+std::make_pair(BinaryMetadataUAR, "uar")};
+for (const auto &F : BinaryMetadataFlags) {
+  if (BinaryMetadataFeatures & F.first)
+CmdArgs.push_back(
+Args.MakeArgString("-fexperimental-sanitize-metadata=" + 
F.second));
+}
+addSpecialCaseListOpt(Args, CmdArgs,
+  "-fexperimental-sanitize-metadata-ignorelist=",
+  BinaryMetadataIgnorelistFiles);
   }
-  addSpecialCaseListOpt(Args, CmdArgs,
-"-fexperimental-sanitize-metadata-ignorelist=",
-BinaryMetadataIgnorelistFiles);
 
   if (TC.getTriple().isOSWindows() && needsUbsanRt()) {
 // Instruct the code generator to embed linker directives in the object 
file

diff  --git a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp 
b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
index 563d3e4039a9b..a1309a4ec0f45 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -131,6 +131,8 @@ class SanitizerBinaryMetadata {
 IRB(M.getContext()) {
 // FIXME: Make it work with other formats.
 assert(TargetTriple.isOSBinFormatELF() && "ELF only");
+assert(!(TargetTriple.isNVPTX() || TargetTriple.isAMDGPU()) &&
+   "Device targets are not supported");
   }
 
   bool run();



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

[clang] 421215b - [SanitizerBinaryMetadata] Support ignore list

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2023-02-10T10:25:48+01:00
New Revision: 421215b919d037a912cd4fffa73242852da41fc0

URL: 
https://github.com/llvm/llvm-project/commit/421215b919d037a912cd4fffa73242852da41fc0
DIFF: 
https://github.com/llvm/llvm-project/commit/421215b919d037a912cd4fffa73242852da41fc0.diff

LOG: [SanitizerBinaryMetadata] Support ignore list

For large projects it will be required to opt out entire subdirectories.
In the absence of fine-grained control over the flags passed via the
build system, introduce -fexperimental-sanitize-metadata-ignorelist=.

The format is identical to other sanitizer ignore lists, and its effect
will be to simply not instrument either functions or entire modules
based on the rules in the ignore list file.

Reviewed By: dvyukov

Differential Revision: https://reviews.llvm.org/D143664

Added: 
clang/test/CodeGen/sanitize-metadata-ignorelist.c
clang/test/Driver/fsanitize-metadata-ignorelist.c

Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/SanitizerArgs.cpp
llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 4175fe3072ab8..04a516df1911e 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -417,6 +417,11 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// coverage pass should actually not be instrumented.
   std::vector SanitizeCoverageIgnorelistFiles;
 
+  /// Path to ignorelist file specifying which objects
+  /// (files, functions) listed for instrumentation by sanitizer
+  /// binary metadata pass should not be instrumented.
+  std::vector SanitizeMetadataIgnorelistFiles;
+
   /// Name of the stack usage file (i.e., .su file) if user passes
   /// -fstack-usage. If empty, it can be implied that -fstack-usage is not
   /// passed on the command line.

diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 800819779890a..8d557846c6ff0 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -215,6 +215,8 @@ def err_drv_malformed_sanitizer_coverage_allowlist : Error<
   "malformed sanitizer coverage allowlist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_ignorelist : Error<
   "malformed sanitizer coverage ignorelist: '%0'">;
+def err_drv_malformed_sanitizer_metadata_ignorelist : Error<
+  "malformed sanitizer metadata ignorelist: '%0'">;
 def err_drv_unsupported_static_ubsan_darwin : Error<
   "static UndefinedBehaviorSanitizer runtime is not supported on darwin">;
 def err_drv_duplicate_config : Error<

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 9766a087eb6d9..96904518a51d7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1756,6 +1756,10 @@ def fexperimental_sanitize_metadata_EQ : 
CommaJoined<["-"], "fexperimental-sanit
 def fno_experimental_sanitize_metadata_EQ : CommaJoined<["-"], 
"fno-experimental-sanitize-metadata=">,
   Group, Flags<[CoreOption]>,
   HelpText<"Disable emitting metadata for binary analysis sanitizers">;
+def fexperimental_sanitize_metadata_ignorelist_EQ : Joined<["-"], 
"fexperimental-sanitize-metadata-ignorelist=">,
+Group, Flags<[CoreOption]>,
+HelpText<"Disable sanitizer metadata for modules and functions that match 
the provided special case list">,
+
MarshallingInfoStringVector>;
 def fsanitize_memory_track_origins_EQ : Joined<["-"], 
"fsanitize-memory-track-origins=">,
 Group,
 HelpText<"Enable origins tracking in 
MemorySanitizer">,

diff  --git a/clang/include/clang/Driver/SanitizerArgs.h 
b/clang/include/clang/Driver/SanitizerArgs.h
index abc5271b0a811..61b7c7f809ae4 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -30,6 +30,7 @@ class SanitizerArgs {
   std::vector SystemIgnorelistFiles;
   std::vector CoverageAllowlistFiles;
   std::vector CoverageIgnorelistFiles;
+  std::vector BinaryMetadataIgnorelistFiles;
   int CoverageFeatures = 0;
   int BinaryMetadataFeatures = 0;
   int MsanTrackOrigins = 0;

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index f38d2347c9723..b03d326dd0654 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -665,7 +665,8 @@ static void addSanitizers(const Triple &TargetTriple,

[clang] bb8bd8c - [SanitizerBinaryMetadata] Fix ignorelist test under Windows

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2023-02-10T11:24:25+01:00
New Revision: bb8bd8c232e893441937d057a8b32760065c6e1d

URL: 
https://github.com/llvm/llvm-project/commit/bb8bd8c232e893441937d057a8b32760065c6e1d
DIFF: 
https://github.com/llvm/llvm-project/commit/bb8bd8c232e893441937d057a8b32760065c6e1d.diff

LOG: [SanitizerBinaryMetadata] Fix ignorelist test under Windows

Windows paths confuse the regular expression. Just use the test source
name directly.

Fixes: 421215b919d0 ("[SanitizerBinaryMetadata] Support ignore list")

Added: 


Modified: 
clang/test/CodeGen/sanitize-metadata-ignorelist.c

Removed: 




diff  --git a/clang/test/CodeGen/sanitize-metadata-ignorelist.c 
b/clang/test/CodeGen/sanitize-metadata-ignorelist.c
index 9cc9215a9e43..1458b681672c 100644
--- a/clang/test/CodeGen/sanitize-metadata-ignorelist.c
+++ b/clang/test/CodeGen/sanitize-metadata-ignorelist.c
@@ -1,7 +1,7 @@
 // RUN: %clang -O -fexperimental-sanitize-metadata=all -target 
x86_64-gnu-linux -x c -S -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=ALLOW
 // RUN: echo "fun:foo" > %t.fun
 // RUN: %clang -O -fexperimental-sanitize-metadata=all 
-fexperimental-sanitize-metadata-ignorelist=%t.fun -target x86_64-gnu-linux -x 
c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=FUN
-// RUN: echo "src:%s" > %t.src
+// RUN: echo "src:*sanitize-metadata-ignorelist.c" > %t.src
 // RUN: %clang -O -fexperimental-sanitize-metadata=all 
-fexperimental-sanitize-metadata-ignorelist=%t.src -target x86_64-gnu-linux -x 
c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=SRC
 
 int y;



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

[clang] dac423b - [SanitizerBinaryMetadata] Fix ignorelist test with -Assert

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2023-02-11T00:33:13+01:00
New Revision: dac423bd571858a85f3b388904392f0e55421d7d

URL: 
https://github.com/llvm/llvm-project/commit/dac423bd571858a85f3b388904392f0e55421d7d
DIFF: 
https://github.com/llvm/llvm-project/commit/dac423bd571858a85f3b388904392f0e55421d7d.diff

LOG: [SanitizerBinaryMetadata] Fix ignorelist test with -Assert

%clang and %clang_cc1 have different behaviour, and with -Assert %clang
seems to not match +Assert behaviour. Fix it by switching to %clang_cc1
for the test.

Fixes: 421215b919d0 ("[SanitizerBinaryMetadata] Support ignore list")

Added: 


Modified: 
clang/test/CodeGen/sanitize-metadata-ignorelist.c

Removed: 




diff  --git a/clang/test/CodeGen/sanitize-metadata-ignorelist.c 
b/clang/test/CodeGen/sanitize-metadata-ignorelist.c
index 1458b681672c..b5656fd0781d 100644
--- a/clang/test/CodeGen/sanitize-metadata-ignorelist.c
+++ b/clang/test/CodeGen/sanitize-metadata-ignorelist.c
@@ -1,15 +1,15 @@
-// RUN: %clang -O -fexperimental-sanitize-metadata=all -target 
x86_64-gnu-linux -x c -S -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=ALLOW
+// RUN: %clang_cc1 -O -fexperimental-sanitize-metadata=atomics -triple 
x86_64-gnu-linux -x c -S -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=ALLOW
 // RUN: echo "fun:foo" > %t.fun
-// RUN: %clang -O -fexperimental-sanitize-metadata=all 
-fexperimental-sanitize-metadata-ignorelist=%t.fun -target x86_64-gnu-linux -x 
c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=FUN
+// RUN: %clang_cc1 -O -fexperimental-sanitize-metadata=atomics 
-fexperimental-sanitize-metadata-ignorelist=%t.fun -triple x86_64-gnu-linux -x 
c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=FUN
 // RUN: echo "src:*sanitize-metadata-ignorelist.c" > %t.src
-// RUN: %clang -O -fexperimental-sanitize-metadata=all 
-fexperimental-sanitize-metadata-ignorelist=%t.src -target x86_64-gnu-linux -x 
c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=SRC
+// RUN: %clang_cc1 -O -fexperimental-sanitize-metadata=atomics 
-fexperimental-sanitize-metadata-ignorelist=%t.src -triple x86_64-gnu-linux -x 
c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=SRC
 
 int y;
 
 // ALLOW-LABEL: define {{[^@]+}}@foo
-// ALLOW-SAME: () local_unnamed_addr #[[ATTR0:[0-9]+]] !pcsections !5 {
+// ALLOW-SAME: () local_unnamed_addr #[[ATTR0:[0-9]+]] !pcsections !2 {
 // ALLOW-NEXT:  entry:
-// ALLOW-NEXT:[[TMP0:%.*]] = atomicrmw add ptr @y, i32 1 monotonic, align 
4, !pcsections !7
+// ALLOW-NEXT:[[TMP0:%.*]] = atomicrmw add ptr @y, i32 1 monotonic, align 
4, !pcsections !4
 // ALLOW-NEXT:ret void
 //
 // FUN-LABEL: define {{[^@]+}}@foo
@@ -29,15 +29,15 @@ void foo() {
 }
 
 // ALLOW-LABEL: define {{[^@]+}}@bar
-// ALLOW-SAME: () local_unnamed_addr #[[ATTR0]] !pcsections !5 {
+// ALLOW-SAME: () local_unnamed_addr #[[ATTR0]] !pcsections !2 {
 // ALLOW-NEXT:  entry:
-// ALLOW-NEXT:[[TMP0:%.*]] = atomicrmw add ptr @y, i32 2 monotonic, align 
4, !pcsections !7
+// ALLOW-NEXT:[[TMP0:%.*]] = atomicrmw add ptr @y, i32 2 monotonic, align 
4, !pcsections !4
 // ALLOW-NEXT:ret void
 //
 // FUN-LABEL: define {{[^@]+}}@bar
-// FUN-SAME: () local_unnamed_addr #[[ATTR0]] !pcsections !5 {
+// FUN-SAME: () local_unnamed_addr #[[ATTR0]] !pcsections !2 {
 // FUN-NEXT:  entry:
-// FUN-NEXT:[[TMP0:%.*]] = atomicrmw add ptr @y, i32 2 monotonic, align 4, 
!pcsections !7
+// FUN-NEXT:[[TMP0:%.*]] = atomicrmw add ptr @y, i32 2 monotonic, align 4, 
!pcsections !4
 // FUN-NEXT:ret void
 //
 // SRC-LABEL: define {{[^@]+}}@bar



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

[clang] 5f605e2 - [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2023-04-19T14:49:56+02:00
New Revision: 5f605e254a0f81a41fd69025c572d597f3059ebc

URL: 
https://github.com/llvm/llvm-project/commit/5f605e254a0f81a41fd69025c572d597f3059ebc
DIFF: 
https://github.com/llvm/llvm-project/commit/5f605e254a0f81a41fd69025c572d597f3059ebc.diff

LOG: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute

To avoid false positives, respect no_sanitize("thread") when atomics
metadata is enabled.

Reviewed By: dvyukov

Differential Revision: https://reviews.llvm.org/D148694

Added: 
clang/test/CodeGen/sanitize-metadata-nosanitize.c

Modified: 
clang/lib/CodeGen/CodeGenFunction.cpp
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index a5cbf7223ad65..f6f92ebd85cf2 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -730,31 +730,38 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
QualType RetTy,
 
   if (D) {
 const bool SanitizeBounds = SanOpts.hasOneOf(SanitizerKind::Bounds);
+SanitizerMask no_sanitize_mask;
 bool NoSanitizeCoverage = false;
 
 for (auto *Attr : D->specific_attrs()) {
-  // Apply the no_sanitize* attributes to SanOpts.
-  SanitizerMask mask = Attr->getMask();
-  SanOpts.Mask &= ~mask;
-  if (mask & SanitizerKind::Address)
-SanOpts.set(SanitizerKind::KernelAddress, false);
-  if (mask & SanitizerKind::KernelAddress)
-SanOpts.set(SanitizerKind::Address, false);
-  if (mask & SanitizerKind::HWAddress)
-SanOpts.set(SanitizerKind::KernelHWAddress, false);
-  if (mask & SanitizerKind::KernelHWAddress)
-SanOpts.set(SanitizerKind::HWAddress, false);
-
+  no_sanitize_mask |= Attr->getMask();
   // SanitizeCoverage is not handled by SanOpts.
   if (Attr->hasCoverage())
 NoSanitizeCoverage = true;
 }
 
+// Apply the no_sanitize* attributes to SanOpts.
+SanOpts.Mask &= ~no_sanitize_mask;
+if (no_sanitize_mask & SanitizerKind::Address)
+  SanOpts.set(SanitizerKind::KernelAddress, false);
+if (no_sanitize_mask & SanitizerKind::KernelAddress)
+  SanOpts.set(SanitizerKind::Address, false);
+if (no_sanitize_mask & SanitizerKind::HWAddress)
+  SanOpts.set(SanitizerKind::KernelHWAddress, false);
+if (no_sanitize_mask & SanitizerKind::KernelHWAddress)
+  SanOpts.set(SanitizerKind::HWAddress, false);
+
 if (SanitizeBounds && !SanOpts.hasOneOf(SanitizerKind::Bounds))
   Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
 
 if (NoSanitizeCoverage && CGM.getCodeGenOpts().hasSanitizeCoverage())
   Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
+
+// Some passes need the non-negated no_sanitize attribute. Pass them on.
+if (CGM.getCodeGenOpts().hasSanitizeBinaryMetadata()) {
+  if (no_sanitize_mask & SanitizerKind::Thread)
+Fn->addFnAttr("no_sanitize_thread");
+}
   }
 
   if (ShouldSkipSanitizerInstrumentation()) {

diff  --git a/clang/test/CodeGen/sanitize-metadata-nosanitize.c 
b/clang/test/CodeGen/sanitize-metadata-nosanitize.c
new file mode 100644
index 0..488714fe6078e
--- /dev/null
+++ b/clang/test/CodeGen/sanitize-metadata-nosanitize.c
@@ -0,0 +1,111 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --check-attributes --check-globals --version 2
+// RUN: %clang_cc1 -O -fexperimental-sanitize-metadata=covered 
-fexperimental-sanitize-metadata=atomics -fexperimental-sanitize-metadata=uar 
-triple x86_64-gnu-linux -x c -S -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=CHECK
+
+//.
+// CHECK: @__start_sanmd_covered = extern_weak hidden global ptr
+// CHECK: @__stop_sanmd_covered = extern_weak hidden global ptr
+// CHECK: @__start_sanmd_atomics = extern_weak hidden global ptr
+// CHECK: @__stop_sanmd_atomics = extern_weak hidden global ptr
+// CHECK: @llvm.used = appending global [4 x ptr] [ptr 
@__sanitizer_metadata_covered.module_ctor, ptr 
@__sanitizer_metadata_covered.module_dtor, ptr 
@__sanitizer_metadata_atomics.module_ctor, ptr 
@__sanitizer_metadata_atomics.module_dtor], section "llvm.metadata"
+// CHECK: @llvm.global_ctors = appending global [2 x { i32, ptr, ptr }] [{ 
i32, ptr, ptr } { i32 2, ptr @__sanitizer_metadata_covered.module_ctor, ptr 
@__sanitizer_metadata_covered.module_ctor }, { i32, ptr, ptr } { i32 2, ptr 
@__sanitizer_metadata_atomics.module_ctor, ptr 
@__sanitizer_metadata_atomics.module_ctor }]
+// CHECK: @llvm.global_dtors = appending global [2 x { i32, ptr, ptr }] [{ 
i32, ptr, ptr } { i32 2, ptr @__sanitizer_metadata_covered.module_dtor, ptr 
@__sanitizer_metadata_covered.module_dtor }, { i32, ptr, ptr } { i32 2, ptr 
@__sanitizer_metadata_atomics.module_dtor, ptr 
@__sanitizer_metadata_atomics.module_dtor }]

[clang] [llvm] [SanitizerBinaryMetadata] Fix multi-version sanitizer metadata (PR #97848)

[Marco Elver via cfe-commits]

https://github.com/melver created 
https://github.com/llvm/llvm-project/pull/97848

It should be valid to combine TUs that have different versions of sanitizer 
metadata. However, this had not been possible due to giving sanitizer metadata 
sections, constructors, and destructors (that call callbacks) the same name for 
different versions.

This would then result in the linker attempting to merge sections that contain 
metadata of different versions, as well as picking any one of the constructors 
or destructors due to having the same COMDAT key. The end result is that 
consumers of this data would end up interpreting the metadata incorrectly.

Although combining old and new versions is not recommended, more realistic is 
combining TUs that have been compiled with different target code models (which 
are also encoded in the sanitizer metadata version).

To fix, and properly support multi-version sanitizer metadata, attach the 
version to section names and internal constructor and destructor names. The ABI 
remains unchanged.

>From 8478cb8475785ad64523e81ebd89ba53b0bfa8f7 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Thu, 4 Jul 2024 11:08:16 +0200
Subject: [PATCH] [SanitizerBinaryMetadata] Fix multi-version sanitizer
 metadata

It should be valid to combine TUs that have different versions of
sanitizer metadata. However, this had not been possible due to giving
sanitizer metadata sections, constructors, and destructors (that call
callbacks) the same name for different versions.

This would then result in the linker attempting to merge sections that
contain metadata of different versions, as well as picking any one of
the constructors or destructors due to having the same COMDAT key. The
end result is that consumers of this data would end up interpreting the
metadata incorrectly.

Although combining old and new versions is not recommended, more
realistic is combining TUs that have been compiled with different target
code models (which are also encoded in the sanitizer metadata version).

To fix, and properly support multi-version sanitizer metadata, attach
the version to section names and internal constructor and destructor
names. The ABI remains unchanged.
---
 .../CodeGen/sanitize-metadata-ignorelist.c|  6 ++--
 .../CodeGen/sanitize-metadata-nosanitize.c| 22 ++---
 clang/test/CodeGen/sanitize-metadata.c| 28 
 .../SanitizerBinaryMetadata.cpp   | 32 +--
 .../SanitizerBinaryMetadata/atomics.ll| 28 
 .../SanitizerBinaryMetadata/ctor.ll   | 18 +--
 6 files changed, 73 insertions(+), 61 deletions(-)

diff --git a/clang/test/CodeGen/sanitize-metadata-ignorelist.c 
b/clang/test/CodeGen/sanitize-metadata-ignorelist.c
index 24fb4fa62cc53..4dc8c0c35fefe 100644
--- a/clang/test/CodeGen/sanitize-metadata-ignorelist.c
+++ b/clang/test/CodeGen/sanitize-metadata-ignorelist.c
@@ -50,6 +50,6 @@ void bar() {
   __atomic_fetch_add(&y, 2, __ATOMIC_RELAXED);
 }
 
-// ALLOW: __sanitizer_metadata_covered.module_ctor
-// FUN: __sanitizer_metadata_covered.module_ctor
-// SRC-NOT: __sanitizer_metadata_covered.module_ctor
+// ALLOW: __sanitizer_metadata_covered2.module_ctor
+// FUN: __sanitizer_metadata_covered2.module_ctor
+// SRC-NOT: __sanitizer_metadata_covered{{.*}}.module_ctor
diff --git a/clang/test/CodeGen/sanitize-metadata-nosanitize.c 
b/clang/test/CodeGen/sanitize-metadata-nosanitize.c
index 6414956fb6796..388a3df547a73 100644
--- a/clang/test/CodeGen/sanitize-metadata-nosanitize.c
+++ b/clang/test/CodeGen/sanitize-metadata-nosanitize.c
@@ -2,13 +2,13 @@
 // RUN: %clang_cc1 -O -fexperimental-sanitize-metadata=covered 
-fexperimental-sanitize-metadata=atomics -fexperimental-sanitize-metadata=uar 
-triple x86_64-gnu-linux -x c -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=CHECK
 
 //.
-// CHECK: @__start_sanmd_covered = extern_weak hidden global ptr
-// CHECK: @__stop_sanmd_covered = extern_weak hidden global ptr
-// CHECK: @__start_sanmd_atomics = extern_weak hidden global ptr
-// CHECK: @__stop_sanmd_atomics = extern_weak hidden global ptr
-// CHECK: @llvm.used = appending global [4 x ptr] [ptr 
@__sanitizer_metadata_covered.module_ctor, ptr 
@__sanitizer_metadata_covered.module_dtor, ptr 
@__sanitizer_metadata_atomics.module_ctor, ptr 
@__sanitizer_metadata_atomics.module_dtor], section "llvm.metadata"
-// CHECK: @llvm.global_ctors = appending global [2 x { i32, ptr, ptr }] [{ 
i32, ptr, ptr } { i32 2, ptr @__sanitizer_metadata_covered.module_ctor, ptr 
@__sanitizer_metadata_covered.module_ctor }, { i32, ptr, ptr } { i32 2, ptr 
@__sanitizer_metadata_atomics.module_ctor, ptr 
@__sanitizer_metadata_atomics.module_ctor }]
-// CHECK: @llvm.global_dtors = appending global [2 x { i32, ptr, ptr }] [{ 
i32, ptr, ptr } { i32 2, ptr @__sanitizer_metadata_covered.module_dtor, ptr 
@__sanitizer_metadata_covered.module_dtor }, { i32, ptr, ptr } { i32 2, ptr 
@__sanitizer_metadata_atomics.module_dtor, ptr 
@__sanitiz

[clang] [llvm] [SanitizerBinaryMetadata] Fix multi-version sanitizer metadata (PR #97848)

[Marco Elver via cfe-commits]

https://github.com/melver closed https://github.com/llvm/llvm-project/pull/97848
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c65186c - [clang] Improve -Wdeclaration-after-statement

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2022-01-20T19:56:34+01:00
New Revision: c65186c89f35b7b599c41183def666a2bde62ddd

URL: 
https://github.com/llvm/llvm-project/commit/c65186c89f35b7b599c41183def666a2bde62ddd
DIFF: 
https://github.com/llvm/llvm-project/commit/c65186c89f35b7b599c41183def666a2bde62ddd.diff

LOG: [clang] Improve -Wdeclaration-after-statement

With 118f966b46cf, Clang matches GCC's behaviour and allows enabling
-Wdeclaration-after-statement with C99 and later.

However, the check for mixing declarations and code is not a constant time
algorithm, and therefore should be guarded with Diags.isIgnored().

Furthermore, improve test coverage with: non-pedantic C89 with the
warning; C11 with the warning; and when using -Wall.

Finally, mention the changed behaviour in ReleaseNotes.rst.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D117232

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaStmt.cpp
clang/test/Sema/warn-mixed-decls.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c787d355a3148..2eec63901932e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -58,6 +58,11 @@ Improvements to Clang's diagnostics
   release being diagnosed against). These new groups are automatically implied
   when passing ``-Wc++N-extensions``. Resolves PR33518.
 
+- Support ``-Wdeclaration-after-statement`` with C99 and later standards, and
+  not just C89, matching GCC's behaviour. A notable usecase is supporting style
+  guides that forbid mixing declarations and code, but want to move to newer C
+  standards.
+
 Non-comprehensive list of changes in this release
 -
 

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index ef498f9a52282..746eb82a5bdc7 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -413,7 +413,10 @@ StmtResult Sema::ActOnCompoundStmt(SourceLocation L, 
SourceLocation R,
   // If we're in C mode, check that we don't have any decls after stmts.  If
   // so, emit an extension diagnostic in C89 and potentially a warning in later
   // versions.
-  if (!getLangOpts().CPlusPlus) {
+  const unsigned MixedDeclsCodeID = getLangOpts().C99
+? diag::warn_mixed_decls_code
+: diag::ext_mixed_decls_code;
+  if (!getLangOpts().CPlusPlus && !Diags.isIgnored(MixedDeclsCodeID, L)) {
 // Note that __extension__ can be around a decl.
 unsigned i = 0;
 // Skip over all declarations.
@@ -426,8 +429,7 @@ StmtResult Sema::ActOnCompoundStmt(SourceLocation L, 
SourceLocation R,
 
 if (i != NumElts) {
   Decl *D = *cast(Elts[i])->decl_begin();
-  Diag(D->getLocation(), !getLangOpts().C99 ? diag::ext_mixed_decls_code
-: diag::warn_mixed_decls_code);
+  Diag(D->getLocation(), MixedDeclsCodeID);
 }
   }
 

diff  --git a/clang/test/Sema/warn-mixed-decls.c 
b/clang/test/Sema/warn-mixed-decls.c
index 219d64472b589..b8a7dc1e2bc09 100644
--- a/clang/test/Sema/warn-mixed-decls.c
+++ b/clang/test/Sema/warn-mixed-decls.c
@@ -1,13 +1,23 @@
 /* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -pedantic %s
  */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 
-Wdeclaration-after-statement %s
+ */
 /* RUN: %clang_cc1 -fsyntax-only -verify -std=c99 
-Wdeclaration-after-statement %s
  */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c11 
-Wdeclaration-after-statement %s
+ */
 
 /* Should not emit diagnostic when not pedantic, not enabled or in C++ Code*/
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c89 %s
  */
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 %s
  */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c89 -Wall %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 -Wall -pedantic %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c11 -Wall -pedantic %s
+ */
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ %s
  */
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ 
-Wdeclaration-after-statement %s



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

[clang] 17ce89f - [SanitizerBounds] Add support for NoSanitizeBounds function

[Marco Elver via cfe-commits]

Author: Tong Zhang
Date: 2022-03-01T18:47:02+01:00
New Revision: 17ce89fa8016758be2ec879c5560e506cad4c362

URL: 
https://github.com/llvm/llvm-project/commit/17ce89fa8016758be2ec879c5560e506cad4c362
DIFF: 
https://github.com/llvm/llvm-project/commit/17ce89fa8016758be2ec879c5560e506cad4c362.diff

LOG: [SanitizerBounds] Add support for NoSanitizeBounds function

Currently adding attribute no_sanitize("bounds") isn't disabling
-fsanitize=local-bounds (also enabled in -fsanitize=bounds). The Clang
frontend handles fsanitize=array-bounds which can already be disabled by
no_sanitize("bounds"). However, instrumentation added by the
BoundsChecking pass in the middle-end cannot be disabled by the
attribute.

The fix is very similar to D102772 that added the ability to selectively
disable sanitizer pass on certain functions.

In this patch, if no_sanitize("bounds") is provided, an additional
function attribute (NoSanitizeBounds) is attached to IR to let the
BoundsChecking pass know we want to disable local-bounds checking. In
order to support this feature, the IR is extended (similar to D102772)
to make Clang able to preserve the information and let BoundsChecking
pass know bounds checking is disabled for certain function.

Reviewed By: melver

Differential Revision: https://reviews.llvm.org/D119816

Added: 
llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll

Modified: 
clang/lib/CodeGen/CodeGenFunction.cpp
clang/test/CodeGen/bounds-checking.c
clang/test/CodeGen/sanitize-coverage.c
llvm/bindings/go/llvm/ir_test.go
llvm/docs/BitCodeFormat.rst
llvm/docs/LangRef.rst
llvm/include/llvm/AsmParser/LLToken.h
llvm/include/llvm/Bitcode/LLVMBitCodes.h
llvm/include/llvm/IR/Attributes.td
llvm/lib/AsmParser/LLLexer.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/test/Bitcode/attributes.ll
llvm/test/Bitcode/compatibility.ll
llvm/utils/emacs/llvm-mode.el
llvm/utils/llvm.grm
llvm/utils/vim/syntax/llvm.vim
llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 9c3e5d5460014..c1f3a3014a192 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -740,6 +740,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType 
RetTy,
   } while (false);
 
   if (D) {
+const bool SanitizeBounds = SanOpts.hasOneOf(SanitizerKind::Bounds);
 bool NoSanitizeCoverage = false;
 
 for (auto Attr : D->specific_attrs()) {
@@ -760,6 +761,9 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType 
RetTy,
 NoSanitizeCoverage = true;
 }
 
+if (SanitizeBounds && !SanOpts.hasOneOf(SanitizerKind::Bounds))
+  Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
+
 if (NoSanitizeCoverage && CGM.getCodeGenOpts().hasSanitizeCoverage())
   Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
   }

diff  --git a/clang/test/CodeGen/bounds-checking.c 
b/clang/test/CodeGen/bounds-checking.c
index ba8c18934388c..25b767c7fa0d9 100644
--- a/clang/test/CodeGen/bounds-checking.c
+++ b/clang/test/CodeGen/bounds-checking.c
@@ -49,3 +49,12 @@ int f5(union U *u, int i) {
   return u->c[i];
   // CHECK: }
 }
+
+__attribute__((no_sanitize("bounds")))
+int f6(int i) {
+   int b[64];
+   // CHECK-NOT: call void @llvm.trap()
+   // CHECK-NOT: trap:
+   // CHECK-NOT: cont:
+   return b[i];
+}

diff  --git a/clang/test/CodeGen/sanitize-coverage.c 
b/clang/test/CodeGen/sanitize-coverage.c
index 6fd0adcb4a880..984076fed0ce4 100644
--- a/clang/test/CodeGen/sanitize-coverage.c
+++ b/clang/test/CodeGen/sanitize-coverage.c
@@ -53,6 +53,7 @@ void test_no_sanitize_combined(int n) {
   // ASAN-NOT: call void @__asan_report_store
   // MSAN-NOT: call void @__msan_warning
   // BOUNDS-NOT: call void @__ubsan_handle_out_of_bounds
+  // BOUNDS-NOT: call void @llvm.trap()
   // TSAN-NOT: call void @__tsan_func_entry
   // UBSAN-NOT: call void @__ubsan_handle
   if (n)
@@ -72,6 +73,7 @@ void test_no_sanitize_separate(int n) {
   // ASAN-NOT: call void @__asan_report_store
   // MSAN-NOT: call void @__msan_warning
   // BOUNDS-NOT: call void @__ubsan_handle_out_of_bounds
+  // BOUNDS-NOT: call void @llvm.trap()
   // TSAN-NOT: call void @__tsan_func_entry
   // UBSAN-NOT: call void @__ubsan_handle
   if (n)

diff  --git a/llvm/bindings/go/llvm/ir_test.go 
b/llvm/bindings/go/llvm/ir_test.go
index 61b482f2ef9a2..1aeb6e69bafb2 100644
--- a/llvm/bindings/go/llvm/ir_test.go
+++ b/llvm/bindings/go/llvm/ir_test.go
@@ -69,6 +69,7 @@ func TestAttributes(t *testing.T) {
"noredzone",
"noreturn",
"nounwind",
+   "nosanitize_

[clang] 732ad8e - [clang][auto-init] Provide __builtin_alloca*_uninitialized variants

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2022-01-12T15:13:10+01:00
New Revision: 732ad8ea62edc403727af57537b5d83dcfa937aa

URL: 
https://github.com/llvm/llvm-project/commit/732ad8ea62edc403727af57537b5d83dcfa937aa
DIFF: 
https://github.com/llvm/llvm-project/commit/732ad8ea62edc403727af57537b5d83dcfa937aa.diff

LOG: [clang][auto-init] Provide __builtin_alloca*_uninitialized variants

When `-ftrivial-auto-var-init=` is enabled, allocas unconditionally
receive auto-initialization since [1].

In certain cases, it turns out, this is causing problems. For example,
when using alloca to add a random stack offset, as the Linux kernel does
on syscall entry [2]. In this case, none of the alloca'd stack memory is
ever used, and initializing it should be controllable; furthermore, it
is not always possible to safely call memset (see [2]).

Introduce `__builtin_alloca_uninitialized()` (and
`__builtin_alloca_with_align_uninitialized`), which never performs
initialization when `-ftrivial-auto-var-init=` is enabled.

[1] https://reviews.llvm.org/D60548
[2] https://lkml.kernel.org/r/ybhtkujeejzcl...@elver.google.com

Reviewed By: glider

Differential Revision: https://reviews.llvm.org/D115440

Added: 


Modified: 
clang/include/clang/Basic/Builtins.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGenCXX/trivial-auto-var-init.cpp
clang/test/Sema/warn-alloca.c

Removed: 




diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index bfaa7e9f5a9fa..c7c47cf99abac 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -641,7 +641,9 @@ BUILTIN(__builtin_unreachable, "v", "nr")
 BUILTIN(__builtin_shufflevector, "v."   , "nct")
 BUILTIN(__builtin_convertvector, "v."   , "nct")
 BUILTIN(__builtin_alloca, "v*z"   , "Fn")
+BUILTIN(__builtin_alloca_uninitialized, "v*z", "Fn")
 BUILTIN(__builtin_alloca_with_align, "v*zIz", "Fn")
+BUILTIN(__builtin_alloca_with_align_uninitialized, "v*zIz", "Fn")
 BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
 
 BUILTIN(__builtin_elementwise_abs, "v.", "nct")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 7b5d202afbba2..e9dd41a7daa10 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3427,6 +3427,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 
   case Builtin::BIalloca:
   case Builtin::BI_alloca:
+  case Builtin::BI__builtin_alloca_uninitialized:
   case Builtin::BI__builtin_alloca: {
 Value *Size = EmitScalarExpr(E->getArg(0));
 const TargetInfo &TI = getContext().getTargetInfo();
@@ -3437,10 +3438,12 @@ RValue CodeGenFunction::EmitBuiltinExpr(const 
GlobalDecl GD, unsigned BuiltinID,
 .getAsAlign();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(SuitableAlignmentInBytes);
-initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
+if (BuiltinID != Builtin::BI__builtin_alloca_uninitialized)
+  initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
 return RValue::get(AI);
   }
 
+  case Builtin::BI__builtin_alloca_with_align_uninitialized:
   case Builtin::BI__builtin_alloca_with_align: {
 Value *Size = EmitScalarExpr(E->getArg(0));
 Value *AlignmentInBitsValue = EmitScalarExpr(E->getArg(1));
@@ -3450,7 +3453,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getAsAlign();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(AlignmentInBytes);
-initializeAlloca(*this, AI, Size, AlignmentInBytes);
+if (BuiltinID != Builtin::BI__builtin_alloca_with_align_uninitialized)
+  initializeAlloca(*this, AI, Size, AlignmentInBytes);
 return RValue::get(AI);
   }
 

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 908cb78fbb0a7..d067ac31dc1e8 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1750,10 +1750,12 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, 
unsigned BuiltinID,
   return ExprError();
 break;
   case Builtin::BI__builtin_alloca_with_align:
+  case Builtin::BI__builtin_alloca_with_align_uninitialized:
 if (SemaBuiltinAllocaWithAlign(TheCall))
   return ExprError();
 LLVM_FALLTHROUGH;
   case Builtin::BI__builtin_alloca:
+  case Builtin::BI__builtin_alloca_uninitialized:
 Diag(TheCall->getBeginLoc(), diag::warn_alloca)
 << TheCall->getDirectCallee();
 break;

diff  --git a/clang/test/CodeGenCXX/trivial-auto-var-init.cpp 
b/clang/test/CodeGenCXX/trivial-auto-var-init.cpp
index 513222cb3f1d1..58fba5d96b577 100644
--- a/clang/test/CodeGenCXX/trivial-auto-var-init.cpp
+++ b/clang/test/CodeGenCXX/trivial-auto-v

[clang] [ubsan] Suppression by type for `-fsanitize=enum` (PR #114754)

[Marco Elver via cfe-commits]

https://github.com/melver approved this pull request.


https://github.com/llvm/llvm-project/pull/114754
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement labelled type filtering for overflow/truncation sanitizers w/ SSCLs (PR #107332)

[Marco Elver via cfe-commits]

melver wrote:

> > I wished that we could just attach attributes to type, e.g. `typedef int 
> > __attribute__((no_sanitize("signed-integer-overflow")) wrapping_int` or 
> > something. One thing here is that this should _not_ be a type modifier 
> > (like volatile and such), so it does not change the type, but that means 
> > nothing enforces it's propagated through conversions. But I suppose that 
> > the filter list has the same problem as you depend on specific type names.
> 
> I have the implementation done for this and I should get the PR up soon.
> 
> Then we'll have something like this:
> 
> ```
> typedef int __attribute__((wraps)) wrapping_int
> ```
> 
> I'll be sure to CC you 😄

Oh nice. If you have that almost done, why do you still want the filterlist? 
For the Linux kernel usecase, wouldn't the attribute be preferrable? My 
intuition tells me that maintaining a filterlist detached from the code will 
cause headaches, esp. during refactors or code moves (and you don't want to be 
the one chasing after the breakages).

https://github.com/llvm/llvm-project/pull/107332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement labelled type filtering for overflow/truncation sanitizers w/ SSCLs (PR #107332)

[Marco Elver via cfe-commits]

melver wrote:

> We could make custom types that are filtered out in an ignorelist, allowing 
> for types to be more expressive -- without the need for annotations

Having this sort of semantic information detached from the code may cause some 
maintenance headaches. For example, if the type is renamed, developers have to 
remember to adjust the filter list as well.

I wished that we could just attach attributes to type, e.g. `typedef int 
__attribute__((no_sanitize("signed-integer-overflow")) wrapping_int` or 
something. One thing here is that this should _not_ be a type modifier (like 
volatile and such), so it does not change the type, but that means nothing 
enforces it's propagated through conversions. But I suppose that the filter 
list has the same problem as you depend on specific type names.

Just something to consider - I suspect it's much more complex to implement in 
Clang though. So that's a +1 for the filterlist in terms of complexity.

https://github.com/llvm/llvm-project/pull/107332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement labelled type filtering for overflow/truncation sanitizers w/ SSCLs (PR #107332)

[Marco Elver via cfe-commits]

@@ -15,8 +15,9 @@ file at compile-time.
 Goal and usage
 ==
 
-Users of sanitizer tools, such as :doc:`AddressSanitizer`, 
:doc:`ThreadSanitizer`
-or :doc:`MemorySanitizer` may want to disable or alter some checks for
+Users of sanitizer tools, such as :doc:`AddressSanitizer`,
+:doc:`ThreadSanitizer`, :doc:`MemorySanitizer` or :doc:
+`UndefinedBehaviorSanitizer` may want to disable or alter some checks for

melver wrote:

This newline adds a space and the link is invalid.

https://github.com/llvm/llvm-project/pull/107332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement labelled type filtering for overflow/truncation sanitizers w/ SSCLs (PR #107332)

[Marco Elver via cfe-commits]

https://github.com/melver approved this pull request.


https://github.com/llvm/llvm-project/pull/107332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] add wraps and no_wraps attributes (PR #115094)

[Marco Elver via cfe-commits]

https://github.com/melver requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/115094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] add wraps and no_wraps attributes (PR #115094)

[Marco Elver via cfe-commits]

@@ -8710,3 +8710,103 @@ Declares that a function potentially allocates heap 
memory, and prevents any pot
 of ``nonallocating`` by the compiler.
   }];
 }
+
+def WrapsDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+The ``wraps`` attribute can be used with type or variable declarations to
+denote that arithmetic containing attributed types or variables have defined
+overflow behavior. Specifically, the behavior is defined as being consistent
+with two's complement wrap-around. For the purposes of sanitizers or warnings
+that concern themselves with the definedness of integer arithmetic, they will
+cease to instrument or warn about arithmetic that directly involves operands
+attributed with the ``wraps`` attribute.
+
+The ``signed-integer-overflow``, ``unsigned-integer-overflow``,
+``implicit-signed-integer-truncation`` and the
+``implicit-unsigned-integer-truncation`` sanitizers will not instrument
+arithmetic containing any operands attributed by ``wraps``. Similarly, the
+``-Winteger-overflow`` warning is disabled for these instances.
+
+The following example shows how one may disable ``signed-integer-overflow``
+sanitizer instrumentation using ``__attribute__((wraps))`` on a type definition
+when building with ``-fsanitize=signed-integer-overflow``:
+
+.. code-block:: c
+
+  typedef int __attribute__((wraps)) wrapping_int;
+
+  void foo(void) {
+wrapping_int A = INT_MAX;
+++A; // no sanitizer instrumentation
+  }
+
+``wraps`` may also be used with function parameters or declarations of
+variables as well as members of structures. Using ``wraps`` on non-integer
+types will result in a ``-Wuseless-wraps-attribute`` warning. One may disable
+this warning with ``-Wno-useless-wraps-attribute``.
+
+``wraps`` persists through implicit type promotions and will be applied to the
+result type of arithmetic expressions containing a wrapping operand.
+``-Wimplicitly-discarded-wraps-attribute`` warnings can be caused in situations
+where the ``wraps`` attribute cannot persist through implicit type conversions.
+Disable this with ``-Wno-implicitly-discarded-wraps-attribute``.
+}];
+}
+
+def NoWrapsDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+The ``no_wraps`` attribute can be used to annotate types or variables as
+non-wrapping. This may serve as a helpful annotation to readers of code that
+particular arithmetic expressions involving these types or variables are not
+meant to wrap-around.
+
+When overflow or truncation sanitizer instrumentation is modified at the
+type-level through `SSCLs
+`_, ``no_wraps`` or
+``wraps`` may be used to override sanitizer behavior.
+
+For example, one may specify an ignorelist (with ``-fsanitize-ignorelist=``) to
+disable the ``signed-integer-overflow`` sanitizer for all types:
+
+.. code-block:: text
+
+  [signed-integer-overflow]
+  type:*
+
+``no_wraps`` can override the behavior provided by the ignorelist to
+effectively re-enable instrumentation for specific types or variables.
+
+.. code-block:: c
+
+  typedef int __attribute__((no_wraps)) non_wrapping_int;
+
+  void foo(non_wrapping_int A, int B) {
+  ++A; // will be instrumented if built with 
-fsanitize=signed-integer-overflow
+  ++B; // won't be instrumented as it is ignored by the ignorelist
+  }
+
+Like ``wraps``, ``no_wraps`` persists through implicit type promotions and will
+be automatically applied to the result type of arithmetic expressions
+containing a wrapping operand.
+
+If a type or variable is attributed by both ``wraps`` and ``no_wraps``, then

melver wrote:

Is there a requirement for this somewhere that these can be mixed?

It would seem more intuitive to me to produce a warning if both are applied and 
effectively pretend neither attribute is specified because whoever applied the 
attributes is confused.

https://github.com/llvm/llvm-project/pull/115094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] add wraps and no_wraps attributes (PR #115094)

[Marco Elver via cfe-commits]

@@ -8710,3 +8710,103 @@ Declares that a function potentially allocates heap 
memory, and prevents any pot
 of ``nonallocating`` by the compiler.
   }];
 }
+
+def WrapsDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+The ``wraps`` attribute can be used with type or variable declarations to
+denote that arithmetic containing attributed types or variables have defined
+overflow behavior. Specifically, the behavior is defined as being consistent
+with two's complement wrap-around. For the purposes of sanitizers or warnings
+that concern themselves with the definedness of integer arithmetic, they will
+cease to instrument or warn about arithmetic that directly involves operands
+attributed with the ``wraps`` attribute.
+
+The ``signed-integer-overflow``, ``unsigned-integer-overflow``,
+``implicit-signed-integer-truncation`` and the
+``implicit-unsigned-integer-truncation`` sanitizers will not instrument
+arithmetic containing any operands attributed by ``wraps``. Similarly, the
+``-Winteger-overflow`` warning is disabled for these instances.
+
+The following example shows how one may disable ``signed-integer-overflow``
+sanitizer instrumentation using ``__attribute__((wraps))`` on a type definition
+when building with ``-fsanitize=signed-integer-overflow``:
+
+.. code-block:: c
+
+  typedef int __attribute__((wraps)) wrapping_int;
+
+  void foo(void) {
+wrapping_int A = INT_MAX;
+++A; // no sanitizer instrumentation
+  }
+
+``wraps`` may also be used with function parameters or declarations of
+variables as well as members of structures. Using ``wraps`` on non-integer
+types will result in a ``-Wuseless-wraps-attribute`` warning. One may disable
+this warning with ``-Wno-useless-wraps-attribute``.
+
+``wraps`` persists through implicit type promotions and will be applied to the
+result type of arithmetic expressions containing a wrapping operand.
+``-Wimplicitly-discarded-wraps-attribute`` warnings can be caused in situations
+where the ``wraps`` attribute cannot persist through implicit type conversions.
+Disable this with ``-Wno-implicitly-discarded-wraps-attribute``.
+}];
+}
+
+def NoWrapsDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+The ``no_wraps`` attribute can be used to annotate types or variables as
+non-wrapping. This may serve as a helpful annotation to readers of code that
+particular arithmetic expressions involving these types or variables are not
+meant to wrap-around.
+
+When overflow or truncation sanitizer instrumentation is modified at the
+type-level through `SSCLs
+`_, ``no_wraps`` or
+``wraps`` may be used to override sanitizer behavior.
+
+For example, one may specify an ignorelist (with ``-fsanitize-ignorelist=``) to
+disable the ``signed-integer-overflow`` sanitizer for all types:
+
+.. code-block:: text
+
+  [signed-integer-overflow]
+  type:*
+
+``no_wraps`` can override the behavior provided by the ignorelist to
+effectively re-enable instrumentation for specific types or variables.
+
+.. code-block:: c
+
+  typedef int __attribute__((no_wraps)) non_wrapping_int;
+
+  void foo(non_wrapping_int A, int B) {
+  ++A; // will be instrumented if built with 
-fsanitize=signed-integer-overflow
+  ++B; // won't be instrumented as it is ignored by the ignorelist
+  }
+
+Like ``wraps``, ``no_wraps`` persists through implicit type promotions and will
+be automatically applied to the result type of arithmetic expressions
+containing a wrapping operand.
+
+If a type or variable is attributed by both ``wraps`` and ``no_wraps``, then
+``no_wraps`` takes precedence -- regardless of the order of attribution.
+
+Note that ``no_wraps`` makes no guarantees about the definedness of arithmetic

melver wrote:

This sentence is confusing. Is that implied that if I add `no_wraps`, that 
overflow is now UB? Why does fwrapv help here?

In general, I think there need to be more explicit mentions of -fwrapv and how 
these attribtues interact with these flags - afaik the attributes give us 
selective fwrapv and fno-wrapv respectively.

https://github.com/llvm/llvm-project/pull/115094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] add wraps and no_wraps attributes (PR #115094)

[Marco Elver via cfe-commits]

@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu
+typedef int __attribute__((wraps)) wrapping_int;

melver wrote:

Can we test more types?

Can the attributes be applied to pointers? Both the pointer itself and the 
pointee? It's worth adding tests, just to ensure nothing regresses here.

https://github.com/llvm/llvm-project/pull/115094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] add wraps and no_wraps attributes (PR #115094)

[Marco Elver via cfe-commits]

@@ -433,6 +433,26 @@ Attribute Changes in Clang
 - Fix a bug where clang doesn't automatically apply the ``[[gsl::Owner]]`` or
   ``[[gsl::Pointer]]`` to STL explicit template specialization decls. 
(#GH109442)
 
+- Introduced ``__attribute__((wraps))`` which can be added to type or variable
+  declarations. Using an attributed type or variable in an arithmetic
+  expression will define the overflow behavior for that expression as having
+  two's complement wrap-around. These expressions will not be instrumented by
+  overflow sanitizers nor will they cause integer overflow warnings. They also
+  cannot be optimized away by some eager UB optimizations as the behavior of
+  the arithmetic is no longer "undefined".
+
+  There is also ``__attribute__((no_wraps))`` which can be added to types or
+  variable declarations. Types or variables with this attribute may be
+  instrumented by overflow sanitizers, if enabled. Note that this matches the
+  default behavior of integer types. So, in most cases, ``no_wraps`` serves
+  purely as an annotation to readers of code that a type or variable really
+  shouldn't wrap-around. ``__attribute__((no_wraps))`` has the most function
+  when paired with `Sanitizer Special Case Lists (SSCL)
+  `_.
+
+  These attributes are only valid for C, as there are built-in language

melver wrote:

If the attribute is used with C++ code will it break the build?
I.e. if someone includes a C header into a C++ project that uses these 
attributes, are we going to run into problems? If yes, this doesn't sound right 
to me.

https://github.com/llvm/llvm-project/pull/115094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] add wraps and no_wraps attributes (PR #115094)

[Marco Elver via cfe-commits]

@@ -433,6 +433,26 @@ Attribute Changes in Clang
 - Fix a bug where clang doesn't automatically apply the ``[[gsl::Owner]]`` or
   ``[[gsl::Pointer]]`` to STL explicit template specialization decls. 
(#GH109442)
 
+- Introduced ``__attribute__((wraps))`` which can be added to type or variable
+  declarations. Using an attributed type or variable in an arithmetic
+  expression will define the overflow behavior for that expression as having
+  two's complement wrap-around. These expressions will not be instrumented by
+  overflow sanitizers nor will they cause integer overflow warnings. They also
+  cannot be optimized away by some eager UB optimizations as the behavior of
+  the arithmetic is no longer "undefined".
+
+  There is also ``__attribute__((no_wraps))`` which can be added to types or
+  variable declarations. Types or variables with this attribute may be
+  instrumented by overflow sanitizers, if enabled. Note that this matches the
+  default behavior of integer types. So, in most cases, ``no_wraps`` serves
+  purely as an annotation to readers of code that a type or variable really
+  shouldn't wrap-around. ``__attribute__((no_wraps))`` has the most function
+  when paired with `Sanitizer Special Case Lists (SSCL)
+  `_.
+
+  These attributes are only valid for C, as there are built-in language

melver wrote:

My view is that the semantics of C code should, where possible, not change when 
compiling as C++. This also holds for your type-based proposal.

https://github.com/llvm/llvm-project/pull/115094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] add wraps and no_wraps attributes (PR #115094)

[Marco Elver via cfe-commits]

melver wrote:

> RFC regarding canonical wrapping/non-wrapping types in Clang: 
> https://discourse.llvm.org/t/rfc-clang-canonical-wrapping-and-non-wrapping-types/84356
> 
> Ultimately, a type like what the RFC describes would supersede this PR in 
> terms of feature completeness and usefulness. I'll close this PR if that RFC 
> suggests a new direction is required.
> 
> pings: @erichkeane pinging you because you asked about an RFC.

+1 - I'd be in favor of the type-based approach. It does make more sense that 
this is a type qualifier, vs. an attribute which the compiler might loose as 
soon as type conversions or take-pointer operations are involved.

https://github.com/llvm/llvm-project/pull/115094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

FWIW, the Linux kernel integration (draft, WIP) currently lives here: 
https://github.com/google/kernel-sanitizers/tree/cap-analysis
It currently enables -Wthread-safety-addressof if available. Thus far, I have 
not found any false positives due to -Wthread-safety-addressof in the 2 
subsystems I converted over (more to follow).

And I want to re-iterate that without -Wthread-safety-addressof, the feature's 
coverage is significantly reduced, and I predict it to be one of the first 
complaints in later review.

Kindly take another look.

cc @bvanassche

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

> Doesn't `counted_by` have the same requirements? If not, why does guarded_by?

It does, AFAIK.

> Sure, I can imagine where there might be pushback, but perhaps its worthwhile 
> to see how far you can get with either marking structs that don't need 
> `-fexperimental-late-parse-attributes`, or propose such member declaration 
> re-orderings. At least that would let you land something upstream and begin 
> protecting the kernel/finding bugs.

Fair point, in the worst case individual TUs could still add the flag if needed.

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

> Looks like the kernel patches 
> [use](https://github.com/google/kernel-sanitizers/commit/81883e1fa377d866c288192b6eb8334bcf10f38f)
>  `-fexperimental-late-parse-attributes`? :(

Yeah, no way around it if we want to have guarded_by in structs work properly. 
If the problem is cost, still planning to allow it to be disabled via Kconfig 
option.

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

> Can any of the members in the structs be reorganized to put the mutex member 
> declaration BEFORE the members they guard? Probably not always, but perhaps 
> that's possible for most structures?

It's an option I considered, but I can already hear "what is this crap ... 
NACK". In many cases it might be possible, but where data layout or cacheline 
organization is important for performance, definitely not an option.

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on taking address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/123063

>From 6727047d25b8b72f8e23b03a84c0b23f6dad566a Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 15 Jan 2025 13:23:14 +0100
Subject: [PATCH 1/3] Thread Safety Analysis: Support warning on obtaining
 address of guarded variables

Add the optional ability, via `-Wthread-safety-addressof`, to warn when
obtaining the address of guarded variables.

This is required to avoid false negatives in large C codebases, where
data structures are typically implemented through helpers that take
pointers to instances of a data structure.

We also argue that, while obtaining the address itself does not yet
constitute a potential race (in the presence of missing locking),
placing the requirement on the pointer-recipient to obtain locks to
access the pointed-to data is most likely poor style. This is analogous
to passing C++ references to guarded variables, which produces warnings
by default.

Given that existing codebases using `-Wthread-safety` likely have cases
where obtaining the pointer to a guarded variable is benign, the feature
is not enabled by default but requires explicit opt-in.
---
 clang/docs/ReleaseNotes.rst   |  3 ++
 clang/docs/ThreadSafetyAnalysis.rst   | 12 -
 .../clang/Analysis/Analyses/ThreadSafety.h|  3 ++
 clang/include/clang/Basic/DiagnosticGroups.td |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td|  8 
 clang/lib/Analysis/ThreadSafety.cpp   |  3 ++
 clang/lib/Sema/AnalysisBasedWarnings.cpp  | 26 ---
 clang/test/Sema/warn-thread-safety-analysis.c | 11 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 44 ++-
 9 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 794943b24a003c..abcf0ef236cc2e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -788,6 +788,9 @@ Improvements to Clang's diagnostics
   require(scope); // Warning!  Requires mu1.
 }
 
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-addressof``,
+  which enables warning if the address of guarded variables is obtained.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 9c1c32e46989bc..c506844e7c94dc 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -515,8 +515,16 @@ Warning flags
   + ``-Wthread-safety-analysis``: The core analysis.
   + ``-Wthread-safety-precise``: Requires that mutex expressions match 
precisely.
This warning can be disabled for code which has a lot of aliases.
-  + ``-Wthread-safety-reference``: Checks when guarded members are passed by 
reference.
-
+  + ``-Wthread-safety-reference``: Checks when guarded variables are passed by 
reference.
+
+* ``-Wthread-safety-addressof``: Warn when the address of guarded variables is
+  obtained (``&var``). Since obtaining the address of a variable doesn't
+  necessarily imply a read or write, the warning is off by default. In
+  codebases that prefer passing pointers rather than references (for C++
+  codebases), or passing pointers is ubiquitous (for C codebases), enabling
+  this warning will result in fewer false negatives; for example, where the
+  manipulation of common data structures is done via functions that take
+  pointers to instances of the data structure.
 
 :ref:`negative` are an experimental feature, which are enabled with:
 
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0fcf5bed1285a4..a61c60cbf531e9 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -54,6 +54,9 @@ enum ProtectedOperationKind {
 
   /// Returning a pt-guarded variable by reference.
   POK_PtReturnByRef,
+
+  /// Obtaining address of a variable (e.g. &x).
+  POK_AddressOf,
 };
 
 /// This enum distinguishes between different kinds of lock actions. For
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 594e99a19b64d6..8bd5d043cefa80 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1126,6 +1126,7 @@ def ThreadSafetyReferenceReturn  : 
DiagGroup<"thread-safety-reference-return">;
 def ThreadSafetyReference  : DiagGroup<"thread-safety-reference",
  [ThreadSafetyReferenceReturn]>;
 def ThreadSafetyNegative   : DiagGroup<"thread-safety-negative">;
+def ThreadSafetyAddressof  : DiagGroup<"thread-safety-addressof">;
 def ThreadSafety : DiagGroup<"thread-safety",
  [ThreadSafetyAttributes,
   ThreadSafetyAnalysis,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKin

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

> We had a discussion on https://reviews.llvm.org/D52395 that might be relevant 
> here. To quote @delesley:
> 
> > When you pass an object to a function, either by pointer or by reference, 
> > no actual load from memory has yet occurred. Thus, there is a real risk of 
> > false positives; what you are saying is that the function _might_ read or 
> > write from protected memory, not that it actually will.

Right. Though in the absence of better pointer tracking / alias analysis, I 
believe there's no way to avoid resulting false positives with pointers. It's 
something that a user of Wthread-safety-addressof would opt-in explicitly - 
documentation needs elaboration on this perhaps.

> Another aspect is that we don't check if the lock is still held when we 
> dereference the pointer, so there are also false negatives:
> 
> ```c++
> Mutex mu;
> int x GUARDED_BY(mu);
> 
> void f() {
>   mu.Lock();
>   int *p = &x;
>   mu.Unlock();
>   *p = 0;
> }
> ```

Good point - though I'd prefer few false negatives + false positives over no 
checks at all. It's a tradeoff, and therefore we definitely shouldn't enable 
Wthread-safety-addressof by default.

> You use the analogy of C++ references. Interestingly, we don't warn on 
> "taking a reference", partly because no such thing exists. (You can only 
> initialize something of reference type with an expression.) We warn on 
> passing a variable by reference _to a function_, or in other words, 
> initializing a parameter of reference type with a guarded variable. (Since 
> https://reviews.llvm.org/D153131, we also warn on returning a reference when 
> the lock is no longer held after return. Note that the initialization of the 
> reference might still happen under lock!)
> 
> However, we also track references in a way that pointers are not tracked. The 
> reason is probably that references are immutable (that is, `T&` is 
> semantically the same thing as `T* const`).
> 
> ```c++
> void g() {
>   mu.Lock();
>   int &ref = x;
>   mu.Unlock();
>   ref = 0;  // warning: writing variable 'x' requires holding mutex 'mu' 
> exclusively
> }
> ```
> 
> If you "take the reference" outside of the lock and do the assignment inside, 
> there is no warning. Because the assignment is what needs the lock, not 
> taking the address.

Good points, and I missed that reference handling is much better in this regard.

> However, with functions it's somewhat reasonable to assume that the function 
> accesses through the pointer, and that the pointer doesn't escape. (This is 
> of course not always true, but often enough.) So I wonder whether we should 
> restrict this to pointers passed as function arguments.

That's a reasonable option to make it more conservative, although I'm not sure 
it's necessary (yet).

> But if the number of false positives is manageable, we can also leave it like 
> you implemented it. Did you try it out on some code base that uses thread 
> safety annotations?

I'm working on bringing Wthread-safety to the Linux kernel. The strategy chosen 
is to convert one subsystem (or single TU) at a time, based on an opt-in basis. 
Since Linux already has a rather special dialect of C, those that would choose 
to opt in their TUs would opt into another variant of Linux's C dialect (one 
with Wthread-safety enabled).

My experience in converting a subsystem I maintain is that applying 
Wthread-safety already requires small refactors to avoid false positives, and 
I'm not opposed to add additional constraints via Wthread-safety-addressof. 
Without additional checking of "obtaining address of guarded variables", I find 
that Wthread-safety barely provides benefits, as passing pointers to 
datastructures is so common.

Resolving this will improve the usefulness, and ultimately the chances of us 
succeeding to upstream this to the Linux kernel.

I don't think we will ever convert 100% of the kernel to use Wthread-safety, 
but the plan is that willing subsystem maintainers opt in and deal with 
warnings produced by Wthread-safety. Having Wthread-safety-addressof will add 
additional coverage. For cases where it produces too many false positives, I 
intend to add a simple option that can enable/disable addressof checking for 
individual TUs. Or if almost all places where obtaining the address of a 
particular lock-guarded variable is _not_ actually under a lock (but still 
correct), leaving off `guarded_by` might simply be the right choice.

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/123063

>From 6727047d25b8b72f8e23b03a84c0b23f6dad566a Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 15 Jan 2025 13:23:14 +0100
Subject: [PATCH 1/3] Thread Safety Analysis: Support warning on obtaining
 address of guarded variables

Add the optional ability, via `-Wthread-safety-addressof`, to warn when
obtaining the address of guarded variables.

This is required to avoid false negatives in large C codebases, where
data structures are typically implemented through helpers that take
pointers to instances of a data structure.

We also argue that, while obtaining the address itself does not yet
constitute a potential race (in the presence of missing locking),
placing the requirement on the pointer-recipient to obtain locks to
access the pointed-to data is most likely poor style. This is analogous
to passing C++ references to guarded variables, which produces warnings
by default.

Given that existing codebases using `-Wthread-safety` likely have cases
where obtaining the pointer to a guarded variable is benign, the feature
is not enabled by default but requires explicit opt-in.
---
 clang/docs/ReleaseNotes.rst   |  3 ++
 clang/docs/ThreadSafetyAnalysis.rst   | 12 -
 .../clang/Analysis/Analyses/ThreadSafety.h|  3 ++
 clang/include/clang/Basic/DiagnosticGroups.td |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td|  8 
 clang/lib/Analysis/ThreadSafety.cpp   |  3 ++
 clang/lib/Sema/AnalysisBasedWarnings.cpp  | 26 ---
 clang/test/Sema/warn-thread-safety-analysis.c | 11 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 44 ++-
 9 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 794943b24a003c..abcf0ef236cc2e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -788,6 +788,9 @@ Improvements to Clang's diagnostics
   require(scope); // Warning!  Requires mu1.
 }
 
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-addressof``,
+  which enables warning if the address of guarded variables is obtained.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 9c1c32e46989bc..c506844e7c94dc 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -515,8 +515,16 @@ Warning flags
   + ``-Wthread-safety-analysis``: The core analysis.
   + ``-Wthread-safety-precise``: Requires that mutex expressions match 
precisely.
This warning can be disabled for code which has a lot of aliases.
-  + ``-Wthread-safety-reference``: Checks when guarded members are passed by 
reference.
-
+  + ``-Wthread-safety-reference``: Checks when guarded variables are passed by 
reference.
+
+* ``-Wthread-safety-addressof``: Warn when the address of guarded variables is
+  obtained (``&var``). Since obtaining the address of a variable doesn't
+  necessarily imply a read or write, the warning is off by default. In
+  codebases that prefer passing pointers rather than references (for C++
+  codebases), or passing pointers is ubiquitous (for C codebases), enabling
+  this warning will result in fewer false negatives; for example, where the
+  manipulation of common data structures is done via functions that take
+  pointers to instances of the data structure.
 
 :ref:`negative` are an experimental feature, which are enabled with:
 
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0fcf5bed1285a4..a61c60cbf531e9 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -54,6 +54,9 @@ enum ProtectedOperationKind {
 
   /// Returning a pt-guarded variable by reference.
   POK_PtReturnByRef,
+
+  /// Obtaining address of a variable (e.g. &x).
+  POK_AddressOf,
 };
 
 /// This enum distinguishes between different kinds of lock actions. For
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 594e99a19b64d6..8bd5d043cefa80 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1126,6 +1126,7 @@ def ThreadSafetyReferenceReturn  : 
DiagGroup<"thread-safety-reference-return">;
 def ThreadSafetyReference  : DiagGroup<"thread-safety-reference",
  [ThreadSafetyReferenceReturn]>;
 def ThreadSafetyNegative   : DiagGroup<"thread-safety-negative">;
+def ThreadSafetyAddressof  : DiagGroup<"thread-safety-addressof">;
 def ThreadSafety : DiagGroup<"thread-safety",
  [ThreadSafetyAttributes,
   ThreadSafetyAnalysis,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKin

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

For additional background: I'm trying to work out how to bring -Wthread-safety 
to the Linux kernel. Since -fexperimental-late-parse-attributes made the 
feature practical for complex C structs, I started to look at a strategy to 
enable the feature. 

The current strategy is to create an opt-in mechanism (per subsystem), and 
since the constraints imposed by Wthread-safety already require subtly 
restructuring most code that would opt-in, I think having 
Wthread-safety-addressof part of the initial constraints imposed by 
Wthread-safety would be very helpful.

In converting one subsystem I maintain, I was disappointed that things like 
`list_add(¬e, &guarded_list)` aren't caught. In general, I believe that the 
situation is analogous to C++ reference passing, and thus 
-Wthread-safety-addressof would be the right choice for new C projects.

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] thread-safety: Support the new capability-based names for all related attributes. (PR #99919)

[Marco Elver via cfe-commits]

melver wrote:

Was this dropped? It'd be nice to see this change land.

https://github.com/llvm/llvm-project/pull/99919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver created 
https://github.com/llvm/llvm-project/pull/123063

Add the optional ability, via `-Wthread-safety-addressof`, to warn when 
obtaining the address of guarded variables.

This is required to avoid false negatives in large C codebases, where data 
structures are typically implemented through helpers that take pointers to 
instances of a data structure.

We also argue that, while obtaining the address itself does not yet constitute 
a potential race (in the presence of missing locking), placing the requirement 
on the pointer-recipient to obtain locks to access the pointed-to data is most 
likely poor style. This is analogous to passing C++ references to guarded 
variables, which produces warnings by default.

Given that existing codebases using `-Wthread-safety` likely have cases where 
obtaining the pointer to a guarded variable is benign, the feature is not 
enabled by default but requires explicit opt-in.

>From 60e3b1d7c2dd7f9c070b25c25ff546adb1a68143 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 15 Jan 2025 13:23:14 +0100
Subject: [PATCH] Thread Safety Analysis: Support warning on obtaining address
 of guarded variables

Add the optional ability, via `-Wthread-safety-addressof`, to warn when
obtaining the address of guarded variables.

This is required to avoid false negatives in large C codebases, where
data structures are typically implemented through helpers that take
pointers to instances of a data structure.

We also argue that, while obtaining the address itself does not yet
constitute a potential race (in the presence of missing locking),
placing the requirement on the pointer-recipient to obtain locks to
access the pointed-to data is most likely poor style. This is analogous
to passing C++ references to guarded variables, which produces warnings
by default.

Given that existing codebases using `-Wthread-safety` likely have cases
where obtaining the pointer to a guarded variable is benign, the feature
is not enabled by default but requires explicit opt-in.
---
 clang/docs/ReleaseNotes.rst   |  3 ++
 clang/docs/ThreadSafetyAnalysis.rst   | 12 -
 .../clang/Analysis/Analyses/ThreadSafety.h|  3 ++
 clang/include/clang/Basic/DiagnosticGroups.td |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td|  8 
 clang/lib/Analysis/ThreadSafety.cpp   |  3 ++
 clang/lib/Sema/AnalysisBasedWarnings.cpp  | 26 ---
 clang/test/Sema/warn-thread-safety-analysis.c | 11 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 44 ++-
 9 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 794943b24a003c..abcf0ef236cc2e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -788,6 +788,9 @@ Improvements to Clang's diagnostics
   require(scope); // Warning!  Requires mu1.
 }
 
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-addressof``,
+  which enables warning if the address of guarded variables is obtained.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 9c1c32e46989bc..c506844e7c94dc 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -515,8 +515,16 @@ Warning flags
   + ``-Wthread-safety-analysis``: The core analysis.
   + ``-Wthread-safety-precise``: Requires that mutex expressions match 
precisely.
This warning can be disabled for code which has a lot of aliases.
-  + ``-Wthread-safety-reference``: Checks when guarded members are passed by 
reference.
-
+  + ``-Wthread-safety-reference``: Checks when guarded variables are passed by 
reference.
+
+* ``-Wthread-safety-addressof``: Warn when the address of guarded variables is
+  obtained (``&var``). Since obtaining the address of a variable doesn't
+  necessarily imply a read or write, the warning is off by default. In
+  codebases that prefer passing pointers rather than references (for C++
+  codebases), or passing pointers is ubiquitous (for C codebases), enabling
+  this warning will result in fewer false negatives; for example, where the
+  manipulation of common data structures is done via functions that take
+  pointers to instances of the data structure.
 
 :ref:`negative` are an experimental feature, which are enabled with:
 
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0fcf5bed1285a4..a61c60cbf531e9 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -54,6 +54,9 @@ enum ProtectedOperationKind {
 
   /// Returning a pt-guarded variable by reference.
   POK_PtReturnByRef,
+
+  /// Obtaining address of a variable (e.g. &x).
+  POK_AddressOf,
 };
 
 /// This enum distinguishes between different kinds of loc

[clang] Thread Safety Analysis: Support warning on obtaining address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/123063

>From 6727047d25b8b72f8e23b03a84c0b23f6dad566a Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 15 Jan 2025 13:23:14 +0100
Subject: [PATCH] Thread Safety Analysis: Support warning on obtaining address
 of guarded variables

Add the optional ability, via `-Wthread-safety-addressof`, to warn when
obtaining the address of guarded variables.

This is required to avoid false negatives in large C codebases, where
data structures are typically implemented through helpers that take
pointers to instances of a data structure.

We also argue that, while obtaining the address itself does not yet
constitute a potential race (in the presence of missing locking),
placing the requirement on the pointer-recipient to obtain locks to
access the pointed-to data is most likely poor style. This is analogous
to passing C++ references to guarded variables, which produces warnings
by default.

Given that existing codebases using `-Wthread-safety` likely have cases
where obtaining the pointer to a guarded variable is benign, the feature
is not enabled by default but requires explicit opt-in.
---
 clang/docs/ReleaseNotes.rst   |  3 ++
 clang/docs/ThreadSafetyAnalysis.rst   | 12 -
 .../clang/Analysis/Analyses/ThreadSafety.h|  3 ++
 clang/include/clang/Basic/DiagnosticGroups.td |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td|  8 
 clang/lib/Analysis/ThreadSafety.cpp   |  3 ++
 clang/lib/Sema/AnalysisBasedWarnings.cpp  | 26 ---
 clang/test/Sema/warn-thread-safety-analysis.c | 11 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 44 ++-
 9 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 794943b24a003c..abcf0ef236cc2e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -788,6 +788,9 @@ Improvements to Clang's diagnostics
   require(scope); // Warning!  Requires mu1.
 }
 
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-addressof``,
+  which enables warning if the address of guarded variables is obtained.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 9c1c32e46989bc..c506844e7c94dc 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -515,8 +515,16 @@ Warning flags
   + ``-Wthread-safety-analysis``: The core analysis.
   + ``-Wthread-safety-precise``: Requires that mutex expressions match 
precisely.
This warning can be disabled for code which has a lot of aliases.
-  + ``-Wthread-safety-reference``: Checks when guarded members are passed by 
reference.
-
+  + ``-Wthread-safety-reference``: Checks when guarded variables are passed by 
reference.
+
+* ``-Wthread-safety-addressof``: Warn when the address of guarded variables is
+  obtained (``&var``). Since obtaining the address of a variable doesn't
+  necessarily imply a read or write, the warning is off by default. In
+  codebases that prefer passing pointers rather than references (for C++
+  codebases), or passing pointers is ubiquitous (for C codebases), enabling
+  this warning will result in fewer false negatives; for example, where the
+  manipulation of common data structures is done via functions that take
+  pointers to instances of the data structure.
 
 :ref:`negative` are an experimental feature, which are enabled with:
 
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0fcf5bed1285a4..a61c60cbf531e9 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -54,6 +54,9 @@ enum ProtectedOperationKind {
 
   /// Returning a pt-guarded variable by reference.
   POK_PtReturnByRef,
+
+  /// Obtaining address of a variable (e.g. &x).
+  POK_AddressOf,
 };
 
 /// This enum distinguishes between different kinds of lock actions. For
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 594e99a19b64d6..8bd5d043cefa80 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1126,6 +1126,7 @@ def ThreadSafetyReferenceReturn  : 
DiagGroup<"thread-safety-reference-return">;
 def ThreadSafetyReference  : DiagGroup<"thread-safety-reference",
  [ThreadSafetyReferenceReturn]>;
 def ThreadSafetyNegative   : DiagGroup<"thread-safety-negative">;
+def ThreadSafetyAddressof  : DiagGroup<"thread-safety-addressof">;
 def ThreadSafety : DiagGroup<"thread-safety",
  [ThreadSafetyAttributes,
   ThreadSafetyAnalysis,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.t

[clang] Thread Safety Analysis: Support warning on passing/returning pointers to guarded variables (PR #127396)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/127396

>From a70f021becb2888d6c2a63b2d1e619393a996058 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Sun, 16 Feb 2025 14:53:41 +0100
Subject: [PATCH 1/2] Thread Safety Analysis: Handle address-of followed by
 dereference

Correctly analyze expressions where the address of a guarded variable is
taken and immediately dereferenced, such as (*(type-specifier *)&x).
Previously, such patterns would result in false negatives.
---
 clang/lib/Analysis/ThreadSafety.cpp   | 10 ++
 clang/test/Sema/warn-thread-safety-analysis.c | 13 +
 2 files changed, 23 insertions(+)

diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index bfaf1a0e7c7ff..260505b71c17c 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1765,6 +1765,8 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet 
&FSet, const Expr *Exp,
 void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
  AccessKind AK,
  ProtectedOperationKind POK) {
+  // Strips off paren- and cast-expressions, checking if we encounter any other
+  // operator that should be delegated to checkAccess() instead.
   while (true) {
 if (const auto *PE = dyn_cast(Exp)) {
   Exp = PE->getSubExpr();
@@ -1780,6 +1782,14 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet 
&FSet, const Expr *Exp,
   Exp = CE->getSubExpr();
   continue;
 }
+if (const auto *UO = dyn_cast(Exp)) {
+  if (UO->getOpcode() == UO_AddrOf) {
+// Pointer access via pointer taken of variable, so the dereferenced
+// variable is not actually a pointer.
+checkAccess(FSet, UO->getSubExpr(), AK, POK);
+return;
+  }
+}
 break;
   }
 
diff --git a/clang/test/Sema/warn-thread-safety-analysis.c 
b/clang/test/Sema/warn-thread-safety-analysis.c
index 73b4e4798f644..8a0d44cd4bea9 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -24,6 +24,9 @@
   __attribute__ ((shared_locks_required(__VA_ARGS__)))
 #define NO_THREAD_SAFETY_ANALYSIS  __attribute__ ((no_thread_safety_analysis))
 
+#define __READ_ONCE(x)(*(const volatile __typeof__(x) *)&(x))
+#define __WRITE_ONCE(x, val)  do { *(volatile __typeof__(x) *)&(x) = (val); } 
while (0)
+
 // Define the mutex struct.
 // Simplified only for test purpose.
 struct LOCKABLE Mutex {};
@@ -142,9 +145,19 @@ int main(void) {
   (void)(get_value(b_) == 1);
   mutex_unlock(foo_.mu_);
 
+  a_ = 0; // expected-warning{{writing variable 'a_' requires holding mutex 
'foo_.mu_'}}
+  __WRITE_ONCE(a_, 0); // expected-warning{{writing variable 'a_' requires 
holding mutex 'foo_.mu_'}}
+  (void)(a_ == 0); // expected-warning{{reading variable 'a_' requires holding 
mutex 'foo_.mu_'}}
+  (void)(__READ_ONCE(a_) == 0); // expected-warning{{reading variable 'a_' 
requires holding mutex 'foo_.mu_'}}
+  (void)(*b_ == 0); // expected-warning{{reading the value pointed to by 'b_' 
requires holding mutex 'foo_.mu_'}}
   c_ = 0; // expected-warning{{writing variable 'c_' requires holding any 
mutex exclusively}}
   (void)(*d_ == 0); // expected-warning{{reading the value pointed to by 'd_' 
requires holding any mutex}}
   mutex_exclusive_lock(foo_.mu_);
+  a_ = 0;
+  __WRITE_ONCE(a_, 0);
+  (void)(a_ == 0);
+  (void)(__READ_ONCE(a_) == 0);
+  (void)(*b_ == 0);
   c_ = 1;
   (void)(*d_ == 1);
   mutex_unlock(foo_.mu_);

>From 6683e2aced3c36af6b672e65bcd6779eb8664ac4 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Sun, 16 Feb 2025 12:42:06 +0100
Subject: [PATCH 2/2] Thread Safety Analysis: Support warning on
 passing/returning pointers to guarded variables

Introduce `-Wthread-safety-pointer` (under `-Wthread-safety-beta`) to
warn when passing or returning pointers to guarded variables or guarded
data. This is is analogous to `-Wthread-safety-reference`, which
performs similar checks for C++ references.

Adding checks for pointer passing is required to avoid false negatives
in large C codebases, where data structures are typically implemented
through helpers that take pointers to instances of a data structure.
---
 clang/docs/ReleaseNotes.rst   |   5 +
 clang/docs/ThreadSafetyAnalysis.rst   |   6 +-
 .../clang/Analysis/Analyses/ThreadSafety.h|  12 ++
 clang/include/clang/Basic/DiagnosticGroups.td |   4 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  18 ++
 clang/lib/Analysis/ThreadSafety.cpp   |  24 ++-
 clang/lib/Sema/AnalysisBasedWarnings.cpp  |  24 +++
 clang/test/Sema/warn-thread-safety-analysis.c |   4 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 158 +-
 9 files changed, 250 insertions(+), 5 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index efaacdf18d50a..830433853437d 

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/137133

>From d3324c1023533bf784a3c3c3ef095d07c865e6f9 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../clang/Analysis/Analyses/ThreadSafetyCommon.h   | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..6e46a2d721463 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,28 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
   CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  : CapExpr(E, Neg ? FlagNegative : 0), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
   template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind, !negative());
   }
 
   bool equals(const CapabilityExpr &other) const {

>From 89ceb0d45d7fb30885f793a19cfae7ee26dae3fc Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Thu, 24 Apr 2025 09:02:08 +0200
Subject: [PATCH 2/2] Thread Safety Analysis: Support reentrant capabilities

Introduce the `reentrant_capability` attribute, which may be specified
alongside the `capability(..)` attribute to denote that the defined
capability type is reentrant. Marking a capability as reentrant means
that acquiring the same capability multiple times is safe, and does not
produce warnings on attempted re-acquisition.

The most significant changes required are plumbing to propagate if the
attribute is present to a CapabilityExpr, and then introducing a
ReentrancyCount to FactEntry that can be incremented while a fact
remains in the FactSet.

Care was taken to avoid increasing the size of both CapabilityExpr and
FactEntry by carefully allocating free bits of CapabilityExpr::CapExpr
and the bitset respectively.
---
 clang/docs/ReleaseNotes.rst   |   1 +
 clang/docs/ThreadSafetyAnalysis.rst   |  18 +
 .../clang/Analysis/Analyses/ThreadSafety.h|   4 +
 .../Analysis/Analyses/ThreadSafetyCommon.h|  17 +-
 clang/include/clang/Basic/Attr.td |   7 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/lib/Analysis/ThreadSafety.cpp   | 127 ++--
 clang/lib/Analysis/ThreadSafetyCommon.cpp |  39 +--
 clang/lib/Sema/AnalysisBasedWarnings.cpp  |   7 +
 ...a-attribute-supported-attributes-list.test |   1 +
 clang/test/Sema/warn-thread-safety-analysis.c |  20 ++
 .../test/SemaCXX/thread-safety-annotations.h  |   1 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 308 ++
 clang/unittests/AST/ASTImporterTest.cpp   |   7 +
 14 files changed, 509 insertions(+), 51 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d1f24fb23d44d..7afa2ef359de9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -343,6 +343,7 @@ Improvements to Clang's diagnostics
   as function arguments or return value respectively. Note that
   :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
   feature will be default-enabled with ``-Wthread-safety`` in a future release.
+- The :doc:`ThreadSafetyAnalysis` now supports reentrant capabilities.
 - Clang will now do a better job producing common nested names, when producing
   common types for ternary operator, template argument deduction and multiple 
return auto deduction.
 - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) 
and minus(-) as signed integers
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 130069c5659d6..4fc7ff28e

[clang] 4bf93c0 - Thread Safety Analysis: Fix style

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2025-04-30T08:49:15+02:00
New Revision: 4bf93c098c8b04a06f228b05732d691d0ce2babc

URL: 
https://github.com/llvm/llvm-project/commit/4bf93c098c8b04a06f228b05732d691d0ce2babc
DIFF: 
https://github.com/llvm/llvm-project/commit/4bf93c098c8b04a06f228b05732d691d0ce2babc.diff

LOG: Thread Safety Analysis: Fix style

Comment fix and apply new clang-format style in
ScopedLockableFactEntry::unlock().

Factored out from https://github.com/llvm/llvm-project/pull/137133

NFC.

Added: 


Modified: 
clang/lib/Analysis/ThreadSafety.cpp

Removed: 




diff  --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index f50c2adda4bc0..7e86af6b4a317 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -119,7 +119,7 @@ class FactEntry : public CapabilityExpr {
   /// Exclusive or shared.
   LockKind LKind : 8;
 
-  // How it was acquired.
+  /// How it was acquired.
   SourceKind Source : 8;
 
   /// Where it was acquired.
@@ -1011,8 +1011,8 @@ class ScopedLockableFactEntry : public FactEntry {
   SourceLocation loc, ThreadSafetyHandler *Handler) const {
 if (FSet.findLock(FactMan, Cp)) {
   FSet.removeLock(FactMan, Cp);
-  FSet.addLock(FactMan, std::make_unique(
-!Cp, LK_Exclusive, loc));
+  FSet.addLock(FactMan,
+   std::make_unique(!Cp, LK_Exclusive, 
loc));
 } else if (Handler) {
   SourceLocation PrevLoc;
   if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))



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

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/137133

>From a8319028f08192ca6140beed7f27a83a829c6d84 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../clang/Analysis/Analyses/ThreadSafetyCommon.h   | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..6e46a2d721463 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,28 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
   CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  : CapExpr(E, Neg ? FlagNegative : 0), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
   template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind, !negative());
   }
 
   bool equals(const CapabilityExpr &other) const {

>From 427dcd2c396fbd454129abb3d3f2d572f87eadbc Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Thu, 24 Apr 2025 09:02:08 +0200
Subject: [PATCH 2/2] Thread Safety Analysis: Support reentrant capabilities

Introduce the `reentrant_capability` attribute, which may be specified
alongside the `capability(..)` attribute to denote that the defined
capability type is reentrant. Marking a capability as reentrant means
that acquiring the same capability multiple times is safe, and does not
produce warnings on attempted re-acquisition.

The most significant changes required are plumbing to propagate if the
attribute is present to a CapabilityExpr, and then introducing a
ReentrancyCount to FactEntry that can be incremented while a fact
remains in the FactSet.

Care was taken to avoid increasing the size of both CapabilityExpr and
FactEntry by carefully allocating free bits of CapabilityExpr::CapExpr
and the bitset respectively.
---
 clang/docs/ReleaseNotes.rst   |   1 +
 clang/docs/ThreadSafetyAnalysis.rst   |  18 +
 .../clang/Analysis/Analyses/ThreadSafety.h|  13 +-
 .../Analysis/Analyses/ThreadSafetyCommon.h|  21 +-
 clang/include/clang/Basic/Attr.td |   7 +
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/Analysis/ThreadSafety.cpp   | 134 +--
 clang/lib/Analysis/ThreadSafetyCommon.cpp |  66 ++--
 clang/lib/Sema/AnalysisBasedWarnings.cpp  |   3 +
 clang/lib/Sema/SemaDeclAttr.cpp   |  10 +
 ...a-attribute-supported-attributes-list.test |   1 +
 clang/test/Sema/warn-thread-safety-analysis.c |  20 +
 .../test/SemaCXX/thread-safety-annotations.h  |   1 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 356 ++
 .../SemaCXX/warn-thread-safety-parsing.cpp|   7 +
 clang/unittests/AST/ASTImporterTest.cpp   |   9 +
 16 files changed, 598 insertions(+), 75 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 80399e458aec3..2a6565e37e256 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -369,6 +369,7 @@ Improvements to Clang's diagnostics
   as function arguments or return value respectively. Note that
   :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
   feature will be default-enabled with ``-Wthread-safety`` in a future release.
+- The :doc:`ThreadSafetyAnalysis` now supports reentrant capabilities.
 - Clang will now do a better job producing common nested names, when producing
   common types for ternary operator, template argument deduction and multiple 
return auto deduction.
 - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) 
and minus(-) as signed integers
diff --g

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

https://github.com/melver approved this pull request.


https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -2361,6 +2361,13 @@ def fsanitize_coverage_ignorelist : Joined<["-"], 
"fsanitize-coverage-ignorelist
 HelpText<"Disable sanitizer coverage instrumentation for modules and 
functions "
  "that match the provided special case list, even the allowed 
ones">,
 
MarshallingInfoStringVector>;
+def fsanitize_coverage_stack_depth_callback_min_EQ

melver wrote:

Is there other user facing documentation besides this?

There's https://clang.llvm.org/docs/SanitizerCoverage.html - but I don't see it 
have an entry for stack-depth yet.

I'm asking because it's not entirely clear by looking at this where the 
callback is done. I can see that it's after allocas at entry. Like the inline 
version, in the callback you can then do __builtin_frame_address(). Are there 
other caveats (allocas in later branches, etc.)?

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

https://github.com/melver commented:

Generally LGTM - but let's also wait for others to comment.

Documentation of this feature is lacking (and afaik also wasn't added in 
https://reviews.llvm.org/D36839). Given this will be used in the kernel, some 
kind of official documentation might be good to have.

Btw, if you want a runtime test, it's possible to add one similar to the one in 
`compiler-rt/trunk/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cc`.

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

https://github.com/melver requested changes to this pull request.

This is also missing flag and IR tests.

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -1078,22 +1092,44 @@ void 
ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
 Store->setNoSanitizeMetadata();
   }
   if (Options.StackDepth && IsEntryBB && !IsLeafFunc) {
-// Check stack depth.  If it's the deepest so far, record it.
 Module *M = F.getParent();
-auto FrameAddrPtr = IRB.CreateIntrinsic(
-Intrinsic::frameaddress,
-IRB.getPtrTy(M->getDataLayout().getAllocaAddrSpace()),
-{Constant::getNullValue(Int32Ty)});
-auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
-auto LowestStack = IRB.CreateLoad(IntptrTy, SanCovLowestStack);
-auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
-auto ThenTerm = SplitBlockAndInsertIfThen(
-IsStackLower, &*IP, false,
-MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
-IRBuilder<> ThenIRB(ThenTerm);
-auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
-LowestStack->setNoSanitizeMetadata();
-Store->setNoSanitizeMetadata();
+if (Options.StackDepthCallbackMin) {
+  // In callback mode, only add call when stack depth reaches minimum.
+  const DataLayout &DL = M->getDataLayout();
+  uint32_t EstimatedStackSize = 0;
+
+  // Make an estimate on the stack usage.
+  for (auto &I : F.getEntryBlock()) {
+if (auto *AI = dyn_cast(&I)) {
+  if (AI->isStaticAlloca()) {
+uint32_t TypeSize = DL.getTypeAllocSize(AI->getAllocatedType());
+EstimatedStackSize += TypeSize;
+  } else {
+// Over-estimate dynamic sizes.
+EstimatedStackSize += 4096;

melver wrote:

This just seems too arbitrary. Can we do something more reliable? Because if 
this is used beyond the kernel (where we shouldn't even have all that many VLAs 
or dynamic allocas left - right?), it's anyone's guess if 4K is even a generous 
over estimation.

I see several options:
- Make this a cl::opt and not a hard-coded constant. The default should be much 
larger (8 MiB?), so that even we can't accurately calculate stack size we 
always get a callback.
- Just give up if we found a dynamic alloca, and always do the callback.

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -34,6 +34,7 @@ class SanitizerArgs {
   std::vector CoverageIgnorelistFiles;
   std::vector BinaryMetadataIgnorelistFiles;
   int CoverageFeatures = 0;
+  int StackDepthCallbackMin = 0;

melver wrote:

`CoverageStackDepthCallbackMin`

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -385,6 +385,49 @@ Users need to implement a single function to capture the 
CF table at startup:
 // the collected control flow.
   }
 
+Tracing Stack Depth
+===
+
+With ``-fsanitize-coverage=stack-depth`` the compiler will track how much
+stack space has been used for a function call chain. Leaf functions are
+not included in this tracing.
+
+The maximum depth of a function call graph is stored in the thread-local
+``__sancov_lowest_stack`` variable. Instrumentation is inserted in every
+non-leaf function to check the stack pointer against this variable,
+and if it is lower, store the current stack pointer. This effectively
+inserts the following:
+
+.. code-block:: c++
+
+  thread_local uintptr_t __sancov_lowest_stack;
+
+  uintptr_t stack = (uintptr_t)__builtin_frame_address(0);
+  if (stack < __sancov_lowest_stack)
+__sancov_lowest_stack = stack;
+
+If ``-fsanitize-coverage-stack-depth-callback-min=N`` is also used, the
+tracking is delegated to a callback, ``__sanitizer_cov_stack_depth``,
+instead of adding instrumentation to update ``__sancov_lowest_stack``.
+The ``N`` of the argument is used to determine which functions to
+instrument. Only functions estimated to be using ``N`` bytes or more of
+stack space will be instrumented to call the tracing callback. In the
+case of a dynamically sized stack, the callback is unconditionally added.
+
+The callback takes no arguments and is responsible for determining the
+stack pointer and doing any needed comparisons and storage. A roughtly

melver wrote:

s/roughtly/roughly/

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -385,6 +385,49 @@ Users need to implement a single function to capture the 
CF table at startup:
 // the collected control flow.
   }
 
+Tracing Stack Depth
+===
+
+With ``-fsanitize-coverage=stack-depth`` the compiler will track how much
+stack space has been used for a function call chain. Leaf functions are
+not included in this tracing.
+
+The maximum depth of a function call graph is stored in the thread-local
+``__sancov_lowest_stack`` variable. Instrumentation is inserted in every
+non-leaf function to check the stack pointer against this variable,

melver wrote:

You write "stack pointer" here, but in the below it's "frame pointer", which 
are different.

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -385,6 +385,49 @@ Users need to implement a single function to capture the 
CF table at startup:
 // the collected control flow.
   }
 
+Tracing Stack Depth
+===
+
+With ``-fsanitize-coverage=stack-depth`` the compiler will track how much
+stack space has been used for a function call chain. Leaf functions are
+not included in this tracing.
+
+The maximum depth of a function call graph is stored in the thread-local
+``__sancov_lowest_stack`` variable. Instrumentation is inserted in every
+non-leaf function to check the stack pointer against this variable,
+and if it is lower, store the current stack pointer. This effectively
+inserts the following:
+
+.. code-block:: c++
+
+  thread_local uintptr_t __sancov_lowest_stack;

melver wrote:

extern ?

Because the compiler doesn't define the TLS variable.

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -385,6 +385,49 @@ Users need to implement a single function to capture the 
CF table at startup:
 // the collected control flow.
   }
 
+Tracing Stack Depth
+===
+
+With ``-fsanitize-coverage=stack-depth`` the compiler will track how much
+stack space has been used for a function call chain. Leaf functions are
+not included in this tracing.
+
+The maximum depth of a function call graph is stored in the thread-local
+``__sancov_lowest_stack`` variable. Instrumentation is inserted in every
+non-leaf function to check the stack pointer against this variable,
+and if it is lower, store the current stack pointer. This effectively
+inserts the following:
+
+.. code-block:: c++
+
+  thread_local uintptr_t __sancov_lowest_stack;
+
+  uintptr_t stack = (uintptr_t)__builtin_frame_address(0);
+  if (stack < __sancov_lowest_stack)
+__sancov_lowest_stack = stack;
+
+If ``-fsanitize-coverage-stack-depth-callback-min=N`` is also used, the

melver wrote:

"If ``-fsanitize-coverage-stack-depth-callback-min=N`` (where ``N`` > 0) is 
also used ..." ?

Because right now it's unclear what behaviour is supposed to be if N==0. Per 
current implementation N==0 disables this feature.

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -1078,22 +1091,65 @@ void 
ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
 Store->setNoSanitizeMetadata();
   }
   if (Options.StackDepth && IsEntryBB && !IsLeafFunc) {
-// Check stack depth.  If it's the deepest so far, record it.
 Module *M = F.getParent();
-auto FrameAddrPtr = IRB.CreateIntrinsic(
-Intrinsic::frameaddress,
-IRB.getPtrTy(M->getDataLayout().getAllocaAddrSpace()),
-{Constant::getNullValue(Int32Ty)});
-auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
-auto LowestStack = IRB.CreateLoad(IntptrTy, SanCovLowestStack);
-auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
-auto ThenTerm = SplitBlockAndInsertIfThen(
-IsStackLower, &*IP, false,
-MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
-IRBuilder<> ThenIRB(ThenTerm);
-auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
-LowestStack->setNoSanitizeMetadata();
-Store->setNoSanitizeMetadata();
+const DataLayout &DL = M->getDataLayout();
+
+if (Options.StackDepthCallbackMin) {
+  // In callback mode, only add call when stack depth reaches minimum.
+  uint32_t EstimatedStackSize = 0;
+  // If dynamic alloca found, always add call.
+  bool dynamic_alloca = false;

melver wrote:

HasDynamicAlloc?

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -1078,22 +1091,65 @@ void 
ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
 Store->setNoSanitizeMetadata();
   }
   if (Options.StackDepth && IsEntryBB && !IsLeafFunc) {
-// Check stack depth.  If it's the deepest so far, record it.
 Module *M = F.getParent();
-auto FrameAddrPtr = IRB.CreateIntrinsic(
-Intrinsic::frameaddress,
-IRB.getPtrTy(M->getDataLayout().getAllocaAddrSpace()),
-{Constant::getNullValue(Int32Ty)});
-auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
-auto LowestStack = IRB.CreateLoad(IntptrTy, SanCovLowestStack);
-auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
-auto ThenTerm = SplitBlockAndInsertIfThen(
-IsStackLower, &*IP, false,
-MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
-IRBuilder<> ThenIRB(ThenTerm);
-auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
-LowestStack->setNoSanitizeMetadata();
-Store->setNoSanitizeMetadata();
+const DataLayout &DL = M->getDataLayout();
+
+if (Options.StackDepthCallbackMin) {
+  // In callback mode, only add call when stack depth reaches minimum.
+  uint32_t EstimatedStackSize = 0;
+  // If dynamic alloca found, always add call.
+  bool dynamic_alloca = false;
+  // Find an insertion point after last "alloca".
+  llvm::Instruction *InsertBefore = NULL;

melver wrote:

nullptr

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [sancov] Introduce optional callback for stack-depth tracking (PR #138323)

[Marco Elver via cfe-commits]

@@ -2361,6 +2361,13 @@ def fsanitize_coverage_ignorelist : Joined<["-"], 
"fsanitize-coverage-ignorelist
 HelpText<"Disable sanitizer coverage instrumentation for modules and 
functions "
  "that match the provided special case list, even the allowed 
ones">,
 
MarshallingInfoStringVector>;
+def fsanitize_coverage_stack_depth_callback_min_EQ

melver wrote:

Yeah LLVM has a bunch of these hidden features, and when you find them to solve 
your problem it's like hitting the jackpot. ;-)

https://github.com/llvm/llvm-project/pull/138323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver created 
https://github.com/llvm/llvm-project/pull/137133

Introduce the `reentrant_capability` attribute, which may be specified
alongside the `capability(..)` attribute to denote that the defined
capability type is reentrant. Marking a capability as reentrant means
that acquiring the same capability multiple times is safe, and does not
produce warnings on attempted re-acquisition.

The most significant changes required are plumbing to propagate if the
attribute is present to a CapabilityExpr, and then introducing a
ReentrancyCount to FactEntry that can be incremented while a fact
remains in the FactSet.

Care was taken to avoid increasing the size of both CapabilityExpr and
FactEntry by carefully allocating free bits of CapabilityExpr::CapExpr
and the bitset respectively.

>From c60ccbc31de8e81e6a4af915a83b8271f58f8e7e Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../Analysis/Analyses/ThreadSafetyCommon.h| 20 +++
 clang/lib/Analysis/ThreadSafety.cpp   |  4 ++--
 clang/lib/Analysis/ThreadSafetyCommon.cpp | 14 ++---
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..f328d4c7f481a 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,30 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
-  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
+  CapabilityExpr(const til::SExpr *E, StringRef Kind, unsigned Flags)
+  : CapExpr(E, Flags), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
-  template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
+  template 
+  CapabilityExpr(const til::SExpr *, T, unsigned) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind,
+  CapExpr.getInt() ^ FlagNegative);
   }
 
   bool equals(const CapabilityExpr &other) const {
diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 42fb0fe7dcdaa..96e79bc4dcfcc 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1839,7 +1839,7 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
   if (isa(Exp))
 Self = Placeholder.first;
   if (TagT->getDecl()->hasAttr())
-Scp = CapabilityExpr(Placeholder.first, Placeholder.second, false);
+Scp = CapabilityExpr(Placeholder.first, Placeholder.second, 0);
 }
 
 assert(Loc.isInvalid());
@@ -1982,7 +1982,7 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
   Cp.isInvalid() && CBTE) {
 if (auto Object = 
Analyzer->ConstructedObjects.find(CBTE->getSubExpr());
 Object != Analyzer->ConstructedObjects.end())
-  Cp = CapabilityExpr(Object->second, StringRef("mutex"), false);
+  Cp = CapabilityExpr(Object->second, StringRef("mutex"), 0);
   }
   const FactEntry *Fact = FSet.findLock(Analyzer->FactMan, Cp);
   if (!Fact) {
diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp 
b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index 13cd7e26dc16f..255e6413344da 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -173,7 +173,7 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr 
*AttrExp,
   Self,
   ClassifyDiagnostic(
   cast(D)->getFunctionObjectParameterType()),
-  false);
+  0);
 else  // For most attributes.
   return translateAttrExpr(AttrExp, &Ctx);
   }
@@ -197,22 +197,22 @@ CapabilityExpr SExprBui

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

melver wrote:

In 
https://lore.kernel.org/all/CANpmjNPquO=W1JAh1FNQb8pMQjgeZAKCPQUAd7qUg=5pjJ6x=q...@mail.gmail.com/
 I wrote:

> 1. Re-entrant acquires: rcu_read_lock(), preempt_disable(), etc. are
> all re-entrant locks. My proposal is to introduce an attribute that
> can be added to "ACQUIRE(..)" annotated functions to indicate they are
> re-entrant. Release-count needs to then match acquire-count to fully
> release a capability.

But thinking about this more, this is unnecessarily complex and introduces 
other headaches in the analysis. The simpler option is to just declare the 
whole capability/lock as reentrant, as done in this PR. My conclusion is that 
the more complex version would be a case of YAGNI, and in the odd cases where 
it might need to be more fine-grained, marking the whole capability as 
reentrant will be traded off against potential false negatives. For user space 
locking primitives I've never seen this kind of mixed interface -- only in the 
kernel I could imaging this to be something we could cook up, but I'm happy to 
take the false negatives for having a simpler-to-use and saner feature in Clang.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/137133

>From c60ccbc31de8e81e6a4af915a83b8271f58f8e7e Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../Analysis/Analyses/ThreadSafetyCommon.h| 20 +++
 clang/lib/Analysis/ThreadSafety.cpp   |  4 ++--
 clang/lib/Analysis/ThreadSafetyCommon.cpp | 14 ++---
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..f328d4c7f481a 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,30 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
-  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
+  CapabilityExpr(const til::SExpr *E, StringRef Kind, unsigned Flags)
+  : CapExpr(E, Flags), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
-  template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
+  template 
+  CapabilityExpr(const til::SExpr *, T, unsigned) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind,
+  CapExpr.getInt() ^ FlagNegative);
   }
 
   bool equals(const CapabilityExpr &other) const {
diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 42fb0fe7dcdaa..96e79bc4dcfcc 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1839,7 +1839,7 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
   if (isa(Exp))
 Self = Placeholder.first;
   if (TagT->getDecl()->hasAttr())
-Scp = CapabilityExpr(Placeholder.first, Placeholder.second, false);
+Scp = CapabilityExpr(Placeholder.first, Placeholder.second, 0);
 }
 
 assert(Loc.isInvalid());
@@ -1982,7 +1982,7 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
   Cp.isInvalid() && CBTE) {
 if (auto Object = 
Analyzer->ConstructedObjects.find(CBTE->getSubExpr());
 Object != Analyzer->ConstructedObjects.end())
-  Cp = CapabilityExpr(Object->second, StringRef("mutex"), false);
+  Cp = CapabilityExpr(Object->second, StringRef("mutex"), 0);
   }
   const FactEntry *Fact = FSet.findLock(Analyzer->FactMan, Cp);
   if (!Fact) {
diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp 
b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index 13cd7e26dc16f..255e6413344da 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -173,7 +173,7 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr 
*AttrExp,
   Self,
   ClassifyDiagnostic(
   cast(D)->getFunctionObjectParameterType()),
-  false);
+  0);
 else  // For most attributes.
   return translateAttrExpr(AttrExp, &Ctx);
   }
@@ -197,22 +197,22 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr 
*AttrExp,
   // The "*" expr is a universal lock, which essentially turns off
   // checks until it is removed from the lockset.
   return CapabilityExpr(new (Arena) til::Wildcard(), StringRef("wildcard"),
-false);
+0);
 else
   // Ignore other string literals for now.
   return CapabilityExpr();
   }
 
-  bool Neg = false;
+  unsigned ExprFlags = 0;
   if (const auto *OE = dyn_cast(AttrExp)) {
 if (OE->getOperator() == OO_Exclaim) {
-  Neg = true;
+  ExprFlags |= CapabilityExpr::FlagNegative;
   AttrExp = OE->getArg(0);
 }
   }
   else if (const auto *UO = dyn_cast(AttrExp)) {
 i

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/137133

>From c60ccbc31de8e81e6a4af915a83b8271f58f8e7e Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../Analysis/Analyses/ThreadSafetyCommon.h| 20 +++
 clang/lib/Analysis/ThreadSafety.cpp   |  4 ++--
 clang/lib/Analysis/ThreadSafetyCommon.cpp | 14 ++---
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..f328d4c7f481a 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,30 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
-  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
+  CapabilityExpr(const til::SExpr *E, StringRef Kind, unsigned Flags)
+  : CapExpr(E, Flags), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
-  template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
+  template 
+  CapabilityExpr(const til::SExpr *, T, unsigned) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind,
+  CapExpr.getInt() ^ FlagNegative);
   }
 
   bool equals(const CapabilityExpr &other) const {
diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 42fb0fe7dcdaa..96e79bc4dcfcc 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1839,7 +1839,7 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
   if (isa(Exp))
 Self = Placeholder.first;
   if (TagT->getDecl()->hasAttr())
-Scp = CapabilityExpr(Placeholder.first, Placeholder.second, false);
+Scp = CapabilityExpr(Placeholder.first, Placeholder.second, 0);
 }
 
 assert(Loc.isInvalid());
@@ -1982,7 +1982,7 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
   Cp.isInvalid() && CBTE) {
 if (auto Object = 
Analyzer->ConstructedObjects.find(CBTE->getSubExpr());
 Object != Analyzer->ConstructedObjects.end())
-  Cp = CapabilityExpr(Object->second, StringRef("mutex"), false);
+  Cp = CapabilityExpr(Object->second, StringRef("mutex"), 0);
   }
   const FactEntry *Fact = FSet.findLock(Analyzer->FactMan, Cp);
   if (!Fact) {
diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp 
b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index 13cd7e26dc16f..255e6413344da 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -173,7 +173,7 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr 
*AttrExp,
   Self,
   ClassifyDiagnostic(
   cast(D)->getFunctionObjectParameterType()),
-  false);
+  0);
 else  // For most attributes.
   return translateAttrExpr(AttrExp, &Ctx);
   }
@@ -197,22 +197,22 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr 
*AttrExp,
   // The "*" expr is a universal lock, which essentially turns off
   // checks until it is removed from the lockset.
   return CapabilityExpr(new (Arena) til::Wildcard(), StringRef("wildcard"),
-false);
+0);
 else
   // Ignore other string literals for now.
   return CapabilityExpr();
   }
 
-  bool Neg = false;
+  unsigned ExprFlags = 0;
   if (const auto *OE = dyn_cast(AttrExp)) {
 if (OE->getOperator() == OO_Exclaim) {
-  Neg = true;
+  ExprFlags |= CapabilityExpr::FlagNegative;
   AttrExp = OE->getArg(0);
 }
   }
   else if (const auto *UO = dyn_cast(AttrExp)) {
 i

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/137133

>From d3324c1023533bf784a3c3c3ef095d07c865e6f9 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../clang/Analysis/Analyses/ThreadSafetyCommon.h   | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..6e46a2d721463 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,28 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
   CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  : CapExpr(E, Neg ? FlagNegative : 0), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
   template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind, !negative());
   }
 
   bool equals(const CapabilityExpr &other) const {

>From cadff841b5ddc812e1ae44bcd4f87960173ef7c8 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Thu, 24 Apr 2025 09:02:08 +0200
Subject: [PATCH 2/2] Thread Safety Analysis: Support reentrant capabilities

Introduce the `reentrant_capability` attribute, which may be specified
alongside the `capability(..)` attribute to denote that the defined
capability type is reentrant. Marking a capability as reentrant means
that acquiring the same capability multiple times is safe, and does not
produce warnings on attempted re-acquisition.

The most significant changes required are plumbing to propagate if the
attribute is present to a CapabilityExpr, and then introducing a
ReentrancyCount to FactEntry that can be incremented while a fact
remains in the FactSet.

Care was taken to avoid increasing the size of both CapabilityExpr and
FactEntry by carefully allocating free bits of CapabilityExpr::CapExpr
and the bitset respectively.
---
 clang/docs/ReleaseNotes.rst   |   1 +
 clang/docs/ThreadSafetyAnalysis.rst   |  18 +
 .../clang/Analysis/Analyses/ThreadSafety.h|   4 +
 .../Analysis/Analyses/ThreadSafetyCommon.h|  17 +-
 clang/include/clang/Basic/Attr.td |   7 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/lib/Analysis/ThreadSafety.cpp   | 127 ++--
 clang/lib/Analysis/ThreadSafetyCommon.cpp |  39 +--
 clang/lib/Sema/AnalysisBasedWarnings.cpp  |   7 +
 ...a-attribute-supported-attributes-list.test |   1 +
 clang/test/Sema/warn-thread-safety-analysis.c |  20 ++
 .../test/SemaCXX/thread-safety-annotations.h  |   1 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 308 ++
 clang/unittests/AST/ASTImporterTest.cpp   |   7 +
 14 files changed, 509 insertions(+), 51 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d1f24fb23d44d..7afa2ef359de9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -343,6 +343,7 @@ Improvements to Clang's diagnostics
   as function arguments or return value respectively. Note that
   :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
   feature will be default-enabled with ``-Wthread-safety`` in a future release.
+- The :doc:`ThreadSafetyAnalysis` now supports reentrant capabilities.
 - Clang will now do a better job producing common nested names, when producing
   common types for ternary operator, template argument deduction and multiple 
return auto deduction.
 - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) 
and minus(-) as signed integers
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 130069c5659d6..4fc7ff28e

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -434,6 +434,16 @@ class can be used as a capability.  The string argument 
specifies the kind of
 capability in error messages, e.g. ``"mutex"``.  See the ``Container`` example
 given above, or the ``Mutex`` class in :ref:`mutexheader`.
 
+REENTRANT
+-
+
+``REENTRANT`` is an attribute on capability classes, denoting that they are
+reentrant. Marking a capability as reentrant means that acquiring the same
+capability multiple times is safe.
+
+Note that acquiring the same capability with different access privileges
+(exclusive vs. shared) again is not considered reentrant by the analysis.

melver wrote:

Added a Note. PTAL.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/137133

>From d3324c1023533bf784a3c3c3ef095d07c865e6f9 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../clang/Analysis/Analyses/ThreadSafetyCommon.h   | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..6e46a2d721463 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,28 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
   CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  : CapExpr(E, Neg ? FlagNegative : 0), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
   template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind, !negative());
   }
 
   bool equals(const CapabilityExpr &other) const {

>From 1a2594caf99b5693e5e804b8c25c7a52b35f28b4 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Thu, 24 Apr 2025 09:02:08 +0200
Subject: [PATCH 2/2] Thread Safety Analysis: Support reentrant capabilities

Introduce the `reentrant_capability` attribute, which may be specified
alongside the `capability(..)` attribute to denote that the defined
capability type is reentrant. Marking a capability as reentrant means
that acquiring the same capability multiple times is safe, and does not
produce warnings on attempted re-acquisition.

The most significant changes required are plumbing to propagate if the
attribute is present to a CapabilityExpr, and then introducing a
ReentrancyCount to FactEntry that can be incremented while a fact
remains in the FactSet.

Care was taken to avoid increasing the size of both CapabilityExpr and
FactEntry by carefully allocating free bits of CapabilityExpr::CapExpr
and the bitset respectively.
---
 clang/docs/ReleaseNotes.rst   |   1 +
 clang/docs/ThreadSafetyAnalysis.rst   |  18 +
 .../clang/Analysis/Analyses/ThreadSafety.h|   4 +
 .../Analysis/Analyses/ThreadSafetyCommon.h|  17 +-
 clang/include/clang/Basic/Attr.td |   7 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/lib/Analysis/ThreadSafety.cpp   | 127 ++--
 clang/lib/Analysis/ThreadSafetyCommon.cpp |  39 +--
 clang/lib/Sema/AnalysisBasedWarnings.cpp  |   7 +
 ...a-attribute-supported-attributes-list.test |   1 +
 clang/test/Sema/warn-thread-safety-analysis.c |  20 ++
 .../test/SemaCXX/thread-safety-annotations.h  |   1 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 308 ++
 clang/unittests/AST/ASTImporterTest.cpp   |   7 +
 14 files changed, 509 insertions(+), 51 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d1f24fb23d44d..7afa2ef359de9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -343,6 +343,7 @@ Improvements to Clang's diagnostics
   as function arguments or return value respectively. Note that
   :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
   feature will be default-enabled with ``-Wthread-safety`` in a future release.
+- The :doc:`ThreadSafetyAnalysis` now supports reentrant capabilities.
 - Clang will now do a better job producing common nested names, when producing
   common types for ternary operator, template argument deduction and multiple 
return auto deduction.
 - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) 
and minus(-) as signed integers
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 130069c5659d6..4fc7ff28e

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

melver wrote:

> I think the biggest issue is that removing `const` from `FactEntry` does not 
> work. You'll have to undo all those changes and instead create a new 
> `FactEntry` for every lock/unlock.

Good catch, reworked this.
PTAL.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a9788e3 - Thread Safety Analysis: Test: Minor style fix

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2025-04-25T09:56:54+02:00
New Revision: a9788e3a86cab64ae92dd7d041718b0722f43c3a

URL: 
https://github.com/llvm/llvm-project/commit/a9788e3a86cab64ae92dd7d041718b0722f43c3a
DIFF: 
https://github.com/llvm/llvm-project/commit/a9788e3a86cab64ae92dd7d041718b0722f43c3a.diff

LOG: Thread Safety Analysis: Test: Minor style fix

Factored out from https://github.com/llvm/llvm-project/pull/137133

NFC.

Added: 


Modified: 
clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp 
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index ac3ca5e0c12a8..2a04e820eb095 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -6708,7 +6708,7 @@ int testAdoptShared() {
 
 } // namespace ReturnScopedLockable
 
-#endif
+#endif // __cpp_guaranteed_copy_elision
 
 namespace PR38640 {
 void f() {
@@ -6716,7 +6716,7 @@ void f() {
   // safety analysis was enabled.
   int &i = i; // expected-warning {{reference 'i' is not yet bound to a value 
when used within its own initialization}}
 }
-}
+} // namespace PR38640
 
 namespace Derived_Smart_Pointer {
 template 
@@ -6811,4 +6811,4 @@ class PointerGuard {
 mu1.Unlock();
   }
 };
-}
+} // namespace Derived_Smart_Pointer



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

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -271,26 +271,32 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
-  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  static constexpr unsigned FlagNegative = 1u << 0;
+  static constexpr unsigned FlagReentrant = 1u << 1;

melver wrote:

I don't follow. We need a way to query both, so we need at least 2 bits to 
store that info.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/137133

>From d3324c1023533bf784a3c3c3ef095d07c865e6f9 Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 23 Apr 2025 11:31:25 +0200
Subject: [PATCH 1/2] Thread Safety Analysis: Convert CapabilityExpr::CapExpr
 to hold flags

Rather than holding a single bool, switch it to contain flags, which is
both more descriptive and simplifies adding more flags in subsequent
changes.

NFC.
---
 .../clang/Analysis/Analyses/ThreadSafetyCommon.h   | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..6e46a2d721463 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -271,26 +271,28 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  static constexpr unsigned FlagNegative = 1u << 0;
+
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
+  CapabilityExpr() : CapExpr(nullptr, 0) {}
   CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  : CapExpr(E, Neg ? FlagNegative : 0), CapKind(Kind) {}
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
   template  CapabilityExpr(const til::SExpr *, T, bool) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
-  bool negative() const { return CapExpr.getInt(); }
+  bool negative() const { return CapExpr.getInt() & FlagNegative; }
 
   CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind, !negative());
   }
 
   bool equals(const CapabilityExpr &other) const {

>From 1e01f552b4f3e53d1f42a7af7962e15a1fa46ecd Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Thu, 24 Apr 2025 09:02:08 +0200
Subject: [PATCH 2/2] Thread Safety Analysis: Support reentrant capabilities

Introduce the `reentrant_capability` attribute, which may be specified
alongside the `capability(..)` attribute to denote that the defined
capability type is reentrant. Marking a capability as reentrant means
that acquiring the same capability multiple times is safe, and does not
produce warnings on attempted re-acquisition.

The most significant changes required are plumbing to propagate if the
attribute is present to a CapabilityExpr, and then introducing a
ReentrancyCount to FactEntry that can be incremented while a fact
remains in the FactSet.

Care was taken to avoid increasing the size of both CapabilityExpr and
FactEntry by carefully allocating free bits of CapabilityExpr::CapExpr
and the bitset respectively.
---
 clang/docs/ReleaseNotes.rst   |   1 +
 clang/docs/ThreadSafetyAnalysis.rst   |  13 +
 .../clang/Analysis/Analyses/ThreadSafety.h|   4 +
 .../Analysis/Analyses/ThreadSafetyCommon.h|  17 +-
 clang/include/clang/Basic/Attr.td |   7 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/lib/Analysis/ThreadSafety.cpp   | 127 ++--
 clang/lib/Analysis/ThreadSafetyCommon.cpp |  39 +--
 clang/lib/Sema/AnalysisBasedWarnings.cpp  |   7 +
 ...a-attribute-supported-attributes-list.test |   1 +
 clang/test/Sema/warn-thread-safety-analysis.c |  20 ++
 .../test/SemaCXX/thread-safety-annotations.h  |   1 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 308 ++
 clang/unittests/AST/ASTImporterTest.cpp   |   7 +
 14 files changed, 504 insertions(+), 51 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d1f24fb23d44d..7afa2ef359de9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -343,6 +343,7 @@ Improvements to Clang's diagnostics
   as function arguments or return value respectively. Note that
   :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
   feature will be default-enabled with ``-Wthread-safety`` in a future release.
+- The :doc:`ThreadSafetyAnalysis` now supports reentrant capabilities.
 - Clang will now do a better job producing common nested names, when producing
   common types for ternary operator, template argument deduction and multiple 
return auto deduction.
 - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) 
and minus(-) as signed integers
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 130069c5659d6..beecf09b9

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -163,15 +184,15 @@ using FactID = unsigned short;
 /// the analysis of a single routine.
 class FactManager {
 private:
-  std::vector> Facts;
+  std::vector> Facts;

melver wrote:

Fixed.

I guess we have to do clone-mutate-replace of the FactEntries - a little less 
efficient, but given these structs are relatively small (+ vtables) shouldn't 
be too bad.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -3990,6 +3990,13 @@ def LocksExcluded : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
+def ReentrantCapability : InheritableAttr {
+  let Spellings = [Clang<"reentrant_capability">];
+  let Subjects = SubjectList<[Record, TypedefName]>;
+  let Documentation = [Undocumented];

melver wrote:

Ack. Yes, style follows the rest of these attributes.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -1831,15 +1852,15 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
 assert(!Self);
 const auto *TagT = Exp->getType()->getAs();
 if (D->hasAttrs() && TagT && Exp->isPRValue()) {
-  std::pair Placeholder =
-  Analyzer->SxBuilder.createThisPlaceholder(Exp);
+  auto Placeholder = Analyzer->SxBuilder.createThisPlaceholder(Exp);

melver wrote:

Growing this pair/tuple become a little too complex, so I just made 
createThisPlaceholder return a CapabilityExpr struct right away.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -235,6 +266,20 @@ class FactSet {
 return false;
   }
 
+  std::optional replaceLock(FactManager &FM, iterator It,
+std::unique_ptr Entry) {
+if (It == end())
+  return std::nullopt;
+FactID F = FM.newFact(std::move(Entry));
+*It = F;
+return F;
+  }
+
+  std::optional replaceLock(FactManager &FM, const CapabilityExpr 
&CapE,
+std::unique_ptr Entry) {
+return replaceLock(FM, findLockIter(FM, CapE), std::move(Entry));
+  }
+

melver wrote:

We could, but it's less efficient, esp. if we used findLockIter():
1. without findLockIter: we save a pop_back() and push_back().
2. with preceding findLockIter: like (1), but also saves a search.

Sure, maybe we're just shaving off some constant overheads, but I also find it 
makes it less verbose and more readable.

I have this later patch I would send out as a separate PR later:
```
commit 9a41f2b68a28e6a2239b45a11a0cd360dce320af
Author: Marco Elver 
Date:   Fri Apr 25 18:51:16 2025 +0200

Thread Safety Analysis: Use replaceLock instead of removeLock+addLock

In ScopedLockableFactEntry::unlock(), we can avoid a second search,
pop_back(), and push_back() if we use the already obtained iterator into
the FactSet to replace the old FactEntry and take its position in the
vector.

diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index cb62f11d6f15..429f22715886 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1090,9 +1090,9 @@ private:
 return;
   }

-  FSet.removeLock(FactMan, Cp);
-  FSet.addLock(FactMan,
-   std::make_unique(!Cp, LK_Exclusive, 
loc));
+  FSet.replaceLock(
+  FactMan, It,
+  std::make_unique(!Cp, LK_Exclusive, loc));
 } else if (Handler) {
   SourceLocation PrevLoc;
   if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))
```


https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -271,26 +271,32 @@ class CFGWalker {
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
-  /// The capability expression and whether it's negated.
-  llvm::PointerIntPair CapExpr;
+  /// The capability expression and flags.
+  llvm::PointerIntPair CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
-  CapabilityExpr() : CapExpr(nullptr, false) {}
-  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-  : CapExpr(E, Neg), CapKind(Kind) {}
+  static constexpr unsigned FlagNegative = 1u << 0;
+  static constexpr unsigned FlagReentrant = 1u << 1;

melver wrote:

We need 4 states.

I'm adding this test:
```
+class TestNegativeWithReentrantMutex {
+  ReentrantMutex rmu;
+  int a GUARDED_BY(rmu);
+
+public:
+  void baz() EXCLUSIVE_LOCKS_REQUIRED(!rmu) {
+rmu.Lock();
+a = 0;
+rmu.Unlock();
+  }
+};
```

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -388,7 +395,7 @@ class SExprBuilder {
   til::LiteralPtr *createVariable(const VarDecl *VD);
 
   // Create placeholder for this: we don't know the VarDecl on construction 
yet.
-  std::pair
+  std::pair

melver wrote:

I think this code is just more complex than it needs to be. I do not understand 
the need for createThisPlaceholder(): 

- Placeholder.second is only used if there's a ScopedLockableAttr;
- Placeholder.first is equivalent to a createVariable(nullptr);

Therefore, we can just defer the second step (getting the string+bool out) for 
when we determine it's a ScopedLockableAttr, and use createVariable(nullptr) 
instead of createThisPlaceholder(). The main problem was that the function that 
extracts the string (and now also the reentrant bool) wasn't available here.

So it'll just be much clearer if we have a constructor that does that for us, 
and just use that rather than through this complex indirection of "prefetching" 
the string+bool.

Also saves a few cycles if we simplify it. Let me try that.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support reentrant capabilities (PR #137133)

[Marco Elver via cfe-commits]

@@ -81,26 +81,25 @@ static bool isCalleeArrow(const Expr *E) {
   return ME ? ME->isArrow() : false;
 }
 
-static StringRef ClassifyDiagnostic(const CapabilityAttr *A) {
-  return A->getName();
-}
-
-static StringRef ClassifyDiagnostic(QualType VDT) {
+static CapabilityExpr makeCapabilityExpr(const til::SExpr *E, QualType VDT,

melver wrote:

I'm simplifying this. The only place where it's not yet know if it's a 
capability is the ScopedLockableAttr case. If we solve that, then we can have 
makeCapabilityExpr() - or rather, just a constructor that takes QualType.

https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Improved pointer handling (PR #127396)

[Marco Elver via cfe-commits]

melver wrote:

FWIW, I'm ready for sending a v2 series to the Linux kernel mailing list: 
https://git.kernel.org/pub/scm/linux/kernel/git/melver/linux.git/log/?h=cap-analysis/dev

But I'd like to sort out this PR first before sending the v2 series.

https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on taking address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

Addressed comments so far.

> > The equivalent of passing a `pt_guarded_by` variable by value doesn't seem 
> > to warn.
> 
> This inconsistency is probably my largest concern. If you have
> 
> ```c
> int x GUARDED_BY(mu);
> int *p PT_GUARDED_BY(mu);
> ```
> 
> then `&x` should basically behave like `p`, and `x` like `*p`. But with the 
> current implementation, that cannot be the case, because `p` is just a no-op.

Right, PT_GUARDED_BY() coverage is lacking in this case.

> That's why I think we should rather check function arguments, and warn on 
> passing (1) the address of a guarded variable, (2) a pointee-guarded variable.

I agree this might be more conservative.

> There is already functionality that makes sure (1) and (2) work together: 
> `checkAccess` and `checkPtAccess` call each other when necessary. This 
> doesn't seem to handle unary `&` at the moment, but I suppose this would be 
> easy to add. The check for passing arguments itself is in `examineArguments`. 
> We currently only check reference type arguments:
> 
> ```c++
> if (Qt->isReferenceType())
>   checkAccess(*Arg, AK_Read, POK_PassByRef);
> ```
> 
> Here we might simply add a branch for pointer types with the appropriate 
> `POK` that you already introduced (though we might need to rename it 
> slightly).
> 
> This still doesn't cover local pointer variables, but here I guess we need a 
> better understanding of the patterns and how much alias analysis we'd need to 
> unravel them. Local pointer variables as obfuscation device are hopefully not 
> common, so there is likely some more complex logic that would be intractable 
> for us anyway. So I'd be curious to see some interesting examples.

The Linux kernel has odd idioms like this one:
```
#define __READ_ONCE(x)  (*(const volatile __unqual_scalar_typeof(x) *)&(x))
```
When passed a guarded variable (e.g. `READ_ONCE(my_guarded_var)`), this will no 
longer warn if we do not check addressof.

My intent with Wthread-safety-addressof was to blindly warn on "addressof", so 
we get this coverage.

But I suppose if we switch it to just cover cases when passing to functions, 
I'd propose:
- introduce Wthread-safety-pointer as a counterpart to Wthread-safety-reference
- enable it by default similar to Wthread-safety-reference (but initially under 
Wthread-safety-beta)

I still wouldn't mind if Wthread-safety-addressof existed (for the plethora of 
weird macros the Linux kernel has), but I understand that maintaining more 
features might not be what we want.

Preferences?

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on taking address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver updated 
https://github.com/llvm/llvm-project/pull/123063

>From 6727047d25b8b72f8e23b03a84c0b23f6dad566a Mon Sep 17 00:00:00 2001
From: Marco Elver 
Date: Wed, 15 Jan 2025 13:23:14 +0100
Subject: [PATCH 1/5] Thread Safety Analysis: Support warning on obtaining
 address of guarded variables

Add the optional ability, via `-Wthread-safety-addressof`, to warn when
obtaining the address of guarded variables.

This is required to avoid false negatives in large C codebases, where
data structures are typically implemented through helpers that take
pointers to instances of a data structure.

We also argue that, while obtaining the address itself does not yet
constitute a potential race (in the presence of missing locking),
placing the requirement on the pointer-recipient to obtain locks to
access the pointed-to data is most likely poor style. This is analogous
to passing C++ references to guarded variables, which produces warnings
by default.

Given that existing codebases using `-Wthread-safety` likely have cases
where obtaining the pointer to a guarded variable is benign, the feature
is not enabled by default but requires explicit opt-in.
---
 clang/docs/ReleaseNotes.rst   |  3 ++
 clang/docs/ThreadSafetyAnalysis.rst   | 12 -
 .../clang/Analysis/Analyses/ThreadSafety.h|  3 ++
 clang/include/clang/Basic/DiagnosticGroups.td |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td|  8 
 clang/lib/Analysis/ThreadSafety.cpp   |  3 ++
 clang/lib/Sema/AnalysisBasedWarnings.cpp  | 26 ---
 clang/test/Sema/warn-thread-safety-analysis.c | 11 +
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 44 ++-
 9 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 794943b24a003..abcf0ef236cc2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -788,6 +788,9 @@ Improvements to Clang's diagnostics
   require(scope); // Warning!  Requires mu1.
 }
 
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-addressof``,
+  which enables warning if the address of guarded variables is obtained.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 9c1c32e46989b..c506844e7c94d 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -515,8 +515,16 @@ Warning flags
   + ``-Wthread-safety-analysis``: The core analysis.
   + ``-Wthread-safety-precise``: Requires that mutex expressions match 
precisely.
This warning can be disabled for code which has a lot of aliases.
-  + ``-Wthread-safety-reference``: Checks when guarded members are passed by 
reference.
-
+  + ``-Wthread-safety-reference``: Checks when guarded variables are passed by 
reference.
+
+* ``-Wthread-safety-addressof``: Warn when the address of guarded variables is
+  obtained (``&var``). Since obtaining the address of a variable doesn't
+  necessarily imply a read or write, the warning is off by default. In
+  codebases that prefer passing pointers rather than references (for C++
+  codebases), or passing pointers is ubiquitous (for C codebases), enabling
+  this warning will result in fewer false negatives; for example, where the
+  manipulation of common data structures is done via functions that take
+  pointers to instances of the data structure.
 
 :ref:`negative` are an experimental feature, which are enabled with:
 
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0fcf5bed1285a..a61c60cbf531e 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -54,6 +54,9 @@ enum ProtectedOperationKind {
 
   /// Returning a pt-guarded variable by reference.
   POK_PtReturnByRef,
+
+  /// Obtaining address of a variable (e.g. &x).
+  POK_AddressOf,
 };
 
 /// This enum distinguishes between different kinds of lock actions. For
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 594e99a19b64d..8bd5d043cefa8 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1126,6 +1126,7 @@ def ThreadSafetyReferenceReturn  : 
DiagGroup<"thread-safety-reference-return">;
 def ThreadSafetyReference  : DiagGroup<"thread-safety-reference",
  [ThreadSafetyReferenceReturn]>;
 def ThreadSafetyNegative   : DiagGroup<"thread-safety-negative">;
+def ThreadSafetyAddressof  : DiagGroup<"thread-safety-addressof">;
 def ThreadSafety : DiagGroup<"thread-safety",
  [ThreadSafetyAttributes,
   ThreadSafetyAnalysis,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b

[clang] Thread Safety Analysis: Support warning on taking address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

@@ -515,8 +515,18 @@ Warning flags
   + ``-Wthread-safety-analysis``: The core analysis.
   + ``-Wthread-safety-precise``: Requires that mutex expressions match 
precisely.
This warning can be disabled for code which has a lot of aliases.
-  + ``-Wthread-safety-reference``: Checks when guarded members are passed by 
reference.
-
+  + ``-Wthread-safety-reference``: Checks when guarded variables are passed by 
reference.
+
+* ``-Wthread-safety-addressof``: Warn when the address of guarded variables is
+  taken (``&var``). Since taking the address of a variable does *not
+  necessarily imply a read or write*, the warning is off by default to avoid
+  false positives. In codebases that prefer passing pointers rather than
+  references (for C++ codebases), or passing pointers is ubiquitous (for C
+  codebases), enabling this warning will result in fewer false negatives; for
+  example, where the manipulation of common data structures is done via
+  functions that take pointers to instances of the data structure. Note,
+  however, that the analysis does not track pointers, and false positives *and*
+  negatives are still possible.

melver wrote:

Made it depend on Wthread-safety-beta.

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Handle address-of followed by dereference (PR #127397)

[Marco Elver via cfe-commits]

https://github.com/melver closed 
https://github.com/llvm/llvm-project/pull/127397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Handle address-of followed by dereference (PR #127397)

[Marco Elver via cfe-commits]

melver wrote:

Superseded by https://github.com/llvm/llvm-project/pull/127396

https://github.com/llvm/llvm-project/pull/127397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on taking address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

https://github.com/melver converted_to_draft 
https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support warning on taking address of guarded variables (PR #123063)

[Marco Elver via cfe-commits]

melver wrote:

 [...]
> Then we only need to make sure that `checkPtAccess` can look through `&`, as 
> mentioned above. (Casts should already be unwrapped.) This might not even 
> need a new flag, it's just closing a gap in the existing analysis.

Thanks for the suggestions!

I implemented both -- please see 
https://github.com/llvm/llvm-project/pull/127396.

https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Improved pointer handling (PR #127396)

[Marco Elver via cfe-commits]

https://github.com/melver edited 
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits