[ODRHash] Hash ObjCProtocolDecl and diagnose discovered mismatches.

Differential Revision: https://reviews.llvm.org/D130324
This commit is contained in:
Volodymyr Sapsai 2022-10-17 16:21:59 -07:00
parent 40b494396b
commit 9c79eab7fd
12 changed files with 420 additions and 10 deletions

View File

@ -2055,6 +2055,12 @@ class ObjCProtocolDecl : public ObjCContainerDecl,
/// Referenced protocols
ObjCProtocolList ReferencedProtocols;
/// Tracks whether a ODR hash has been computed for this protocol.
unsigned HasODRHash : 1;
/// A hash of parts of the class to help in ODR checking.
unsigned ODRHash = 0;
};
/// Contains a pointer to the data associated with this class,
@ -2091,10 +2097,15 @@ class ObjCProtocolDecl : public ObjCContainerDecl,
return getMostRecentDecl();
}
/// True if a valid hash is stored in ODRHash.
bool hasODRHash() const;
void setHasODRHash(bool HasHash);
public:
friend class ASTDeclReader;
friend class ASTDeclWriter;
friend class ASTReader;
friend class ODRDiagsEmitter;
static ObjCProtocolDecl *Create(ASTContext &C, DeclContext *DC,
IdentifierInfo *Id,
@ -2250,6 +2261,9 @@ public:
ProtocolPropertySet &PS,
PropertyDeclOrder &PO) const;
/// Get precomputed ODRHash or add a new one.
unsigned getODRHash();
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == ObjCProtocol; }
};

View File

@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
@ -44,6 +45,16 @@ public:
const CXXRecordDecl *SecondRecord,
const struct CXXRecordDecl::DefinitionData *SecondDD) const;
/// Diagnose ODR mismatch between 2 ObjCProtocolDecl.
///
/// Returns true if found a mismatch and diagnosed it.
/// To compare 2 declarations with merged and identical definition data
/// you need to provide pre-merge definition data in \p SecondDD.
bool diagnoseMismatch(
const ObjCProtocolDecl *FirstProtocol,
const ObjCProtocolDecl *SecondProtocol,
const struct ObjCProtocolDecl::DefinitionData *SecondDD) const;
/// Get the best name we know for the module that owns the given
/// declaration, or an empty string if the declaration is not from a module.
static std::string getOwningModuleNameForDiagnostic(const Decl *D);
@ -116,6 +127,16 @@ private:
const VarDecl *FirstVD,
const VarDecl *SecondVD) const;
/// Check if protocol lists are the same and diagnose if they are different.
///
/// Returns true if found a mismatch and diagnosed it.
bool diagnoseSubMismatchProtocols(const ObjCProtocolList &FirstProtocols,
const ObjCContainerDecl *FirstContainer,
StringRef FirstModule,
const ObjCProtocolList &SecondProtocols,
const ObjCContainerDecl *SecondContainer,
StringRef SecondModule) const;
private:
DiagnosticsEngine &Diags;
const ASTContext &Context;

View File

@ -64,6 +64,10 @@ public:
// more information than the AddDecl class.
void AddEnumDecl(const EnumDecl *Enum);
// Use this for ODR checking ObjC protocols. This
// method compares more information than the AddDecl class.
void AddObjCProtocolDecl(const ObjCProtocolDecl *P);
// Process SubDecls of the main Decl. This method calls the DeclVisitor
// while AddDecl does not.
void AddSubDecl(const Decl *D);

View File

@ -848,6 +848,19 @@ def note_module_odr_violation_enum : Note<"but in '%0' found "
"%ordinal2 element %3 has different initializer|"
"}1">;
def err_module_odr_violation_referenced_protocols : Error <
"%q0 has different definitions in different modules; first difference is "
"%select{definition in module '%2'|defined here}1 found "
"%select{"
"%4 referenced %plural{1:protocol|:protocols}4|"
"%ordinal4 referenced protocol with name %5"
"}3">;
def note_module_odr_violation_referenced_protocols : Note <"but in '%0' found "
"%select{"
"%2 referenced %plural{1:protocol|:protocols}2|"
"%ordinal2 referenced protocol with different name %3"
"}1">;
def err_module_odr_violation_mismatch_decl_unknown : Error<
"%q0 %select{with definition in module '%2'|defined here}1 has different "
"definitions in different modules; first difference is this "

View File

@ -1131,6 +1131,8 @@ private:
using DataPointers =
std::pair<CXXRecordDecl *, struct CXXRecordDecl::DefinitionData *>;
using ObjCProtocolDataPointers =
std::pair<ObjCProtocolDecl *, struct ObjCProtocolDecl::DefinitionData *>;
/// Record definitions in which we found an ODR violation.
llvm::SmallDenseMap<CXXRecordDecl *, llvm::SmallVector<DataPointers, 2>, 2>
@ -1144,6 +1146,11 @@ private:
llvm::SmallDenseMap<EnumDecl *, llvm::SmallVector<EnumDecl *, 2>, 2>
PendingEnumOdrMergeFailures;
/// ObjCProtocolDecl in which we found an ODR violation.
llvm::SmallDenseMap<ObjCProtocolDecl *,
llvm::SmallVector<ObjCProtocolDataPointers, 2>, 2>
PendingObjCProtocolOdrMergeFailures;
/// DeclContexts in which we have diagnosed an ODR violation.
llvm::SmallPtrSet<DeclContext*, 2> DiagnosedOdrMergeFailures;

View File

@ -16,6 +16,7 @@
#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/ODRHash.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/Type.h"
#include "clang/AST/TypeLoc.h"
@ -1985,6 +1986,7 @@ void ObjCProtocolDecl::allocateDefinitionData() {
assert(!Data.getPointer() && "Protocol already has a definition!");
Data.setPointer(new (getASTContext()) DefinitionData);
Data.getPointer()->Definition = this;
Data.getPointer()->HasODRHash = false;
}
void ObjCProtocolDecl::startDefinition() {
@ -2037,6 +2039,33 @@ ObjCProtocolDecl::getObjCRuntimeNameAsString() const {
return getName();
}
unsigned ObjCProtocolDecl::getODRHash() {
assert(hasDefinition() && "ODRHash only for records with definitions");
// Previously calculated hash is stored in DefinitionData.
if (hasODRHash())
return data().ODRHash;
// Only calculate hash on first call of getODRHash per record.
ODRHash Hasher;
Hasher.AddObjCProtocolDecl(getDefinition());
data().ODRHash = Hasher.CalculateHash();
setHasODRHash(true);
return data().ODRHash;
}
bool ObjCProtocolDecl::hasODRHash() const {
if (!hasDefinition())
return false;
return data().HasODRHash;
}
void ObjCProtocolDecl::setHasODRHash(bool HasHash) {
assert(hasDefinition() && "Cannot set ODRHash without definition");
data().HasODRHash = HasHash;
}
//===----------------------------------------------------------------------===//
// ObjCCategoryDecl
//===----------------------------------------------------------------------===//

View File

@ -266,6 +266,65 @@ bool ODRDiagsEmitter::diagnoseSubMismatchVar(const NamedDecl *FirstRecord,
return false;
}
bool ODRDiagsEmitter::diagnoseSubMismatchProtocols(
const ObjCProtocolList &FirstProtocols,
const ObjCContainerDecl *FirstContainer, StringRef FirstModule,
const ObjCProtocolList &SecondProtocols,
const ObjCContainerDecl *SecondContainer, StringRef SecondModule) const {
// Keep in sync with err_module_odr_violation_referenced_protocols.
enum ODRReferencedProtocolDifference {
NumProtocols,
ProtocolType,
};
auto DiagRefProtocolError = [FirstContainer, FirstModule,
this](SourceLocation Loc, SourceRange Range,
ODRReferencedProtocolDifference DiffType) {
return Diag(Loc, diag::err_module_odr_violation_referenced_protocols)
<< FirstContainer << FirstModule.empty() << FirstModule << Range
<< DiffType;
};
auto DiagRefProtocolNote = [SecondModule,
this](SourceLocation Loc, SourceRange Range,
ODRReferencedProtocolDifference DiffType) {
return Diag(Loc, diag::note_module_odr_violation_referenced_protocols)
<< SecondModule << Range << DiffType;
};
auto GetProtoListSourceRange = [](const ObjCProtocolList &PL) {
if (PL.empty())
return SourceRange();
return SourceRange(*PL.loc_begin(), *std::prev(PL.loc_end()));
};
if (FirstProtocols.size() != SecondProtocols.size()) {
DiagRefProtocolError(FirstContainer->getLocation(),
GetProtoListSourceRange(FirstProtocols), NumProtocols)
<< FirstProtocols.size();
DiagRefProtocolNote(SecondContainer->getLocation(),
GetProtoListSourceRange(SecondProtocols), NumProtocols)
<< SecondProtocols.size();
return true;
}
for (unsigned I = 0, E = FirstProtocols.size(); I != E; ++I) {
const ObjCProtocolDecl *FirstProtocol = FirstProtocols[I];
const ObjCProtocolDecl *SecondProtocol = SecondProtocols[I];
DeclarationName FirstProtocolName = FirstProtocol->getDeclName();
DeclarationName SecondProtocolName = SecondProtocol->getDeclName();
if (FirstProtocolName != SecondProtocolName) {
SourceLocation FirstLoc = *(FirstProtocols.loc_begin() + I);
SourceLocation SecondLoc = *(SecondProtocols.loc_begin() + I);
SourceRange EmptyRange;
DiagRefProtocolError(FirstLoc, EmptyRange, ProtocolType)
<< (I + 1) << FirstProtocolName;
DiagRefProtocolNote(SecondLoc, EmptyRange, ProtocolType)
<< (I + 1) << SecondProtocolName;
return true;
}
}
return false;
}
ODRDiagsEmitter::DiffResult
ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes,
DeclHashes &SecondHashes) {
@ -1533,3 +1592,92 @@ bool ODRDiagsEmitter::diagnoseMismatch(const EnumDecl *FirstEnum,
}
return false;
}
bool ODRDiagsEmitter::diagnoseMismatch(
const ObjCProtocolDecl *FirstProtocol,
const ObjCProtocolDecl *SecondProtocol,
const struct ObjCProtocolDecl::DefinitionData *SecondDD) const {
if (FirstProtocol == SecondProtocol)
return false;
std::string FirstModule = getOwningModuleNameForDiagnostic(FirstProtocol);
std::string SecondModule = getOwningModuleNameForDiagnostic(SecondProtocol);
const ObjCProtocolDecl::DefinitionData *FirstDD = &FirstProtocol->data();
assert(FirstDD && SecondDD && "Definitions without DefinitionData");
// Diagnostics from ObjCProtocol DefinitionData are emitted here.
if (FirstDD != SecondDD) {
// Check both protocols reference the same protocols.
const ObjCProtocolList &FirstProtocols =
FirstProtocol->getReferencedProtocols();
const ObjCProtocolList &SecondProtocols = SecondDD->ReferencedProtocols;
if (diagnoseSubMismatchProtocols(FirstProtocols, FirstProtocol, FirstModule,
SecondProtocols, SecondProtocol,
SecondModule))
return true;
}
auto PopulateHashes = [](DeclHashes &Hashes, const ObjCProtocolDecl *ID,
const DeclContext *DC) {
for (const Decl *D : ID->decls()) {
if (!ODRHash::isDeclToBeProcessed(D, DC))
continue;
Hashes.emplace_back(D, computeODRHash(D));
}
};
DeclHashes FirstHashes;
DeclHashes SecondHashes;
// Use definition as DeclContext because definitions are merged when
// DeclContexts are merged and separate when DeclContexts are separate.
PopulateHashes(FirstHashes, FirstProtocol, FirstProtocol->getDefinition());
PopulateHashes(SecondHashes, SecondProtocol, SecondProtocol->getDefinition());
DiffResult DR = FindTypeDiffs(FirstHashes, SecondHashes);
ODRMismatchDecl FirstDiffType = DR.FirstDiffType;
ODRMismatchDecl SecondDiffType = DR.SecondDiffType;
const Decl *FirstDecl = DR.FirstDecl;
const Decl *SecondDecl = DR.SecondDecl;
if (FirstDiffType == Other || SecondDiffType == Other) {
diagnoseSubMismatchUnexpected(DR, FirstProtocol, FirstModule,
SecondProtocol, SecondModule);
return true;
}
if (FirstDiffType != SecondDiffType) {
diagnoseSubMismatchDifferentDeclKinds(DR, FirstProtocol, FirstModule,
SecondProtocol, SecondModule);
return true;
}
assert(FirstDiffType == SecondDiffType);
switch (FirstDiffType) {
// Already handled.
case EndOfClass:
case Other:
// Cannot be contained by ObjCProtocolDecl, invalid in this context.
case Field:
case TypeDef:
case Var:
// C++ only, invalid in this context.
case PublicSpecifer:
case PrivateSpecifer:
case ProtectedSpecifer:
case StaticAssert:
case CXXMethod:
case TypeAlias:
case Friend:
case FunctionTemplate:
llvm_unreachable("Invalid diff type");
}
Diag(FirstDecl->getLocation(),
diag::err_module_odr_violation_mismatch_decl_unknown)
<< FirstProtocol << FirstModule.empty() << FirstModule << FirstDiffType
<< FirstDecl->getSourceRange();
Diag(SecondDecl->getLocation(),
diag::note_module_odr_violation_mismatch_decl_unknown)
<< SecondModule << FirstDiffType << SecondDecl->getSourceRange();
return true;
}

View File

@ -628,6 +628,31 @@ void ODRHash::AddEnumDecl(const EnumDecl *Enum) {
}
void ODRHash::AddObjCProtocolDecl(const ObjCProtocolDecl *P) {
AddDecl(P);
// Hash referenced protocols.
ID.AddInteger(P->getReferencedProtocols().size());
for (const ObjCProtocolDecl *RefP : P->protocols()) {
// Hash the name only as a referenced protocol can be a forward declaration.
AddDeclarationName(RefP->getDeclName());
}
// Filter out sub-Decls which will not be processed in order to get an
// accurate count of Decl's.
llvm::SmallVector<const Decl *, 16> Decls;
for (Decl *SubDecl : P->decls()) {
if (isDeclToBeProcessed(SubDecl, P)) {
Decls.push_back(SubDecl);
}
}
ID.AddInteger(Decls.size());
for (auto *SubDecl : Decls) {
AddSubDecl(SubDecl);
}
}
void ODRHash::AddDecl(const Decl *D) {
assert(D && "Expecting non-null pointer.");
D = D->getCanonicalDecl();

View File

@ -9434,7 +9434,8 @@ void ASTReader::finishPendingActions() {
void ASTReader::diagnoseOdrViolations() {
if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty() &&
PendingFunctionOdrMergeFailures.empty() &&
PendingEnumOdrMergeFailures.empty())
PendingEnumOdrMergeFailures.empty() &&
PendingObjCProtocolOdrMergeFailures.empty())
return;
// Trigger the import of the full definition of each class that had any
@ -9480,6 +9481,16 @@ void ASTReader::diagnoseOdrViolations() {
}
}
// Trigger the import of the full protocol definition.
auto ObjCProtocolOdrMergeFailures =
std::move(PendingObjCProtocolOdrMergeFailures);
PendingObjCProtocolOdrMergeFailures.clear();
for (auto &Merge : ObjCProtocolOdrMergeFailures) {
Merge.first->decls_begin();
for (auto &ProtocolPair : Merge.second)
ProtocolPair.first->decls_begin();
}
// For each declaration from a merged context, check that the canonical
// definition of that context also contains a declaration of the same
// entity.
@ -9564,7 +9575,7 @@ void ASTReader::diagnoseOdrViolations() {
}
if (OdrMergeFailures.empty() && FunctionOdrMergeFailures.empty() &&
EnumOdrMergeFailures.empty())
EnumOdrMergeFailures.empty() && ObjCProtocolOdrMergeFailures.empty())
return;
// Ensure we don't accidentally recursively enter deserialization while
@ -9635,6 +9646,25 @@ void ASTReader::diagnoseOdrViolations() {
(void)Diagnosed;
assert(Diagnosed && "Unable to emit ODR diagnostic.");
}
for (auto &Merge : ObjCProtocolOdrMergeFailures) {
// If we've already pointed out a specific problem with this protocol,
// don't bother issuing a general "something's different" diagnostic.
if (!DiagnosedOdrMergeFailures.insert(Merge.first).second)
continue;
ObjCProtocolDecl *FirstProtocol = Merge.first;
bool Diagnosed = false;
for (auto &ProtocolPair : Merge.second) {
if (DiagsEmitter.diagnoseMismatch(FirstProtocol, ProtocolPair.first,
ProtocolPair.second)) {
Diagnosed = true;
break;
}
}
(void)Diagnosed;
assert(Diagnosed && "Unable to emit ODR diagnostic.");
}
}
void ASTReader::StartedDeserializing() {

View File

@ -1328,18 +1328,23 @@ void ASTDeclReader::ReadObjCDefinitionData(
ProtoLocs.push_back(readSourceLocation());
Data.ReferencedProtocols.set(ProtoRefs.data(), NumProtoRefs,
ProtoLocs.data(), Reader.getContext());
Data.ODRHash = Record.readInt();
Data.HasODRHash = true;
}
void ASTDeclReader::MergeDefinitionData(ObjCProtocolDecl *D,
struct ObjCProtocolDecl::DefinitionData &&NewDD) {
void ASTDeclReader::MergeDefinitionData(
ObjCProtocolDecl *D, struct ObjCProtocolDecl::DefinitionData &&NewDD) {
struct ObjCProtocolDecl::DefinitionData &DD = D->data();
if (DD.Definition != NewDD.Definition) {
Reader.MergedDeclContexts.insert(
std::make_pair(NewDD.Definition, DD.Definition));
Reader.mergeDefinitionVisibility(DD.Definition, NewDD.Definition);
}
if (DD.Definition == NewDD.Definition)
return;
// FIXME: odr checking?
Reader.MergedDeclContexts.insert(
std::make_pair(NewDD.Definition, DD.Definition));
Reader.mergeDefinitionVisibility(DD.Definition, NewDD.Definition);
if (D->getODRHash() != NewDD.ODRHash)
Reader.PendingObjCProtocolOdrMergeFailures[DD.Definition].push_back(
{NewDD.Definition, &NewDD});
}
void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {

View File

@ -828,6 +828,7 @@ void ASTDeclWriter::VisitObjCProtocolDecl(ObjCProtocolDecl *D) {
Record.AddDeclRef(I);
for (const auto &PL : D->protocol_locs())
Record.AddSourceLocation(PL);
Record.push_back(D->getODRHash());
}
Code = serialization::DECL_OBJC_PROTOCOL;

View File

@ -0,0 +1,113 @@
// RUN: rm -rf %t
// RUN: split-file %s %t
// Build first header file
// RUN: echo "#define FIRST" >> %t/include/first.h
// RUN: cat %t/test.m >> %t/include/first.h
// RUN: echo "#undef FIRST" >> %t/include/first.h
// Build second header file
// RUN: echo "#define SECOND" >> %t/include/second.h
// RUN: cat %t/test.m >> %t/include/second.h
// RUN: echo "#undef SECOND" >> %t/include/second.h
// Test that each header can compile
// RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/first.h -fblocks -fobjc-arc
// RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/second.h -fblocks -fobjc-arc
// Run test
// RUN: %clang_cc1 -I%t/include -verify %t/test.m -fblocks -fobjc-arc \
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
// In non-modular case we ignore protocol redefinitions. But with modules
// previous definition can come from a hidden [sub]module. And in this case we
// allow a new definition if it is equivalent to the hidden one.
//
// This test case is to verify equivalence checks.
//--- include/common.h
#ifndef COMMON_H
#define COMMON_H
@protocol CommonProtocol @end
@protocol ExtraProtocol @end
#endif
//--- include/first-empty.h
//--- include/module.modulemap
module Common {
header "common.h"
export *
}
module First {
module Empty {
header "first-empty.h"
}
module Hidden {
header "first.h"
export *
}
}
module Second {
header "second.h"
export *
}
//--- test.m
#if defined(FIRST) || defined(SECOND)
# include "common.h"
#endif
#if !defined(FIRST) && !defined(SECOND)
# include "first-empty.h"
# include "second.h"
#endif
#if defined(FIRST)
@protocol CompareForwardDeclaration1;
@protocol CompareForwardDeclaration2<CommonProtocol> @end
#elif defined(SECOND)
@protocol CompareForwardDeclaration1<CommonProtocol> @end
@protocol CompareForwardDeclaration2;
#else
id<CompareForwardDeclaration1> compareForwardDeclaration1;
id<CompareForwardDeclaration2> compareForwardDeclaration2;
#endif
#if defined(FIRST)
@protocol CompareMatchingConformingProtocols<CommonProtocol> @end
@protocol ForwardProtocol;
@protocol CompareMatchingConformingForwardProtocols<ForwardProtocol> @end
@protocol CompareProtocolPresence1<CommonProtocol> @end
@protocol CompareProtocolPresence2 @end
@protocol CompareDifferentProtocols<CommonProtocol> @end
@protocol CompareProtocolOrder<CommonProtocol, ExtraProtocol> @end
#elif defined(SECOND)
@protocol CompareMatchingConformingProtocols<CommonProtocol> @end
@protocol ForwardProtocol @end
@protocol CompareMatchingConformingForwardProtocols<ForwardProtocol> @end
@protocol CompareProtocolPresence1 @end
@protocol CompareProtocolPresence2<CommonProtocol> @end
@protocol CompareDifferentProtocols<ExtraProtocol> @end
@protocol CompareProtocolOrder<ExtraProtocol, CommonProtocol> @end
#else
id<CompareMatchingConformingProtocols> compareMatchingConformingProtocols;
id<CompareMatchingConformingForwardProtocols> compareMatchingConformingForwardProtocols;
id<CompareProtocolPresence1> compareProtocolPresence1;
// expected-error@first.h:* {{'CompareProtocolPresence1' has different definitions in different modules; first difference is definition in module 'First.Hidden' found 1 referenced protocol}}
// expected-note@second.h:* {{but in 'Second' found 0 referenced protocols}}
id<CompareProtocolPresence2> compareProtocolPresence2;
// expected-error@first.h:* {{'CompareProtocolPresence2' has different definitions in different modules; first difference is definition in module 'First.Hidden' found 0 referenced protocols}}
// expected-note@second.h:* {{but in 'Second' found 1 referenced protocol}}
id<CompareDifferentProtocols> compareDifferentProtocols;
// expected-error@first.h:* {{'CompareDifferentProtocols' has different definitions in different modules; first difference is definition in module 'First.Hidden' found 1st referenced protocol with name 'CommonProtocol'}}
// expected-note@second.h:* {{but in 'Second' found 1st referenced protocol with different name 'ExtraProtocol'}}
id<CompareProtocolOrder> compareProtocolOrder;
// expected-error@first.h:* {{'CompareProtocolOrder' has different definitions in different modules; first difference is definition in module 'First.Hidden' found 1st referenced protocol with name 'CommonProtocol'}}
// expected-note@second.h:* {{but in 'Second' found 1st referenced protocol with different name 'ExtraProtocol'}}
#endif