vsavchenko created this revision. vsavchenko added a reviewer: NoQ. Herald added a subscriber: Charusso. vsavchenko requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Cleanup attribute allows users to attach a destructor-like functions to variable declarations to be called whenever they leave the scope. The logic of such functions is not supported by the Clang's CFG and is too hard to be reasoned about. In order to avoid false positives in this situation, we assume that we didn't see ALL of the executtion paths of the function and, thus, can warn only about multiple call violation. rdar://74441906 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D98694 Files: clang/lib/Analysis/CalledOnceCheck.cpp clang/test/SemaObjC/warn-called-once.m Index: clang/test/SemaObjC/warn-called-once.m =================================================================== --- clang/test/SemaObjC/warn-called-once.m +++ clang/test/SemaObjC/warn-called-once.m @@ -1193,4 +1193,46 @@ escape(handler); } +// rdar://74441906 +typedef void (^DeferredBlock)(void); +static inline void DefferedCallback(DeferredBlock *inBlock) { (*inBlock)(); } +#define _DEFERCONCAT(a, b) a##b +#define _DEFERNAME(a) _DEFERCONCAT(__DeferredVar_, a) +#define DEFER __extension__ __attribute__((cleanup(DefferedCallback), unused)) \ + DeferredBlock _DEFERNAME(__COUNTER__) = ^ + +- (void)test_cleanup_1:(int)cond + withCompletion:(void (^)(void))handler { + int error = 0; + DEFER { + if (error) + handler(); + }; + + if (cond) { + error = 1; + } else { + // no-warning + handler(); + } +} + +- (void)test_cleanup_2:(int)cond + withCompletion:(void (^)(void))handler { + int error = 0; + DEFER { + if (error) + handler(); + }; + + if (cond) { + error = 1; + } else { + handler(); // expected-note{{previous call is here}} + } + + // We still can warn about double call even in this case. + handler(); // expected-warning{{completion handler is called twice}} +} + @end Index: clang/lib/Analysis/CalledOnceCheck.cpp =================================================================== --- clang/lib/Analysis/CalledOnceCheck.cpp +++ clang/lib/Analysis/CalledOnceCheck.cpp @@ -812,8 +812,12 @@ } } - // Early exit if we don't have parameters for extra analysis. - if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none()) + // Early exit if we don't have parameters for extra analysis... + if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none() && + // ... or if we've seen variables with cleanup functions. + // We can't reason that we've seen every path in this case, + // and thus abandon reporting any warnings that imply that. + !FunctionHasCleanupVars) return; // We are looking for a pair of blocks A, B so that the following is true: @@ -1601,6 +1605,10 @@ if (Var->getInit()) { checkEscapee(Var->getInit()); } + + if (Var->hasAttr<CleanupAttr>()) { + FunctionHasCleanupVars = true; + } } } } @@ -1669,6 +1677,13 @@ // around. bool SuppressOnConventionalErrorPaths = false; + // The user can annotate variable declarations with cleanup functions, which + // essentially imposes a custom destructor logic on that variable. + // It is possible to use it, however, to call tracked parameters on all exits + // from the function. For this reason, we track the fact that the function + // actually has these. + bool FunctionHasCleanupVars = false; + State CurrentState; ParamSizedVector<const ParmVarDecl *> TrackedParams; CFGSizedVector<State> States;
Index: clang/test/SemaObjC/warn-called-once.m =================================================================== --- clang/test/SemaObjC/warn-called-once.m +++ clang/test/SemaObjC/warn-called-once.m @@ -1193,4 +1193,46 @@ escape(handler); } +// rdar://74441906 +typedef void (^DeferredBlock)(void); +static inline void DefferedCallback(DeferredBlock *inBlock) { (*inBlock)(); } +#define _DEFERCONCAT(a, b) a##b +#define _DEFERNAME(a) _DEFERCONCAT(__DeferredVar_, a) +#define DEFER __extension__ __attribute__((cleanup(DefferedCallback), unused)) \ + DeferredBlock _DEFERNAME(__COUNTER__) = ^ + +- (void)test_cleanup_1:(int)cond + withCompletion:(void (^)(void))handler { + int error = 0; + DEFER { + if (error) + handler(); + }; + + if (cond) { + error = 1; + } else { + // no-warning + handler(); + } +} + +- (void)test_cleanup_2:(int)cond + withCompletion:(void (^)(void))handler { + int error = 0; + DEFER { + if (error) + handler(); + }; + + if (cond) { + error = 1; + } else { + handler(); // expected-note{{previous call is here}} + } + + // We still can warn about double call even in this case. + handler(); // expected-warning{{completion handler is called twice}} +} + @end Index: clang/lib/Analysis/CalledOnceCheck.cpp =================================================================== --- clang/lib/Analysis/CalledOnceCheck.cpp +++ clang/lib/Analysis/CalledOnceCheck.cpp @@ -812,8 +812,12 @@ } } - // Early exit if we don't have parameters for extra analysis. - if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none()) + // Early exit if we don't have parameters for extra analysis... + if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none() && + // ... or if we've seen variables with cleanup functions. + // We can't reason that we've seen every path in this case, + // and thus abandon reporting any warnings that imply that. + !FunctionHasCleanupVars) return; // We are looking for a pair of blocks A, B so that the following is true: @@ -1601,6 +1605,10 @@ if (Var->getInit()) { checkEscapee(Var->getInit()); } + + if (Var->hasAttr<CleanupAttr>()) { + FunctionHasCleanupVars = true; + } } } } @@ -1669,6 +1677,13 @@ // around. bool SuppressOnConventionalErrorPaths = false; + // The user can annotate variable declarations with cleanup functions, which + // essentially imposes a custom destructor logic on that variable. + // It is possible to use it, however, to call tracked parameters on all exits + // from the function. For this reason, we track the fact that the function + // actually has these. + bool FunctionHasCleanupVars = false; + State CurrentState; ParamSizedVector<const ParmVarDecl *> TrackedParams; CFGSizedVector<State> States;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits