llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Karthik P (Caryoake) <details> <summary>Changes</summary> For context, this PR solves the issue raised in #<!-- -->197250 .It is regarding C not having an inherent "this" pointer. The problem caused by this is that, whenever certain bounds safety and threads safety annotations need to reference other fields within the same struct, it ends up using DeclRefExpr to refer directly to a FieldDecl. This is an illegal hack and causes crashes in type builders like BuildCountAttributedArrayOrPointerType. FieldDecl should always be referenced by MemberExpr which has a base object, unlike DeclRefExpr which is for free standing variables. The solution for this (as hinted by @<!-- -->rapidsna ) was to create a C equivalent of C++'s this pointer. Hence I decided to add CThisExpr which mirrors the semantics of CXXThisExpr in C++. This is a lightweight AST node that acts as a phantom context object for C structs, hence allowing the semantic analyzer to create legally correct MemberExpr nodes. **Architectural Changes made :-** * **AST Core:** Defined CThisExpr in StmtNodes.td and implemented its C++ anatomy in Expr.h / Expr.cpp. Anatomy was mostly based off of CXXThisExpr, as it has already been implemented before. Added corresponding stubs for AST traversal (TreeTransform), printing (StmtPrinter), profiling (StmtProfile), and serialization (ASTReaderStmt & ASTWriterStmt). * **Semantic Analysis (SemaDeclAttr.cpp):** Updated handleCountedByAttrField to intercept the legacy DeclRefExpr generated for the attribute argument. Implemented an AST mutation that dynamically forges a CThisExpr pointer to the parent struct and constructs a valid MemberExpr (CThis->size). * **Type Building (SemaType.cpp):** Refactored BuildTypeCoupledDecls to safely route and extract declarations from both DeclRefExpr (to preserve support for standard function parameters) and MemberExpr (for our new struct implementation), eliminating the hardcoded `cast<DeclRefExpr>` that was causing compiler crashes. **Testing** * I also added clang/test/AST/ast-dump-cthis-counted-by.c to verify that CThisExpr and MemberExpr are successfully generated. It also makes sure that it is locked into the AST for struct fields using the __counted_by attribute. Assisted by: Gemini- 3, for debugging, PR markdown formatting and code review --- Full diff: https://github.com/llvm/llvm-project/pull/199241.diff 12 Files Affected: - (modified) clang/include/clang/AST/Expr.h (+40) - (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+1) - (modified) clang/include/clang/Basic/StmtNodes.td (+1) - (modified) clang/lib/AST/Expr.cpp (+10) - (modified) clang/lib/AST/StmtPrinter.cpp (+4) - (modified) clang/lib/AST/StmtProfile.cpp (+4) - (modified) clang/lib/Sema/SemaDeclAttr.cpp (+15) - (modified) clang/lib/Sema/SemaType.cpp (+11-3) - (modified) clang/lib/Sema/TreeTransform.h (+5) - (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+4) - (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+4) - (added) clang/test/AST/ast-dump-cthis-counted-by.c (+11) ``````````diff diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index b91bf4a5375fb..1547157719bbe 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -1510,6 +1510,46 @@ class DeclRefExpr final } }; +class CThisExpr : public Expr { + SourceLocation Loc; + bool IsImplicit; + + CThisExpr(SourceLocation L, QualType Ty, bool IsImplicit) + : Expr(CThisExprClass, Ty, VK_PRValue, OK_Ordinary), Loc(L), + IsImplicit(IsImplicit) { + setDependence(ExprDependence::None); + } + + CThisExpr(EmptyShell Empty) : Expr(CThisExprClass, Empty) {} + +public: + static CThisExpr *Create(const ASTContext &Ctx, SourceLocation L, + QualType Ty, bool IsImplicit); + + static CThisExpr *CreateEmpty(const ASTContext &Ctx); + + SourceLocation getLocation() const { return Loc; } + void setLocation(SourceLocation L) { Loc = L; } + + SourceLocation getBeginLoc() const { return getLocation(); } + SourceLocation getEndLoc() const { return getLocation(); } + + bool isImplicit() const { return IsImplicit; } + void setImplicit(bool I) { IsImplicit = I; } + + static bool classof(const Stmt *T) { + return T->getStmtClass() == CThisExprClass; + } + + child_range children() { + return child_range(child_iterator(), child_iterator()); + } + + const_child_range children() const { + return const_child_range(const_child_iterator(), const_child_iterator()); + } +}; + class IntegerLiteral : public Expr, public APIntStorage { SourceLocation Loc; diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h index febdf715698d9..9726a7363980a 100644 --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -2975,6 +2975,7 @@ DEF_TRAVERSE_STMT(CXXPseudoDestructorExpr, { }) DEF_TRAVERSE_STMT(CXXThisExpr, {}) +DEF_TRAVERSE_STMT(CThisExpr, {}) DEF_TRAVERSE_STMT(CXXThrowExpr, {}) DEF_TRAVERSE_STMT(UserDefinedLiteral, {}) DEF_TRAVERSE_STMT(DesignatedInitExpr, {}) diff --git a/clang/include/clang/Basic/StmtNodes.td b/clang/include/clang/Basic/StmtNodes.td index e166894ea024b..1eb9a7b6faf01 100644 --- a/clang/include/clang/Basic/StmtNodes.td +++ b/clang/include/clang/Basic/StmtNodes.td @@ -137,6 +137,7 @@ def UserDefinedLiteral : StmtNode<CallExpr>; def CXXBoolLiteralExpr : StmtNode<Expr>; def CXXNullPtrLiteralExpr : StmtNode<Expr>; def CXXThisExpr : StmtNode<Expr>; +def CThisExpr : StmtNode<Expr>; def CXXThrowExpr : StmtNode<Expr>; def CXXDefaultArgExpr : StmtNode<Expr>; def CXXDefaultInitExpr : StmtNode<Expr>; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 90747be4208e1..17fdf3c9eab0e 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -432,6 +432,16 @@ APValue ConstantExpr::getAPValueResult() const { llvm_unreachable("invalid ResultKind"); } +CThisExpr *CThisExpr::Create(const ASTContext &Ctx, SourceLocation L, + QualType Ty, bool IsImplicit) { + return new (Ctx) CThisExpr(L, Ty, IsImplicit); +} + +CThisExpr *CThisExpr::CreateEmpty(const ASTContext &Ctx) { + return new (Ctx) CThisExpr(EmptyShell()); +} + + DeclRefExpr::DeclRefExpr(const ASTContext &Ctx, ValueDecl *D, bool RefersToEnclosingVariableOrCapture, QualType T, ExprValueKind VK, SourceLocation L, diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp index 6c3294573e9d4..d569d8637f3fb 100644 --- a/clang/lib/AST/StmtPrinter.cpp +++ b/clang/lib/AST/StmtPrinter.cpp @@ -2281,6 +2281,10 @@ void StmtPrinter::VisitCXXThisExpr(CXXThisExpr *Node) { OS << "this"; } +void StmtPrinter::VisitCThisExpr(CThisExpr *Node) { + OS << "this"; +} + void StmtPrinter::VisitCXXThrowExpr(CXXThrowExpr *Node) { if (!Node->getSubExpr()) OS << "throw"; diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp index eb25e5260fd1a..d1abd6d75509b 100644 --- a/clang/lib/AST/StmtProfile.cpp +++ b/clang/lib/AST/StmtProfile.cpp @@ -2124,6 +2124,10 @@ void StmtProfiler::VisitCXXThisExpr(const CXXThisExpr *S) { ID.AddBoolean(S->isCapturedByCopyInLambdaWithExplicitObjectParameter()); } +void StmtProfiler::VisitCThisExpr(const CThisExpr *S) { + VisitExpr(S); +} + void StmtProfiler::VisitCXXThrowExpr(const CXXThrowExpr *S) { VisitExpr(S); } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index ae04d3855f01c..d935bd08d3737 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7016,6 +7016,21 @@ static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) { if (S.CheckCountedByAttrOnField(FD, CountExpr, CountInBytes, OrNull)) return; + // --- CThisExpr Interception Block (This block swaps the DeclRefExpr out)--- + if (auto *DRE = dyn_cast<DeclRefExpr>(CountExpr)) { + if (auto *TargetField = dyn_cast<FieldDecl>(DRE->getDecl())) { + QualType StructTy = S.Context.getTypeDeclType(cast<TypeDecl>(FD->getParent())); + QualType ThisPtrTy = S.Context.getPointerType(StructTy); + + Expr *CThis = CThisExpr::Create(S.Context, DRE->getBeginLoc(), ThisPtrTy, /*IsImplicit=*/true); + + CountExpr = MemberExpr::CreateImplicit(S.Context, CThis, /*IsArrow=*/true, + TargetField, TargetField->getType(), + VK_LValue, OK_Ordinary); + } + } + // ------------------------------------ + QualType CAT = S.BuildCountAttributedArrayOrPointerType( FD->getType(), CountExpr, CountInBytes, OrNull); FD->setType(CAT); diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 44ac4f6630690..6475c8555b417 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -9967,9 +9967,17 @@ QualType Sema::BuildTypeofExprType(Expr *E, TypeOfKind Kind) { static void BuildTypeCoupledDecls(Expr *E, llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls) { - // Currently, 'counted_by' only allows direct DeclRefExpr to FieldDecl. - auto *CountDecl = cast<DeclRefExpr>(E)->getDecl(); - Decls.push_back(TypeCoupledDeclRefInfo(CountDecl, /*IsDref*/ false)); + ValueDecl *CountDecl = nullptr; + + if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { + CountDecl = DRE->getDecl(); + } else if (auto *ME = dyn_cast<MemberExpr>(E)) { + CountDecl = ME->getMemberDecl(); + } else { + llvm_unreachable("CountExpr must be a DeclRefExpr or a MemberExpr"); + } + + Decls.push_back(TypeCoupledDeclRefInfo(CountDecl, /*IsDref=*/false)); } QualType Sema::BuildCountAttributedArrayOrPointerType(QualType WrappedTy, diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index c3bf71dbf10df..ba9e5abfe080e 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -14758,6 +14758,11 @@ TreeTransform<Derived>::TransformCXXThisExpr(CXXThisExpr *E) { return getDerived().RebuildCXXThisExpr(E->getBeginLoc(), T, E->isImplicit()); } +template<typename Derived> +ExprResult TreeTransform<Derived>::TransformCThisExpr(CThisExpr *E) { + return E; +} + template<typename Derived> ExprResult TreeTransform<Derived>::TransformCXXThrowExpr(CXXThrowExpr *E) { diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index 7e51ce8c0aca2..ebf59c6149576 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -1940,6 +1940,10 @@ void ASTStmtReader::VisitCXXThisExpr(CXXThisExpr *E) { E->setCapturedByCopyInLambdaWithExplicitObjectParameter(Record.readInt()); } +void ASTStmtReader::VisitCThisExpr(CThisExpr *E) { + VisitExpr(E); +} + void ASTStmtReader::VisitCXXThrowExpr(CXXThrowExpr *E) { VisitExpr(E); E->CXXThrowExprBits.ThrowLoc = readSourceLocation(); diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index a7e815a1ef438..be420c4e19514 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -1951,6 +1951,10 @@ void ASTStmtWriter::VisitCXXThisExpr(CXXThisExpr *E) { Code = serialization::EXPR_CXX_THIS; } +void ASTStmtWriter::VisitCThisExpr(CThisExpr *E) { + VisitExpr(E); +} + void ASTStmtWriter::VisitCXXThrowExpr(CXXThrowExpr *E) { VisitExpr(E); Record.AddSourceLocation(E->getThrowLoc()); diff --git a/clang/test/AST/ast-dump-cthis-counted-by.c b/clang/test/AST/ast-dump-cthis-counted-by.c new file mode 100644 index 0000000000000..48e01cbe591c4 --- /dev/null +++ b/clang/test/AST/ast-dump-cthis-counted-by.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s + +// Proving the CThisExpr Indiana Jones swap works for structs +struct Packet { + int size; + int *data __attribute__((counted_by(size))); +}; + +// CHECK: RecordDecl {{.*}} struct Packet definition +// CHECK: FieldDecl {{.*}} size 'int' +// CHECK: FieldDecl {{.*}} data 'int * __counted_by(this->size)':'int *' `````````` </details> https://github.com/llvm/llvm-project/pull/199241 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
