[clang] [WIP][ObjC] objc_direct method visibility ABI (PR #76608)
Puyan Lotfi via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 29 23:43:01 PST 2023
https://github.com/plotfi created https://github.com/llvm/llvm-project/pull/76608
_**Posting this PR for posterity a bit earlier than I had intended because he old Phabricator is crashing (https://discourse.llvm.org/t/cant-access-https-reviews-llvm-org/75905):**_
This patch adds the ability to expose direct ObjC methods as visible across link boundaries (dylibs). It is done so by using existing visibility attributes:
```
@interface C
- (void)testMethod:(int)arg1 bar:(float)arg2
__attribute__((objc_direct))
__attribute__((visibility("default")));
@end
```
This is a work in progress and there are a few pieces that should be landed with this within an LLVM release cycle so as not to cause any inconsistency in the ABI:
* First, we want some checks that inform the developer when they are using this feature as part of a framework that this is potentially brittle. Something similar to the following: https://github.com/llvm/llvm-project/commit/1b3b69fbda70d
* Next, we want to move the ObjC method nil checks from the callee side to something more useful for analysis and optimization that can be more visible from the caller side. Having the nil checks straight up in the caller side isn't so great for code size so a good alternative is to have a thunk between the call site and the callee for handling nil checks.
* Finally, any mangling decisions should be finalized before this is landed.
Much of this has been discussed ahead of time, and there is a more extensive document on this proposal at:
https://docs.google.com/document/d/16CsNCA2OKWkUM_LCw_qabTcMtPiq9ODVkZHALlDvzR8
>From 5e69de7ad9b67280db6d62a1c77362d37c343f47 Mon Sep 17 00:00:00 2001
From: Puyan Lotfi <puyan at puyan.org>
Date: Fri, 29 Dec 2023 23:13:02 -0800
Subject: [PATCH] [WIP][ObjC] objc_direct method visibility ABI
This patch adds the ability to expose direct ObjC methods as visible
across link boundaries (dylibs). It is done so by using existing
visibility attributes:
```
@interface C
- (void)testMethod:(int)arg1 bar:(float)arg2
__attribute__((objc_direct))
__attribute__((visibility("default")));
@end
```
This is a work in progress and there are a few pieces that should be
landed with this within an LLVM release cycle so as not to cause any
inconsistency in the ABI:
* First, we want some checks that inform the developer when they are
using this feature as part of a framework that this is potentially
brittle. Something similar to the following:
https://github.com/llvm/llvm-project/commit/1b3b69fbda70d
* Next, we want to move the ObjC method nil checks from the callee
side to something more useful for analysis and optimization that can be
more visible from the caller side. Having the nil checks straight up
in the caller side isn't so great for code size so a good
alternative is to have a thunk between the call site and the callee
for handling nil checks.
* Finally, any mangling decisions should be finalized before this is
landed.
Much of this has been discussed ahead of time, and there is a more
extensive document on this proposal at:
https://docs.google.com/document/d/16CsNCA2OKWkUM_LCw_qabTcMtPiq9ODVkZHALlDvzR8
---
clang/include/clang/AST/DeclObjC.h | 14 ++++
clang/lib/AST/Mangle.cpp | 10 ++-
clang/lib/CodeGen/CGObjC.cpp | 4 +-
clang/lib/CodeGen/CGObjCRuntime.cpp | 3 +-
clang/test/CodeGenObjC/objc-direct-wrapper.m | 86 ++++++++++++++++++++
5 files changed, 113 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenObjC/objc-direct-wrapper.m
diff --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h
index f8f894b4b10d19..9a12b4007f09f6 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/include/clang/AST/DeclObjC.h
@@ -482,6 +482,20 @@ class ObjCMethodDecl : public NamedDecl, public DeclContext {
/// True if the method is tagged as objc_direct
bool isDirectMethod() const;
+ /// True if method is meant to be visible.
+ ///
+ /// TODO: For now this is only true for a very narrow case where the method
+ /// must be direct and must also be explicitly marked as
+ /// __attribute__((visibility("default"))). In the future it may be possible
+ /// to assume that all direct (or even both direct and indirect) methods are
+ /// visible but for the time being it is prudent to provide this default
+ /// visibility behavior as an opt-in only.
+ bool hasMethodVisibilityDefault() const {
+ return isDirectMethod() &&
+ getExplicitVisibility(VisibilityForValue)
+ .value_or(HiddenVisibility) == DefaultVisibility;
+ }
+
/// True if the method has a parameter that's destroyed in the callee.
bool hasParamDestroyedInCallee() const;
diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp
index d3a6b61fd2bec9..ec409da0e5da00 100644
--- a/clang/lib/AST/Mangle.cpp
+++ b/clang/lib/AST/Mangle.cpp
@@ -363,10 +363,16 @@ void MangleContext::mangleObjCMethodName(const ObjCMethodDecl *MD,
}
// \01+[ContainerName(CategoryName) SelectorName]
+ // Note: If we are mangling for an objc_direct method with external visibility
+ // then the standard mangling scheme will not work. We must replace the
+ // square braces with angle braces so in the case of visible direct objc
+ // methods it results in: +<ContainerName(CategoryName) SelectorName>
+ // This will include a prefix _ on Darwin.
if (includePrefixByte) {
OS << '\01';
}
- OS << (MD->isInstanceMethod() ? '-' : '+') << '[';
+ OS << (MD->isInstanceMethod() ? '-' : '+');
+ OS << (MD->hasMethodVisibilityDefault() ? '<' : '[');
if (const auto *CID = MD->getCategory()) {
OS << CID->getClassInterface()->getName();
if (includeCategoryNamespace) {
@@ -380,7 +386,7 @@ void MangleContext::mangleObjCMethodName(const ObjCMethodDecl *MD,
}
OS << ' ';
MD->getSelector().print(OS);
- OS << ']';
+ OS << (MD->hasMethodVisibilityDefault() ? '>' : ']');
}
void MangleContext::mangleObjCMethodNameAsSourceName(const ObjCMethodDecl *MD,
diff --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index acc85165a470be..3dcd5a8334d102 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -761,7 +761,9 @@ void CodeGenFunction::StartObjCMethod(const ObjCMethodDecl *OMD,
const CGFunctionInfo &FI = CGM.getTypes().arrangeObjCMethodDeclaration(OMD);
if (OMD->isDirectMethod()) {
- Fn->setVisibility(llvm::Function::HiddenVisibility);
+ Fn->setVisibility(OMD->hasMethodVisibilityDefault()
+ ? llvm::Function::DefaultVisibility
+ : llvm::Function::HiddenVisibility);
CGM.SetLLVMFunctionAttributes(OMD, FI, Fn, /*IsThunk=*/false);
CGM.SetLLVMFunctionAttributesForDefinition(OMD, Fn);
} else {
diff --git a/clang/lib/CodeGen/CGObjCRuntime.cpp b/clang/lib/CodeGen/CGObjCRuntime.cpp
index 424564f9759995..1841c8ed23b5db 100644
--- a/clang/lib/CodeGen/CGObjCRuntime.cpp
+++ b/clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -471,7 +471,8 @@ std::string CGObjCRuntime::getSymbolNameForMethod(const ObjCMethodDecl *OMD,
std::string buffer;
llvm::raw_string_ostream out(buffer);
CGM.getCXXABI().getMangleContext().mangleObjCMethodName(OMD, out,
- /*includePrefixByte=*/true,
+ /*includePrefixByte=*/
+ !OMD->hasMethodVisibilityDefault(),
includeCategoryName);
return buffer;
}
diff --git a/clang/test/CodeGenObjC/objc-direct-wrapper.m b/clang/test/CodeGenObjC/objc-direct-wrapper.m
new file mode 100644
index 00000000000000..2c67f252b41dfd
--- /dev/null
+++ b/clang/test/CodeGenObjC/objc-direct-wrapper.m
@@ -0,0 +1,86 @@
+// RUN: %clang -fobjc-arc -Wno-objc-root-class -ObjC -fobjc-runtime=ios -FFoundation \
+// RUN: -DENABLE_VISIBLE_OBJC_DIRECT=1 \
+// RUN: -target x86_64-apple-macosx10.15.0 -c -o - %s | \
+// RUN: llvm-nm - | FileCheck -check-prefix=CHECK-WRAPPER %s
+
+// RUN: %clang -fobjc-arc -Wno-objc-root-class \
+// RUN: -target x86_64-apple-macosx10.15.0 \
+// RUN: -ObjC -fobjc-runtime=ios -FFoundation -c -o - %s | llvm-nm - | \
+// RUN: FileCheck -check-prefix=CHECK-DEFAULT %s
+
+// RUN: %clang -fobjc-arc -Wno-objc-root-class \
+// RUN: -DENABLE_VISIBLE_OBJC_DIRECT=1 \
+// RUN: -target x86_64-apple-macosx10.15.0 \
+// RUN: -ObjC -fobjc-runtime=ios -FFoundation -c -o - %s -S -emit-llvm | \
+// RUN: FileCheck -check-prefix=CHECK-WRAPPER-IR-DEF %s
+
+// RUN: %clang -fobjc-arc -Wno-objc-root-class -DNO_OBJC_IMPL \
+// RUN: -DENABLE_VISIBLE_OBJC_DIRECT=1 \
+// RUN: -target x86_64-apple-macosx10.15.0 \
+// RUN: -ObjC -fobjc-runtime=ios -FFoundation -c -o - %s -S -emit-llvm | \
+// RUN: FileCheck -check-prefix=CHECK-WRAPPER-IR-DECLARE %s
+
+// RUN: not %clang -fobjc-arc -Wno-objc-root-class -DENABLE_PROTOCOL_DIRECT_FAIL \
+// RUN: -DENABLE_VISIBLE_OBJC_DIRECT=1 \
+// RUN: -target x86_64-apple-macosx10.15.0 \
+// RUN: -ObjC -fobjc-runtime=ios -FFoundation -c -o - %s -S -emit-llvm 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-PROTOCOL-DIRECT-FAIL %s
+
+////////////////////////////////////////////////////////////////////////////////
+
+// RUN: %clang -fobjc-arc -Wno-objc-root-class -ObjC -fobjc-runtime=ios -FFoundation \
+// RUN: -DENABLE_VISIBLE_OBJC_DIRECT=0 \
+// RUN: -target x86_64-apple-macosx10.15.0 -c -o - %s | \
+// RUN: llvm-nm - | FileCheck -check-prefix=CHECK-WRAPPER-INDIRECT %s
+
+// RUN: %clang -fobjc-arc -Wno-objc-root-class \
+// RUN: -DENABLE_VISIBLE_OBJC_DIRECT=0 \
+// RUN: -target x86_64-apple-macosx10.15.0 \
+// RUN: -ObjC -fobjc-runtime=ios -FFoundation -c -o - %s -S -emit-llvm | \
+// RUN: FileCheck -check-prefix=CHECK-WRAPPER-IR-DEF-INDIRECT %s
+
+// RUN: %clang -fobjc-arc -Wno-objc-root-class -DNO_OBJC_IMPL \
+// RUN: -DENABLE_VISIBLE_OBJC_DIRECT=0 \
+// RUN: -target x86_64-apple-macosx10.15.0 \
+// RUN: -ObjC -fobjc-runtime=ios -FFoundation -c -o - %s -S -emit-llvm | \
+// RUN: FileCheck -check-prefix=CHECK-WRAPPER-IR-DECLARE-INDIRECT %s
+
+// CHECK-WRAPPER: T _-<C testMethod:bar:>
+ // TODO: Fix this
+// CHECK-DEFAULT: t -[C testMethod:bar:]
+// CHECK-WRAPPER-IR-DEF: define {{(dso_local )?}}void @"-<C testMethod:bar:>"
+// CHECK-WRAPPER-IR-DECLARE: declare {{(dso_local )?}}void @"-<C testMethod:bar:>"
+// CHECK-PROTOCOL-DIRECT-FAIL: error: 'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol
+
+// CHECK-WRAPPER-INDIRECT-NOT: T _-<C testMethod:bar:>
+// CHECK-WRAPPER-IR-DEF-INDIRECT-NOT: define {{(dso_local )?}}void @"-<C testMethod:bar:>"
+// CHECK-WRAPPER-IR-DECLARE-INDIRECT-NOT: declare {{(dso_local )?}}void @"-<C testMethod:bar:>"
+
+#if ENABLE_VISIBLE_OBJC_DIRECT
+#define OBJC_DIRECT __attribute__((objc_direct)) __attribute__((visibility("default")))
+#else
+#define OBJC_DIRECT
+#endif
+
+ at interface C
+- (void)testMethod:(int)arg1 bar:(float)arg2 OBJC_DIRECT;
+ at end
+
+#ifndef NO_OBJC_IMPL
+ at implementation C
+- (void)testMethod:(int)arg1 bar:(float)arg2 OBJC_DIRECT {
+}
+ at end
+#endif
+
+#ifdef ENABLE_PROTOCOL_DIRECT_FAIL
+ at protocol ProtoDirectVisibleFail
+- (void)protoMethod OBJC_DIRECT; // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+ at end
+#endif
+
+C *c;
+
+void f() {
+ [c testMethod:1 bar:1.0];
+}
More information about the cfe-commits
mailing list