https://github.com/11happy updated https://github.com/llvm/llvm-project/pull/80040
>From 93adb872d0e18ff3a1356ab47527d81b61c920cd Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Tue, 30 Jan 2024 23:19:04 +0530 Subject: [PATCH 1/5] Diagnose misuse of the cleanup attribute Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../include/clang/Basic/DiagnosticSemaKinds.td | 4 ++++ clang/include/clang/Sema/Sema.h | 2 ++ clang/lib/Sema/SemaDeclAttr.cpp | 7 +++++++ clang/lib/Sema/SemaExpr.cpp | 17 +++++++++++++++++ 4 files changed, 30 insertions(+) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 24d32cb87c89e..99ef803b1e0ec 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8265,6 +8265,10 @@ def warn_condition_is_assignment : Warning<"using the result of an " def warn_free_nonheap_object : Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression|object: lambda-to-function-pointer conversion}1">, InGroup<FreeNonHeapObject>; +def warn_free_called_on_unallocated_object : Warning< + "'%0' called on unallocated object '%1'">, + InGroup<FreeNonHeapObject>; + // Completely identical except off by default. def warn_condition_is_idiomatic_assignment : Warning<"using the result " diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b5946e3f3178f..535c479aeb7c5 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12932,6 +12932,8 @@ class Sema final { bool IsStringLiteralToNonConstPointerConversion(Expr *From, QualType ToType); + bool IsPointerToPointer(QualType LHSType, QualType RHSType); + bool CheckExceptionSpecCompatibility(Expr *From, QualType ToType); ExprResult PerformImplicitConversion(Expr *From, QualType ToType, diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 069571fcf7864..e149f745cc2f9 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3782,6 +3782,13 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } D->addAttr(::new (S.Context) CleanupAttr(S.Context, AL, FD)); + + if (S.IsPointerToPointer(ParamTy, Ty)) { + VarDecl *VD = cast<VarDecl>(D); + S.Diag(Loc, diag::warn_free_called_on_unallocated_object) + << NI.getName() << VD; + return; + } } static void handleEnumExtensibilityAttr(Sema &S, Decl *D, diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 2f1ddfb215116..255e0be3cc842 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -10098,6 +10098,23 @@ static bool isVector(QualType QT, QualType ElementType) { return false; } +bool Sema::IsPointerToPointer(QualType LHSType, QualType RHSType) { + if (const PointerType *LHSPointer = dyn_cast<PointerType>(LHSType)) { + // Check if LHS is a single pointer, not a pointer to a pointer. + if (!isa<PointerType>(LHSPointer->getPointeeType())) { + if (isa<PointerType>(RHSType)) { + if (const PointerType *RHSPtr = dyn_cast<PointerType>(RHSType)) { + // If RHSType is a pointer to a pointer type, return True + if (isa<PointerType>(RHSPtr->getPointeeType())) { + return true; + } + } + } + } + } + return false; +} + /// CheckAssignmentConstraints (C99 6.5.16) - This routine currently /// has code to accommodate several GCC extensions when type checking /// pointers. Here are some objectionable examples that GCC considers warnings: >From 730f4d7f088645f4b49649c73328ca25e681339a Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Wed, 31 Jan 2024 05:45:00 +0530 Subject: [PATCH 2/5] remove whitespace Signed-off-by: 11happy <soni5ha...@gmail.com> --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 99ef803b1e0ec..ae9ad757788e8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8268,7 +8268,6 @@ def warn_free_nonheap_object def warn_free_called_on_unallocated_object : Warning< "'%0' called on unallocated object '%1'">, InGroup<FreeNonHeapObject>; - // Completely identical except off by default. def warn_condition_is_idiomatic_assignment : Warning<"using the result " >From 1a3cc18276be89969d5a262ff12499d20fd925ee Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Wed, 31 Jan 2024 20:58:19 +0530 Subject: [PATCH 3/5] change diagnostic Signed-off-by: 11happy <soni5ha...@gmail.com> --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++-- clang/lib/Sema/SemaDeclAttr.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index ae9ad757788e8..e8165394701c9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8265,8 +8265,8 @@ def warn_condition_is_assignment : Warning<"using the result of an " def warn_free_nonheap_object : Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression|object: lambda-to-function-pointer conversion}1">, InGroup<FreeNonHeapObject>; -def warn_free_called_on_unallocated_object : Warning< - "'%0' called on unallocated object '%1'">, +def warn_called_on_unallocated_object : Warning< + "calling function '%0' on an unallocated object '%1'">, InGroup<FreeNonHeapObject>; // Completely identical except off by default. diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index e149f745cc2f9..02d20a25eadf8 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3785,7 +3785,7 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (S.IsPointerToPointer(ParamTy, Ty)) { VarDecl *VD = cast<VarDecl>(D); - S.Diag(Loc, diag::warn_free_called_on_unallocated_object) + S.Diag(Loc, diag::warn_called_on_unallocated_object) << NI.getName() << VD; return; } >From bd676b70e041523b3adc99208265666ffc52bf09 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Thu, 1 Feb 2024 16:05:13 +0530 Subject: [PATCH 4/5] format Signed-off-by: 11happy <soni5ha...@gmail.com> --- clang/lib/Sema/SemaDeclAttr.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 02d20a25eadf8..bd7074b280aa0 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3785,8 +3785,7 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (S.IsPointerToPointer(ParamTy, Ty)) { VarDecl *VD = cast<VarDecl>(D); - S.Diag(Loc, diag::warn_called_on_unallocated_object) - << NI.getName() << VD; + S.Diag(Loc, diag::warn_called_on_unallocated_object) << NI.getName() << VD; return; } } >From f7279ba9dd52e0685106d0b53552a9ea223a0582 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 2 Feb 2024 17:30:38 +0530 Subject: [PATCH 5/5] call CheckFunctionCall to detect misuse Signed-off-by: 11happy <soni5ha...@gmail.com> --- clang/docs/ReleaseNotes.rst | 3 ++ .../clang/Basic/DiagnosticSemaKinds.td | 3 -- clang/include/clang/Sema/Sema.h | 4 +-- clang/lib/Sema/SemaDeclAttr.cpp | 28 +++++++++++++++---- clang/lib/Sema/SemaExpr.cpp | 17 ----------- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 254e0a9cb7297..116b2b9829fdb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -106,6 +106,9 @@ Improvements to Clang's diagnostics - Clang now applies syntax highlighting to the code snippets it prints. +- Clang now provides improved warnings for the cleanup attribute to detect misuse scenarios, + such as attempting to call `free` on unallocated objects. + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index e8165394701c9..24d32cb87c89e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8265,9 +8265,6 @@ def warn_condition_is_assignment : Warning<"using the result of an " def warn_free_nonheap_object : Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression|object: lambda-to-function-pointer conversion}1">, InGroup<FreeNonHeapObject>; -def warn_called_on_unallocated_object : Warning< - "calling function '%0' on an unallocated object '%1'">, - InGroup<FreeNonHeapObject>; // Completely identical except off by default. def warn_condition_is_idiomatic_assignment : Warning<"using the result " diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 535c479aeb7c5..f84804abd0252 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -13873,8 +13873,6 @@ class Sema final { bool AllowOnePastEnd = true, bool IndexNegated = false); void CheckArrayAccess(const Expr *E); - bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall, - const FunctionProtoType *Proto); bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc, ArrayRef<const Expr *> Args); bool CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall, @@ -13973,6 +13971,8 @@ class Sema final { ExprResult SemaConvertVectorExpr(Expr *E, TypeSourceInfo *TInfo, SourceLocation BuiltinLoc, SourceLocation RParenLoc); + bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall, + const FunctionProtoType *Proto); private: bool SemaBuiltinPrefetch(CallExpr *TheCall); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index bd7074b280aa0..fda5d67205d17 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3782,12 +3782,28 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } D->addAttr(::new (S.Context) CleanupAttr(S.Context, AL, FD)); - - if (S.IsPointerToPointer(ParamTy, Ty)) { - VarDecl *VD = cast<VarDecl>(D); - S.Diag(Loc, diag::warn_called_on_unallocated_object) << NI.getName() << VD; - return; - } + VarDecl *VD = cast<VarDecl>(D); + // Create a reference to the variable declaration. This is a fake/dummy + // reference. + DeclRefExpr *VariableReference = DeclRefExpr::Create( + S.Context, NestedNameSpecifierLoc{}, SourceLocation{}, VD, false, + DeclarationNameInfo{VD->getDeclName(), VD->getLocation()}, VD->getType(), + VK_LValue); + + // Create a unary operator expression that represents taking the address of + // the variable. This is a fake/dummy expression. + Expr *AddressOfVariable = UnaryOperator::Create( + S.Context, VariableReference, UnaryOperatorKind::UO_AddrOf, + S.Context.getPointerType(VD->getType()), VK_PRValue, OK_Ordinary, + SourceLocation{}, false, FPOptionsOverride{}); + + // Create a function call expression. This is a fake/dummy call expression. + CallExpr *FunctionCallExpression = CallExpr::Create( + S.Context, E, ArrayRef{AddressOfVariable}, S.Context.VoidTy, VK_PRValue, + SourceLocation{}, FPOptionsOverride{}); + + S.CheckFunctionCall(FD, FunctionCallExpression, + FD->getType()->getAs<FunctionProtoType>()); } static void handleEnumExtensibilityAttr(Sema &S, Decl *D, diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 255e0be3cc842..2f1ddfb215116 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -10098,23 +10098,6 @@ static bool isVector(QualType QT, QualType ElementType) { return false; } -bool Sema::IsPointerToPointer(QualType LHSType, QualType RHSType) { - if (const PointerType *LHSPointer = dyn_cast<PointerType>(LHSType)) { - // Check if LHS is a single pointer, not a pointer to a pointer. - if (!isa<PointerType>(LHSPointer->getPointeeType())) { - if (isa<PointerType>(RHSType)) { - if (const PointerType *RHSPtr = dyn_cast<PointerType>(RHSType)) { - // If RHSType is a pointer to a pointer type, return True - if (isa<PointerType>(RHSPtr->getPointeeType())) { - return true; - } - } - } - } - } - return false; -} - /// CheckAssignmentConstraints (C99 6.5.16) - This routine currently /// has code to accommodate several GCC extensions when type checking /// pointers. Here are some objectionable examples that GCC considers warnings: _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits