[clang] 9dcd96f - Canonicalize declaration pointers when forming APValues.
Arthur Eubanks via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 9 19:12:46 PDT 2020
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 at 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 at -2 {{declared here}}
> +#endif
>
> template <class T>
> T foo() {
> @@ -176,7 +179,7 @@ T foo() {
>
> int foo() {
> #if __cplusplus >= 201103L
> -// expected-note at -2 2 {{declared here}}
> +// expected-note at -2 {{declared here}}
> #endif
> int k;
> #pragma omp for ordered
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201009/a1d6ec39/attachment.html>
More information about the cfe-commits
mailing list