I think this is the cause of https://crbug.com/1134762. Can we speculatively revert while coming up with a repro? Or would you like a repro first?
On Sun, Sep 27, 2020 at 7:06 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > Author: Richard Smith > Date: 2020-09-27T19:05:26-07:00 > New Revision: 9dcd96f728863d40d6f5922ed52732fdd728fb5f > > URL: > https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f > DIFF: > https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f.diff > > LOG: Canonicalize declaration pointers when forming APValues. > > References to different declarations of the same entity aren't different > values, so shouldn't have different representations. > > Recommit of e6393ee813178e9d3306b8e3c6949a4f32f8a2cb with fixed handling > for weak declarations. We now look for attributes on the most recent > declaration when determining whether a declaration is weak. (Second > recommit with further fixes for mishandling of weak declarations. Our > behavior here is fundamentally unsound -- see PR47663 -- but this > approach attempts to not make things worse.) > > Added: > > > Modified: > clang/include/clang/AST/APValue.h > clang/lib/AST/APValue.cpp > clang/lib/AST/Decl.cpp > clang/lib/AST/DeclBase.cpp > clang/lib/AST/ExprConstant.cpp > clang/lib/CodeGen/CGExprConstant.cpp > clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp > clang/test/CodeGenCXX/weak-external.cpp > clang/test/OpenMP/ordered_messages.cpp > > Removed: > > > > > ################################################################################ > diff --git a/clang/include/clang/AST/APValue.h > b/clang/include/clang/AST/APValue.h > index 5103cfa8604e..6307f8a92e5a 100644 > --- a/clang/include/clang/AST/APValue.h > +++ b/clang/include/clang/AST/APValue.h > @@ -174,6 +174,7 @@ class APValue { > return !(LHS == RHS); > } > friend llvm::hash_code hash_value(const LValueBase &Base); > + friend struct llvm::DenseMapInfo<LValueBase>; > > private: > PtrTy Ptr; > @@ -201,8 +202,7 @@ class APValue { > > public: > LValuePathEntry() : Value() {} > - LValuePathEntry(BaseOrMemberType BaseOrMember) > - : > Value{reinterpret_cast<uintptr_t>(BaseOrMember.getOpaqueValue())} {} > + LValuePathEntry(BaseOrMemberType BaseOrMember); > static LValuePathEntry ArrayIndex(uint64_t Index) { > LValuePathEntry Result; > Result.Value = Index; > > diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp > index 08ae0ff3c67d..32d3ff7ce1d0 100644 > --- a/clang/lib/AST/APValue.cpp > +++ b/clang/lib/AST/APValue.cpp > @@ -38,7 +38,7 @@ static_assert( > "Type is insufficiently aligned"); > > APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned > V) > - : Ptr(P), Local{I, V} {} > + : Ptr(P ? cast<ValueDecl>(P->getCanonicalDecl()) : nullptr), Local{I, > V} {} > APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V) > : Ptr(P), Local{I, V} {} > > @@ -82,13 +82,19 @@ bool operator==(const APValue::LValueBase &LHS, > const APValue::LValueBase &RHS) { > if (LHS.Ptr != RHS.Ptr) > return false; > - if (LHS.is<TypeInfoLValue>()) > + if (LHS.is<TypeInfoLValue>() || LHS.is<DynamicAllocLValue>()) > return true; > return LHS.Local.CallIndex == RHS.Local.CallIndex && > LHS.Local.Version == RHS.Local.Version; > } > } > > +APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType BaseOrMember) { > + if (const Decl *D = BaseOrMember.getPointer()) > + BaseOrMember.setPointer(D->getCanonicalDecl()); > + Value = reinterpret_cast<uintptr_t>(BaseOrMember.getOpaqueValue()); > +} > + > namespace { > struct LVBase { > APValue::LValueBase Base; > @@ -113,14 +119,16 @@ APValue::LValueBase::operator bool () const { > > clang::APValue::LValueBase > llvm::DenseMapInfo<clang::APValue::LValueBase>::getEmptyKey() { > - return clang::APValue::LValueBase( > - DenseMapInfo<const ValueDecl*>::getEmptyKey()); > + clang::APValue::LValueBase B; > + B.Ptr = DenseMapInfo<const ValueDecl*>::getEmptyKey(); > + return B; > } > > clang::APValue::LValueBase > llvm::DenseMapInfo<clang::APValue::LValueBase>::getTombstoneKey() { > - return clang::APValue::LValueBase( > - DenseMapInfo<const ValueDecl*>::getTombstoneKey()); > + clang::APValue::LValueBase B; > + B.Ptr = DenseMapInfo<const ValueDecl*>::getTombstoneKey(); > + return B; > } > > namespace clang { > @@ -773,8 +781,10 @@ void APValue::MakeMemberPointer(const ValueDecl > *Member, bool IsDerivedMember, > assert(isAbsent() && "Bad state change"); > MemberPointerData *MPD = new ((void*)(char*)Data.buffer) > MemberPointerData; > Kind = MemberPointer; > - MPD->MemberAndIsDerivedMember.setPointer(Member); > + MPD->MemberAndIsDerivedMember.setPointer( > + Member ? cast<ValueDecl>(Member->getCanonicalDecl()) : nullptr); > MPD->MemberAndIsDerivedMember.setInt(IsDerivedMember); > MPD->resizePath(Path.size()); > - memcpy(MPD->getPath(), Path.data(), Path.size()*sizeof(const > CXXRecordDecl*)); > + for (unsigned I = 0; I != Path.size(); ++I) > + MPD->getPath()[I] = Path[I]->getCanonicalDecl(); > } > > diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp > index 0ee1399d42df..c96450b8a377 100644 > --- a/clang/lib/AST/Decl.cpp > +++ b/clang/lib/AST/Decl.cpp > @@ -4686,11 +4686,9 @@ char *Buffer = new (getASTContext(), 1) > char[Name.size() + 1]; > void ValueDecl::anchor() {} > > bool ValueDecl::isWeak() const { > - for (const auto *I : attrs()) > - if (isa<WeakAttr>(I) || isa<WeakRefAttr>(I)) > - return true; > - > - return isWeakImported(); > + auto *MostRecent = getMostRecentDecl(); > + return MostRecent->hasAttr<WeakAttr>() || > + MostRecent->hasAttr<WeakRefAttr>() || isWeakImported(); > } > > void ImplicitParamDecl::anchor() {} > > diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp > index f4314d0bd961..ab2b55c0762e 100644 > --- a/clang/lib/AST/DeclBase.cpp > +++ b/clang/lib/AST/DeclBase.cpp > @@ -720,7 +720,7 @@ bool Decl::isWeakImported() const { > if (!canBeWeakImported(IsDefinition)) > return false; > > - for (const auto *A : attrs()) { > + for (const auto *A : getMostRecentDecl()->attrs()) { > if (isa<WeakImportAttr>(A)) > return true; > > > diff --git a/clang/lib/AST/ExprConstant.cpp > b/clang/lib/AST/ExprConstant.cpp > index 9bc4afd0b9f8..3bc649b96990 100644 > --- a/clang/lib/AST/ExprConstant.cpp > +++ b/clang/lib/AST/ExprConstant.cpp > @@ -1978,18 +1978,11 @@ static bool HasSameBase(const LValue &A, const > LValue &B) { > return false; > > if (A.getLValueBase().getOpaqueValue() != > - B.getLValueBase().getOpaqueValue()) { > - const Decl *ADecl = GetLValueBaseDecl(A); > - if (!ADecl) > - return false; > - const Decl *BDecl = GetLValueBaseDecl(B); > - if (!BDecl || ADecl->getCanonicalDecl() != BDecl->getCanonicalDecl()) > - return false; > - } > + B.getLValueBase().getOpaqueValue()) > + return false; > > - return IsGlobalLValue(A.getLValueBase()) || > - (A.getLValueCallIndex() == B.getLValueCallIndex() && > - A.getLValueVersion() == B.getLValueVersion()); > + return A.getLValueCallIndex() == B.getLValueCallIndex() && > + A.getLValueVersion() == B.getLValueVersion(); > } > > static void NoteLValueLocation(EvalInfo &Info, APValue::LValueBase Base) { > @@ -3158,7 +3151,8 @@ static bool evaluateVarDeclInit(EvalInfo &Info, > const Expr *E, > > // If we're currently evaluating the initializer of this declaration, > use that > // in-flight value. > - if (Info.EvaluatingDecl.dyn_cast<const ValueDecl*>() == VD) { > + if (declaresSameEntity(Info.EvaluatingDecl.dyn_cast<const ValueDecl > *>(), > + VD)) { > Result = Info.EvaluatingDeclValue; > return true; > } > > diff --git a/clang/lib/CodeGen/CGExprConstant.cpp > b/clang/lib/CodeGen/CGExprConstant.cpp > index b0a37531dfe1..bff4a0c38af9 100644 > --- a/clang/lib/CodeGen/CGExprConstant.cpp > +++ b/clang/lib/CodeGen/CGExprConstant.cpp > @@ -1877,6 +1877,10 @@ ConstantLValue > ConstantLValueEmitter::tryEmitBase(const APValue::LValueBase &base) { > // Handle values. > if (const ValueDecl *D = base.dyn_cast<const ValueDecl*>()) { > + // The constant always points to the canonical declaration. We want > to look > + // at properties of the most recent declaration at the point of > emission. > + D = cast<ValueDecl>(D->getMostRecentDecl()); > + > if (D->hasAttr<WeakRefAttr>()) > return CGM.GetWeakRefReference(D).getPointer(); > > > diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp > b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp > index 8d51dbde7177..3720b277af7a 100644 > --- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp > +++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp > @@ -24,11 +24,10 @@ constexpr double &ni3; // expected-error {{declaration > of reference variable 'ni > > constexpr int nc1 = i; // expected-error {{constexpr variable 'nc1' must > be initialized by a constant expression}} expected-note {{read of non-const > variable 'i' is not allowed in a constant expression}} > constexpr C nc2 = C(); // expected-error {{cannot have non-literal type > 'const C'}} > -int &f(); // expected-note {{declared here}} > +int &f(); // expected-note 2{{declared here}} > constexpr int &nc3 = f(); // expected-error {{constexpr variable 'nc3' > must be initialized by a constant expression}} expected-note > {{non-constexpr function 'f' cannot be used in a constant expression}} > constexpr int nc4(i); // expected-error {{constexpr variable 'nc4' must > be initialized by a constant expression}} expected-note {{read of non-const > variable 'i' is not allowed in a constant expression}} > constexpr C nc5((C())); // expected-error {{cannot have non-literal type > 'const C'}} > -int &f(); // expected-note {{here}} > constexpr int &nc6(f()); // expected-error {{constexpr variable 'nc6' > must be initialized by a constant expression}} expected-note > {{non-constexpr function 'f'}} > > struct pixel { > > diff --git a/clang/test/CodeGenCXX/weak-external.cpp > b/clang/test/CodeGenCXX/weak-external.cpp > index a2c53a59dcd5..433fb3c80624 100644 > --- a/clang/test/CodeGenCXX/weak-external.cpp > +++ b/clang/test/CodeGenCXX/weak-external.cpp > @@ -64,3 +64,15 @@ class _LIBCPP_EXCEPTION_ABI runtime_error > void dummysymbol() { > throw(std::runtime_error("string")); > } > + > +namespace not_weak_on_first { > + int func(); > + // CHECK: {{.*}} extern_weak {{.*}} @_ZN17not_weak_on_first4funcEv( > + int func() __attribute__ ((weak)); > + > + typedef int (*FuncT)(); > + > + extern const FuncT table[] = { > + func, > + }; > +} > > diff --git a/clang/test/OpenMP/ordered_messages.cpp > b/clang/test/OpenMP/ordered_messages.cpp > index f6b9dbd6d27f..8a3a86443eb8 100644 > --- a/clang/test/OpenMP/ordered_messages.cpp > +++ b/clang/test/OpenMP/ordered_messages.cpp > @@ -16,6 +16,9 @@ void xxx(int argc) { > } > > int foo(); > +#if __cplusplus >= 201103L > +// expected-note@-2 {{declared here}} > +#endif > > template <class T> > T foo() { > @@ -176,7 +179,7 @@ T foo() { > > int foo() { > #if __cplusplus >= 201103L > -// expected-note@-2 2 {{declared here}} > +// expected-note@-2 {{declared here}} > #endif > int k; > #pragma omp for ordered > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
