[modules] Merge equivalent extensions and diagnose ivar redeclarations for extensions loaded from different modules.

[vsapsai]

Summary

Emitting metadata for the same ivar multiple times can lead to
miscompilations. Objective-C runtime adds offsets to calculate ivar
position in memory and presence of duplicate offsets causes wrong final
position thus overwriting unrelated memory.

Such a situation is impossible with modules disabled as clang diagnoses
ivar redeclarations during sema checks after parsing
(Sema::ActOnFields). Fix the case with modules enabled by checking
during deserialization if ivar is already declared. We also support
a use case where the same category ends up in multiple modules. We
don't want to treat this case as ivar redeclaration and instead merge
corresponding ivars.

rdar://83468070

[vsapsai]

The main alternative to the current implementation was to change getRedeclContext for ObjCCategoryDecl, so we handle ivar redeclarations and merges in ObjCInterfaceDecl with ASTDeclReader::findExisting. Decided against it because of noticeably different merging behavior:

• should merge not any same-name ivars but only those located in equivalent extensions;
• should error for incompatible same-name ivars;
• should handle ObjCCategoryDecl too but we should check them not during their deserialization but after their ivars are deserialized.

Mostly for these reasons decided not to piggy-back on ASTDeclReader::findExisting but to have a different implementation.

[ChuanqiXu]

I don't know about ObjC. Comments on style/name only.

clang/include/clang/Serialization/ASTReader.h

1110

How about change the name to:

template <typename DeclTy>
using DuplicateObjDecls = std::pair<DeclTy *, DeclTy *>;

to clarify it is only used for ObjC. So the developer of other languages would skip this when reading.

1112–1115

Similarly, I suggest to add words or change the name to clarify it is used in ObjC only.

1117

It looks a little bit odd for me to use Vector for duplicate vars. Especially, the structure is Vector<pair<>>. I feel it is better with llvm::SmallSet. If the order is important here, I think llvm::SmallSetVector might be an option.

clang/lib/Serialization/ASTReaderDecl.cpp

1250–1252

nit:

auto *ParentExt = dyn_cast<ObjCCategoryDecl>(IVD->getDeclContext());
auto *PrevParentExt = dyn_cast<ObjCCategoryDecl>(PrevIvar->getDeclContext());
if (ParentExt && PrevParentExt) {

}

We could reduce one identation in this way. Codes with less identation looks better.

[vsapsai]

Thanks for the review, Chuanqi! I believe you were touching recently isSameEntity. Do you have any concerns about using StructuralEquivalenceContext instead of isSameEntity? I've decided to go with StructuralEquivalenceContext because at this point we are still dealing with deserialization and deduplication while ASTContext::isSameEntity kinda assumes that all deduplication is already done, at least based on the comment

// Objective-C classes and protocols with the same name always match.
if (isa<ObjCInterfaceDecl>(X) || isa<ObjCProtocolDecl>(X))
  return true;

clang/include/clang/Serialization/ASTReader.h

1110

Would DuplicateObjCDecls work? ObjC part is common enough and I want to avoid just Obj because it can imply "Object" which is not the goal. Also we have getObjCObjectType which doesn't help with disambiguating the meaning of "Obj".

1112–1115

Thanks for sharing your perspective, I'll make it more obvious it is ObjC-related.

1117

I'm not sure the use of llvm::SmallSet is warranted here. We deserialize each ObjCIvarDecl from each module (and corresponding DeclContext) only once, so we don't have to ensure there are no repeated pairs of same ObjCIvarDecl. And for stable diagnostic we need to use some kind of ordered container. Also a bunch of other "Pending..." fields don't use sets as uniqueness is enforced in different ways.

clang/lib/Serialization/ASTReaderDecl.cpp

1250–1252

Thanks, good point, will change it.

[ChuanqiXu]

In D121177#3371695, @vsapsai wrote:

Thanks for the review, Chuanqi! I believe you were touching recently isSameEntity. Do you have any concerns about using StructuralEquivalenceContext instead of isSameEntity? I've decided to go with StructuralEquivalenceContext because at this point we are still dealing with deserialization and deduplication while ASTContext::isSameEntity kinda assumes that all deduplication is already done, at least based on the comment

// Objective-C classes and protocols with the same name always match.
if (isa<ObjCInterfaceDecl>(X) || isa<ObjCProtocolDecl>(X))
  return true;

Yeah, I was touching isSameEntity. But I don't know about StructuralEquivalenceContext. I just skipped over the implementation of StructuralEquivalenceContext. I feel like it would be OK if we could assume the input is duplicated indeed.
I see it is guaranteed by lookupInstanceVariable(Identifier) now. I don't know much about ObjC. But I feel a little bit strange. Is it possible that two distinct variable with the same identifier in ObjC? If it is impossible, I think it might be OK.

clang/include/clang/Serialization/ASTReader.h

1110

Yeah, it should be ObjC. It's my typo originally.

1117

Oh, I got it. llvm::SmallVector might not be wrong in this case. And for the mapping relationship, llvm::SmallMapVector is considerable too.

[vsapsai]

Address comments and make the tests use non-fragile ABI (it's not default everywhere).

[vsapsai]

In D121177#3371832, @ChuanqiXu wrote:

I see it is guaranteed by lookupInstanceVariable(Identifier) now. I don't know much about ObjC. But I feel a little bit strange. Is it possible that two distinct variable with the same identifier in ObjC? If it is impossible, I think it might be OK.

Two distinct ivars with the same name in the same class are prohibited. But there are a bunch of situations when ivars don't end up in the same class. For example, the most common case (and a huge reason for non-fragile ABI) is you have @interface MyView: NSView that has ivar color. If NSView decides to add its own ivar color in @implementation, it won't break your MyView and your binary will work without recompilation. That is roughly achieved by treating those ivars as MyView.color and NSView.color (it is very hand-wavy over-simplified explanation). But within the same class ivar redeclarations are prohibited. Without splitting everything into modules, we don't allow redeclarations in

@interface TestSubject {
  int a;
}
@end

@interface TestSubject() {
  int a;  // <- error as it is a redeclaration
  int b;
  int c;
}
@end

@interface TestSubject() {
  int b;  // <- error as it is a redeclaration
}
@end

@implementation TestSubject {
  int c;  // <- error as it is a redeclaration
}
@end

[ChuanqiXu]

OK, if it is prohibited. It looks good to me. Please wait for a few days before landing in case there are other comments.

[vsapsai]

Rebase and update test to use ptr instead of i64*. Also now the added test is failing for implementationIvar, investigating that.

[vsapsai]

clang/lib/Serialization/ASTReaderDecl.cpp

1261–1263

We emit diagnostic for

@implementation TestClass {
@public
  struct { int z; } implementationIvar;
}
@end

from merge-anon-record-definition-in-objc.m because we handle only ObjCCategoryDecl and not ObjCImplementationDecl. It is very uncommon to put implementation in a header file but I still need to check we don't emit duplicate ivar metadata in this case.

[vsapsai]

Handle in VisitObjCIvarDecl only redeclarations in extensions.

Interface+interface redeclarations should be handled in VisitObjCInterfaceDecl
and implementation+implementation in VisitObjCImplementationDecl.

Checked that we don't emit duplicate ivar metadata because unlike extensions,
there can be only a single [non-category] implementation.

clang/include/clang/AST/DeclObjC.h

static ObjCIvarDecl *Create(ASTContext &C, ObjCContainerDecl *DC,
                               bool synthesized=false);
 
   static ObjCIvarDecl *CreateDeserialized(ASTContext &C, unsigned ID);
 
   /// Return the class interface that this ivar is logically contained
   /// in; this is either the interface where the ivar was declared, or the
   /// interface the ivar is conceptually a part of in the case of synthesized
   /// ivars.
-  const ObjCInterfaceDecl *getContainingInterface() const;
+  ObjCInterfaceDecl *getContainingInterface();
+  const ObjCInterfaceDecl *getContainingInterface() const {
+    return const_cast<ObjCIvarDecl *>(this)->getContainingInterface();
+  }
 
   ObjCIvarDecl *getNextIvar() { return NextIvar; }
   const ObjCIvarDecl *getNextIvar() const { return NextIvar; }
   void setNextIvar(ObjCIvarDecl *ivar) { NextIvar = ivar; }
 
   ObjCIvarDecl *getCanonicalDecl() override {
     return cast<ObjCIvarDecl>(FieldDecl::getCanonicalDecl());
   }

 inline ObjCInterfaceDecl::filtered_category_iterator<Filter> &
 ObjCInterfaceDecl::filtered_category_iterator<Filter>::operator++() {
   Current = Current->getNextClassCategoryRaw();
   findAcceptableCategory();
   return *this;
 }
 
 inline bool ObjCInterfaceDecl::isVisibleCategory(ObjCCategoryDecl *Cat) {
-  return Cat->isUnconditionallyVisible();
+  return !Cat->isInvalidDecl() && Cat->isUnconditionallyVisible();
 }
 
 inline bool ObjCInterfaceDecl::isVisibleExtension(ObjCCategoryDecl *Cat) {
-  return Cat->IsClassExtension() && Cat->isUnconditionallyVisible();
+  return !Cat->isInvalidDecl() && Cat->IsClassExtension() &&
+         Cat->isUnconditionallyVisible();
 }
 
 inline bool ObjCInterfaceDecl::isKnownExtension(ObjCCategoryDecl *Cat) {
-  return Cat->IsClassExtension();
+  return !Cat->isInvalidDecl() && Cat->IsClassExtension();
 }
 
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_DECLOBJC_H

clang/include/clang/Serialization/ASTReader.h

private:
 
   /// The set of Decls that have been loaded but their DeclContexts are
   /// not set yet.
   ///
   /// The DeclContexts for these Decls will be set once recursive loading has
   /// been completed.
   std::deque<PendingDeclContextInfo> PendingDeclContextInfos;
 
+  template <typename DeclTy>
+  using DuplicateObjCDecls = std::pair<DeclTy *, DeclTy *>;
+
+  /// When resolving duplicate ivars from Objective-C extensions we don't error
+  /// out immediately but check if can merge identical extensions. Not checking
+  /// extensions for equality immediately because ivar deserialization isn't
+  /// over yet at that point.
+  llvm::SmallMapVector<DuplicateObjCDecls<ObjCCategoryDecl>,
+                       llvm::SmallVector<DuplicateObjCDecls<ObjCIvarDecl>, 4>,
+                       2>
+      PendingObjCExtensionIvarRedeclarations;
+
   /// The set of NamedDecls that have been loaded, but are members of a
   /// context that has been merged into another context where the corresponding
   /// declaration is either missing or has not yet been loaded.
   ///
   /// We will check whether the corresponding declaration is in fact missing
   /// once recursing loading has been completed.
   llvm::SmallVector<NamedDecl *, 16> PendingOdrMergeChecks;
 

clang/lib/AST/DeclObjC.cpp

 /// See also ObjCIvarDecl::Create().
 ObjCIvarDecl *ObjCInterfaceDecl::all_declared_ivar_begin() {
   // FIXME: Should make sure no callers ever do this.
   if (!hasDefinition())
     return nullptr;
 
   ObjCIvarDecl *curIvar = nullptr;
   if (!data().IvarList) {
+    // Force ivar deserialization upfront, before building IvarList.
+    (void)ivar_empty();
+    for (const auto *Ext : known_extensions()) {
+      (void)Ext->ivar_empty();
+    }
     if (!ivar_empty()) {
       ObjCInterfaceDecl::ivar_iterator I = ivar_begin(), E = ivar_end();
       data().IvarList = *I; ++I;
       for (curIvar = data().IvarList; I != E; curIvar = *I, ++I)
         curIvar->setNextIvar(*I);
     }
 
     for (const auto *Ext : known_extensions()) {

 }
 
 ObjCIvarDecl *ObjCIvarDecl::CreateDeserialized(ASTContext &C, unsigned ID) {
   return new (C, ID) ObjCIvarDecl(nullptr, SourceLocation(), SourceLocation(),
                                   nullptr, QualType(), nullptr,
                                   ObjCIvarDecl::None, nullptr, false);
 }
 
-const ObjCInterfaceDecl *ObjCIvarDecl::getContainingInterface() const {
-  const auto *DC = cast<ObjCContainerDecl>(getDeclContext());
+ObjCInterfaceDecl *ObjCIvarDecl::getContainingInterface() {
+  auto *DC = cast<ObjCContainerDecl>(getDeclContext());
 
   switch (DC->getKind()) {
   default:
   case ObjCCategoryImpl:
   case ObjCProtocol:
     llvm_unreachable("invalid ivar container!");
 
     // Ivars can only appear in class extension categories.
   case ObjCCategory: {
-    const auto *CD = cast<ObjCCategoryDecl>(DC);
+    auto *CD = cast<ObjCCategoryDecl>(DC);
     assert(CD->IsClassExtension() && "invalid container for ivar!");
     return CD->getClassInterface();
   }
 
   case ObjCImplementation:
     return cast<ObjCImplementationDecl>(DC)->getClassInterface();
 
   case ObjCInterface:

clang/lib/Serialization/ASTReader.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
 #include "clang/AST/ASTUnresolvedSet.h"
 #include "clang/AST/AbstractTypeReader.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/AST/DeclObjC.h"

 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/CommentOptions.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/ObjCRuntime.h"
std::string ASTReader::getOwningModuleNameForDiagnostic(const Decl *D) {
   // Not from a module.
   return {};
 }
 
 void ASTReader::finishPendingActions() {
   while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() ||
          !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() ||
          !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
-         !PendingUpdateRecords.empty()) {
+         !PendingUpdateRecords.empty() ||
+         !PendingObjCExtensionIvarRedeclarations.empty()) {
     // If any identifiers with corresponding top-level declarations have
     // been loaded, load those declarations now.
     using TopLevelDeclsMap =
         llvm::DenseMap<IdentifierInfo *, SmallVector<Decl *, 2>>;
     TopLevelDeclsMap TopLevelDecls;
 
     while (!PendingIdentifierInfos.empty()) {
       IdentifierInfo *II = PendingIdentifierInfos.back().first;
while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() ||
     }
 
     // Perform any pending declaration updates.
     while (!PendingUpdateRecords.empty()) {
       auto Update = PendingUpdateRecords.pop_back_val();
       ReadingKindTracker ReadingKind(Read_Decl, *this);
       loadDeclUpdateRecords(Update);
     }
+
+    while (!PendingObjCExtensionIvarRedeclarations.empty()) {
+      auto ExtensionsPair = PendingObjCExtensionIvarRedeclarations.back().first;
+      auto DuplicateIvars =
+          PendingObjCExtensionIvarRedeclarations.back().second;
+      llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+      StructuralEquivalenceContext Ctx(
+          ExtensionsPair.first->getASTContext(),
+          ExtensionsPair.second->getASTContext(), NonEquivalentDecls,
+          StructuralEquivalenceKind::Default, /*StrictTypeSpelling =*/false,
+          /*Complain =*/false,
+          /*ErrorOnTagTypeMismatch =*/true);
+      if (Ctx.IsEquivalent(ExtensionsPair.first, ExtensionsPair.second)) {
+        // Merge redeclared ivars with their predecessors.
+        for (auto IvarPair : DuplicateIvars) {
+          ObjCIvarDecl *Ivar = IvarPair.first, *PrevIvar = IvarPair.second;
+          // Change semantic DeclContext but keep the lexical one.
+          Ivar->setDeclContextsImpl(PrevIvar->getDeclContext(),
+                                    Ivar->getLexicalDeclContext(),
+                                    getContext());
+          getContext().setPrimaryMergedDecl(Ivar, PrevIvar->getCanonicalDecl());
+        }
+        // Invalidate duplicate extension and the cached ivar list.
+        ExtensionsPair.first->setInvalidDecl();
+        ExtensionsPair.second->getClassInterface()
+            ->getDefinition()
+            ->setIvarList(nullptr);
+      } else {
+        for (auto IvarPair : DuplicateIvars) {
+          Diag(IvarPair.first->getLocation(),
+               diag::err_duplicate_ivar_declaration)
+              << IvarPair.first->getIdentifier();
+          Diag(IvarPair.second->getLocation(), diag::note_previous_definition);
+        }
+      }
+      PendingObjCExtensionIvarRedeclarations.pop_back();
+    }
   }
 
   // At this point, all update records for loaded decls are in place, so any
   // fake class definitions should have become real.
   assert(PendingFakeDefinitionData.empty() &&
          "faked up a class definition but never saw the real one");
 
   // If we deserialized any C++ or Objective-C class definitions, any

clang/lib/Serialization/ASTReaderDecl.cpp

 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Redeclarable.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/AttrKinds.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/Lambda.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Linkage.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/PragmaKinds.h"

 
 void ASTDeclReader::VisitObjCIvarDecl(ObjCIvarDecl *IVD) {
   VisitFieldDecl(IVD);
   IVD->setAccessControl((ObjCIvarDecl::AccessControl)Record.readInt());
   // This field will be built lazily.
   IVD->setNextIvar(nullptr);
   bool synth = Record.readInt();
   IVD->setSynthesize(synth);
+
+  // Check ivar redeclaration.
+  if (IVD->isInvalidDecl())
+    return;
+  // Don't check ObjCInterfaceDecl as interfaces are named and mismatches can be
+  // detected in VisitObjCInterfaceDecl. Here we are looking for redeclarations
+  // in extensions.
+  if (isa<ObjCInterfaceDecl>(IVD->getDeclContext()))
+    return;
+  ObjCInterfaceDecl *CanonIntf =
+      IVD->getContainingInterface()->getCanonicalDecl();
+  IdentifierInfo *II = IVD->getIdentifier();
+  ObjCIvarDecl *PrevIvar = CanonIntf->lookupInstanceVariable(II);
+  if (PrevIvar && PrevIvar != IVD) {
+    auto *ParentExt = dyn_cast<ObjCCategoryDecl>(IVD->getDeclContext());
+    auto *PrevParentExt =
+        dyn_cast<ObjCCategoryDecl>(PrevIvar->getDeclContext());
+    if (ParentExt && PrevParentExt) {
+      // Postpone diagnostic as we should merge identical extensions from
+      // different modules.
+      Reader
+          .PendingObjCExtensionIvarRedeclarations[std::make_pair(ParentExt,
+                                                                 PrevParentExt)]
+          .push_back(std::make_pair(IVD, PrevIvar));
+    } else if (ParentExt || PrevParentExt) {
+      // Duplicate ivars in extension + implementation are never compatible.
+      // Compatibility of implementation + implementation should be handled in
+      // VisitObjCImplementationDecl.
+      Reader.Diag(IVD->getLocation(), diag::err_duplicate_ivar_declaration)
+          << II;
+      Reader.Diag(PrevIvar->getLocation(), diag::note_previous_definition);
+    }
+  }
 }
 
 void ASTDeclReader::ReadObjCDefinitionData(
          struct ObjCProtocolDecl::DefinitionData &Data) {
     unsigned NumProtoRefs = Record.readInt();
     SmallVector<ObjCProtocolDecl *, 16> ProtoRefs;
     ProtoRefs.reserve(NumProtoRefs);
     for (unsigned I = 0; I != NumProtoRefs; ++I)

clang/test/Modules/merge-extension-ivars.m

+// UNSUPPORTED: -zos, -aix
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -emit-llvm -o %t/test-compatible-extensions.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-compatible-extensions.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=InterfaceAndExtension
+// RUN: FileCheck --input-file=%t/test-compatible-extensions.ll %t/test-compatible-extensions.m
+
+// RUN: %clang_cc1 -emit-llvm -o %t/test-access-extension-ivar.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-access-extension-ivar.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: FileCheck --input-file=%t/test-access-extension-ivar.ll %t/test-access-extension-ivar.m
+
+// RUN: %clang_cc1 -emit-llvm -o %t/test-synthesized-ivar.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-synthesized-ivar.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: FileCheck --input-file=%t/test-synthesized-ivar.ll %t/test-synthesized-ivar.m
+// RUN: %clang_cc1 -emit-llvm -o %t/test-synthesized-ivar-extension.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-synthesized-ivar.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache \
+// RUN:            -DIMPORT_EXTENSION=1
+// RUN: FileCheck --input-file=%t/test-synthesized-ivar-extension.ll %t/test-synthesized-ivar.m
+
+// Test various scenarios where we can end up with the same-name ivars coming from multiple modules.
+// The goal is to avoid duplicate metadata for ivars because it can lead to miscompilations
+// with a wrong ivar offset.
+//
+// See specific .m tests for the details of various scenarios.
+
+//--- Frameworks/InterfaceAndExtension.framework/Headers/Interface.h
+@interface NSObject @end
+@interface ObjCInterface : NSObject
+@end
+
+//--- Frameworks/InterfaceAndExtension.framework/Headers/Extension.h
+#import <InterfaceAndExtension/Interface.h>
+@interface ObjCInterface() {
+  float ivarInExtension;
+  int bitfieldIvarInExtension: 3;
+}
+@end
+
+//--- Frameworks/InterfaceAndExtension.framework/Headers/InterfaceAndExtension.h
+#import <InterfaceAndExtension/Interface.h>
+#import <InterfaceAndExtension/Extension.h>
+
+//--- Frameworks/InterfaceAndExtension.framework/Modules/module.modulemap
+framework module InterfaceAndExtension {
+  umbrella header "InterfaceAndExtension.h"
+  export *
+  module * { export * }
+}
+
+//--- Frameworks/Redirecting.framework/Headers/Redirecting.h
+#import <InterfaceAndExtension/InterfaceAndExtension.h>
+
+//--- Frameworks/Redirecting.framework/Modules/module.modulemap
+framework module Redirecting {
+  header "Redirecting.h"
+  export *
+}
+
+//--- test-compatible-extensions.m
+// Test adding through deserialization an extension with already declared ivars.
+
+// First create `ObjCInterface()` extension by parsing corresponding code.
+#import <InterfaceAndExtension/InterfaceAndExtension.h>
+// Now add the same extension through deserialization from the imported module.
+#import <Redirecting/Redirecting.h>
+@implementation ObjCInterface {
+  int ivarInImplementation;
+}
+@end
+// CHECK: @"_OBJC_$_INSTANCE_VARIABLES_ObjCInterface"
+// CHECK-SAME: [3 x %struct._ivar_t] [%struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.ivarInExtension", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.bitfieldIvarInExtension", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.ivarInImplementation", {{.*}} }]
+
+
+//--- Frameworks/WithInlineIvar.framework/Headers/WithInlineIvar.h
+#import <InterfaceAndExtension/InterfaceAndExtension.h>
+@interface ObjCInterface() {
+@public
+  int accessedIvar;
+}
+@end
+static inline void inlinedIvarAccessor(ObjCInterface *obj) {
+  obj->accessedIvar = 0;
+}
+
+//--- Frameworks/WithInlineIvar.framework/Modules/module.modulemap
+framework module WithInlineIvar {
+  header "WithInlineIvar.h"
+  export *
+}
+
+//--- test-access-extension-ivar.m
+// Test accessing ivars from extensions.
+#import <InterfaceAndExtension/InterfaceAndExtension.h>
+@interface ObjCInterface() {
+@public
+  int accessedIvar;
+}
+@end
+#import <WithInlineIvar/WithInlineIvar.h>
+@implementation ObjCInterface
+- (void)test {
+  inlinedIvarAccessor(self);
+  ivarInExtension = 0;
+}
+@end
+// CHECK: @"_OBJC_$_INSTANCE_VARIABLES_ObjCInterface"
+// CHECK-SAME: [3 x %struct._ivar_t] [%struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.accessedIvar", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.ivarInExtension", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.bitfieldIvarInExtension", {{.*}} }]
+
+
+//--- Frameworks/WithProperty.framework/Headers/WithProperty.h
+@interface NSObject @end
+@interface WithProperty: NSObject
+@property (assign) int propertyName;
+@end
+
+//--- Frameworks/WithProperty.framework/Modules/module.modulemap
+framework module WithProperty {
+  header "WithProperty.h"
+  export *
+}
+
+//--- Frameworks/BackingIvarInExtension.framework/Headers/BackingIvarInExtension.h
+#import <WithProperty/WithProperty.h>
+@interface WithProperty() {
+  int propertyBackingIvar;
+}
+@end
+
+//--- Frameworks/BackingIvarInExtension.framework/Modules/module.modulemap
+framework module BackingIvarInExtension {
+  header "BackingIvarInExtension.h"
+  export *
+}
+
+//--- test-synthesized-ivar.m
+// Test when an ivar is both synthesized and when declared in an extension.
+// Behavior with and without extension should be the same.
+#import <WithProperty/WithProperty.h>
+#ifdef IMPORT_EXTENSION
+#import <BackingIvarInExtension/BackingIvarInExtension.h>
+#endif
+@implementation WithProperty
+@synthesize propertyName = propertyBackingIvar;
+@end
+// CHECK: @"_OBJC_$_INSTANCE_VARIABLES_WithProperty"
+// CHECK-SAME: [1 x %struct._ivar_t] [%struct._ivar_t { ptr @"OBJC_IVAR_$_WithProperty.propertyBackingIvar", {{.*}} }]

clang/test/Modules/redecl-ivars.m

+// UNSUPPORTED: -zos, -aix
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-extension.m
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-extension.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-ivars-number.m
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-ivars-number.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-methods-protocols.m
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-methods-protocols.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-subclass.m
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-subclass.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-implementation.m
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-implementation.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Same class extensions with the same ivars but from different modules aren't considered
+// an error and they are merged together. Test that differences in extensions and/or ivars
+// are still reported as errors.
+
+//--- include/Interfaces.h
+@interface NSObject @end
+@interface ObjCInterface : NSObject
+@end
+@interface ObjCInterfaceLevel2 : ObjCInterface
+@end
+
+@protocol Protocol1 @end
+@protocol Protocol2 @end
+
+//--- include/IvarsInExtensions.h
+#import <Interfaces.h>
+@interface ObjCInterface() {
+  int ivarName;
+}
+@end
+@interface ObjCInterfaceLevel2() {
+  int bitfieldIvarName: 3;
+}
+@end
+
+//--- include/IvarsInExtensionsWithMethodsProtocols.h
+#import <Interfaces.h>
+@interface ObjCInterface() {
+  int methodRelatedIvar;
+}
+- (void)test;
+@end
+@interface ObjCInterfaceLevel2() <Protocol1> {
+  int protocolRelatedIvar;
+}
+@end
+
+//--- include/IvarInImplementation.h
+#import <Interfaces.h>
+@implementation ObjCInterface {
+  int ivarName;
+}
+@end
+
+//--- include/module.modulemap
+module Interfaces {
+  header "Interfaces.h"
+  export *
+}
+module IvarsInExtensions {
+  header "IvarsInExtensions.h"
+  export *
+}
+module IvarsInExtensionsWithMethodsProtocols {
+  header "IvarsInExtensionsWithMethodsProtocols.h"
+  export *
+}
+module IvarInImplementation {
+  header "IvarInImplementation.h"
+  export *
+}
+
+
+//--- test-mismatch-in-extension.m
+// Different ivars with the same name aren't mergeable and constitute an error.
+#import <Interfaces.h>
+@interface ObjCInterface() {
+  float ivarName; // expected-note {{previous definition is here}}
+}
+@end
+@interface ObjCInterfaceLevel2() {
+  int bitfieldIvarName: 5; // expected-note {{previous definition is here}}
+}
+@end
+#import <IvarsInExtensions.h>
+// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}}
+// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}}
+@implementation ObjCInterfaceLevel2
+@end
+
+
+//--- test-mismatch-in-ivars-number.m
+// Extensions with different amount of ivars aren't considered to be the same.
+#import <Interfaces.h>
+@interface ObjCInterface() {
+  int ivarName; // expected-note {{previous definition is here}}
+  float anotherIvar;
+}
+@end
+#import <IvarsInExtensions.h>
+// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}}
+@implementation ObjCInterface
+@end
+
+
+//--- test-mismatch-in-methods-protocols.m
+// Extensions with different methods or protocols aren't considered to be the same.
+#import <Interfaces.h>
+@interface ObjCInterface() {
+  int methodRelatedIvar; // expected-note {{previous definition is here}}
+}
+- (void)differentTest;
+@end
+@interface ObjCInterfaceLevel2() <Protocol2> {
+  int protocolRelatedIvar; // expected-note {{previous definition is here}}
+}
+@end
+#import <IvarsInExtensionsWithMethodsProtocols.h>
+// expected-error@IvarsInExtensionsWithMethodsProtocols.h:* {{instance variable is already declared}}
+// expected-error@IvarsInExtensionsWithMethodsProtocols.h:* {{instance variable is already declared}}
+@implementation ObjCInterfaceLevel2
+@end
+
+
+//--- test-redecl-in-subclass.m
+// Ivar in superclass extension is not added to a subclass, so the ivar with
+// the same name in subclass extension is not considered a redeclaration.
+// expected-no-diagnostics
+#import <Interfaces.h>
+@interface ObjCInterfaceLevel2() {
+  float ivarName;
+}
+@end
+#import <IvarsInExtensions.h>
+@implementation ObjCInterfaceLevel2
+@end
+
+
+//--- test-redecl-in-implementation.m
+// Ivar redeclaration in `@implementation` is always an error and never mergeable.
+#import <IvarsInExtensions.h>
+@interface ObjCInterface() {
+  int triggerExtensionIvarDeserialization;
+}
+@end
+#import <IvarInImplementation.h>
+#if __has_feature(modules)
+// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}}
+// expected-note@IvarInImplementation.h:* {{previous definition is here}}
+#else
+// expected-error@IvarInImplementation.h:* {{instance variable is already declared}}
+// expected-note@IvarsInExtensions.h:* {{previous definition is here}}
+#endif