[clang] [Clang] Diagnose additional ObjC statements as function effect violations (PR #112148)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos updated https://github.com/llvm/llvm-project/pull/112148

>From f34e4ac55d04bcd8e34bef57afec5a39fbf2acb9 Mon Sep 17 00:00:00 2001
From: Doug Wyatt <dwyatt at apple.com>
Date: Sun, 13 Oct 2024 11:35:10 -0700
Subject: [PATCH 1/3] [Clang] Diagnose ObjC @autoreleasepool statements as
 function effect violations.

---
 clang/lib/Sema/SemaFunctionEffects.cpp                | 10 ++++++++++
 clang/test/SemaObjCXX/attr-nonblocking-constraints.mm |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp
index 0ac5de29f66aa7..5c4c71e58c4edd 100644
--- a/clang/lib/Sema/SemaFunctionEffects.cpp
+++ b/clang/lib/Sema/SemaFunctionEffects.cpp
@@ -1140,6 +1140,16 @@ class Analyzer {
       return true;
     }
 
+    bool VisitObjCAutoreleasePoolStmt(ObjCAutoreleasePoolStmt *ARP) {
+      // Under the hood, @autorelease (potentially?) allocates memory and
+      // invokes ObjC methods. We don't currently have memory allocation as
+      // a "language construct" but we do have ObjC messaging, so diagnose that.
+      diagnoseLanguageConstruct(FunctionEffect::FE_ExcludeObjCMessageSend,
+                                ViolationID::AccessesObjCMethodOrProperty,
+                                ARP->getBeginLoc());
+      return true;
+    }
+
     bool VisitSEHExceptStmt(SEHExceptStmt *Exc) {
       diagnoseLanguageConstruct(FunctionEffect::FE_ExcludeCatch,
                                 ViolationID::ThrowsOrCatchesExceptions,
diff --git a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
index abd0938ac321af..93555048562850 100644
--- a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
+++ b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
@@ -24,3 +24,8 @@ void nb4() [[clang::nonblocking]] {
 	@catch (...) { // expected-warning {{function with 'nonblocking' attribute must not throw or catch exceptions}}
 	}
 }
+
+void nb5() [[clang::nonblocking]] {
+	@autoreleasepool { // expected-warning {{function with 'nonblocking' attribute must not access ObjC methods or properties}}
+	}
+}
\ No newline at end of file

>From 930e1fa44fa1ae1c0fd030312748dd7222d587ca Mon Sep 17 00:00:00 2001
From: Doug Wyatt <dwyatt at apple.com>
Date: Sun, 13 Oct 2024 11:48:03 -0700
Subject: [PATCH 2/3] Also diagnose @finally and @synchronized.

---
 clang/lib/Sema/SemaFunctionEffects.cpp          | 17 +++++++++++++++++
 .../SemaObjCXX/attr-nonblocking-constraints.mm  |  8 ++++++++
 2 files changed, 25 insertions(+)

diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp
index 5c4c71e58c4edd..190a0fd77a3699 100644
--- a/clang/lib/Sema/SemaFunctionEffects.cpp
+++ b/clang/lib/Sema/SemaFunctionEffects.cpp
@@ -1133,6 +1133,13 @@ class Analyzer {
       return true;
     }
 
+    bool VisitObjCAtFinallyStmt(ObjCAtFinallyStmt *Finally) {
+      diagnoseLanguageConstruct(FunctionEffect::FE_ExcludeCatch,
+                                ViolationID::ThrowsOrCatchesExceptions,
+                                Finally->getAtFinallyLoc());
+      return true;
+    }
+
     bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
       diagnoseLanguageConstruct(FunctionEffect::FE_ExcludeObjCMessageSend,
                                 ViolationID::AccessesObjCMethodOrProperty,
@@ -1150,6 +1157,16 @@ class Analyzer {
       return true;
     }
 
+    bool VisitObjCAtSynchronizedStmt(ObjCAtSynchronizedStmt *Sync) {
+      // Under the hood, this calls objc_sync_enter and objc_sync_exit, wrapped
+      // in a @try/@finally block. Diagnose this somewhat generically as "ObjC"
+      // messaging.
+      diagnoseLanguageConstruct(FunctionEffect::FE_ExcludeObjCMessageSend,
+                                ViolationID::AccessesObjCMethodOrProperty,
+                                Sync->getBeginLoc());
+      return true;
+    }
+
     bool VisitSEHExceptStmt(SEHExceptStmt *Exc) {
       diagnoseLanguageConstruct(FunctionEffect::FE_ExcludeCatch,
                                 ViolationID::ThrowsOrCatchesExceptions,
diff --git a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
index 93555048562850..cb8c7c3f3d07c0 100644
--- a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
+++ b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
@@ -23,9 +23,17 @@ void nb4() [[clang::nonblocking]] {
 	}
 	@catch (...) { // expected-warning {{function with 'nonblocking' attribute must not throw or catch exceptions}}
 	}
+	@finally { // expected-warning {{function with 'nonblocking' attribute must not throw or catch exceptions}}
+	}
 }
 
+ at class Lock;
+extern Lock *someLock;
+
 void nb5() [[clang::nonblocking]] {
 	@autoreleasepool { // expected-warning {{function with 'nonblocking' attribute must not access ObjC methods or properties}}
 	}
+
+	@synchronized(someLock) { // expected-warning {{function with 'nonblocking' attribute must not access ObjC methods or properties}}
+	}
 }
\ No newline at end of file

>From 04fb5208cf0a15afd1e2a9f3326fd73e517429b2 Mon Sep 17 00:00:00 2001
From: Doug Wyatt <dwyatt at apple.com>
Date: Mon, 14 Oct 2024 08:15:21 -0700
Subject: [PATCH 3/3] Clean up nits from review.

---
 clang/lib/Sema/SemaFunctionEffects.cpp                | 4 ++--
 clang/test/SemaObjCXX/attr-nonblocking-constraints.mm | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp
index 190a0fd77a3699..70e5d78661a835 100644
--- a/clang/lib/Sema/SemaFunctionEffects.cpp
+++ b/clang/lib/Sema/SemaFunctionEffects.cpp
@@ -1159,8 +1159,8 @@ class Analyzer {
 
     bool VisitObjCAtSynchronizedStmt(ObjCAtSynchronizedStmt *Sync) {
       // Under the hood, this calls objc_sync_enter and objc_sync_exit, wrapped
-      // in a @try/@finally block. Diagnose this somewhat generically as "ObjC"
-      // messaging.
+      // in a @try/@finally block. Diagnose this generically as "ObjC
+      // messaging".
       diagnoseLanguageConstruct(FunctionEffect::FE_ExcludeObjCMessageSend,
                                 ViolationID::AccessesObjCMethodOrProperty,
                                 Sync->getBeginLoc());
diff --git a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
index cb8c7c3f3d07c0..d1c9b3cc98a1c0 100644
--- a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
+++ b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
@@ -36,4 +36,4 @@ void nb5() [[clang::nonblocking]] {
 
 	@synchronized(someLock) { // expected-warning {{function with 'nonblocking' attribute must not access ObjC methods or properties}}
 	}
-}
\ No newline at end of file
+}