llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Yeoul Na (rapidsna) <details> <summary>Changes</summary> Introduce the machinery that fills in a late-parsed bounds attribute's argument once the enclosing record is complete, without wiring it up yet. Nothing creates an incomplete CountAttributedType or records one for completion at this point, so this is inert -- the functions are unused and behavior is unchanged. Activation follows in the next commit. Depends on #<!-- -->224545 --- Patch is 21.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/224550.diff 8 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) - (modified) clang/include/clang/Basic/LangOptions.h (+7) - (modified) clang/include/clang/Parse/Parser.h (+15) - (modified) clang/include/clang/Sema/Sema.h (+31) - (modified) clang/lib/AST/Decl.cpp (+7-1) - (modified) clang/lib/Parse/ParseDecl.cpp (+48) - (modified) clang/lib/Sema/SemaBoundsSafety.cpp (+112) - (modified) clang/lib/Sema/SemaType.cpp (+138) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 825d17f49790c..0a13fb09d8ff8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7266,6 +7266,9 @@ def err_builtin_counted_by_ref_invalid_use : Error< "value returned by '__builtin_counted_by_ref' cannot be used in " "%select{an array subscript|a binary}0 expression">; +def err_counted_by_on_nested_pointer : Error< + "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' attribute on nested pointer type is not allowed">; + let CategoryName = "ARC Semantic Issue" in { // ARC-mode diagnostics. diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 7539e000d03f9..0d615824ecc5b 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -710,6 +710,13 @@ class LangOptions : public LangOptionsBase { return ConvergentFunctions; } + /// Returns true when the -fbounds-safety attribute programming model is in + /// effect. There is no attributes-only mode on this base, so this is always + /// false; it exists so the shared Sema::ValidateBoundsAttrTypeShape leaf can + /// gate its -fbounds-safety-only branches with the same predicate used + /// downstream (where those branches carry the extra diagnostics). + bool hasBoundsSafetyAttributes() const { return false; } + /// Return true if atomicrmw operations targeting allocations in private /// memory are undefined. bool threadPrivateMemoryAtomicsAreUndefined() const { diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index e9bab81b095fd..ce97ad25bcd16 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -232,6 +232,16 @@ struct LateParsedAttribute : public LateParsedDeclaration { /// is replaced with a concrete type (e.g., CountAttributedType). struct LateParsedTypeAttribute : public LateParsedAttribute { + /// The type built for this attribute during type construction, still missing + /// the argument that hasn't been parsed yet. Filled in by + /// `Parser::ProcessLateParsedTypeAttrCallback` and completed once the + /// enclosing scope makes the argument parseable. Null if type construction + /// rejected the attribute. + /// + /// Held as the base class so the parser stays agnostic about which bounds + /// attribute this is; Sema dispatches on the concrete kind when completing. + BoundsAttributedType *TypeToComplete = nullptr; + explicit LateParsedTypeAttribute(Parser *P, IdentifierInfo &Name, SourceLocation Loc) : LateParsedAttribute(P, Name, Loc, Kind::Type) {} @@ -1524,6 +1534,11 @@ class Parser : public CodeCompletionHandler { void ParseLexedTypeAttribute(LateParsedTypeAttribute &LA, ParsedAttributes &OutAttrs); + /// Complete every late-parsed type attribute queued for the record whose body + /// just closed. Consumes and clears \p LateTypeAttrs. + void CompleteLateParsedTypeAttributes( + SmallVectorImpl<LateParsedTypeAttribute *> &LateTypeAttrs); + /// Parse cached tokens for a late-parsed attribute and return the parsed /// attributes. Shared implementation used by both ParseLexedAttribute and /// ParseLexedTypeAttribute. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 5becfc9fae152..8533f2823ed14 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2493,6 +2493,30 @@ class Sema final : public SemaBase { /// Implementations are in SemaBoundsSafety.cpp ///@{ public: + struct BoundsAttrFlags { + bool CountInBytes = false; + bool OrNull = false; + bool IsEndedBy = false; + }; + static BoundsAttrFlags getBoundsAttrFlags(AttributeCommonInfo::Kind K); + static BoundsAttributedType::BoundsAttrKind + getBoundsAttrKind(const BoundsAttrFlags &); + + /// Validates that a type is eligible for an "externally counted" bounds + /// attribute (counted_by/sized_by and their _or_null variants). + /// + /// \p Flags selects the attribute variant. \returns true if the type is + /// valid, false on error (diagnostics emitted). For `void *__counted_by(n)` + /// it warns that the count is treated as a byte size and sets + /// \p Flags.CountInBytes; callers that want to preserve a counted_by node + /// pass a scratch copy (see validateBoundsAttrTypeForTypePosition). + bool ValidateBoundsAttrTypeShape(QualType Ty, SourceLocation AttrLoc, + SourceRange AttrRange, + BoundsAttrFlags &Flags, + StringRef AttrSpelling = {}, + bool AllowRedecl = false, + Expr *AttrArg = nullptr); + /// Check if applying the specified attribute variant from the "counted by" /// family of attributes to FieldDecl \p FD is semantically valid. If /// semantically invalid diagnostics will be emitted explaining the problems. @@ -2514,6 +2538,13 @@ class Sema final : public SemaBase { bool CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes, bool OrNull); + /// Supply the parsed argument of a late-parsed bounds attribute to the type + /// built for it by ActOnLateParsedTypeAttr, and run the checks that need the + /// owning declaration. \p FD is the field the type belongs to. Returns false + /// if the attribute was rejected. + bool ActOnLateParsedTypeAttrArgument(BoundsAttributedType *BATy, + FieldDecl *FD, Expr *Arg); + /// Perform Bounds Safety Semantic checks for assigning to a `__counted_by` or /// `__counted_by_or_null` pointer type \param LHSTy. /// diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 8c1418625bf33..4b931d1836dec 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -4924,7 +4924,13 @@ const FieldDecl *FieldDecl::findCountedByField() const { if (!CAT) return nullptr; - const auto *CountDRE = cast<DeclRefExpr>(CAT->getCountExpr()); + // A late-parsed attribute whose argument was rejected keeps the node with the + // raw argument as its count (see Sema::ActOnLateParsedTypeAttrArgument). That + // argument may not be a simple declaration reference (e.g. it may be an error + // expression or a `sizeof`), in which case it refers to no field. + const auto *CountDRE = dyn_cast<DeclRefExpr>(CAT->getCountExpr()); + if (!CountDRE) + return nullptr; const auto *CountDecl = CountDRE->getDecl(); if (const auto *IFD = dyn_cast<IndirectFieldDecl>(CountDecl)) CountDecl = IFD->getAnonField(); diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index 5976f5a7ccdea..9a49ce16447cc 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -4896,6 +4896,54 @@ void Parser::ParseLexedTypeAttribute(LateParsedTypeAttribute &LA, OutAttrs.takeAllAppendingFrom(Attrs); } +void Parser::CompleteLateParsedTypeAttributes( + SmallVectorImpl<LateParsedTypeAttribute *> &LateTypeAttrs) { + for (LateParsedTypeAttribute *LTA : LateTypeAttrs) { + // Read these out before parsing, which destroys the attribute. The type is + // null if construction rejected the attribute, in which case the diagnostic + // has already been emitted and there is nothing to complete. + BoundsAttributedType *BATy = LTA->TypeToComplete; + // Rejected during construction (already diagnosed); the cached tokens are + // self-contained, so there is nothing to drain — just discard it. + if (!BATy) { + delete LTA; + continue; + } + // The fields were + // attached in ParseStructDeclaration as each declarator was completed; more + // than one appears when several declarators share a + // declaration-specifier-position attribute. + SmallVector<Decl *, 2> Fields(LTA->Decls); + + AttributeFactory AF; + ParsedAttributes Attrs(AF); + ParseLexedTypeAttribute(*LTA, Attrs); + delete LTA; + + // An unparseable argument leaves no attribute behind; already diagnosed. + if (Attrs.empty()) + continue; + assert(Attrs.size() == 1); + + Expr *Arg = Attrs[0].getArgAsExpr(0); + assert(Arg); + + // No field means the attribute never reached a field declarator (for + // instance the type was rejected during construction, which unwraps the + // node and leaves it unreferenced), so nothing is left to complete. + bool Valid = !Fields.empty(); + for (Decl *FD : Fields) + Valid &= Actions.ActOnLateParsedTypeAttrArgument( + BATy, cast<FieldDecl>(FD), Arg); + + if (Valid) + Attrs[0].setUsedAsTypeAttr(); + else + Attrs[0].setInvalid(); + } + LateTypeAttrs.clear(); +} + void LateParsedTypeAttribute::ParseInto(ParsedAttributes &OutAttrs) { // Delegate to the Parser that created this attribute Self->ParseLexedTypeAttribute(*this, OutAttrs); diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp index 75041c801b6ff..2afe0812dcf4f 100644 --- a/clang/lib/Sema/SemaBoundsSafety.cpp +++ b/clang/lib/Sema/SemaBoundsSafety.cpp @@ -26,6 +26,34 @@ static CountAttributedType::BoundsAttrKind getCountAttrKind(bool CountInBytes, : CountAttributedType::CountedBy; } +BoundsAttributedType::BoundsAttrKind +Sema::getBoundsAttrKind(const BoundsAttrFlags &Flags) { + // `ended_by` (Flags.IsEndedBy) has no home on this base; the field exists for + // struct parity with the downstream API but is never set here. + return getCountAttrKind(Flags.CountInBytes, Flags.OrNull); +} + +Sema::BoundsAttrFlags Sema::getBoundsAttrFlags(AttributeCommonInfo::Kind K) { + BoundsAttrFlags Flags; + switch (K) { + case ParsedAttr::AT_SizedBy: + Flags.CountInBytes = true; + break; + case ParsedAttr::AT_SizedByOrNull: + Flags.CountInBytes = true; + Flags.OrNull = true; + break; + case ParsedAttr::AT_CountedBy: + break; + case ParsedAttr::AT_CountedByOrNull: + Flags.OrNull = true; + break; + default: + llvm_unreachable("unexpected bounds attribute kind"); + } + return Flags; +} + static const RecordDecl *GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) { const auto *RD = FD->getParent(); // An unnamed struct is treated as anonymous struct at this point. @@ -49,6 +77,90 @@ enum class CountedByInvalidPointeeTypeKind { VALID, }; +bool Sema::ValidateBoundsAttrTypeShape(QualType Ty, SourceLocation AttrLoc, + SourceRange AttrRange, + BoundsAttrFlags &Flags, + StringRef AttrSpelling, bool AllowRedecl, + Expr *AttrArg) { + // The downstream leaf runs a `hasBoundsSafetyAttributes()`-gated + // `checkBoundsAttrTypeConflictsAndMisc` preamble and an `ended_by` early + // path; both depend on machinery (`DynamicRangePointerType`, + // `ValueTerminatedType`, the `err_bounds_safety_*` diagnostics) that does not + // exist here, so they are the omitted bounds-safety arms. The rest matches. + BoundsAttributedType::BoundsAttrKind Kind = getBoundsAttrKind(Flags); + + // counted_by/sized_by: must be pointer or array. + if (!Ty->isPointerType() && !Ty->isArrayType()) { + Diag(AttrLoc, diag::err_count_attr_not_on_ptr_or_flexible_array_member) + << Kind << 0; + return false; + } + + // Arrays with sized_by or _or_null variants are not allowed under the + // non -fbounds-safety path; emit the "did you mean to use 'counted_by'" hint. + if (!getLangOpts().hasBoundsSafetyAttributes() && Ty->isArrayType() && + (Flags.CountInBytes || Flags.OrNull)) { + Diag(AttrLoc, diag::err_count_attr_not_on_ptr_or_flexible_array_member) + << Kind << /*suggest counted_by*/ 1; + return false; + } + + // Pointee/element type validation. + QualType PointeeTy; + int SelectPtrOrArr; + if (Ty->isPointerType()) { + PointeeTy = Ty->getPointeeType(); + SelectPtrOrArr = 0; + } else { + const ArrayType *AT = getASTContext().getAsArrayType(Ty); + PointeeTy = AT->getElementType(); + SelectPtrOrArr = 1; + } + + auto InvalidTypeKind = CountedByInvalidPointeeTypeKind::VALID; + bool ShouldWarn = false; + if (!Flags.CountInBytes && PointeeTy->isAlwaysIncompleteType()) { + // Exception: void has an implicit size of 1 byte for pointer arithmetic + // (following GNU convention). Therefore, counted_by on void* is allowed + // and behaves equivalently to sized_by (treating the count as bytes). + if (PointeeTy->isVoidType() && !getLangOpts().hasBoundsSafetyAttributes()) { + // Emit a warning that this is a GNU extension. + Diag(AttrLoc, diag::ext_gnu_counted_by_void_ptr) << Kind; + Diag(AttrLoc, diag::note_gnu_counted_by_void_ptr_use_sized_by) << Kind; + Flags.CountInBytes = true; + return true; + } + InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE; + } else if (PointeeTy->isSizelessType()) { + InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS; + } else if (PointeeTy->isFunctionType()) { + InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION; + } else if (!Flags.CountInBytes && + PointeeTy->isStructureTypeWithFlexibleArrayMember()) { + if (Ty->isArrayType() && !getLangOpts().BoundsSafety) { + // This is a workaround for the Linux kernel that has already adopted + // `counted_by` on a FAM where the pointee is a struct with a FAM. This + // should be an error because computing the bounds of the array cannot + // be done correctly without manually traversing every struct object in + // the array at runtime. To allow the code to be built this error is + // downgraded to a warning. + ShouldWarn = true; + } + InvalidTypeKind = CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER; + } + + if (InvalidTypeKind != CountedByInvalidPointeeTypeKind::VALID) { + unsigned DiagID = ShouldWarn + ? diag::warn_counted_by_attr_elt_type_unknown_size + : diag::err_counted_by_attr_pointee_unknown_size; + Diag(AttrLoc, DiagID) << SelectPtrOrArr << PointeeTy << (int)InvalidTypeKind + << (ShouldWarn ? 1 : 0) << Kind << AttrRange; + return false; + } + + return true; +} + bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes, bool OrNull) { // Check the context the attribute is used in diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 483f9ab088799..66e5c45d21d79 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -9041,6 +9041,90 @@ static void HandleHLSLParamModifierAttr(TypeProcessingState &State, } } +static CountAttributedType::BoundsAttrKind +getCountAttrKind(bool CountInBytes, bool OrNull) { + if (CountInBytes) + return OrNull ? CountAttributedType::SizedByOrNull + : CountAttributedType::SizedBy; + return OrNull ? CountAttributedType::CountedByOrNull + : CountAttributedType::CountedBy; +} + +/// Calculate the pointer nesting level for counted_by attribute validation. +/// Counts the number of pointer/array/function declarator chunks before the +/// specified chunk index. +/// +/// For example, given \c "int * __counted_by(n) *pp" the declarator chunks +/// are (outermost first): [0]=Pointer(\c int **), [1]=Pointer(\c int *). +/// When processing the inner pointer at \p chunkIndex=1, one Pointer chunk +/// precedes it, so the function returns 1. +/// +/// \param state The type processing state +/// \param chunkIndex The index of the current declarator chunk +/// \return The number of pointer/array/function chunks before chunkIndex +static unsigned getPointerNestLevel(TypeProcessingState &state, + unsigned chunkIndex) { + unsigned pointerNestLevel = 0; + const auto &stateDeclarator = state.getDeclarator(); + assert(chunkIndex <= stateDeclarator.getNumTypeObjects()); + // DeclChunks are ordered identifier out. Index 0 is the outer most type + // object. Find outer pointer, array or function. + for (unsigned i = 0; i < chunkIndex; ++i) { + auto TypeObject = stateDeclarator.getTypeObject(i); + switch (TypeObject.Kind) { + case DeclaratorChunk::Function: + case DeclaratorChunk::Array: + case DeclaratorChunk::Pointer: + pointerNestLevel++; + break; + default: + break; + } + } + return pointerNestLevel; +} + +/// The single "is this type valid for a counted_by-family attribute in type +/// position" leaf, shared by the eager path (HandleCountedByAttrOnType) and the +/// late-parsed path (Sema::ActOnLateParsedTypeAttr). +/// +/// Delegates the type-shape checks to Sema::ValidateBoundsAttrTypeShape so +/// there is exactly one copy of those diagnostics, and adds the nested-pointer +/// rejection that only applies in type position. +/// +/// \p Flags is set from \p AttrKind and returned to the caller for building the +/// type. +static bool validateBoundsAttrTypeForTypePosition( + Sema &S, QualType Ty, ParsedAttr::Kind AttrKind, SourceLocation AttrLoc, + SourceRange AttrRange, unsigned PointerNestLevel, + Sema::BoundsAttrFlags &Flags) { + Flags = Sema::getBoundsAttrFlags(AttrKind); + + // ValidateBoundsAttrTypeShape may rewrite Flags.CountInBytes: for + // `void *__counted_by(n)` it warns "treated as 'sized_by'" and sets + // CountInBytes so the -fbounds-safety path builds a SizedBy node. The + // pre-existing non-BoundsSafety field path discards that rewrite and builds a + // CountedBy node, so absorb it in a scratch copy to keep this + // diagnostics-only. + // + // FIXME: Reconcile with the -fbounds-safety path, which honors the rewrite. + Sema::BoundsAttrFlags Scratch = Flags; + if (!S.ValidateBoundsAttrTypeShape(Ty, AttrLoc, AttrRange, Scratch)) + return false; + + // A counted_by-family attribute has to end up at the outermost level of the + // declared type; a nested one would be buried where the bounds cannot be + // maintained. + if (PointerNestLevel > 0) { + S.Diag(AttrLoc, diag::err_counted_by_on_nested_pointer) + << Sema::getBoundsAttrKind(Flags); + return false; + } + + return true; +} + + static void processTypeAttrs(TypeProcessingState &state, QualType &type, TypeAttrLocation TAL, const ParsedAttributesView &attrs, @@ -10001,6 +10085,60 @@ BuildTypeCoupledDecls(Expr *E, Decls.push_back(TypeCoupledDeclRefInfo(CountDecl, /*IsDref*/ false)); } +bool Sema::ActOnLateParsedTypeAttrArgument(BoundsAttributedType *BATy, + FieldDecl *FD, Expr *Arg) { + assert(Arg); + + // Only the counted_by family exists so far. + auto *CATy = cast<CountAttributedType>(BATy); + + // A nested counted_by (buried under a pointer or array) was diagnosed and + // dropped to its wrapped type while the declarator was built, orphaning this + // node -- it is no longer part of the field's type. getAs finds only a + // top-level (through-sugar) CountAttributedType, so when it can't find this + // node the node was dropped: skip it, leaving the field as-is. This matches + // the eager path, which drops the attribute for a nested counted_by. + if (FD->getType()->getAs<CountAttributedType>() != CATy) + return false; + + // Rejected: complete the node in place with the raw argument and no coupled + // decls. The argument isn't a valid count reference, so there are none -- + // and BuildTypeCoupledDecls would assert on a non-DeclRefExpr. Mark the field + // invalid; consumers bail on a non-DeclRefExpr count. Guarded so shared + // declarators (`IP __counted_by(n) a, b;`) only complete the node once. + auto Reject = [&]() -> bool { + if (!CATy->getCountExpr()) + Context.completeCountAttributedType(CATy, Arg, {}); + FD->setInvalidDecl(); + return false; + }; + + // A failed parse was already diagnosed; skip the checks (they would only add + // noise) and recover the node directly. + if (Arg->containsErrors()) + return Reject(); + + // Rejec... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/224550 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
