[clang] 9fad9de - [modules] Fix IRGen assertion on accessing ObjC ivar inside a method.
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 7 17:09:56 PDT 2021
Author: Volodymyr Sapsai
Date: 2021-10-07T17:09:31-07:00
New Revision: 9fad9de5c0032898a481e06bf5f696ca50c804c1
URL: https://github.com/llvm/llvm-project/commit/9fad9de5c0032898a481e06bf5f696ca50c804c1
DIFF: https://github.com/llvm/llvm-project/commit/9fad9de5c0032898a481e06bf5f696ca50c804c1.diff
LOG: [modules] Fix IRGen assertion on accessing ObjC ivar inside a method.
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 IRGen.
rdar://82854574
Differential Revision: https://reviews.llvm.org/D110280
Added:
clang/test/Modules/merge-objc-interface.m
Modified:
clang/include/clang/AST/DeclObjC.h
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h
index 6bb9cdf67034d..f484dfedbf544 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/include/clang/AST/DeclObjC.h
@@ -1955,6 +1955,13 @@ class ObjCIvarDecl : public FieldDecl {
const ObjCIvarDecl *getNextIvar() const { return NextIvar; }
void setNextIvar(ObjCIvarDecl *ivar) { NextIvar = ivar; }
+ ObjCIvarDecl *getCanonicalDecl() override {
+ return cast<ObjCIvarDecl>(FieldDecl::getCanonicalDecl());
+ }
+ const ObjCIvarDecl *getCanonicalDecl() const {
+ return const_cast<ObjCIvarDecl *>(this)->getCanonicalDecl();
+ }
+
void setAccessControl(AccessControl ac) { DeclAccess = ac; }
AccessControl getAccessControl() const { return AccessControl(DeclAccess); }
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index d49b0aec42fec..d17d1e26742c2 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3404,6 +3404,7 @@ uint64_t ASTContext::getFieldOffset(const ValueDecl *VD) const {
uint64_t ASTContext::lookupFieldBitOffset(const ObjCInterfaceDecl *OID,
const ObjCImplementationDecl *ID,
const ObjCIvarDecl *Ivar) const {
+ Ivar = Ivar->getCanonicalDecl();
const ObjCInterfaceDecl *Container = Ivar->getContainingInterface();
// FIXME: We should eliminate the need to have ObjCImplementationDecl passed
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index d011d14612d13..f2c2023cbf233 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3350,6 +3350,9 @@ DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader,
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))
diff --git a/clang/test/Modules/merge-objc-interface.m b/clang/test/Modules/merge-objc-interface.m
new file mode 100644
index 0000000000000..fba06294a26af
--- /dev/null
+++ b/clang/test/Modules/merge-objc-interface.m
@@ -0,0 +1,105 @@
+// 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
diff erent modules.
+
+//--- Frameworks/Foundation.framework/Headers/Foundation.h
+ at interface NSObject
+ at end
+
+//--- Frameworks/Foundation.framework/Modules/module.modulemap
+framework module Foundation {
+ header "Foundation.h"
+ export *
+}
+
+//--- Frameworks/ObjCInterface.framework/Headers/ObjCInterface.h
+#import <Foundation/Foundation.h>
+ at interface ObjCInterface : NSObject {
+ at public
+ id _item;
+}
+ at end
+
+ at interface WithBitFields : NSObject {
+ at public
+ int x: 3;
+ int y: 4;
+}
+ at end
+
+//--- Frameworks/ObjCInterface.framework/Modules/module.modulemap
+framework module ObjCInterface {
+ header "ObjCInterface.h"
+ export *
+}
+
+//--- Frameworks/ObjCInterfaceCopy.framework/Headers/ObjCInterfaceCopy.h
+#import <Foundation/Foundation.h>
+ at interface ObjCInterface : NSObject {
+ at public
+ id _item;
+}
+ at end
+
+ at interface WithBitFields : NSObject {
+ at public
+ int x: 3;
+ int y: 4;
+}
+ at end
+
+// Inlined function present only in Copy.framework to make sure it uses decls from Copy module.
+__attribute__((always_inline)) void inlinedIVarAccessor(ObjCInterface *obj, WithBitFields *bitFields) {
+ obj->_item = 0;
+ bitFields->x = 0;
+}
+
+//--- Frameworks/ObjCInterfaceCopy.framework/Modules/module.modulemap
+framework module ObjCInterfaceCopy {
+ header "ObjCInterfaceCopy.h"
+ export *
+}
+
+//--- test.m
+#import <ObjCInterface/ObjCInterface.h>
+#import <ObjCInterfaceCopy/ObjCInterfaceCopy.h>
+
+ at implementation ObjCInterface
+- (void)test:(id)item {
+ _item = item;
+}
+ at end
+
+ at implementation WithBitFields
+- (void)reset {
+ x = 0;
+ y = 0;
+}
+ at 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;
+}
+void testInlinedFunction(ObjCInterface *obj, WithBitFields *bitFields) {
+ inlinedIVarAccessor(obj, bitFields);
+}
More information about the cfe-commits
mailing list