================ @@ -186,4 +218,216 @@ bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes, return false; } +SourceRange Sema::BoundsSafetySourceRangeFor(const CountAttributedType *CATy) { + // This is an approximation that's not quite right. This points to the + // the expression inside the attribute rather than the attribute itself. + // + // TODO: Implement logic to find the relevant TypeLoc for the attribute and + // get the SourceRange from that (#113582). + return CATy->getCountExpr()->getSourceRange(); +} + +static void EmitIncompleteCountedByPointeeNotes(Sema &S, + const CountAttributedType *CATy, + NamedDecl *IncompleteTyDecl, + bool NoteAttrLocation = true) { + assert(IncompleteTyDecl == nullptr || isa<TypeDecl>(IncompleteTyDecl)); + + if (NoteAttrLocation) { + // Note where the attribute is declared + auto AttrSrcRange = S.BoundsSafetySourceRangeFor(CATy); + S.Diag(AttrSrcRange.getBegin(), diag::note_named_attribute) + << CATy->getAttributeName(/*WithMacroPrefix=*/true) << AttrSrcRange; + } + + if (!IncompleteTyDecl) + return; + + // If there's an associated forward declaration display it to emphasize + // why the type is incomplete (all we have is a forward declaration). + + // Note the `IncompleteTyDecl` type is the underlying type which might not + // be the same as `CATy->getPointeeType()` which could be a typedef. + // + // The diagnostic printed will be at the location of the underlying type but + // the diagnostic text will print the type of `CATy->getPointeeType()` which + // could be a typedef name rather than the underlying type. This is ok + // though because the diagnostic will print the underlying type name too. + // E.g: + // + // `forward declaration of 'Incomplete_Struct_t' + // (aka 'struct IncompleteStructTy')` + // + // If this ends up being confusing we could emit a second diagnostic (one + // explaining where the typedef is) but that seems overly verbose. + + S.Diag(IncompleteTyDecl->getBeginLoc(), diag::note_forward_declaration) + << CATy->getPointeeType(); +} + +static bool +HasCountedByAttrOnIncompletePointee(QualType Ty, NamedDecl **ND, + const CountAttributedType **CATyOut, + QualType *PointeeTyOut) { + auto *CATy = Ty->getAs<CountAttributedType>(); + if (!CATy) + return false; + + // Incomplete pointee type is only a problem for + // counted_by/counted_by_or_null + if (CATy->isCountInBytes()) + return false; + + auto PointeeTy = CATy->getPointeeType(); + if (PointeeTy.isNull()) + return false; // Reachable? + + if (!PointeeTy->isIncompleteType(ND)) + return false; + + if (CATyOut) + *CATyOut = CATy; + if (PointeeTyOut) + *PointeeTyOut = PointeeTy; + return true; +} + +/// Perform Checks for assigning to a `__counted_by` or +/// `__counted_by_or_null` pointer type \param LHSTy where the pointee type +/// is incomplete which is invalid. +/// +/// \param S The Sema instance. +/// \param LHSTy The type being assigned to. Checks will only be performed if +/// the type is a `counted_by` or `counted_by_or_null ` pointer. +/// \param RHSExpr The expression being assigned from. +/// \param Action The type assignment being performed +/// \param Loc The SourceLocation to use for error diagnostics +/// \param ComputeAssignee If provided this function will be called before +/// emitting a diagnostic. The function should return the name of +/// entity being assigned to or an empty string if this cannot be +/// determined. +/// +/// \returns True iff no diagnostic where emitted, false otherwise. +static bool CheckAssignmentToCountAttrPtrWithIncompletePointeeTy( + Sema &S, QualType LHSTy, Expr *RHSExpr, AssignmentAction Action, + SourceLocation Loc, llvm::function_ref<std::string()> ComputeAssignee) { + NamedDecl *IncompleteTyDecl = nullptr; + const CountAttributedType *CATy = nullptr; + QualType PointeeTy; + if (!HasCountedByAttrOnIncompletePointee(LHSTy, &IncompleteTyDecl, &CATy, + &PointeeTy)) + return true; + assert(CATy && !CATy->isCountInBytes() && !PointeeTy.isNull()); + + // It's not expected that the diagnostic be emitted in these cases. + // It's not necessarily a problem but we should catch when this starts + // to happen. + assert(Action != AssignmentAction::Converting && + Action != AssignmentAction::Sending && + Action != AssignmentAction::Casting && + Action != AssignmentAction::Passing_CFAudited); + + // By having users provide a function we only pay the cost of allocation the + // string and computing when a diagnostic is emitted. + std::string Assignee = ComputeAssignee ? ComputeAssignee() : ""; + { + auto D = S.Diag(Loc, diag::err_counted_by_on_incomplete_type_on_assign) + << (int)Action << Assignee << (Assignee.size() > 0) + << isa<ImplicitValueInitExpr>(RHSExpr) << LHSTy + << CATy->getAttributeName(/*WithMacroPrefix=*/true) << PointeeTy + << CATy->isOrNull(); + + if (RHSExpr->getSourceRange().isValid()) + D << RHSExpr->getSourceRange(); + } + + EmitIncompleteCountedByPointeeNotes(S, CATy, IncompleteTyDecl); + return false; // check failed +} + +bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtr( + QualType LHSTy, Expr *RHSExpr, AssignmentAction Action, SourceLocation Loc, + llvm::function_ref<std::string()> ComputeAssignee) { + return CheckAssignmentToCountAttrPtrWithIncompletePointeeTy( + *this, LHSTy, RHSExpr, Action, Loc, ComputeAssignee); +} + +bool Sema::BoundsSafetyCheckInitialization(const InitializedEntity &Entity, + const InitializationKind &Kind, + AssignmentAction Action, + QualType LHSType, Expr *RHSExpr) { + bool ChecksPassed = true; + auto SL = Kind.getLocation(); + + // Note: We don't call `BoundsSafetyCheckAssignmentToCountAttrPtr` here + // because we need conditionalize what is checked. In downstream + // Clang `counted_by` is supported on variable definitions and in that + // implementation an error diagnostic will be emitted on the variable + // definition if the pointee is an incomplete type. To avoid warning about the + // same problem twice (once when the variable is defined, once when Sema + // checks the initializer) we skip checking the initializer if it's a + // variable. + if (Action == AssignmentAction::Initializing && + Entity.getKind() != InitializedEntity::EK_Variable) { + + if (!CheckAssignmentToCountAttrPtrWithIncompletePointeeTy( + *this, LHSType, RHSExpr, Action, SL, [&Entity]() -> std::string { + if (const auto *VD = + dyn_cast_or_null<ValueDecl>(Entity.getDecl())) { + return VD->getQualifiedNameAsString(); + } + return ""; + })) { + ChecksPassed = false; + + // It's not necessarily bad if this assert fails but we should catch + // if this happens. + assert(Entity.getKind() == InitializedEntity::EK_Member); ---------------- Sirraide wrote:
Also, same concerns about the comment on the assertion here. https://github.com/llvm/llvm-project/pull/106321 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits