https://github.com/delcypher created
https://github.com/llvm/llvm-project/pull/99330
This patch adds a new `SemaBoundsSafety.cpp` source file and moves the existing
`CheckCountedByAttrOnField` function and related helper functions and types
from `SemaDeclAttr.cpp` into the new source file. The
`CheckCountedByAttrOnField` function is now a method on the Sema class and now
has doxygen comments.
The goal behind this refactor is to clearly separate the `-fbounds-safety` Sema
code from everything else.
Although `counted_by(_or_null)` and `sized_by(_or_null)` attributes have a
meaning outside of `-fbounds-safety` it seems reasonable to also have the Sema
logic live in `SemaBoundsSafety.cpp` since the intention is that the attributes
will have the same semantics (but not necessarily the same enforcement).
As `-fbounds-safety` is upstreamed additional Sema checks will be added to
`SemaBoundsSafety.cpp`.
rdar://131777237
>From 050c328e90109c3f8a2c98da8df50f2865d67095 Mon Sep 17 00:00:00 2001
From: Dan Liew <d...@su-root.co.uk>
Date: Wed, 17 Jul 2024 14:53:52 +0100
Subject: [PATCH] [Bounds Safety][NFC] Move Bounds Safety Sema code to
`SemaBoundsSafety.cpp`
This patch adds a new `SemaBoundsSafety.cpp` source file and moves the
existing `CheckCountedByAttrOnField` function and related helper
functions and types from `SemaDeclAttr.cpp` into the new source file.
The `CheckCountedByAttrOnField` function is now a method on the Sema
class and now has doxygen comments.
The goal behind this refactor is to clearly separate the
`-fbounds-safety` Sema code from everything else.
Although `counted_by(_or_null)` and `sized_by(_or_null)` attributes have
a meaning outside of `-fbounds-safety` it seems reasonable to also have
the Sema logic live in `SemaBoundsSafety.cpp` since the intention is
that the attributes will have the same semantics (but not necessarily
the same enforcement).
As `-fbounds-safety` is upstreamed additional Sema checks will be
added to `SemaBoundsSafety.cpp`.
rdar://131777237
---
clang/include/clang/Sema/Sema.h | 38 ++++++
clang/lib/Sema/CMakeLists.txt | 1 +
clang/lib/Sema/SemaBoundsSafety.cpp | 193 ++++++++++++++++++++++++++++
clang/lib/Sema/SemaDeclAttr.cpp | 177 +------------------------
4 files changed, 233 insertions(+), 176 deletions(-)
create mode 100644 clang/lib/Sema/SemaBoundsSafety.cpp
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 48dff1b76cc57..0850ed456bcf6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -15046,6 +15046,44 @@ class Sema final : public SemaBase {
void ProcessAPINotes(Decl *D);
///@}
+
+ //
+ //
+ // -------------------------------------------------------------------------
+ //
+ //
+
+ /// \name Bounds Safety
+ /// Implementations are in SemaBoundsSafety.cpp
+ ///@{
+public:
+ /// 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.
+ ///
+ /// \param FD The FieldDecl to apply the attribute to
+ /// \param E The count expression on the attribute
+ /// \param[out] Decls If the attribute is semantically valid \p Decls
+ /// is populated with TypeCoupledDeclRefInfo objects, each
+ /// describing Decls referred to in \p E.
+ /// \param CountInBytes If true the attribute is from the "sized_by" family
of
+ /// attributes. If the false the attribute is from
+ /// "counted_by" family of attributes.
+ /// \param OrNull If true the attribute is from the "_or_null" suffixed
family
+ /// of attributes. If false the attribute does not have the
+ /// suffix.
+ ///
+ /// Together \p CountInBytes and \p OrNull decide the attribute variant. E.g.
+ /// \p CountInBytes and \p OrNull both being true indicates the
+ /// `counted_by_or_null` attribute.
+ ///
+ /// \returns false iff semantically valid.
+ bool CheckCountedByAttrOnField(
+ FieldDecl *FD, Expr *E,
+ llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls, bool CountInBytes,
+ bool OrNull);
+
+ ///@}
};
DeductionFailureInfo
diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 5934c8c30daf9..2cee4f5ef6e99 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -36,6 +36,7 @@ add_clang_library(clangSema
SemaAvailability.cpp
SemaBPF.cpp
SemaBase.cpp
+ SemaBoundsSafety.cpp
SemaCXXScopeSpec.cpp
SemaCast.cpp
SemaChecking.cpp
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp
b/clang/lib/Sema/SemaBoundsSafety.cpp
new file mode 100644
index 0000000000000..5bb1c8b276c85
--- /dev/null
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -0,0 +1,193 @@
+//===-- SemaBoundsSafety.cpp - Bounds Safety specific routines-*- C++
-*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file declares semantic analysis functions specific to
`-fbounds-safety`
+/// (Bounds Safety) and also its attributes when used without `-fbounds-safety`
+/// (e.g. `counted_by`)
+///
+//===----------------------------------------------------------------------===//
+#include "clang/Sema/Sema.h"
+
+namespace clang {
+
+static CountAttributedType::DynamicCountPointerKind
+getCountAttrKind(bool CountInBytes, bool OrNull) {
+ if (CountInBytes)
+ return OrNull ? CountAttributedType::SizedByOrNull
+ : CountAttributedType::SizedBy;
+ return OrNull ? CountAttributedType::CountedByOrNull
+ : CountAttributedType::CountedBy;
+}
+
+static const RecordDecl *GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD)
{
+ const auto *RD = FD->getParent();
+ // An unnamed struct is anonymous struct only if it's not instantiated.
+ // However, the struct may not be fully processed yet to determine
+ // whether it's anonymous or not. In that case, this function treats it as
+ // an anonymous struct and tries to find a named parent.
+ while (RD && (RD->isAnonymousStructOrUnion() ||
+ (!RD->isCompleteDefinition() && RD->getName().empty()))) {
+ const auto *Parent = dyn_cast<RecordDecl>(RD->getParent());
+ if (!Parent)
+ break;
+ RD = Parent;
+ }
+ return RD;
+}
+
+enum class CountedByInvalidPointeeTypeKind {
+ INCOMPLETE,
+ SIZELESS,
+ FUNCTION,
+ FLEXIBLE_ARRAY_MEMBER,
+ VALID,
+};
+
+bool Sema::CheckCountedByAttrOnField(
+ FieldDecl *FD, Expr *E,
+ llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls, bool CountInBytes,
+ bool OrNull) {
+ // Check the context the attribute is used in
+
+ unsigned Kind = getCountAttrKind(CountInBytes, OrNull);
+
+ if (FD->getParent()->isUnion()) {
+ Diag(FD->getBeginLoc(), diag::err_count_attr_in_union)
+ << Kind << FD->getSourceRange();
+ return true;
+ }
+
+ const auto FieldTy = FD->getType();
+ if (FieldTy->isArrayType() && (CountInBytes || OrNull)) {
+ Diag(FD->getBeginLoc(),
+ diag::err_count_attr_not_on_ptr_or_flexible_array_member)
+ << Kind << FD->getLocation() << /* suggest counted_by */ 1;
+ return true;
+ }
+ if (!FieldTy->isArrayType() && !FieldTy->isPointerType()) {
+ Diag(FD->getBeginLoc(),
+ diag::err_count_attr_not_on_ptr_or_flexible_array_member)
+ << Kind << FD->getLocation() << /* do not suggest counted_by */ 0;
+ return true;
+ }
+
+ LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
+ LangOptions::StrictFlexArraysLevelKind::IncompleteOnly;
+ if (FieldTy->isArrayType() &&
+ !Decl::isFlexibleArrayMemberLike(getASTContext(), FD, FieldTy,
+ StrictFlexArraysLevel, true)) {
+ Diag(FD->getBeginLoc(),
+ diag::err_counted_by_attr_on_array_not_flexible_array_member)
+ << Kind << FD->getLocation();
+ return true;
+ }
+
+ CountedByInvalidPointeeTypeKind InvalidTypeKind =
+ CountedByInvalidPointeeTypeKind::VALID;
+ QualType PointeeTy;
+ int SelectPtrOrArr = 0;
+ if (FieldTy->isPointerType()) {
+ PointeeTy = FieldTy->getPointeeType();
+ SelectPtrOrArr = 0;
+ } else {
+ assert(FieldTy->isArrayType());
+ const ArrayType *AT = getASTContext().getAsArrayType(FieldTy);
+ PointeeTy = AT->getElementType();
+ SelectPtrOrArr = 1;
+ }
+ // Note: The `Decl::isFlexibleArrayMemberLike` check earlier on means
+ // only `PointeeTy->isStructureTypeWithFlexibleArrayMember()` is reachable
+ // when `FieldTy->isArrayType()`.
+ bool ShouldWarn = false;
+ if (PointeeTy->isIncompleteType() && !CountInBytes) {
+ InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
+ } else if (PointeeTy->isSizelessType()) {
+ InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS;
+ } else if (PointeeTy->isFunctionType()) {
+ InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION;
+ } else if (PointeeTy->isStructureTypeWithFlexibleArrayMember()) {
+ if (FieldTy->isArrayType()) {
+ // 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(FD->getBeginLoc(), DiagID)
+ << SelectPtrOrArr << PointeeTy << (int)InvalidTypeKind
+ << (ShouldWarn ? 1 : 0) << Kind << FD->getSourceRange();
+ return true;
+ }
+
+ // Check the expression
+
+ if (!E->getType()->isIntegerType() || E->getType()->isBooleanType()) {
+ Diag(E->getBeginLoc(), diag::err_count_attr_argument_not_integer)
+ << Kind << E->getSourceRange();
+ return true;
+ }
+
+ auto *DRE = dyn_cast<DeclRefExpr>(E);
+ if (!DRE) {
+ Diag(E->getBeginLoc(),
+ diag::err_count_attr_only_support_simple_decl_reference)
+ << Kind << E->getSourceRange();
+ return true;
+ }
+
+ auto *CountDecl = DRE->getDecl();
+ FieldDecl *CountFD = dyn_cast<FieldDecl>(CountDecl);
+ if (auto *IFD = dyn_cast<IndirectFieldDecl>(CountDecl)) {
+ CountFD = IFD->getAnonField();
+ }
+ if (!CountFD) {
+ Diag(E->getBeginLoc(), diag::err_count_attr_must_be_in_structure)
+ << CountDecl << Kind << E->getSourceRange();
+
+ Diag(CountDecl->getBeginLoc(),
+ diag::note_flexible_array_counted_by_attr_field)
+ << CountDecl << CountDecl->getSourceRange();
+ return true;
+ }
+
+ if (FD->getParent() != CountFD->getParent()) {
+ if (CountFD->getParent()->isUnion()) {
+ Diag(CountFD->getBeginLoc(), diag::err_count_attr_refer_to_union)
+ << Kind << CountFD->getSourceRange();
+ return true;
+ }
+ // Whether CountRD is an anonymous struct is not determined at this
+ // point. Thus, an additional diagnostic in case it's not anonymous struct
+ // is done later in `Parser::ParseStructDeclaration`.
+ auto *RD = GetEnclosingNamedOrTopAnonRecord(FD);
+ auto *CountRD = GetEnclosingNamedOrTopAnonRecord(CountFD);
+
+ if (RD != CountRD) {
+ Diag(E->getBeginLoc(), diag::err_count_attr_param_not_in_same_struct)
+ << CountFD << Kind << FieldTy->isArrayType() << E->getSourceRange();
+ Diag(CountFD->getBeginLoc(),
+ diag::note_flexible_array_counted_by_attr_field)
+ << CountFD << CountFD->getSourceRange();
+ return true;
+ }
+ }
+
+ Decls.push_back(TypeCoupledDeclRefInfo(CountFD, /*IsDref*/ false));
+ return false;
+}
+
+} // namespace clang
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 20f46c003a464..3b3c352332c31 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5852,181 +5852,6 @@ static void handleZeroCallUsedRegsAttr(Sema &S, Decl
*D, const ParsedAttr &AL) {
D->addAttr(ZeroCallUsedRegsAttr::Create(S.Context, Kind, AL));
}
-static const RecordDecl *GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD)
{
- const auto *RD = FD->getParent();
- // An unnamed struct is anonymous struct only if it's not instantiated.
- // However, the struct may not be fully processed yet to determine
- // whether it's anonymous or not. In that case, this function treats it as
- // an anonymous struct and tries to find a named parent.
- while (RD && (RD->isAnonymousStructOrUnion() ||
- (!RD->isCompleteDefinition() && RD->getName().empty()))) {
- const auto *Parent = dyn_cast<RecordDecl>(RD->getParent());
- if (!Parent)
- break;
- RD = Parent;
- }
- return RD;
-}
-
-static CountAttributedType::DynamicCountPointerKind
-getCountAttrKind(bool CountInBytes, bool OrNull) {
- if (CountInBytes)
- return OrNull ? CountAttributedType::SizedByOrNull
- : CountAttributedType::SizedBy;
- return OrNull ? CountAttributedType::CountedByOrNull
- : CountAttributedType::CountedBy;
-}
-
-enum class CountedByInvalidPointeeTypeKind {
- INCOMPLETE,
- SIZELESS,
- FUNCTION,
- FLEXIBLE_ARRAY_MEMBER,
- VALID,
-};
-
-static bool
-CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
- llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls,
- bool CountInBytes, bool OrNull) {
- // Check the context the attribute is used in
-
- unsigned Kind = getCountAttrKind(CountInBytes, OrNull);
-
- if (FD->getParent()->isUnion()) {
- S.Diag(FD->getBeginLoc(), diag::err_count_attr_in_union)
- << Kind << FD->getSourceRange();
- return true;
- }
-
- const auto FieldTy = FD->getType();
- if (FieldTy->isArrayType() && (CountInBytes || OrNull)) {
- S.Diag(FD->getBeginLoc(),
- diag::err_count_attr_not_on_ptr_or_flexible_array_member)
- << Kind << FD->getLocation() << /* suggest counted_by */ 1;
- return true;
- }
- if (!FieldTy->isArrayType() && !FieldTy->isPointerType()) {
- S.Diag(FD->getBeginLoc(),
- diag::err_count_attr_not_on_ptr_or_flexible_array_member)
- << Kind << FD->getLocation() << /* do not suggest counted_by */ 0;
- return true;
- }
-
- LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
- LangOptions::StrictFlexArraysLevelKind::IncompleteOnly;
- if (FieldTy->isArrayType() &&
- !Decl::isFlexibleArrayMemberLike(S.getASTContext(), FD, FieldTy,
- StrictFlexArraysLevel, true)) {
- S.Diag(FD->getBeginLoc(),
- diag::err_counted_by_attr_on_array_not_flexible_array_member)
- << Kind << FD->getLocation();
- return true;
- }
-
- CountedByInvalidPointeeTypeKind InvalidTypeKind =
- CountedByInvalidPointeeTypeKind::VALID;
- QualType PointeeTy;
- int SelectPtrOrArr = 0;
- if (FieldTy->isPointerType()) {
- PointeeTy = FieldTy->getPointeeType();
- SelectPtrOrArr = 0;
- } else {
- assert(FieldTy->isArrayType());
- const ArrayType *AT = S.getASTContext().getAsArrayType(FieldTy);
- PointeeTy = AT->getElementType();
- SelectPtrOrArr = 1;
- }
- // Note: The `Decl::isFlexibleArrayMemberLike` check earlier on means
- // only `PointeeTy->isStructureTypeWithFlexibleArrayMember()` is reachable
- // when `FieldTy->isArrayType()`.
- bool ShouldWarn = false;
- if (PointeeTy->isIncompleteType() && !CountInBytes) {
- InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
- } else if (PointeeTy->isSizelessType()) {
- InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS;
- } else if (PointeeTy->isFunctionType()) {
- InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION;
- } else if (PointeeTy->isStructureTypeWithFlexibleArrayMember()) {
- if (FieldTy->isArrayType()) {
- // 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;
- S.Diag(FD->getBeginLoc(), DiagID)
- << SelectPtrOrArr << PointeeTy << (int)InvalidTypeKind
- << (ShouldWarn ? 1 : 0) << Kind << FD->getSourceRange();
- return true;
- }
-
- // Check the expression
-
- if (!E->getType()->isIntegerType() || E->getType()->isBooleanType()) {
- S.Diag(E->getBeginLoc(), diag::err_count_attr_argument_not_integer)
- << Kind << E->getSourceRange();
- return true;
- }
-
- auto *DRE = dyn_cast<DeclRefExpr>(E);
- if (!DRE) {
- S.Diag(E->getBeginLoc(),
- diag::err_count_attr_only_support_simple_decl_reference)
- << Kind << E->getSourceRange();
- return true;
- }
-
- auto *CountDecl = DRE->getDecl();
- FieldDecl *CountFD = dyn_cast<FieldDecl>(CountDecl);
- if (auto *IFD = dyn_cast<IndirectFieldDecl>(CountDecl)) {
- CountFD = IFD->getAnonField();
- }
- if (!CountFD) {
- S.Diag(E->getBeginLoc(), diag::err_count_attr_must_be_in_structure)
- << CountDecl << Kind << E->getSourceRange();
-
- S.Diag(CountDecl->getBeginLoc(),
- diag::note_flexible_array_counted_by_attr_field)
- << CountDecl << CountDecl->getSourceRange();
- return true;
- }
-
- if (FD->getParent() != CountFD->getParent()) {
- if (CountFD->getParent()->isUnion()) {
- S.Diag(CountFD->getBeginLoc(), diag::err_count_attr_refer_to_union)
- << Kind << CountFD->getSourceRange();
- return true;
- }
- // Whether CountRD is an anonymous struct is not determined at this
- // point. Thus, an additional diagnostic in case it's not anonymous struct
- // is done later in `Parser::ParseStructDeclaration`.
- auto *RD = GetEnclosingNamedOrTopAnonRecord(FD);
- auto *CountRD = GetEnclosingNamedOrTopAnonRecord(CountFD);
-
- if (RD != CountRD) {
- S.Diag(E->getBeginLoc(), diag::err_count_attr_param_not_in_same_struct)
- << CountFD << Kind << FieldTy->isArrayType() << E->getSourceRange();
- S.Diag(CountFD->getBeginLoc(),
- diag::note_flexible_array_counted_by_attr_field)
- << CountFD << CountFD->getSourceRange();
- return true;
- }
- }
-
- Decls.push_back(TypeCoupledDeclRefInfo(CountFD, /*IsDref*/ false));
- return false;
-}
-
static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) {
auto *FD = dyn_cast<FieldDecl>(D);
assert(FD);
@@ -6059,7 +5884,7 @@ static void handleCountedByAttrField(Sema &S, Decl *D,
const ParsedAttr &AL) {
}
llvm::SmallVector<TypeCoupledDeclRefInfo, 1> Decls;
- if (CheckCountedByAttrOnField(S, FD, CountExpr, Decls, CountInBytes, OrNull))
+ if (S.CheckCountedByAttrOnField(FD, CountExpr, Decls, CountInBytes, OrNull))
return;
QualType CAT = S.BuildCountAttributedArrayOrPointerType(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
