================ @@ -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); + } + } + + return ChecksPassed; +} + +bool Sema::BoundsSafetyCheckUseOfCountAttrPtr(Expr *E) { + QualType T = E->getType(); + if (!T->isPointerType()) + return true; + + const CountAttributedType *CATy = nullptr; + QualType PointeeTy; + NamedDecl *IncompleteTyDecl = nullptr; + if (!HasCountedByAttrOnIncompletePointee(T, &IncompleteTyDecl, &CATy, + &PointeeTy)) + return true; + assert(CATy && !CATy->isCountInBytes() && !PointeeTy.isNull()); + + // Generate a string for the diagnostic that describes the "use". + // The string is specialized for direct calls to produce a better + // diagnostic. + StringRef DirectCallFn; + std::string UseStr; + if (const auto *CE = dyn_cast<CallExpr>(E->IgnoreParens())) { + if (const auto *FD = CE->getDirectCallee()) { + DirectCallFn = FD->getName(); + } + } + int SelectExprKind = DirectCallFn.size() > 0 ? 1 : 0; + if (SelectExprKind) { + UseStr = DirectCallFn; + } else { + llvm::raw_string_ostream SS(UseStr); + E->printPretty(SS, nullptr, PrintingPolicy(getPrintingPolicy())); + } + assert(UseStr.size() > 0); + + Diag(E->getBeginLoc(), diag::err_counted_by_on_incomplete_type_on_use) + << SelectExprKind << UseStr << T << PointeeTy ---------------- Sirraide wrote:
```suggestion SmallString<64> UseStr; bool IsDirectCall = false; if (const auto *CE = dyn_cast<CallExpr>(E->IgnoreParens())) { if (const auto *FD = CE->getDirectCallee()) { UseStr = FD->getName(); IsDirectCall = true; } } if (!IsDirectCall) { llvm::raw_svector_ostream SS(UseStr); E->printPretty(SS, nullptr, getPrintingPolicy()); } Diag(E->getBeginLoc(), diag::err_counted_by_on_incomplete_type_on_use) << IsDirectCall << UseStr << T << PointeeTy ``` I think we can simplify this a bit (and maybe use a `SmallString<>` so we don’t have to heap-allocate in as many cases). This should be equivalent unless I made a mistake somewhere, so please double-check. 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