Skip to content

Commit 62abd46

Browse files
committed
Add general ODR hash support for ObjCInterfaceDecl.
Add ODR diagnostics for mismatches with super classes and number of protocols. There's still a lot to cover: properties, ivars, methods and other relevant bits which are going to be added in follow up commits. rdar://problem/53021360
1 parent 4ee4536 commit 62abd46

File tree

11 files changed

+522
-5
lines changed

11 files changed

+522
-5
lines changed

clang/include/clang/AST/DeclObjC.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1196,6 +1196,7 @@ class ObjCContainerDecl : public NamedDecl, public DeclContext {
11961196
class ObjCInterfaceDecl : public ObjCContainerDecl
11971197
, public Redeclarable<ObjCInterfaceDecl> {
11981198
friend class ASTContext;
1199+
friend class ASTReader;
11991200

12001201
/// TypeForDecl - This indicates the Type object that represents this
12011202
/// TypeDecl. It is a cache maintained by ASTContext::getObjCInterfaceType
@@ -1253,6 +1254,12 @@ class ObjCInterfaceDecl : public ObjCContainerDecl
12531254
/// One of the \c InheritedDesignatedInitializersState enumeratos.
12541255
mutable unsigned InheritedDesignatedInitializers : 2;
12551256

1257+
/// Tracks whether a ODR hash has been computed for this interface.
1258+
unsigned HasODRHash : 1;
1259+
1260+
/// A hash of parts of the class to help in ODR checking.
1261+
unsigned ODRHash = 0;
1262+
12561263
/// The location of the last location in this declaration, before
12571264
/// the properties/methods. For example, this will be the '>', '}', or
12581265
/// identifier,
@@ -1261,7 +1268,7 @@ class ObjCInterfaceDecl : public ObjCContainerDecl
12611268
DefinitionData()
12621269
: ExternallyCompleted(false), IvarListMissingImplementation(true),
12631270
HasDesignatedInitializers(false),
1264-
InheritedDesignatedInitializers(IDI_Unknown) {}
1271+
InheritedDesignatedInitializers(IDI_Unknown), HasODRHash(false) {}
12651272
};
12661273

12671274
/// The type parameters associated with this class, if any.
@@ -1946,7 +1953,14 @@ class ObjCInterfaceDecl : public ObjCContainerDecl
19461953
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
19471954
static bool classofKind(Kind K) { return K == ObjCInterface; }
19481955

1956+
/// Get precomputed ODRHash or add a new one.
1957+
unsigned getODRHash();
1958+
19491959
private:
1960+
/// True if a valid hash is stored in ODRHash.
1961+
bool hasODRHash() const;
1962+
void setHasODRHash(bool Hash = true);
1963+
19501964
const ObjCInterfaceDecl *findInterfaceWithDesignatedInitializers() const;
19511965
bool inheritsDesignatedInitializers() const;
19521966
};

clang/include/clang/AST/ODRHash.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ class ODRHash {
5959
// method compares more information than the AddDecl class.
6060
void AddRecordDecl(const RecordDecl *Record);
6161

62+
// Use this for ODR checking ObjC interfaces. This
63+
// method compares more information than the AddDecl class.
64+
void AddObjCInterfaceDecl(const ObjCInterfaceDecl *Record);
65+
6266
// Use this for ODR checking functions between modules. This method compares
6367
// more information than the AddDecl class. SkipBody will process the
6468
// hash as if the function has no body.

clang/include/clang/Basic/DiagnosticSerializationKinds.td

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,21 @@ def note_module_odr_violation_definition_data : Note <
153153
"%ordinal2 base class %3 with "
154154
"%select{public|protected|private|no}4 access specifier}1">;
155155

156+
def err_module_odr_violation_obj_interface_definition_data : Error <
157+
"%q0 has different definitions in different modules; first difference is "
158+
"%select{definition in module '%2'|defined here}1 found "
159+
"%select{"
160+
"%select{no super class|super class with type %5}4|"
161+
"%4 referenced %plural{1:protocol|:protocols}4|"
162+
"}3">;
163+
164+
def note_module_odr_violation_obj_interface_definition_data : Note <
165+
"but in '%0' found "
166+
"%select{"
167+
"%select{no super class|super class with type %3}2|"
168+
"%2 referenced %plural{1:protocol|:protocols}2|"
169+
"}1">;
170+
156171
def err_module_odr_violation_template_parameter : Error <
157172
"%q0 has different definitions in different modules; first difference is "
158173
"%select{definition in module '%2'|defined here}1 found "

clang/include/clang/Serialization/ASTReader.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef LLVM_CLANG_SERIALIZATION_ASTREADER_H
1414
#define LLVM_CLANG_SERIALIZATION_ASTREADER_H
1515

16+
#include "clang/AST/DeclObjC.h"
1617
#include "clang/AST/Type.h"
1718
#include "clang/Basic/Diagnostic.h"
1819
#include "clang/Basic/DiagnosticOptions.h"
@@ -1087,6 +1088,8 @@ class ASTReader
10871088

10881089
using DataPointers =
10891090
std::pair<CXXRecordDecl *, struct CXXRecordDecl::DefinitionData *>;
1091+
using IDDataPointers = std::pair<ObjCInterfaceDecl *,
1092+
struct ObjCInterfaceDecl::DefinitionData *>;
10901093

10911094
/// Record definitions in which we found an ODR violation.
10921095
llvm::SmallDenseMap<CXXRecordDecl *, llvm::SmallVector<DataPointers, 2>, 2>
@@ -1104,6 +1107,11 @@ class ASTReader
11041107
llvm::SmallDenseMap<RecordDecl *, llvm::SmallVector<RecordDecl *, 2>, 2>
11051108
PendingRecordOdrMergeFailures;
11061109

1110+
/// ObjCInterfaceDecl in which we found an ODR violation.
1111+
llvm::SmallDenseMap<ObjCInterfaceDecl *, llvm::SmallVector<IDDataPointers, 2>,
1112+
2>
1113+
PendingObjCInterfaceOdrMergeFailures;
1114+
11071115
/// DeclContexts in which we have diagnosed an ODR violation.
11081116
llvm::SmallPtrSet<DeclContext*, 2> DiagnosedOdrMergeFailures;
11091117

clang/lib/AST/DeclObjC.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "clang/AST/Attr.h"
1717
#include "clang/AST/Decl.h"
1818
#include "clang/AST/DeclBase.h"
19+
#include "clang/AST/ODRHash.h"
1920
#include "clang/AST/Stmt.h"
2021
#include "clang/AST/Type.h"
2122
#include "clang/AST/TypeLoc.h"
@@ -771,6 +772,33 @@ ObjCMethodDecl *ObjCInterfaceDecl::lookupPrivateMethod(
771772
return Method;
772773
}
773774

775+
unsigned ObjCInterfaceDecl::getODRHash() {
776+
assert(hasDefinition() && "ODRHash only for records with definitions");
777+
778+
// Previously calculated hash is stored in DefinitionData.
779+
if (hasODRHash())
780+
return data().ODRHash;
781+
782+
// Only calculate hash on first call of getODRHash per record.
783+
class ODRHash Hash;
784+
Hash.AddObjCInterfaceDecl(getDefinition());
785+
setHasODRHash();
786+
data().ODRHash = Hash.CalculateHash();
787+
788+
return data().ODRHash;
789+
}
790+
791+
bool ObjCInterfaceDecl::hasODRHash() const {
792+
if (!hasDefinition())
793+
return false;
794+
return data().HasODRHash;
795+
}
796+
797+
void ObjCInterfaceDecl::setHasODRHash(bool Hash) {
798+
assert(hasDefinition() && "Cannot set ODRHash without definition");
799+
data().HasODRHash = Hash;
800+
}
801+
774802
//===----------------------------------------------------------------------===//
775803
// ObjCMethodDecl
776804
//===----------------------------------------------------------------------===//

clang/lib/AST/ODRHash.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,27 @@ void ODRHash::AddSubDecl(const Decl *D) {
464464
ODRDeclVisitor(ID, *this).Visit(D);
465465
}
466466

467+
void ODRHash::AddObjCInterfaceDecl(const ObjCInterfaceDecl *IF) {
468+
AddDecl(IF);
469+
470+
auto *SuperClass = IF->getSuperClass();
471+
AddBoolean(SuperClass);
472+
if (SuperClass)
473+
ID.AddInteger(SuperClass->getODRHash());
474+
ID.AddInteger(IF->getReferencedProtocols().size());
475+
476+
// Filter out sub-Decls which will not be processed in order to get an
477+
// accurate count of Decl's.
478+
llvm::SmallVector<const Decl *, 16> Decls;
479+
for (Decl *SubDecl : IF->decls())
480+
if (isWhitelistedDecl(SubDecl, IF))
481+
Decls.push_back(SubDecl);
482+
483+
ID.AddInteger(Decls.size());
484+
for (auto *SubDecl : Decls)
485+
AddSubDecl(SubDecl);
486+
}
487+
467488
void ODRHash::AddRecordDecl(const RecordDecl *Record) {
468489
AddDecl(Record);
469490

clang/lib/Serialization/ASTReader.cpp

Lines changed: 180 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9270,7 +9270,8 @@ void ASTReader::diagnoseOdrViolations() {
92709270
if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty() &&
92719271
PendingFunctionOdrMergeFailures.empty() &&
92729272
PendingEnumOdrMergeFailures.empty() &&
9273-
PendingRecordOdrMergeFailures.empty())
9273+
PendingRecordOdrMergeFailures.empty() &&
9274+
PendingObjCInterfaceOdrMergeFailures.empty())
92749275
return;
92759276

92769277
// Trigger the import of the full definition of each class that had any
@@ -9301,6 +9302,16 @@ void ASTReader::diagnoseOdrViolations() {
93019302
D->decls_begin();
93029303
}
93039304

9305+
// Trigger the import of the full interface definition.
9306+
auto ObjCInterfaceOdrMergeFailures =
9307+
std::move(PendingObjCInterfaceOdrMergeFailures);
9308+
PendingObjCInterfaceOdrMergeFailures.clear();
9309+
for (auto &Merge : ObjCInterfaceOdrMergeFailures) {
9310+
Merge.first->decls_begin();
9311+
for (auto &InterfacePair : Merge.second)
9312+
InterfacePair.first->decls_begin();
9313+
}
9314+
93049315
// Trigger the import of functions.
93059316
auto FunctionOdrMergeFailures = std::move(PendingFunctionOdrMergeFailures);
93069317
PendingFunctionOdrMergeFailures.clear();
@@ -9413,7 +9424,8 @@ void ASTReader::diagnoseOdrViolations() {
94139424
}
94149425

94159426
if (OdrMergeFailures.empty() && FunctionOdrMergeFailures.empty() &&
9416-
EnumOdrMergeFailures.empty() && RecordOdrMergeFailures.empty())
9427+
EnumOdrMergeFailures.empty() && RecordOdrMergeFailures.empty() &&
9428+
ObjCInterfaceOdrMergeFailures.empty())
94179429
return;
94189430

94199431
// Ensure we don't accidentally recursively enter deserialization while
@@ -11199,6 +11211,172 @@ void ASTReader::diagnoseOdrViolations() {
1119911211
}
1120011212
}
1120111213

11214+
for (auto &Merge : ObjCInterfaceOdrMergeFailures) {
11215+
// If we've already pointed out a specific problem with this interface,
11216+
// don't bother issuing a general "something's different" diagnostic.
11217+
if (!DiagnosedOdrMergeFailures.insert(Merge.first).second)
11218+
continue;
11219+
11220+
bool Diagnosed = false;
11221+
ObjCInterfaceDecl *FirstID = Merge.first;
11222+
std::string FirstModule = getOwningModuleNameForDiagnostic(FirstID);
11223+
for (auto &InterfacePair : Merge.second) {
11224+
ObjCInterfaceDecl *SecondID = InterfacePair.first;
11225+
// Multiple different declarations got merged together; tell the user
11226+
// where they came from.
11227+
if (FirstID == SecondID)
11228+
continue;
11229+
11230+
std::string SecondModule = getOwningModuleNameForDiagnostic(SecondID);
11231+
11232+
auto *FirstDD = &FirstID->data();
11233+
auto *SecondDD = InterfacePair.second;
11234+
assert(FirstDD && SecondDD && "Definitions without DefinitionData");
11235+
// Diagnostics from ObjCInterfaces DefinitionData are emitted here.
11236+
// FIXME: as part of definition data handling, add support for checking
11237+
// matching protocols, ivars and categories
11238+
if (FirstDD != SecondDD) {
11239+
// Keep this in sync with
11240+
// err_module_odr_violation_obj_interface_definition_data and
11241+
// note_module_odr_violation_obj_interface_definition_data.
11242+
enum ODRDefinitionDataDifference {
11243+
SuperClassType,
11244+
ReferencedProtocols
11245+
};
11246+
auto ODRDiagBaseError = [FirstID, &FirstModule,
11247+
this](SourceLocation Loc, SourceRange Range,
11248+
ODRDefinitionDataDifference DiffType) {
11249+
using namespace diag;
11250+
return Diag(Loc,
11251+
err_module_odr_violation_obj_interface_definition_data)
11252+
<< FirstID << FirstModule.empty() << FirstModule << Range
11253+
<< DiffType;
11254+
};
11255+
auto ODRDiagBaseNote = [&SecondModule,
11256+
this](SourceLocation Loc, SourceRange Range,
11257+
ODRDefinitionDataDifference DiffType) {
11258+
using namespace diag;
11259+
return Diag(Loc,
11260+
note_module_odr_violation_obj_interface_definition_data)
11261+
<< SecondModule << Range << DiffType;
11262+
};
11263+
11264+
// Check for matching super class.
11265+
auto GetSuperClassSourceRange = [](TypeSourceInfo *SuperInfo,
11266+
ObjCInterfaceDecl *ID) {
11267+
if (!SuperInfo)
11268+
return ID->getSourceRange();
11269+
TypeLoc Loc = SuperInfo->getTypeLoc();
11270+
return SourceRange(Loc.getBeginLoc(), Loc.getEndLoc());
11271+
};
11272+
11273+
ObjCInterfaceDecl *FirstSuperClass = FirstID->getSuperClass();
11274+
ObjCInterfaceDecl *SecondSuperClass = nullptr;
11275+
auto *FirstSuperInfo = FirstID->getSuperClassTInfo();
11276+
auto *SecondSuperInfo = SecondDD->SuperClassTInfo;
11277+
if (SecondSuperInfo)
11278+
SecondSuperClass = SecondSuperInfo->getType()
11279+
->castAs<ObjCObjectType>()
11280+
->getInterface();
11281+
11282+
if ((FirstSuperClass && SecondSuperClass &&
11283+
FirstSuperClass->getODRHash() != SecondSuperClass->getODRHash()) ||
11284+
(FirstSuperClass && !SecondSuperClass) ||
11285+
(!FirstSuperClass && SecondSuperClass)) {
11286+
QualType FirstType;
11287+
if (FirstSuperInfo)
11288+
FirstType = FirstSuperInfo->getType();
11289+
11290+
ODRDiagBaseError(FirstID->getLocation(),
11291+
GetSuperClassSourceRange(FirstSuperInfo, FirstID),
11292+
SuperClassType)
11293+
<< (bool)FirstSuperInfo << FirstType;
11294+
11295+
QualType SecondType;
11296+
if (SecondSuperInfo)
11297+
SecondType = SecondSuperInfo->getType();
11298+
11299+
ODRDiagBaseNote(SecondID->getLocation(),
11300+
GetSuperClassSourceRange(SecondSuperInfo, SecondID),
11301+
SuperClassType)
11302+
<< (bool)SecondSuperInfo << SecondType;
11303+
Diagnosed = true;
11304+
break;
11305+
}
11306+
11307+
// Check both interfaces reference the same number of protocols.
11308+
// FIXME: add support for checking the actual protocols once support
11309+
// for ODR hash in protocols land.
11310+
auto GetProtoListSourceRange = [](ObjCInterfaceDecl *ID,
11311+
const ObjCProtocolList &PL) {
11312+
if (!PL.size())
11313+
return ID->getSourceRange();
11314+
return SourceRange(*PL.loc_begin(), *std::prev(PL.loc_end()));
11315+
};
11316+
auto &FirstProtos = FirstID->getReferencedProtocols();
11317+
auto &SecondProtos = SecondDD->ReferencedProtocols;
11318+
if (FirstProtos.size() != SecondProtos.size()) {
11319+
ODRDiagBaseError(FirstID->getLocation(),
11320+
GetProtoListSourceRange(FirstID, FirstProtos),
11321+
ReferencedProtocols)
11322+
<< FirstProtos.size();
11323+
ODRDiagBaseNote(SecondID->getLocation(),
11324+
GetProtoListSourceRange(SecondID, SecondProtos),
11325+
ReferencedProtocols)
11326+
<< SecondProtos.size();
11327+
Diagnosed = true;
11328+
break;
11329+
}
11330+
}
11331+
11332+
// FIXME: Improve PopulateHashes above and only have one version.
11333+
auto PopulateHashes = [&ComputeSubDeclODRHash](DeclHashes &Hashes,
11334+
ObjCInterfaceDecl *ID,
11335+
const DeclContext *DC) {
11336+
for (auto *D : ID->decls()) {
11337+
if (!ODRHash::isWhitelistedDecl(D, DC))
11338+
continue;
11339+
Hashes.emplace_back(D, ComputeSubDeclODRHash(D));
11340+
}
11341+
};
11342+
11343+
DeclHashes FirstHashes;
11344+
DeclHashes SecondHashes;
11345+
PopulateHashes(FirstHashes, FirstID, FirstID);
11346+
PopulateHashes(SecondHashes, SecondID, SecondID);
11347+
11348+
auto DR = FindTypeDiffs(FirstHashes, SecondHashes);
11349+
ODRMismatchDecl FirstDiffType = DR.FirstDiffType;
11350+
ODRMismatchDecl SecondDiffType = DR.SecondDiffType;
11351+
Decl *FirstDecl = DR.FirstDecl;
11352+
Decl *SecondDecl = DR.SecondDecl;
11353+
11354+
if (FirstDiffType == Other || SecondDiffType == Other) {
11355+
DiagnoseODRUnexpected(DR, FirstID, FirstModule, SecondID, SecondModule);
11356+
Diagnosed = true;
11357+
break;
11358+
}
11359+
11360+
if (FirstDiffType != SecondDiffType) {
11361+
DiagnoseODRMismatch(DR, FirstID, FirstModule, SecondID, SecondModule);
11362+
Diagnosed = true;
11363+
break;
11364+
}
11365+
11366+
if (Diagnosed)
11367+
continue;
11368+
11369+
Diag(FirstDecl->getLocation(),
11370+
diag::err_module_odr_violation_mismatch_decl_unknown)
11371+
<< FirstID << FirstModule.empty() << FirstModule << FirstDiffType
11372+
<< FirstDecl->getSourceRange();
11373+
Diag(SecondDecl->getLocation(),
11374+
diag::note_module_odr_violation_mismatch_decl_unknown)
11375+
<< SecondModule << FirstDiffType << SecondDecl->getSourceRange();
11376+
Diagnosed = true;
11377+
}
11378+
}
11379+
1120211380
// Issue ODR failures diagnostics for functions.
1120311381
for (auto &Merge : FunctionOdrMergeFailures) {
1120411382
enum ODRFunctionDifference {

0 commit comments

Comments
 (0)