From addcfac0307b815a96e889d88fb1d6da8836afd2 Mon Sep 17 00:00:00 2001 From: PyTorch Bot Date: Mon, 31 Aug 2026 22:00:30 -0700 Subject: [PATCH] Pass the Apple event tracer's C++ handle opaquely across the framework boundary The event tracer's construction seam took and returned a std::unique_ptr through a category in a private ObjC++ header, Internal/ExecuTorchEventTracer+Internal.h. The core module used it within the target, and the ETDump framework, a separate target, subclassed the tracer and called it across the framework boundary. That cross-target C++ header is the problem. Under Buck a private Internal header is not visible to another target, and exporting it pulls a C++-typed header into the module umbrella, which the Swift module precompile cannot parse. It also let the two targets hold independent declarations of a C++ signature whose layout varies with ET_EVENT_TRACER_ENABLED, an unchecked ABI handshake. Cross the boundary the way ExecuTorchTensor, Value and Module already do: an opaque handle. The base class takes a void * designated initializer pointing at the std::unique_ptr and exposes it back through a nativeInstance property, both NS_SWIFT_UNAVAILABLE. No shared header carries a C++ type, so the private seam header is deleted, its cross-target export is removed, and the dump subclass hands its tracer up through the public initializer with no private import and no designated-initializer warning to suppress. Test Plan: Compiled the ObjC++ pattern (derived-to-base unique_ptr handoff, opaque move-out) with ARC on Xcode 26; it builds warning-clean and moves the tracer through correctly. The shape mirrors ExecuTorchTensor's initWithNativeInstance:, which ships today. --- extension/apple/BUCK | 5 -- .../Exported/ExecuTorchEventTracer.h | 18 +++++++ .../Exported/ExecuTorchEventTracer.mm | 16 ++++--- .../ExecuTorch/Exported/ExecuTorchModule.mm | 6 ++- .../Internal/ExecuTorchEventTracer+Internal.h | 48 ------------------- extension/apple/dump/CMakeLists.txt | 10 ++-- .../Exported/ExecuTorchDumpTracer.mm | 18 +++---- 7 files changed, 41 insertions(+), 80 deletions(-) delete mode 100644 extension/apple/ExecuTorch/Internal/ExecuTorchEventTracer+Internal.h diff --git a/extension/apple/BUCK b/extension/apple/BUCK index 3288cbdfcdf..0c04eea9ca1 100644 --- a/extension/apple/BUCK +++ b/extension/apple/BUCK @@ -8,11 +8,6 @@ oncall("executorch") non_fbcode_target(_kind = fb_apple_library, name = "ExecuTorch", - # The ETDump extension subclasses ExecuTorchEventTracer, which needs the C++ - # tracer seam this header declares, so export it beyond the target. - autoglob_additional_exported_headers = [ - "ExecuTorch/Internal/ExecuTorchEventTracer+Internal.h", - ], autoglob_mode = "EXPORT_UNLESS_INTERNAL", extension_api_only = True, frameworks = [ diff --git a/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.h b/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.h index e168e8ce5ef..4b19b38a0d5 100644 --- a/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.h +++ b/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.h @@ -30,6 +30,24 @@ NS_SWIFT_NAME(EventTracer) + (instancetype)new NS_UNAVAILABLE; - (instancetype)init NS_UNAVAILABLE; +/** + * Initializes a tracer with a native std::unique_ptr instance. + * + * @param nativeInstance A pointer to a native std::unique_ptr + * instance. + * @return An initialized ExecuTorchEventTracer instance. + */ +- (instancetype)initWithNativeInstance:(void *)nativeInstance + NS_DESIGNATED_INITIALIZER NS_SWIFT_UNAVAILABLE(""); + +/** + * Pointer to the underlying native std::unique_ptr instance. + * + * @return A raw pointer to the native std::unique_ptr held by this + * class. + */ +@property(nonatomic, readonly) void *nativeInstance NS_SWIFT_UNAVAILABLE(""); + @end NS_ASSUME_NONNULL_END diff --git a/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.mm b/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.mm index 5429a541bcd..f202ae0997b 100644 --- a/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.mm +++ b/extension/apple/ExecuTorch/Exported/ExecuTorchEventTracer.mm @@ -8,7 +8,7 @@ #import "ExecuTorchEventTracer.h" -#import "ExecuTorchEventTracer+Internal.h" +#import using executorch::runtime::EventTracer; @@ -16,16 +16,18 @@ @implementation ExecuTorchEventTracer { std::unique_ptr _tracer; } -- (instancetype)initWithCppTracer:(std::unique_ptr)tracer { - self = [super init]; - if (self) { - _tracer = std::move(tracer); +- (instancetype)initWithNativeInstance:(void *)nativeInstance { + ET_CHECK(nativeInstance); + if (self = [super init]) { + _tracer = std::move( + *reinterpret_cast *>(nativeInstance)); + ET_CHECK(_tracer); } return self; } -- (std::unique_ptr)takeCppTracer { - return std::move(_tracer); +- (void *)nativeInstance { + return &_tracer; } @end diff --git a/extension/apple/ExecuTorch/Exported/ExecuTorchModule.mm b/extension/apple/ExecuTorch/Exported/ExecuTorchModule.mm index 83561030a93..ce2b86406b7 100644 --- a/extension/apple/ExecuTorch/Exported/ExecuTorchModule.mm +++ b/extension/apple/ExecuTorch/Exported/ExecuTorchModule.mm @@ -12,7 +12,6 @@ #import "ExecuTorchBackendOptionsMap+Internal.h" #import "ExecuTorchBackendOptionsMap.h" #import "ExecuTorchError.h" -#import "ExecuTorchEventTracer+Internal.h" #import "ExecuTorchUtils.h" #import @@ -330,7 +329,10 @@ - (instancetype)initWithFilePath:(NSString *)filePath // Taking it empties the handle, so a tracer handed to a second module would // yield nothing and that module would run and record silently into nowhere. // Catch it here rather than letting it surface as an empty profile. - auto cppTracer = [eventTracer takeCppTracer]; + auto *cppTracerPtr = reinterpret_cast *>( + eventTracer.nativeInstance); + ET_CHECK(cppTracerPtr); + auto cppTracer = std::move(*cppTracerPtr); NSAssert(cppTracer != nullptr, @"This event tracer was already given to another module. Create one " @"tracer per module."); diff --git a/extension/apple/ExecuTorch/Internal/ExecuTorchEventTracer+Internal.h b/extension/apple/ExecuTorch/Internal/ExecuTorchEventTracer+Internal.h deleted file mode 100644 index f220dceef63..00000000000 --- a/extension/apple/ExecuTorch/Internal/ExecuTorchEventTracer+Internal.h +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. - */ - -#if __has_include() -#import -#else -#import "ExecuTorchEventTracer.h" -#endif - -#ifdef __cplusplus - -#import - -#import - -NS_ASSUME_NONNULL_BEGIN - -// Internal extension header exposing the underlying C++ tracer to other ObjC++ -// translation units in this module (ExecuTorchModule.mm) and to concrete tracer -// subclasses in other frameworks. Not part of the public umbrella header. The -// C++ types in the method signatures mean this header is ObjC++-only, guarded -// against accidental import from a `.m` file. -@interface ExecuTorchEventTracer (Internal) - -/** - * Wraps a C++ tracer. A concrete subclass calls this to hand its implementation - * to the base class, which owns it until a module takes it. - */ -- (instancetype)initWithCppTracer: - (std::unique_ptr)tracer; - -/** - * Moves the C++ tracer out of the receiver. Called once, by the module that - * takes ownership of the tracer at construction. The receiver holds nothing - * after this. - */ -- (std::unique_ptr)takeCppTracer; - -@end - -NS_ASSUME_NONNULL_END - -#endif // __cplusplus diff --git a/extension/apple/dump/CMakeLists.txt b/extension/apple/dump/CMakeLists.txt index 38ea0586f5b..e6fdf0706fe 100644 --- a/extension/apple/dump/CMakeLists.txt +++ b/extension/apple/dump/CMakeLists.txt @@ -25,15 +25,13 @@ target_sources(extension_etdump_apple PRIVATE ${OBJC_SOURCES}) # The public header exposes ExecuTorchModule and so imports it as # , the path a consumer of the assembled # framework uses. That layout does not exist during this build, so stage the -# core Exported and Internal headers under one ExecuTorch directory and put it -# on the include path, mirroring what the framework packaging produces. Staging -# them together in one directory is what lets the Internal header's own quoted -# import of "ExecuTorchModule.h" find its sibling, and keeps the module header -# reachable by a single path so it is not seen as two conflicting definitions. +# core Exported headers under one ExecuTorch directory and put it on the include +# path, mirroring what the framework packaging produces. Staging them together +# in one directory keeps the module header reachable by a single path so it is +# not seen as two conflicting definitions. set(_etdump_core_headers_dir ${CMAKE_CURRENT_BINARY_DIR}/include) file(GLOB _etdump_core_headers ${EXECUTORCH_ROOT}/extension/apple/ExecuTorch/Exported/*.h - ${EXECUTORCH_ROOT}/extension/apple/ExecuTorch/Internal/*.h ) # configure_file(... COPYONLY) rather than file(COPY): it registers each source # header as a reconfigure dependency, so editing a core header re-stages it. A diff --git a/extension/apple/dump/ExecuTorchDump/Exported/ExecuTorchDumpTracer.mm b/extension/apple/dump/ExecuTorchDump/Exported/ExecuTorchDumpTracer.mm index 2b9e0b40600..6e33bdd89a6 100644 --- a/extension/apple/dump/ExecuTorchDump/Exported/ExecuTorchDumpTracer.mm +++ b/extension/apple/dump/ExecuTorchDump/Exported/ExecuTorchDumpTracer.mm @@ -11,8 +11,6 @@ #import "ExecuTorchDumpError.h" #import "ExecuTorchDumpTracer+Internal.h" -#import - #import using namespace executorch::etdump; @@ -39,25 +37,21 @@ @implementation ExecuTorchDumpTracer { NSLock *_lock; } -// init chains to the base class's real designated initializer, initWithCppTracer:, -// which takes a C++ type and so lives in an Objective-C++ category rather than the -// public interface. Clang cannot see a category initializer as designated, so it -// wrongly flags this chain; the pattern is deliberate and correct. -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wobjc-designated-initializers" - - (instancetype)init { auto generator = std::make_unique(); _generator = generator.get(); - self = [super initWithCppTracer:std::move(generator)]; + // The base moves out of the pointee as a std::unique_ptr, so the + // local must be that type, not unique_ptr. _generator keeps the + // concrete pointer for readback. + std::unique_ptr tracer = + std::move(generator); + self = [super initWithNativeInstance:&tracer]; if (self) { _lock = [NSLock new]; } return self; } -#pragma clang diagnostic pop - - (ETDumpGen *)generator { return _generator; }