llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Med Ismail Bennani (medismailben) <details> <summary>Changes</summary> When a protocol method is annotated with `DesignatedInit: true` in an .apinotes file, `APINotesWriter::addObjCMethod` would crash with: Assertion failed: (Implementation->Contexts.contains(CtxKey)), function addObjCMethod The bug was introduced in 8dc789a2: when reconstructing the `ContextTableKey` to mark the parent context as having designated initializers, the code hardcoded `ContextKind::ObjCClass`. This is wrong when the context is a protocol — the key won't be found in `Implementation->Contexts`, triggering the assertion. Fix by storing the context kind in a new `ContextKinds` map on the `Implementation` struct (populated in `addContext`), then using the stored kind in `addObjCMethod` instead of the hardcoded value. rdar://171361188 --- Full diff: https://github.com/llvm/llvm-project/pull/183799.diff 4 Files Affected: - (modified) clang/lib/APINotes/APINotesWriter.cpp (+5-1) - (modified) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes (+6) - (modified) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h (+4) - (added) clang/test/APINotes/objc_designated_init_protocol.m (+15) ``````````diff diff --git a/clang/lib/APINotes/APINotesWriter.cpp b/clang/lib/APINotes/APINotesWriter.cpp index 47ed93a567c0e..390aea57f3915 100644 --- a/clang/lib/APINotes/APINotesWriter.cpp +++ b/clang/lib/APINotes/APINotesWriter.cpp @@ -50,6 +50,9 @@ class APINotesWriter::Implementation { /// Indexed by context ID, provides the parent context ID. llvm::DenseMap<uint32_t, uint32_t> ParentContexts; + /// Mapping from context IDs to the kind of context. + llvm::DenseMap<unsigned, uint8_t> ContextKinds; + /// Mapping from context IDs to the identifier ID holding the name. llvm::DenseMap<unsigned, unsigned> ContextNames; @@ -1461,6 +1464,7 @@ ContextID APINotesWriter::addContext(std::optional<ContextID> ParentCtxID, Implementation->ContextNames[NextID] = NameID; Implementation->ParentContexts[NextID] = RawParentCtxID; + Implementation->ContextKinds[NextID] = static_cast<uint8_t>(Kind); } // Add this version information. @@ -1505,7 +1509,7 @@ void APINotesWriter::addObjCMethod(ContextID CtxID, ObjCSelectorRef Selector, assert(Implementation->ParentContexts.contains(CtxID.Value)); uint32_t ParentCtxID = Implementation->ParentContexts[CtxID.Value]; ContextTableKey CtxKey(ParentCtxID, - static_cast<uint8_t>(ContextKind::ObjCClass), + Implementation->ContextKinds[CtxID.Value], Implementation->ContextNames[CtxID.Value]); assert(Implementation->Contexts.contains(CtxKey)); auto &VersionedVec = Implementation->Contexts[CtxKey].second; diff --git a/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes index 817af123fc77b..79f0bdb44371c 100644 --- a/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes +++ b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes @@ -45,6 +45,12 @@ Classes: - Name: intPropertyToMangle PropertyKind: Instance Type: 'double *' +Protocols: + - Name: InitializableProtocol + Methods: + - Selector: "initWithValue:" + MethodKind: Instance + DesignatedInit: true Functions: - Name: global_int_fun ResultType: 'char *' diff --git a/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h index 1a192f5432fd1..0af2c03aa1597 100644 --- a/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h +++ b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h @@ -49,6 +49,10 @@ __attribute__((objc_root_class)) @property int *intPropertyToMangle; @end +@protocol InitializableProtocol +- (instancetype)initWithValue:(int)value; +@end + @interface A(ImplicitGetterSetters) @property (nonatomic, readonly, retain) A *implicitGetOnlyInstance; @property (class, nonatomic, readonly, retain) A *implicitGetOnlyClass; diff --git a/clang/test/APINotes/objc_designated_init_protocol.m b/clang/test/APINotes/objc_designated_init_protocol.m new file mode 100644 index 0000000000000..ff8c9b298c5b5 --- /dev/null +++ b/clang/test/APINotes/objc_designated_init_protocol.m @@ -0,0 +1,15 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache -fapinotes-modules -Wno-private-module -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -verify + +// Regression test for APINotesWriter::addObjCMethod asserting when a protocol +// method is annotated with DesignatedInit: true in an .apinotes file. +// The writer was reconstructing the ContextTableKey with a hardcoded +// ContextKind::ObjCClass, causing the Contexts lookup to fail when the +// context was actually a protocol. Just importing the module is enough to +// trigger the writer path that previously crashed. + +// expected-no-diagnostics + +#import <SomeKit/SomeKit.h> + +void useProtocol(id<InitializableProtocol> p) {} `````````` </details> https://github.com/llvm/llvm-project/pull/183799 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
