[clang] [BoundsSafety] build TypeLoc for CountAttributedType (PR #167287)
Henrik G. Olsson via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 10 01:49:12 PST 2025
https://github.com/hnrklssn updated https://github.com/llvm/llvm-project/pull/167287
>From 8804ce91e34c60bea50c8adaf516204fb8680a00 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Sun, 9 Nov 2025 19:01:09 -0800
Subject: [PATCH 1/4] [BoundsSafety] build TypeLoc for CountAttributedType
This adds attribute SourceRange to CountAttributedTypeLoc and populates
it. The following commit will change diagnostics to use this
information.
Fixes https://github.com/llvm/llvm-project/issues/113582
---
clang/include/clang/AST/TypeLoc.h | 17 +++++++++++------
clang/lib/AST/TypeLoc.cpp | 4 ----
clang/lib/Sema/SemaDeclAttr.cpp | 9 +++++++++
3 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/AST/TypeLoc.h b/clang/include/clang/AST/TypeLoc.h
index 2cefaa9611c98..2999c086af3b6 100644
--- a/clang/include/clang/AST/TypeLoc.h
+++ b/clang/include/clang/AST/TypeLoc.h
@@ -19,6 +19,7 @@
#include "clang/AST/NestedNameSpecifierBase.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/TypeBase.h"
+#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
@@ -1303,7 +1304,9 @@ class ObjCInterfaceTypeLoc : public ConcreteTypeLoc<ObjCObjectTypeLoc,
}
};
-struct BoundsAttributedLocInfo {};
+struct BoundsAttributedLocInfo {
+ SourceRange Range;
+};
class BoundsAttributedTypeLoc
: public ConcreteTypeLoc<UnqualTypeLoc, BoundsAttributedTypeLoc,
BoundsAttributedType, BoundsAttributedLocInfo> {
@@ -1311,10 +1314,14 @@ class BoundsAttributedTypeLoc
TypeLoc getInnerLoc() const { return getInnerTypeLoc(); }
QualType getInnerType() const { return getTypePtr()->desugar(); }
void initializeLocal(ASTContext &Context, SourceLocation Loc) {
- // nothing to do
+ setAttrRange({Loc, Loc});
+ }
+ void setAttrRange(SourceRange Range) {
+ getLocalData()->Range = Range;
}
- // LocalData is empty and TypeLocBuilder doesn't handle DataSize 1.
- unsigned getLocalDataSize() const { return 0; }
+ SourceRange getAttrRange() const { return getLocalData()->Range; }
+
+ unsigned getLocalDataSize() const { return sizeof(BoundsAttributedLocInfo); }
};
class CountAttributedTypeLoc final
@@ -1325,8 +1332,6 @@ class CountAttributedTypeLoc final
Expr *getCountExpr() const { return getTypePtr()->getCountExpr(); }
bool isCountInBytes() const { return getTypePtr()->isCountInBytes(); }
bool isOrNull() const { return getTypePtr()->isOrNull(); }
-
- SourceRange getLocalSourceRange() const;
};
struct MacroQualifiedLocInfo {
diff --git a/clang/lib/AST/TypeLoc.cpp b/clang/lib/AST/TypeLoc.cpp
index f54ccf0932bc7..66f9e3c67df84 100644
--- a/clang/lib/AST/TypeLoc.cpp
+++ b/clang/lib/AST/TypeLoc.cpp
@@ -590,10 +590,6 @@ SourceRange AttributedTypeLoc::getLocalSourceRange() const {
return getAttr() ? getAttr()->getRange() : SourceRange();
}
-SourceRange CountAttributedTypeLoc::getLocalSourceRange() const {
- return getCountExpr() ? getCountExpr()->getSourceRange() : SourceRange();
-}
-
SourceRange BTFTagAttributedTypeLoc::getLocalSourceRange() const {
return getAttr() ? getAttr()->getRange() : SourceRange();
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a9e7b44ac9d73..1753261822755 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -10,10 +10,12 @@
//
//===----------------------------------------------------------------------===//
+#include "TypeLocBuilder.h"
#include "clang/AST/APValue.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
@@ -24,6 +26,8 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Mangle.h"
#include "clang/AST/Type.h"
+#include "clang/AST/TypeBase.h"
+#include "clang/AST/TypeLoc.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Cuda.h"
#include "clang/Basic/DarwinSDKInfo.h"
@@ -6580,9 +6584,14 @@ static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) {
if (S.CheckCountedByAttrOnField(FD, CountExpr, CountInBytes, OrNull))
return;
+ TypeLocBuilder TLB;
QualType CAT = S.BuildCountAttributedArrayOrPointerType(
FD->getType(), CountExpr, CountInBytes, OrNull);
+ TLB.pushFullCopy(FD->getTypeSourceInfo()->getTypeLoc());
+ CountAttributedTypeLoc CATL = TLB.push<CountAttributedTypeLoc>(CAT);
+ CATL.setAttrRange(AL.getRange());
FD->setType(CAT);
+ FD->setTypeSourceInfo(TLB.getTypeSourceInfo(S.getASTContext(), CAT));
}
static void handleFunctionReturnThunksAttr(Sema &S, Decl *D,
>From e92dd11798bc3dea17cff61f0ebf185099671d8a Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Sun, 9 Nov 2025 23:32:45 -0800
Subject: [PATCH 2/4] [BoundsSafety] get counted_by spelling from source range
This replaces the count expr source location used for
note_counted_by_consider_using_sized_by with the actual attribute
location fetched from TypeLof, except in the rare case where we couldn't
find a TypeLoc. It also attaches a fix-it hint, now that we have the
proper source range.
Fixes https://github.com/llvm/llvm-project/issues/113585
---
clang/include/clang/AST/TypeLoc.h | 4 +
clang/lib/Sema/SemaBoundsSafety.cpp | 121 ++++++++++++++++++++++++----
2 files changed, 109 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/AST/TypeLoc.h b/clang/include/clang/AST/TypeLoc.h
index 2999c086af3b6..0cf17c463d6cd 100644
--- a/clang/include/clang/AST/TypeLoc.h
+++ b/clang/include/clang/AST/TypeLoc.h
@@ -1304,6 +1304,7 @@ class ObjCInterfaceTypeLoc : public ConcreteTypeLoc<ObjCObjectTypeLoc,
}
};
+class Sema;
struct BoundsAttributedLocInfo {
SourceRange Range;
};
@@ -1321,6 +1322,9 @@ class BoundsAttributedTypeLoc
}
SourceRange getAttrRange() const { return getLocalData()->Range; }
+ StringRef getAttrNameAsWritten(Sema &S) const;
+ SourceRange getAttrNameRange(Sema &S) const;
+
unsigned getLocalDataSize() const { return sizeof(BoundsAttributedLocInfo); }
};
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp
index de9adf8ef5a1b..a699c9d116b15 100644
--- a/clang/lib/Sema/SemaBoundsSafety.cpp
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -11,6 +11,14 @@
/// (e.g. `counted_by`)
///
//===----------------------------------------------------------------------===//
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TypeBase.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Sema.h"
@@ -231,9 +239,69 @@ bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
return false;
}
+// FIXME: for some reason diagnostics highlight the end character, while
+// getSourceText() does not include the end character.
+static SourceRange getAttrNameRangeImpl(Sema &S, SourceLocation Begin,
+ bool IsForDiagnostics) {
+ SourceManager &SM = S.getSourceManager();
+ SourceLocation TokenStart = Begin;
+ while (TokenStart.isMacroID())
+ TokenStart = SM.getImmediateExpansionRange(TokenStart).getBegin();
+ unsigned Offset = IsForDiagnostics ? 1 : 0;
+ SourceLocation End = S.getLocForEndOfToken(TokenStart, Offset);
+ return {TokenStart, End};
+}
+
+StringRef BoundsAttributedTypeLoc::getAttrNameAsWritten(Sema &S) const {
+ SourceRange Range = getAttrNameRangeImpl(S, getAttrRange().getBegin(), false);
+ CharSourceRange NameRange = CharSourceRange::getCharRange(Range);
+ return Lexer::getSourceText(NameRange, S.getSourceManager(), S.getLangOpts());
+}
+
+SourceRange BoundsAttributedTypeLoc::getAttrNameRange(Sema &S) const {
+ return getAttrNameRangeImpl(S, getAttrRange().getBegin(), true);
+}
+
+static TypeSourceInfo *getTSI(const Decl *D) {
+ if (const auto* DD = dyn_cast<DeclaratorDecl>(D)) {
+ return DD->getTypeSourceInfo();
+ }
+ return nullptr;
+}
+
+struct TypeLocFinder : public ConstStmtVisitor<TypeLocFinder, TypeLoc> {
+ TypeLoc VisitParenExpr(const ParenExpr* E) {
+ return Visit(E->getSubExpr());
+ }
+
+ TypeLoc VisitDeclRefExpr(const DeclRefExpr *E) {
+ return getTSI(E->getDecl())->getTypeLoc();
+ }
+
+ TypeLoc VisitMemberExpr(const MemberExpr *E) {
+ return getTSI(E->getMemberDecl())->getTypeLoc();
+ }
+
+ TypeLoc VisitExplicitCastExpr(const ExplicitCastExpr *E) {
+ return E->getTypeInfoAsWritten()->getTypeLoc();
+ }
+
+ TypeLoc VisitCallExpr(const CallExpr *E) {
+ if (const auto *D = E->getCalleeDecl()) {
+ FunctionTypeLoc FTL = getTSI(D)->getTypeLoc().getAs<FunctionTypeLoc>();
+ if (FTL.isNull()) {
+ return FTL;
+ }
+ return FTL.getReturnLoc();
+ }
+ return {};
+ }
+};
+
static void EmitIncompleteCountedByPointeeNotes(Sema &S,
const CountAttributedType *CATy,
- NamedDecl *IncompleteTyDecl) {
+ NamedDecl *IncompleteTyDecl,
+ TypeLoc TL) {
assert(IncompleteTyDecl == nullptr || isa<TypeDecl>(IncompleteTyDecl));
if (IncompleteTyDecl) {
@@ -253,20 +321,36 @@ static void EmitIncompleteCountedByPointeeNotes(Sema &S,
<< CATy->getPointeeType();
}
- // Suggest using __sized_by(_or_null) instead of __counted_by(_or_null) as
- // __sized_by(_or_null) doesn't have the complete type restriction.
- //
- // We use the source range of the expression on the CountAttributedType as an
- // approximation for the source range of the attribute. This isn't quite right
- // but isn't easy to fix right now.
- //
- // TODO: Implement logic to find the relevant TypeLoc for the attribute and
- // get the SourceRange from that (#113582).
- //
- // TODO: We should emit a fix-it here.
- SourceRange AttrSrcRange = CATy->getCountExpr()->getSourceRange();
+ CountAttributedTypeLoc CATL;
+ if (!TL.isNull())
+ CATL = TL.getAs<CountAttributedTypeLoc>();
+
+ if (CATL.isNull()) {
+ // Fall back to pointing to the count expr - not great, but close enough.
+ // This should happen rarely, if ever.
+ S.Diag(CATy->getCountExpr()->getExprLoc(),
+ diag::note_counted_by_consider_using_sized_by)
+ << CATy->isOrNull();
+ return;
+ }
+ SourceRange AttrSrcRange = CATL.getAttrNameRange(S);
+
+ StringRef Spelling = CATL.getAttrNameAsWritten(S);
+ StringRef FixedSpelling;
+ if (Spelling == "__counted_by")
+ FixedSpelling = "__sized_by";
+ else if (Spelling == "counted_by")
+ FixedSpelling = "sized_by";
+ else if (Spelling == "__counted_by_or_null")
+ FixedSpelling = "__sized_by_or_null";
+ else if (Spelling == "counted_by_or_null")
+ FixedSpelling = "sized_by_or_null";
+ FixItHint Fix;
+ if (!FixedSpelling.empty())
+ Fix = FixItHint::CreateReplacement(AttrSrcRange, FixedSpelling);
+
S.Diag(AttrSrcRange.getBegin(), diag::note_counted_by_consider_using_sized_by)
- << CATy->isOrNull() << AttrSrcRange;
+ << CATy->isOrNull() << AttrSrcRange << Fix;
}
static std::tuple<const CountAttributedType *, QualType>
@@ -335,7 +419,11 @@ static bool CheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
<< CATy->getAttributeName(/*WithMacroPrefix=*/true) << PointeeTy
<< CATy->isOrNull() << RHSExpr->getSourceRange();
- EmitIncompleteCountedByPointeeNotes(S, CATy, IncompleteTyDecl);
+ TypeLoc TL;
+ if (TypeSourceInfo *TSI = getTSI(Assignee))
+ TL = TSI->getTypeLoc();
+
+ EmitIncompleteCountedByPointeeNotes(S, CATy, IncompleteTyDecl, TL);
return false; // check failed
}
@@ -408,7 +496,8 @@ bool Sema::BoundsSafetyCheckUseOfCountAttrPtr(const Expr *E) {
<< CATy->getAttributeName(/*WithMacroPrefix=*/true) << CATy->isOrNull()
<< E->getSourceRange();
- EmitIncompleteCountedByPointeeNotes(*this, CATy, IncompleteTyDecl);
+ TypeLoc TL = TypeLocFinder().Visit(E);
+ EmitIncompleteCountedByPointeeNotes(*this, CATy, IncompleteTyDecl, TL);
return false;
}
>From f38fc71ea77911e9f227411ebf161e51c34237f8 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Sun, 9 Nov 2025 23:57:37 -0800
Subject: [PATCH 3/4] format
---
clang/include/clang/AST/TypeLoc.h | 4 +---
clang/lib/Sema/SemaBoundsSafety.cpp | 6 ++----
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/AST/TypeLoc.h b/clang/include/clang/AST/TypeLoc.h
index 0cf17c463d6cd..24373361f68ca 100644
--- a/clang/include/clang/AST/TypeLoc.h
+++ b/clang/include/clang/AST/TypeLoc.h
@@ -1317,9 +1317,7 @@ class BoundsAttributedTypeLoc
void initializeLocal(ASTContext &Context, SourceLocation Loc) {
setAttrRange({Loc, Loc});
}
- void setAttrRange(SourceRange Range) {
- getLocalData()->Range = Range;
- }
+ void setAttrRange(SourceRange Range) { getLocalData()->Range = Range; }
SourceRange getAttrRange() const { return getLocalData()->Range; }
StringRef getAttrNameAsWritten(Sema &S) const;
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp
index a699c9d116b15..befef566f01da 100644
--- a/clang/lib/Sema/SemaBoundsSafety.cpp
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -263,16 +263,14 @@ SourceRange BoundsAttributedTypeLoc::getAttrNameRange(Sema &S) const {
}
static TypeSourceInfo *getTSI(const Decl *D) {
- if (const auto* DD = dyn_cast<DeclaratorDecl>(D)) {
+ if (const auto *DD = dyn_cast<DeclaratorDecl>(D)) {
return DD->getTypeSourceInfo();
}
return nullptr;
}
struct TypeLocFinder : public ConstStmtVisitor<TypeLocFinder, TypeLoc> {
- TypeLoc VisitParenExpr(const ParenExpr* E) {
- return Visit(E->getSubExpr());
- }
+ TypeLoc VisitParenExpr(const ParenExpr *E) { return Visit(E->getSubExpr()); }
TypeLoc VisitDeclRefExpr(const DeclRefExpr *E) {
return getTSI(E->getDecl())->getTypeLoc();
>From aef6bc4e23f3fba898269ad6b4c2a3551a4698ec Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Mon, 10 Nov 2025 01:44:30 -0800
Subject: [PATCH 4/4] if-else -> StringSwitch
---
clang/lib/Sema/SemaBoundsSafety.cpp | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp
index befef566f01da..5d123228b200a 100644
--- a/clang/lib/Sema/SemaBoundsSafety.cpp
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -23,6 +23,8 @@
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Sema.h"
+#include "llvm/ADT/StringSwitch.h"
+
namespace clang {
static CountAttributedType::DynamicCountPointerKind
@@ -334,15 +336,13 @@ static void EmitIncompleteCountedByPointeeNotes(Sema &S,
SourceRange AttrSrcRange = CATL.getAttrNameRange(S);
StringRef Spelling = CATL.getAttrNameAsWritten(S);
- StringRef FixedSpelling;
- if (Spelling == "__counted_by")
- FixedSpelling = "__sized_by";
- else if (Spelling == "counted_by")
- FixedSpelling = "sized_by";
- else if (Spelling == "__counted_by_or_null")
- FixedSpelling = "__sized_by_or_null";
- else if (Spelling == "counted_by_or_null")
- FixedSpelling = "sized_by_or_null";
+ StringRef FixedSpelling =
+ llvm::StringSwitch<StringRef>(Spelling)
+ .Case("__counted_by", "__sized_by")
+ .Case("counted_by", "sized_by")
+ .Case("__counted_by_or_null", "__sized_by_or_null")
+ .Case("counted_by_or_null", "sized_by_or_null")
+ .Default("");
FixItHint Fix;
if (!FixedSpelling.empty())
Fix = FixItHint::CreateReplacement(AttrSrcRange, FixedSpelling);
More information about the cfe-commits
mailing list