vsapsai created this revision.
vsapsai added reviewers: rsmith, bruno, jansvoboda11.
Herald added a subscriber: ributzka.
vsapsai requested review of this revision.
Herald added a project: clang.
When have ObjCInterfaceDecl with the same name in 2 different modules,
hitting the assertion
> Assertion failed: (Index < RL->getFieldCount() && "Ivar is not inside record
> layout!"),
> function lookupFieldBitOffset, file
> llvm-project/clang/lib/AST/RecordLayoutBuilder.cpp, line 3434.
on accessing an ivar inside a method. The assertion happens because
ivar belongs to one module while its containing interface belongs to
another module and then we fail to find the ivar inside the containing
interface. We already keep a single ObjCInterfaceDecl definition in
redecleration chain and in this case containing interface was correct.
The issue is with ObjCIvarDecl. IVar decl for IRGen is taken from
ObjCIvarRefExpr that is created in `Sema::BuildIvarRefExpr` using ivar
decl returned from `Sema::LookupIvarInObjCMethod`. And ivar lookup
returns a wrong decl because basically we take the first ObjCIvarDecl
found in `ASTReader::FindExternalVisibleDeclsByName` (called by
`DeclContext::lookup`). And in `ASTReader.Lookups` lookup table for a
wrong module comes first because `ASTReader::finishPendingActions`
processes `PendingUpdateRecords` in reverse order and the first
encountered ObjCIvarDecl will end up the last in `ASTReader.Lookups`.
Fix by merging ObjCIvarDecl from different modules correctly and by
using a canonical one in name lookup.
rdar://82854574
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D110280
Files:
clang/lib/AST/DeclObjC.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/Modules/merge-objc-interface.m
Index: clang/test/Modules/merge-objc-interface.m
===================================================================
--- /dev/null
+++ clang/test/Modules/merge-objc-interface.m
@@ -0,0 +1,96 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%t/Frameworks %t/test.m \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%t/Frameworks %t/test-functions.m \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test a case when Objective-C interface ivars are present in two different modules.
+
+//--- Frameworks/Foundation.framework/Headers/Foundation.h
+@interface NSObject
+@end
+
+//--- Frameworks/Foundation.framework/Modules/module.modulemap
+framework module Foundation {
+ header "Foundation.h"
+ export *
+}
+
+//--- Frameworks/ObjCInterface.framework/Headers/ObjCInterface.h
+#import <Foundation/Foundation.h>
+@interface ObjCInterface : NSObject {
+@public
+ id _item;
+}
+@end
+
+@interface WithBitFields : NSObject {
+@public
+ int x: 3;
+ int y: 4;
+}
+@end
+
+//--- Frameworks/ObjCInterface.framework/Modules/module.modulemap
+framework module ObjCInterface {
+ header "ObjCInterface.h"
+ export *
+}
+
+//--- Frameworks/ObjCInterfaceCopy.framework/Headers/ObjCInterfaceCopy.h
+#import <Foundation/Foundation.h>
+@interface ObjCInterface : NSObject {
+@public
+ id _item;
+}
+@end
+
+@interface WithBitFields : NSObject {
+@public
+ int x: 3;
+ int y: 4;
+}
+@end
+
+//--- Frameworks/ObjCInterfaceCopy.framework/Modules/module.modulemap
+framework module ObjCInterfaceCopy {
+ header "ObjCInterfaceCopy.h"
+ export *
+}
+
+//--- test.m
+#import <ObjCInterface/ObjCInterface.h>
+#import <ObjCInterfaceCopy/ObjCInterfaceCopy.h>
+
+@implementation ObjCInterface
+- (void)test:(id)item {
+ _item = item;
+}
+@end
+
+@implementation WithBitFields
+- (void)reset {
+ x = 0;
+ y = 0;
+}
+@end
+
+//--- test-functions.m
+#import <ObjCInterface/ObjCInterface.h>
+
+void testAccessIVar(ObjCInterface *obj, id item) {
+ obj->_item = item;
+}
+void testAccessBitField(WithBitFields *obj) {
+ obj->x = 0;
+}
+
+#import <ObjCInterfaceCopy/ObjCInterfaceCopy.h>
+
+void testAccessIVarLater(ObjCInterface *obj, id item) {
+ obj->_item = item;
+}
+void testAccessBitFieldLater(WithBitFields *obj) {
+ obj->y = 0;
+}
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3350,6 +3350,9 @@
return ED->getASTContext().getLangOpts().CPlusPlus? ED->getDefinition()
: nullptr;
+ if (auto *OID = dyn_cast<ObjCInterfaceDecl>(DC))
+ return OID->getDefinition();
+
// We can see the TU here only if we have no Sema object. In that case,
// there's no TU scope to look in, so using the DC alone is sufficient.
if (auto *TU = dyn_cast<TranslationUnitDecl>(DC))
Index: clang/lib/AST/DeclObjC.cpp
===================================================================
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -81,7 +81,7 @@
lookup_result R = lookup(Id);
for (lookup_iterator Ivar = R.begin(), IvarEnd = R.end();
Ivar != IvarEnd; ++Ivar) {
- if (auto *ivar = dyn_cast<ObjCIvarDecl>(*Ivar))
+ if (auto *ivar = dyn_cast<ObjCIvarDecl>((*Ivar)->getCanonicalDecl()))
return ivar;
}
return nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
