llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> It used to cause an assertion failure inside `ASTContext::getTypeSizeInChars()` when the reocord decl was not defined. --- Full diff: https://github.com/llvm/llvm-project/pull/199987.diff 4 Files Affected: - (modified) clang/lib/AST/ByteCode/Interp.h (+10-6) - (modified) clang/lib/AST/ByteCode/Pointer.cpp (+28-9) - (modified) clang/lib/AST/ByteCode/Pointer.h (+2-1) - (modified) clang/test/AST/ByteCode/const-eval.c (+4) ``````````diff diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 4d2055cdb8385..4d028d7042821 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -1237,9 +1237,11 @@ inline bool CmpHelper<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { } } - unsigned VL = LHS.computeOffsetForComparison(S.getASTContext()); - unsigned VR = RHS.computeOffsetForComparison(S.getASTContext()); - S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR)))); + std::optional<size_t> VL = LHS.computeOffsetForComparison(S.getASTContext()); + std::optional<size_t> VR = RHS.computeOffsetForComparison(S.getASTContext()); + if (!VL || !VR) + return Invalid(S, OpPC); + S.Stk.push<BoolT>(BoolT::from(Fn(Compare(*VL, *VR)))); return true; } @@ -1310,10 +1312,12 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { } if (Pointer::hasSameBase(LHS, RHS)) { - size_t A = LHS.computeOffsetForComparison(S.getASTContext()); - size_t B = RHS.computeOffsetForComparison(S.getASTContext()); + std::optional<size_t> A = LHS.computeOffsetForComparison(S.getASTContext()); + std::optional<size_t> B = RHS.computeOffsetForComparison(S.getASTContext()); + if (!A || !B) + return Invalid(S, OpPC); - S.Stk.push<BoolT>(BoolT::from(Fn(Compare(A, B)))); + S.Stk.push<BoolT>(BoolT::from(Fn(Compare(*A, *B)))); return true; } diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp index 51ef1401c3603..93357653c6f5d 100644 --- a/clang/lib/AST/ByteCode/Pointer.cpp +++ b/clang/lib/AST/ByteCode/Pointer.cpp @@ -359,9 +359,13 @@ void Pointer::print(llvm::raw_ostream &OS) const { /// with the same base. To get accurate results, we basically _have to_ compute /// the lvalue offset using the ASTRecordLayout. /// -/// FIXME: We're still mixing values from the record layout with our internal -/// offsets, which will inevitably lead to cryptic errors. -size_t Pointer::computeOffsetForComparison(const ASTContext &ASTCtx) const { +/// This function will fail if we're trying to get the type size of a forward +/// declaration. +/// +// FIXME: We're still mixing values from the record layout with our internal +// offsets, which will inevitably lead to cryptic errors. +std::optional<size_t> +Pointer::computeOffsetForComparison(const ASTContext &ASTCtx) const { switch (StorageKind) { case Storage::Int: return Int.Value + Offset; @@ -374,6 +378,15 @@ size_t Pointer::computeOffsetForComparison(const ASTContext &ASTCtx) const { return reinterpret_cast<uintptr_t>(asTypeidPointer().TypePtr) + Offset; } + auto getTypeSize = [&](QualType T) -> std::optional<size_t> { + if (const RecordType *RT = T->getAs<RecordType>()) { + // We cannot get the type size of a forward declaration. + if (!RT->getDecl()->getDefinition()) + return std::nullopt; + } + return ASTCtx.getTypeSizeInChars(T).getQuantity(); + }; + size_t Result = 0; Pointer P = *this; while (true) { @@ -397,9 +410,12 @@ size_t Pointer::computeOffsetForComparison(const ASTContext &ASTCtx) const { } if (P.isRoot()) { - if (P.isOnePastEnd()) - Result += - ASTCtx.getTypeSizeInChars(P.getDeclDesc()->getType()).getQuantity(); + if (P.isOnePastEnd()) { + if (auto Size = getTypeSize(P.getDeclDesc()->getType())) + Result += *Size; + else + return std::nullopt; + } break; } @@ -413,9 +429,12 @@ size_t Pointer::computeOffsetForComparison(const ASTContext &ASTCtx) const { Layout.getFieldOffset(P.getField()->getFieldIndex())) .getQuantity(); - if (P.isOnePastEnd()) - Result += - ASTCtx.getTypeSizeInChars(P.getField()->getType()).getQuantity(); + if (P.isOnePastEnd()) { + if (auto Size = getTypeSize(P.getField()->getType())) + Result += *Size; + else + return std::nullopt; + } P = P.getBase(); if (P.isRoot()) diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h index 3eb9cfc4e53db..50c38f206edba 100644 --- a/clang/lib/AST/ByteCode/Pointer.h +++ b/clang/lib/AST/ByteCode/Pointer.h @@ -843,7 +843,8 @@ class Pointer { /// Compute an integer that can be used to compare this pointer to /// another one. This is usually NOT the same as the pointer offset /// regarding the AST record layout. - size_t computeOffsetForComparison(const ASTContext &ASTCtx) const; + std::optional<size_t> + computeOffsetForComparison(const ASTContext &ASTCtx) const; private: friend class Block; diff --git a/clang/test/AST/ByteCode/const-eval.c b/clang/test/AST/ByteCode/const-eval.c index 9805fc042c470..10d69caa2a445 100644 --- a/clang/test/AST/ByteCode/const-eval.c +++ b/clang/test/AST/ByteCode/const-eval.c @@ -185,3 +185,7 @@ union ToUnion_U { struct ToUnion_X x; double y; int z : 3; }; _Static_assert(((union ToUnion_U)(struct ToUnion_X){67}).x.a == 67, ""); _Static_assert(((union ToUnion_U)1.0).y == 1.0, ""); _Static_assert(((union ToUnion_U)9).z == 1, ""); + +struct FD fd; // both-error {{tentative definition has type 'struct FD' that is never completed}} \ + // both-note {{forward declaration of 'struct FD'}} +EVAL_EXPR(55, &fd < (struct FD *)((int *)&fd + 42)) // both-error {{not an integer constant expression}} `````````` </details> https://github.com/llvm/llvm-project/pull/199987 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
