Skip to content

Commit ccd1608

Browse files
authored
Diagnose misuse of the cleanup attribute (#80040)
This pull request fixes #79443 when the cleanup attribute is intended to be applied to a variable declaration, passing its address to a specified function. The problem arises when standard functions like free, closedir, fclose, etc., are used incorrectly with this attribute, leading to incorrect behavior. Fixes #79443
1 parent bb893fa commit ccd1608

File tree

4 files changed

+57
-4
lines changed

4 files changed

+57
-4
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ Improvements to Clang's diagnostics
237237
- Clang now diagnoses lambda function expressions being implicitly cast to boolean values, under ``-Wpointer-bool-conversion``.
238238
Fixes #GH82512.
239239

240+
- Clang now provides improved warnings for the ``cleanup`` attribute to detect misuse scenarios,
241+
such as attempting to call ``free`` on an unallocated object. Fixes
242+
`#79443 <https://github.com/llvm/llvm-project/issues/79443>`_.
243+
240244
Improvements to Clang's time-trace
241245
----------------------------------
242246

clang/include/clang/Sema/Sema.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2152,14 +2152,15 @@ class Sema final {
21522152

21532153
bool IsLayoutCompatible(QualType T1, QualType T2) const;
21542154

2155+
bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
2156+
const FunctionProtoType *Proto);
2157+
21552158
private:
21562159
void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
21572160
const ArraySubscriptExpr *ASE = nullptr,
21582161
bool AllowOnePastEnd = true, bool IndexNegated = false);
21592162
void CheckArrayAccess(const Expr *E);
21602163

2161-
bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
2162-
const FunctionProtoType *Proto);
21632164
bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc,
21642165
ArrayRef<const Expr *> Args);
21652166
bool CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall,

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3787,6 +3787,30 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
37873787
<< NI.getName() << ParamTy << Ty;
37883788
return;
37893789
}
3790+
VarDecl *VD = cast<VarDecl>(D);
3791+
// Create a reference to the variable declaration. This is a fake/dummy
3792+
// reference.
3793+
DeclRefExpr *VariableReference = DeclRefExpr::Create(
3794+
S.Context, NestedNameSpecifierLoc{}, FD->getLocation(), VD, false,
3795+
DeclarationNameInfo{VD->getDeclName(), VD->getLocation()}, VD->getType(),
3796+
VK_LValue);
3797+
3798+
// Create a unary operator expression that represents taking the address of
3799+
// the variable. This is a fake/dummy expression.
3800+
Expr *AddressOfVariable = UnaryOperator::Create(
3801+
S.Context, VariableReference, UnaryOperatorKind::UO_AddrOf,
3802+
S.Context.getPointerType(VD->getType()), VK_PRValue, OK_Ordinary, Loc,
3803+
+false, FPOptionsOverride{});
3804+
3805+
// Create a function call expression. This is a fake/dummy call expression.
3806+
CallExpr *FunctionCallExpression =
3807+
CallExpr::Create(S.Context, E, ArrayRef{AddressOfVariable},
3808+
S.Context.VoidTy, VK_PRValue, Loc, FPOptionsOverride{});
3809+
3810+
if (S.CheckFunctionCall(FD, FunctionCallExpression,
3811+
FD->getType()->getAs<FunctionProtoType>())) {
3812+
return;
3813+
}
37903814

37913815
D->addAttr(::new (S.Context) CleanupAttr(S.Context, AL, FD));
37923816
}

clang/test/Sema/attr-cleanup.c

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
// RUN: %clang_cc1 %s -verify -fsyntax-only
1+
// RUN: %clang_cc1 -Wfree-nonheap-object -fsyntax-only -verify %s
22

33
void c1(int *a);
4-
4+
typedef __typeof__(sizeof(0)) size_t;
55
extern int g1 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute only applies to local variables}}
66
int g2 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute only applies to local variables}}
77
static int g3 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute only applies to local variables}}
@@ -48,3 +48,27 @@ void t6(void) {
4848
}
4949

5050
void t7(__attribute__((cleanup(c4))) int a) {} // expected-warning {{'cleanup' attribute only applies to local variables}}
51+
52+
extern void free(void *);
53+
extern void *malloc(size_t size);
54+
void t8(void) {
55+
void *p
56+
__attribute__((
57+
cleanup(
58+
free // expected-warning{{attempt to call free on non-heap object 'p'}}
59+
)
60+
))
61+
= malloc(10);
62+
}
63+
typedef __attribute__((aligned(2))) int Aligned2Int;
64+
void t9(void){
65+
Aligned2Int __attribute__((cleanup(c1))) xwarn; // expected-warning{{passing 2-byte aligned argument to 4-byte aligned parameter 1 of 'c1' may result in an unaligned pointer access}}
66+
}
67+
68+
__attribute__((enforce_tcb("TCB1"))) void func1(int *x) {
69+
*x = 5;
70+
}
71+
__attribute__((enforce_tcb("TCB2"))) void t10() {
72+
int __attribute__((cleanup(func1))) x = 5; // expected-warning{{calling 'func1' is a violation of trusted computing base 'TCB2'}}
73+
}
74+

0 commit comments

Comments
 (0)