r352533 - [analyzer] [RetainSummaryManager] [NFC] Split one function into two, as it's really doing two things
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 29 11:29:46 PST 2019
Author: george.karpenkov
Date: Tue Jan 29 11:29:45 2019
New Revision: 352533
URL: http://llvm.org/viewvc/llvm-project?rev=352533&view=rev
Log:
[analyzer] [RetainSummaryManager] [NFC] Split one function into two, as it's really doing two things
Differential Revision: https://reviews.llvm.org/D57201
Modified:
cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
Modified: cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/RetainSummaryManager.h?rev=352533&r1=352532&r2=352533&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/RetainSummaryManager.h (original)
+++ cfe/trunk/include/clang/Analysis/RetainSummaryManager.h Tue Jan 29 11:29:45 2019
@@ -693,10 +693,22 @@ private:
void updateSummaryFromAnnotations(const RetainSummary *&Summ,
const FunctionDecl *FD);
- void updateSummaryForCall(const RetainSummary *&Summ,
- AnyCall C,
- bool HasNonZeroCallbackArg,
- bool IsReceiverUnconsumedSelf);
+ const RetainSummary *updateSummaryForNonZeroCallbackArg(const RetainSummary *S,
+ AnyCall &C);
+
+ /// Special case '[super init];' and '[self init];'
+ ///
+ /// Even though calling '[super init]' without assigning the result to self
+ /// and checking if the parent returns 'nil' is a bad pattern, it is common.
+ /// Additionally, our Self Init checker already warns about it. To avoid
+ /// overwhelming the user with messages from both checkers, we model the case
+ /// of '[super init]' in cases when it is not consumed by another expression
+ /// as if the call preserves the value of 'self'; essentially, assuming it can
+ /// never fail and return 'nil'.
+ /// Note, we don't want to just stop tracking the value since we want the
+ /// RetainCount checker to report leaks and use-after-free if SelfInit checker
+ /// is turned off.
+ void updateSummaryForReceiverUnconsumedSelf(const RetainSummary *&S);
/// Determine whether a declaration {@code D} of correspondent type (return
/// type for functions/methods) {@code QT} has any of the given attributes,
Modified: cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RetainSummaryManager.cpp?rev=352533&r1=352532&r2=352533&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/Analysis/RetainSummaryManager.cpp Tue Jan 29 11:29:45 2019
@@ -557,64 +557,47 @@ static ArgEffect getStopTrackingHardEqui
llvm_unreachable("Unknown ArgEffect kind");
}
-void RetainSummaryManager::updateSummaryForCall(const RetainSummary *&S,
- AnyCall C,
- bool HasNonZeroCallbackArg,
- bool IsReceiverUnconsumedSelf) {
-
- if (HasNonZeroCallbackArg) {
- ArgEffect RecEffect =
- getStopTrackingHardEquivalent(S->getReceiverEffect());
- ArgEffect DefEffect =
- getStopTrackingHardEquivalent(S->getDefaultArgEffect());
-
- ArgEffects ScratchArgs(AF.getEmptyMap());
- ArgEffects CustomArgEffects = S->getArgEffects();
- for (ArgEffects::iterator I = CustomArgEffects.begin(),
- E = CustomArgEffects.end();
- I != E; ++I) {
- ArgEffect Translated = getStopTrackingHardEquivalent(I->second);
- if (Translated.getKind() != DefEffect.getKind())
- ScratchArgs = AF.add(ScratchArgs, I->first, Translated);
- }
-
- RetEffect RE = RetEffect::MakeNoRetHard();
+const RetainSummary *
+RetainSummaryManager::updateSummaryForNonZeroCallbackArg(const RetainSummary *S,
+ AnyCall &C) {
+ ArgEffect RecEffect = getStopTrackingHardEquivalent(S->getReceiverEffect());
+ ArgEffect DefEffect = getStopTrackingHardEquivalent(S->getDefaultArgEffect());
+
+ ArgEffects ScratchArgs(AF.getEmptyMap());
+ ArgEffects CustomArgEffects = S->getArgEffects();
+ for (ArgEffects::iterator I = CustomArgEffects.begin(),
+ E = CustomArgEffects.end();
+ I != E; ++I) {
+ ArgEffect Translated = getStopTrackingHardEquivalent(I->second);
+ if (Translated.getKind() != DefEffect.getKind())
+ ScratchArgs = AF.add(ScratchArgs, I->first, Translated);
+ }
- // Special cases where the callback argument CANNOT free the return value.
- // This can generally only happen if we know that the callback will only be
- // called when the return value is already being deallocated.
- if (C.getKind() == AnyCall::Function) {
- if (const IdentifierInfo *Name = C.getIdentifier()) {
- // When the CGBitmapContext is deallocated, the callback here will free
- // the associated data buffer.
- // The callback in dispatch_data_create frees the buffer, but not
- // the data object.
- if (Name->isStr("CGBitmapContextCreateWithData") ||
- Name->isStr("dispatch_data_create"))
- RE = S->getRetEffect();
- }
- }
+ RetEffect RE = RetEffect::MakeNoRetHard();
- S = getPersistentSummary(RE, ScratchArgs, RecEffect, DefEffect);
+ // Special cases where the callback argument CANNOT free the return value.
+ // This can generally only happen if we know that the callback will only be
+ // called when the return value is already being deallocated.
+ if (const IdentifierInfo *Name = C.getIdentifier()) {
+ // When the CGBitmapContext is deallocated, the callback here will free
+ // the associated data buffer.
+ // The callback in dispatch_data_create frees the buffer, but not
+ // the data object.
+ if (Name->isStr("CGBitmapContextCreateWithData") ||
+ Name->isStr("dispatch_data_create"))
+ RE = S->getRetEffect();
}
- // Special case '[super init];' and '[self init];'
- //
- // Even though calling '[super init]' without assigning the result to self
- // and checking if the parent returns 'nil' is a bad pattern, it is common.
- // Additionally, our Self Init checker already warns about it. To avoid
- // overwhelming the user with messages from both checkers, we model the case
- // of '[super init]' in cases when it is not consumed by another expression
- // as if the call preserves the value of 'self'; essentially, assuming it can
- // never fail and return 'nil'.
- // Note, we don't want to just stop tracking the value since we want the
- // RetainCount checker to report leaks and use-after-free if SelfInit checker
- // is turned off.
- if (IsReceiverUnconsumedSelf) {
- RetainSummaryTemplate ModifiableSummaryTemplate(S, *this);
- ModifiableSummaryTemplate->setReceiverEffect(ArgEffect(DoNothing));
- ModifiableSummaryTemplate->setRetEffect(RetEffect::MakeNoRet());
- }
+ return getPersistentSummary(RE, ScratchArgs, RecEffect, DefEffect);
+}
+
+void RetainSummaryManager::updateSummaryForReceiverUnconsumedSelf(
+ const RetainSummary *&S) {
+
+ RetainSummaryTemplate Template(S, *this);
+
+ Template->setReceiverEffect(ArgEffect(DoNothing));
+ Template->setRetEffect(RetEffect::MakeNoRet());
}
const RetainSummary *
@@ -647,8 +630,11 @@ RetainSummaryManager::getSummary(AnyCall
}
}
- updateSummaryForCall(Summ, C, HasNonZeroCallbackArg,
- IsReceiverUnconsumedSelf);
+ if (HasNonZeroCallbackArg)
+ Summ = updateSummaryForNonZeroCallbackArg(Summ, C);
+
+ if (IsReceiverUnconsumedSelf)
+ updateSummaryForReceiverUnconsumedSelf(Summ);
assert(Summ && "Unknown call type?");
return Summ;
More information about the cfe-commits
mailing list