Turn 'counted_by' into a type attribute and parse it into 'CountAttributedType' #78000
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rapidsna
added
clang
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Yeoul Na (rapidsna) ChangesIn We introduce a new sugar type to support bounds annotated types, This patch also extends the parsing logic to parse the This also adjusts Patch is 50.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78000.diff 33 Files Affected:
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 3e46a5da3fc043..c8354fbb108a27 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -247,6 +247,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
DependentBitIntTypes;
llvm::FoldingSet<BTFTagAttributedType> BTFTagAttributedTypes;
+ mutable llvm::FoldingSet<CountAttributedType> CountAttributedTypes;
+
mutable llvm::FoldingSet<QualifiedTemplateName> QualifiedTemplateNames;
mutable llvm::FoldingSet<DependentTemplateName> DependentTemplateNames;
mutable llvm::FoldingSet<SubstTemplateTemplateParmStorage>
@@ -1338,6 +1340,11 @@ class ASTContext : public RefCountedBase<ASTContext> {
return CanQualType::CreateUnsafe(getPointerType((QualType) T));
}
+ QualType
+ getCountAttributedType(QualType T, Expr *CountExpr, bool CountInBytes,
+ bool OrNull,
+ ArrayRef<TypeCoupledDeclRefInfo> DependentDecls) const;
+
/// Return the uniqued reference to a type adjusted from the original
/// type to a new type.
QualType getAdjustedType(QualType Orig, QualType New) const;
diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index d86c4eba6a2251..25ddfd105ab877 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -143,6 +143,7 @@ def UInt32 : CountPropertyType<"uint32_t">;
def UInt64 : CountPropertyType<"uint64_t">;
def UnaryTypeTransformKind : EnumPropertyType<"UnaryTransformType::UTTKind">;
def VectorKind : EnumPropertyType<"VectorKind">;
+def TypeCoupledDeclRefInfo : PropertyType;
def ExceptionSpecInfo : PropertyType<"FunctionProtoType::ExceptionSpecInfo"> {
let BufferElementTypes = [ QualType ];
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 8f2714e142bbe3..b57ab36939d07c 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1099,6 +1099,12 @@ DEF_TRAVERSE_TYPE(InjectedClassNameType, {})
DEF_TRAVERSE_TYPE(AttributedType,
{ TRY_TO(TraverseType(T->getModifiedType())); })
+DEF_TRAVERSE_TYPE(CountAttributedType, {
+ if (T->getCountExpr())
+ TRY_TO(TraverseStmt(T->getCountExpr()));
+ TRY_TO(TraverseType(T->desugar()));
+})
+
DEF_TRAVERSE_TYPE(BTFTagAttributedType,
{ TRY_TO(TraverseType(T->getWrappedType())); })
@@ -1385,6 +1391,9 @@ DEF_TRAVERSE_TYPELOC(MacroQualifiedType,
DEF_TRAVERSE_TYPELOC(AttributedType,
{ TRY_TO(TraverseTypeLoc(TL.getModifiedLoc())); })
+DEF_TRAVERSE_TYPELOC(CountAttributedType,
+ { TRY_TO(TraverseTypeLoc(TL.getInnerLoc())); })
+
DEF_TRAVERSE_TYPELOC(BTFTagAttributedType,
{ TRY_TO(TraverseTypeLoc(TL.getWrappedLoc())); })
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index d4e5310fb3abc6..489644ca5acf86 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -61,6 +61,7 @@ class BTFTypeTagAttr;
class ExtQuals;
class QualType;
class ConceptDecl;
+class ValueDecl;
class TagDecl;
class TemplateParameterList;
class Type;
@@ -2000,6 +2001,21 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
unsigned NumExpansions;
};
+ class CountAttributedTypeBitfields {
+ friend class CountAttributedType;
+
+ LLVM_PREFERRED_TYPE(TypeBitfields)
+ unsigned : NumTypeBits;
+
+ /// The limit is 15.
+ static constexpr unsigned NumCoupledDeclsBits = 4;
+ unsigned NumCoupledDecls : NumCoupledDeclsBits;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned CountInBytes : 1;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned OrNull : 1;
+ };
+
union {
TypeBitfields TypeBits;
ArrayTypeBitfields ArrayTypeBits;
@@ -2022,6 +2038,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
DependentTemplateSpecializationTypeBitfields
DependentTemplateSpecializationTypeBits;
PackExpansionTypeBitfields PackExpansionTypeBits;
+ CountAttributedTypeBitfields CountAttributedTypeBits;
};
private:
@@ -2719,6 +2736,14 @@ template <> const TemplateSpecializationType *Type::getAs() const;
/// until it reaches an AttributedType or a non-sugared type.
template <> const AttributedType *Type::getAs() const;
+/// This will check for a BoundsAttributedType by removing any existing
+/// sugar until it reaches an BoundsAttributedType or a non-sugared type.
+template <> const BoundsAttributedType *Type::getAs() const;
+
+/// This will check for a CountAttributedType by removing any existing
+/// sugar until it reaches an CountAttributedType or a non-sugared type.
+template <> const CountAttributedType *Type::getAs() const;
+
// We can do canonical leaf types faster, because we don't have to
// worry about preserving child type decoration.
#define TYPE(Class, Base)
@@ -2917,6 +2942,133 @@ class PointerType : public Type, public llvm::FoldingSetNode {
static bool classof(const Type *T) { return T->getTypeClass() == Pointer; }
};
+/// [BoundsSafety] Represents information of declarations referenced by the
+/// arguments of the `counted_by` attribute and the likes.
+class TypeCoupledDeclRefInfo {
+public:
+ using BaseTy = llvm::PointerIntPair<ValueDecl *, 1, unsigned>;
+
+private:
+ enum {
+ DerefShift = 0,
+ DerefMask = 1,
+ };
+ BaseTy Data;
+
+public:
+ /// \p D is to a declaration referenced by the argument of attribute. \p Deref
+ /// indicates whether \p D is referenced as a dereferenced form, e.g., \p
+ /// Deref is true for `*n` in `int *__counted_by(*n)`.
+ TypeCoupledDeclRefInfo(ValueDecl *D = nullptr, bool Deref = false);
+
+ bool isDeref() const;
+ ValueDecl *getDecl() const;
+ unsigned getInt() const;
+ void *getOpaqueValue() const;
+ bool operator==(const TypeCoupledDeclRefInfo &Other) const;
+ void setFromOpaqueValue(void *V);
+};
+
+/// [BoundsSafety] Represents a parent type class for CountAttributedType and
+/// similar sugar types that will be introduced to represent a type with a
+/// bounds attribute.
+///
+/// Provides a common interface to navigate declarations referred to by the
+/// bounds expression.
+
+class BoundsAttributedType : public Type, public llvm::FoldingSetNode {
+ QualType WrappedTy;
+
+protected:
+ ArrayRef<TypeCoupledDeclRefInfo> Decls; // stored in trailing objects
+
+ BoundsAttributedType(TypeClass TC, QualType Wrapped, QualType Canon);
+
+public:
+ bool isSugared() const { return true; }
+ QualType desugar() const { return WrappedTy; }
+
+ using decl_iterator = const TypeCoupledDeclRefInfo *;
+ using decl_range = llvm::iterator_range<decl_iterator>;
+
+ decl_iterator dependent_decl_begin() const { return Decls.begin(); }
+ decl_iterator dependent_decl_end() const { return Decls.end(); }
+
+ unsigned getNumCoupledDecls() const { return Decls.size(); }
+
+ decl_range dependent_decls() const {
+ return decl_range(dependent_decl_begin(), dependent_decl_end());
+ }
+
+ ArrayRef<TypeCoupledDeclRefInfo> getCoupledDecls() const {
+ return {dependent_decl_begin(), dependent_decl_end()};
+ }
+
+ bool referencesFieldDecls() const;
+
+ static bool classof(const Type *T) {
+ switch (T->getTypeClass()) {
+ case CountAttributed:
+ return true;
+ default:
+ return false;
+ }
+ }
+};
+
+/// Represents a sugar type with `__counted_by` or `__sized_by` annotations,
+/// including their `_or_null` variants.
+class CountAttributedType final
+ : public BoundsAttributedType,
+ public llvm::TrailingObjects<CountAttributedType,
+ TypeCoupledDeclRefInfo> {
+ friend class ASTContext;
+
+ Expr *CountExpr;
+ /// \p CountExpr represents the argument of __counted_by or the likes. \p
+ /// CountInBytes indicates that \p CountExpr is a byte count (i.e.,
+ /// __sized_by(_or_null)) \p OrNull means it's an or_null variant (i.e.,
+ /// __counted_by_or_null or __sized_by_or_null) \p CoupledDecls contains the
+ /// list of declarations referenced by \p CountExpr, which the type depends on
+ /// for the bounds information.
+ CountAttributedType(QualType Wrapped, QualType Canon, Expr *CountExpr,
+ bool CountInBytes, bool OrNull,
+ ArrayRef<TypeCoupledDeclRefInfo> CoupledDecls);
+
+ unsigned numTrailingObjects(OverloadToken<TypeCoupledDeclRefInfo>) const {
+ return CountAttributedTypeBits.NumCoupledDecls;
+ }
+
+public:
+ enum DynamicCountPointerKind {
+ CountedBy = 0,
+ SizedBy,
+ CountedByOrNull,
+ SizedByOrNull,
+ };
+
+ Expr *getCountExpr() const { return CountExpr; }
+ bool isCountInBytes() const { return CountAttributedTypeBits.CountInBytes; }
+ bool isOrNull() const { return CountAttributedTypeBits.OrNull; }
+
+ DynamicCountPointerKind getKind() const {
+ if (isOrNull())
+ return isCountInBytes() ? SizedByOrNull : CountedByOrNull;
+ return isCountInBytes() ? SizedBy : CountedBy;
+ }
+
+ void Profile(llvm::FoldingSetNodeID &ID) {
+ Profile(ID, desugar(), CountExpr, isCountInBytes(), isOrNull());
+ }
+
+ static void Profile(llvm::FoldingSetNodeID &ID, QualType WrappedTy,
+ Expr *CountExpr, bool CountInBytes, bool Nullable);
+
+ static bool classof(const Type *T) {
+ return T->getTypeClass() == CountAttributed;
+ }
+};
+
/// Represents a type which was implicitly adjusted by the semantic
/// engine for arbitrary reasons. For example, array and function types can
/// decay, and function types can have their calling conventions adjusted.
diff --git a/clang/include/clang/AST/TypeLoc.h b/clang/include/clang/AST/TypeLoc.h
index 471deb14aba51f..fa3cd46ffd3fc0 100644
--- a/clang/include/clang/AST/TypeLoc.h
+++ b/clang/include/clang/AST/TypeLoc.h
@@ -1110,6 +1110,32 @@ class ObjCInterfaceTypeLoc : public ConcreteTypeLoc<ObjCObjectTypeLoc,
}
};
+struct BoundsAttributedLocInfo {};
+class BoundsAttributedTypeLoc
+ : public ConcreteTypeLoc<UnqualTypeLoc, BoundsAttributedTypeLoc,
+ BoundsAttributedType, BoundsAttributedLocInfo> {
+public:
+ TypeLoc getInnerLoc() const { return this->getInnerTypeLoc(); }
+ QualType getInnerType() const { return this->getTypePtr()->desugar(); }
+ void initializeLocal(ASTContext &Context, SourceLocation Loc) {
+ // nothing to do
+ }
+ unsigned getLocalDataAlignment() const { return 1; }
+ unsigned getLocalDataSize() const { return 0; }
+};
+
+class CountAttributedTypeLoc final
+ : public InheritingConcreteTypeLoc<BoundsAttributedTypeLoc,
+ CountAttributedTypeLoc,
+ CountAttributedType> {
+public:
+ Expr *getCountExpr() const { return getTypePtr()->getCountExpr(); }
+ bool isCountInBytes() const { return getTypePtr()->isCountInBytes(); }
+ bool isOrNull() const { return getTypePtr()->isOrNull(); }
+
+ SourceRange getLocalSourceRange() const;
+};
+
struct MacroQualifiedLocInfo {
SourceLocation ExpansionLoc;
};
diff --git a/clang/include/clang/AST/TypeProperties.td b/clang/include/clang/AST/TypeProperties.td
index 682c869b0c5847..8697903d8d7f6e 100644
--- a/clang/include/clang/AST/TypeProperties.td
+++ b/clang/include/clang/AST/TypeProperties.td
@@ -25,6 +25,25 @@ let Class = PointerType in {
def : Creator<[{ return ctx.getPointerType(pointeeType); }]>;
}
+let Class = CountAttributedType in {
+ def : Property<"WrappedTy", QualType> {
+ let Read = [{ node->desugar() }];
+ }
+ def : Property<"CountExpr", ExprRef> {
+ let Read = [{ node->getCountExpr() }];
+ }
+ def : Property<"CountInBytes", Bool> {
+ let Read = [{ node->isCountInBytes() }];
+ }
+ def : Property<"OrNull", Bool> {
+ let Read = [{ node->isOrNull() }];
+ }
+ def : Property<"CoupledDecls", Array<TypeCoupledDeclRefInfo>> {
+ let Read = [{ node->getCoupledDecls() }];
+ }
+ def : Creator<[{ return ctx.getCountAttributedType(WrappedTy, CountExpr, CountInBytes, OrNull, CoupledDecls); }]>;
+}
+
let Class = AdjustedType in {
def : Property<"originalType", QualType> {
let Read = [{ node->getOriginalType() }];
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index a03b0e44e15f7d..af2680be0da906 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4373,10 +4373,11 @@ def CodeAlign: StmtAttr {
}];
}
-def CountedBy : InheritableAttr {
+def CountedBy : DeclOrTypeAttr {
let Spellings = [Clang<"counted_by">];
- let Subjects = SubjectList<[Field]>;
- let Args = [IdentifierArgument<"CountedByField">];
+ let Subjects = SubjectList<[Field], ErrorDiag>;
+ let Args = [ExprArgument<"Count">, IntArgument<"NestedLevel">];
+ let ParseArgumentsAsUnevaluated = 1;
let Documentation = [CountedByDocs];
let LangOpts = [COnly];
// FIXME: This is ugly. Let using a DeclArgument would be nice, but a Decl
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1a79892e40030a..ae8b90964c3fc9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6445,12 +6445,18 @@ def err_flexible_array_count_not_in_same_struct : Error<
"'counted_by' field %0 isn't within the same struct as the flexible array">;
def err_counted_by_attr_not_on_flexible_array_member : Error<
"'counted_by' only applies to C99 flexible array members">;
-def err_counted_by_attr_refers_to_flexible_array : Error<
- "'counted_by' cannot refer to the flexible array %0">;
+def err_counted_by_attr_refer_to_itself : Error<
+ "'counted_by' cannot refer to the flexible array member %0">;
def err_counted_by_must_be_in_structure : Error<
"field %0 in 'counted_by' not inside structure">;
-def err_flexible_array_counted_by_attr_field_not_integer : Error<
- "field %0 in 'counted_by' must be a non-boolean integer type">;
+def err_counted_by_attr_argument_not_integer : Error<
+ "'counted_by' requires a non-boolean integer type argument">;
+def err_counted_by_attr_only_support_simple_decl_reference : Error<
+ "'counted_by' argument must be a simple declaration reference">;
+def err_counted_by_attr_in_union : Error<
+ "'counted_by' cannot be applied to a union member">;
+def err_counted_by_attr_refer_to_union : Error<
+ "'counted_by' argument cannot refer to a union member">;
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;
diff --git a/clang/include/clang/Basic/TypeNodes.td b/clang/include/clang/Basic/TypeNodes.td
index 649b071cebb940..da733716fa1f66 100644
--- a/clang/include/clang/Basic/TypeNodes.td
+++ b/clang/include/clang/Basic/TypeNodes.td
@@ -107,6 +107,8 @@ def ObjCTypeParamType : TypeNode<Type>, NeverCanonical;
def ObjCObjectType : TypeNode<Type>;
def ObjCInterfaceType : TypeNode<ObjCObjectType>, LeafType;
def ObjCObjectPointerType : TypeNode<Type>;
+def BoundsAttributedType : TypeNode<Type, 1>;
+def CountAttributedType : TypeNode<BoundsAttributedType>, NeverCanonical;
def PipeType : TypeNode<Type>;
def AtomicType : TypeNode<Type>;
def BitIntType : TypeNode<Type>;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index e50a4d05b45991..f88191ef5f52c2 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3054,6 +3054,13 @@ class Parser : public CodeCompletionHandler {
SourceLocation ScopeLoc,
ParsedAttr::Form Form);
+ void ParseBoundsAttribute(IdentifierInfo &AttrName,
+ SourceLocation AttrNameLoc,
+ ParsedAttributes &Attrs,
+ IdentifierInfo *ScopeName,
+ SourceLocation ScopeLoc,
+ ParsedAttr::Form Form);
+
void ParseTypeofSpecifier(DeclSpec &DS);
SourceLocation ParseDecltypeSpecifier(DeclSpec &DS);
void AnnotateExistingDecltypeSpecifier(const DeclSpec &DS,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index cf2d4fbe6d3ba1..027afb5d703d1d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1341,7 +1341,7 @@ class Sema final {
/// \brief Describes whether we are in an expression constext which we have
/// to handle differently.
enum ExpressionKind {
- EK_Decltype, EK_TemplateArgument, EK_Other
+ EK_Decltype, EK_TemplateArgument, EK_BoundsAttrArgument, EK_Other
} ExprContext;
// A context can be nested in both a discarded statement context and
@@ -1436,6 +1436,11 @@ class Sema final {
std::tuple<MangleNumberingContext *, Decl *>
getCurrentMangleNumberContext(const DeclContext *DC);
+ bool isBoundsAttrArgument() const {
+ return ExprEvalContexts.back().ExprContext ==
+ ExpressionEvaluationContextRecord::ExpressionKind::EK_BoundsAttrArgument;
+ }
+
/// SpecialMemberOverloadResult - The overloading result for a special member
/// function.
@@ -2612,6 +2617,9 @@ class Sema final {
QualType BuiltinChangeSignedness(QualType BaseType, UTTKind UKind,
SourceLocation Loc);
+ QualType BuildCountAttributedArrayType(QualType WrappedTy, Expr *CountExpr,
+ const llvm::SmallVector<TypeCoupledDeclRefInfo, 1> &Decls);
+
//===--------------------------------------------------------------------===//
// Symbol table / Decl tracking callbacks: SemaDecl.cpp.
//
@@ -4799,8 +4807,6 @@ class Sema final {
bool CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
const AttributeCommonInfo &A);
- bool CheckCountedByAttr(Scope *Scope, const FieldDecl *FD);
-
/// Adjust the calling convention of a method to be the ABI default if it
/// wasn't specified explicitly. This handles method types formed from
/// function type typedefs and typename template arguments.
diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h
index 80a1359fd16aa0..5d3e95cb5d630f 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -221,6 +221,8 @@ class ASTRecordReader
return Reader->ReadSelector(*F, Record, Idx);
}
+ TypeCoupledDeclRefInfo readTypeCoupledDeclRefInfo();
+
/// Read a declaration name, advancing Idx.
// DeclarationName readDeclarationName(); (inherited)
DeclarationNameLoc readDeclarationNameLoc(DeclarationName Name);
diff --git a/clang/include/clang/Serialization/ASTRecordWriter.h b/clang/include/clang/Serialization/ASTRecordWriter.h
index 9a64735c9fa55d..e007d4a70843a1 100644
--- a/clang/include/clang/Serialization/ASTRecordWriter.h
+++ b/clang/include/clang/Serialization/ASTRecordWriter.h
@@ -141,6 +141,11 @@ class ASTRecordWriter
AddSourceLocation(Loc);
}
+ void writeTypeCoupledDeclRefInfo(TypeCoupledDeclRefInfo Info) {
+ writeDeclRef(Info.getDecl());
+ writeBool(Info.isDeref());
+ }
+
/// Emit a source range.
void AddSourceRange(SourceRange Range, LocSeq *Seq = nullptr) {
return Writer->AddSourceRange(Range, *Record, Seq);
diff --git a/clang/include/clang/Serialization/TypeBitCodes.def b/clang/include/clang/Serialization/TypeBitCodes.def
index 89ae1a2fa39546..c04dacdaa15820 100644
--- a/clang/include/clang/Serialization/TypeBitCodes.def
+++ b/clang/include/clang/Serialization/TypeBitCodes.def
@@ -64,5 +64,6 @@ TYPE_BIT_CODE(ConstantMatrix, CONSTANT_MATRIX, 52)
TYPE_BIT_CODE(DependentSizedMatrix, DEPENDENT_SIZE_MATRIX, 53)
TYPE_BIT_CODE(Using, USING, 54)
TYPE_BIT_CODE(BTFTagAttributed, BTFTAG_ATTRIBUTED, 55)
+TYPE_BIT_CODE(CountAttributed, COUNT_ATTRIBUTED, 56)
#undef TYPE_BIT_CODE
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index d9cefcaa84d7e5..43d2da0e60f367 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2341,6 +2341,9 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
return getTypeInfo(
cast<AttributedType>(T)->getEquivalentType().getTypePtr());
+ case Type::CountAttributed:
+ return getTypeInfo(cast<CountAttributedType>(T)->des...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
rapidsna
changed the title
rapidsna
requested review from
kees,
AaronBallman,
bwendling,
devincoughlin,
apple-fcloutier and
nickdesaulniers
rapidsna
force-pushed
the
bounds-safety/counted_by_type
branch
from
d501646 to
5fd456d
Compare
rapidsna
added
the
clang:frontend
rapidsna
force-pushed
the
bounds-safety/counted_by_type
branch
from
2ada26d to
c39871e
Compare
llvmbot
added
clang:modules
|
@AaronBallman @bwendling I'd appreciate your feedback! |
|
It's generally not a good idea to use sugar to represent constructs that are semantically significant: anything that uses the canonical type will throw it away. We try to avoid throwing away sugar in most cases, but we aren't always successful. It's possible there are other considerations here (maybe it matters less if the attribute is only relevant inside the definition?), but I'd like an explanation of the tradeoffs. |
Thanks @efriedma-quic! Yes, these are really good points. The main reason that we decided it to be a sugar type was that this type of attribute doesn't change the shape of the underlying type so we wanted such as But as you pointed out we encountered cases where the sugar wasn't preserved as it needed to be so we had to specially handle such cases in our own implementation, so we're open for other suggestions. An alternative design could be to create new canonical types that inherit We could also add an additional field in the Similarly, we could add more fields to We haven't experimented these alternative designs so I'm curious about @AaronBallman's thoughts. |
erichkeane
reviewed
Feb 27, 2024
|
@AaronBallman thanks for the review! I addressed your comments. |
|
Can you resolve merge conflicts, so that I can review changes to |
CountAttributedType is a sugar type to represent a type with a 'counted_by' attribute and the likes, which provides bounds information to the underlying type. The type contains an the argument of attribute as an expression. Additionally, the type holds metadata about declarations referenced by the expression in order to make it easier for Sema to access declarations on which the type depends. This also adjusts the CountedBy attribute definition and implements parsing CountAttributedType. __bdos and array-checks sanitizer use CountAttributedType instead of hasAttr<CountedByAttr>. Implements special lookup for counted_by argument in structs. Adjust test/Sema/attr-counted-by.c to match the default diags generated by the expression parser.
rapidsna
force-pushed
the
bounds-safety/counted_by_type
branch
from
a92e090 to
8f18416
Compare
|
@Endilll I just rebased it so you can review |
Endilll
reviewed
Mar 7, 2024
… type attribute with an expression
…'t handle DataSize 1
AaronBallman
approved these changes
Mar 13, 2024
There was a problem hiding this comment.
Thank you for this, it's exciting to see more bounds safety being added to Clang!
I think everything looks good to me (we can clean up any surprises in follow-ups at this point). I'm presuming you'll add a release note & docs once more of the functionality is place?
|
@AaronBallman Thank you! The -fbounds-safety documentation is currently shown in https://clang.llvm.org/docs/BoundsSafety.html, but yes, we're planning to add the feature in the release note once we have more functionalities in place. |
|
I see a crash from an unreachable statement while building the Linux kernel with debug info enabled after this change. A trivial reproducer from struct {
int num_counters;
long value[] __attribute__((__counted_by__(num_counters)));
} agent_send_response_port_num;cc @kees @bwendling |
|
@nathanchance Thanks for reporting this. I'm going to have a quick go at reproducing this to see if fixing this is straight forward. If it is not I will revert this PR and then we can re-land this change with the problem you reported fixed. |
|
Thanks @nathanchance. Opened PR to fix the crash. #86017 |
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
…butedType' (llvm#78000) In `-fbounds-safety`, bounds annotations are considered type attributes rather than declaration attributes. Constructing them as type attributes allows us to extend the attribute to apply nested pointers, which is essential to annotate functions that involve out parameters: `void foo(int *__counted_by(*out_count) *out_buf, int *out_count)`. We introduce a new sugar type to support bounds annotated types, `CountAttributedType`. In order to maintain extra data (the bounds expression and the dependent declaration information) that is not trackable in `AttributedType` we create a new type dedicate to this functionality. This patch also extends the parsing logic to parse the `counted_by` argument as an expression, which will allow us to extend the model to support arguments beyond an identifier, e.g., `__counted_by(n + m)` in the future as specified by `-fbounds-safety`. This also adjusts `__bdos` and array-bounds sanitizer code that already uses `CountedByAttr` to check `CountAttributedType` instead to get the field referred to by the attribute.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In
-fbounds-safety, bounds annotations are considered type attributes rather than declaration attributes. Constructing them as type attributes allows us to extend the attribute to apply nested pointers, which is essential to annotate functions that involve out parameters:void foo(int *__counted_by(*out_count) *out_buf, int *out_count).We introduce a new sugar type to support bounds annotated types,
CountAttributedType. In order to maintain extra data (the bounds expression and the dependent declaration information) that is not trackable inAttributedTypewe create a new type dedicate to this functionality.This patch also extends the parsing logic to parse the
counted_byargument as an expression, which will allow us to extend the model to support arguments beyond an identifier, e.g.,__counted_by(n + m)in the future as specified by-fbounds-safety.This also adjusts
__bdosand array-bounds sanitizer code that already usesCountedByAttrto checkCountAttributedTypeinstead to get the field referred to by the attribute.