[Clang] Diagnose additional ObjC statements as function effect violations

[dougsonos]

Bug fix: @autoreleasepool, @synchronized, and @finally were not being noticed and treated as function effect violations.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Doug Wyatt (dougsonos)

Changes

Bug fix: @<!-- -->autoreleasepool, @<!-- -->synchronized, and @<!-- -->finally were not being noticed and treated as function effect violations.

---

Full diff: https://github.com/llvm/llvm-project/pull/112148.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaFunctionEffects.cpp (+27)
• (modified) clang/test/SemaObjCXX/attr-nonblocking-constraints.mm (+13)

diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp
index 0ac5de29f66aa7..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,
@@ -1140,6 +1147,26 @@ 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 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 abd0938ac321af..cb8c7c3f3d07c0 100644
--- a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
+++ b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm
@@ -23,4 +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}}
+	}
 }
+
+@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

[Sirraide]

LGTM

Using just ‘ObjC messaging’ seems fine to me—though admittedly, I also don’t know much about Objective C.