[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Great, thanks a lot! https://reviews.llvm.org/D49438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing
george.karpenkov added a comment. > I think what pointer chasing should do, is check whether that pointer owns > the pointee But ownership is a convention, and it's not always deducible from a codebase. I think of those as two separate checks, and I think we should only talk about enabling the pointer-chasing after we had established that just checking for uninitialized fields finds lots of valid bugs (and we can only do that after it gets enabled for many projects) https://reviews.llvm.org/D49438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing
george.karpenkov added a comment. @Szelethus in any case, could you commit this change for now? I'm very keen on seeing the results on a few of our internal projects, and it's much easier to do that once the change is committed. https://reviews.llvm.org/D49438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50111: [analyzer] Add ASTContext to CheckerManager
This revision was automatically updated to reflect the committed changes. Closed by commit rC339079: [analyzer] Add ASTContext to CheckerManager (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50111?vs=158834&id=159424#toc Repository: rC Clang https://reviews.llvm.org/D50111 Files: include/clang/StaticAnalyzer/Core/CheckerManager.h include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h lib/StaticAnalyzer/Core/AnalysisManager.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp === --- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp +++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp @@ -112,12 +112,12 @@ } std::unique_ptr ento::createCheckerManager( -AnalyzerOptions &opts, const LangOptions &langOpts, +ASTContext &context, +AnalyzerOptions &opts, ArrayRef plugins, ArrayRef> checkerRegistrationFns, DiagnosticsEngine &diags) { - std::unique_ptr checkerMgr( - new CheckerManager(langOpts, opts)); + auto checkerMgr = llvm::make_unique(context, opts); SmallVector checkerOpts = getCheckerOptList(opts); Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp === --- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -295,13 +295,12 @@ void Initialize(ASTContext &Context) override { Ctx = &Context; -checkerMgr = -createCheckerManager(*Opts, PP.getLangOpts(), Plugins, - CheckerRegistrationFns, PP.getDiagnostics()); +checkerMgr = createCheckerManager( +*Ctx, *Opts, Plugins, CheckerRegistrationFns, PP.getDiagnostics()); Mgr = llvm::make_unique( -*Ctx, PP.getDiagnostics(), PP.getLangOpts(), PathConsumers, -CreateStoreMgr, CreateConstraintMgr, checkerMgr.get(), *Opts, Injector); +*Ctx, PP.getDiagnostics(), PathConsumers, CreateStoreMgr, +CreateConstraintMgr, checkerMgr.get(), *Opts, Injector); } /// Store the top level decls in the set to be processed later on. Index: lib/StaticAnalyzer/Core/AnalysisManager.cpp === --- lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -14,28 +14,28 @@ void AnalysisManager::anchor() { } -AnalysisManager::AnalysisManager( -ASTContext &ASTCtx, DiagnosticsEngine &diags, const LangOptions &lang, -const PathDiagnosticConsumers &PDC, StoreManagerCreator storemgr, -ConstraintManagerCreator constraintmgr, CheckerManager *checkerMgr, -AnalyzerOptions &Options, CodeInjector *injector) -: AnaCtxMgr(ASTCtx, Options.UnoptimizedCFG, -Options.includeImplicitDtorsInCFG(), -/*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(), -Options.includeLifetimeInCFG(), -// Adding LoopExit elements to the CFG is a requirement for loop -// unrolling. -Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(), -Options.includeScopesInCFG(), -Options.shouldSynthesizeBodies(), -Options.shouldConditionalizeStaticInitializers(), -/*addCXXNewAllocator=*/true, -Options.includeRichConstructorsInCFG(), -Options.shouldElideConstructors(), -injector), - Ctx(ASTCtx), Diags(diags), LangOpts(lang), PathConsumers(PDC), - CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr), - CheckerMgr(checkerMgr), options(Options) { +AnalysisManager::AnalysisManager(ASTContext &ASTCtx, DiagnosticsEngine &diags, + const PathDiagnosticConsumers &PDC, + StoreManagerCreator storemgr, + ConstraintManagerCreator constraintmgr, + CheckerManager *checkerMgr, + AnalyzerOptions &Options, + CodeInjector *injector) +: AnaCtxMgr( + ASTCtx, Options.UnoptimizedCFG, Options.includeImplicitDtorsInCFG(), + /*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(), + Options.includeLifetimeInCFG(), + // Adding LoopExit elements to the CFG is a requirement for loop + // unrolling. + Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(), + Options.includeScopesInCFG(), Options.shouldSynthesizeBodies(), + Options.shouldConditionalizeStaticInitializers(), + /*addCXXNewAllocator=*/true,
[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence
george.karpenkov added a comment. @rnk As discussed, would it be acceptable for you to just have empty sanitizer runtime files in the resource directory? Repository: rL LLVM https://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50408: [analyzer] Avoid querying this-pointers for static-methods.
george.karpenkov added a reviewer: NoQ. george.karpenkov added a subscriber: NoQ. george.karpenkov added a comment. Looks reasonable. @NoQ any further comments? https://reviews.llvm.org/D50408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50523: [analyzer] Fix the bug in UninitializedObjectChecker caused by not handling block pointers
This revision was automatically updated to reflect the committed changes. Closed by commit rC339369: [analyzer] Fix the bug in UninitializedObjectChecker caused by not handling… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50523?vs=159968&id=159971#toc Repository: rC Clang https://reviews.llvm.org/D50523 Files: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp test/Analysis/objcpp-uninitialized-object.mm Index: test/Analysis/objcpp-uninitialized-object.mm === --- test/Analysis/objcpp-uninitialized-object.mm +++ test/Analysis/objcpp-uninitialized-object.mm @@ -0,0 +1,22 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -std=c++11 -fblocks -verify %s + +typedef void (^myBlock) (); + +struct StructWithBlock { + int a; + myBlock z; // expected-note{{uninitialized field 'this->z'}} + + StructWithBlock() : a(0), z(^{}) {} + + // Miss initialization of field `z`. + StructWithBlock(int pA) : a(pA) {} // expected-warning{{1 uninitialized field at the end of the constructor call}} + +}; + +void warnOnUninitializedBlock() { + StructWithBlock a(10); +} + +void noWarningWhenInitialized() { + StructWithBlock a; +} Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -417,7 +417,7 @@ continue; } -if (T->isPointerType() || T->isReferenceType()) { +if (T->isPointerType() || T->isReferenceType() || T->isBlockPointerType()) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; @@ -478,7 +478,8 @@ const FieldRegion *FR, FieldChainInfo LocalChain) { assert((FR->getDecl()->getType()->isPointerType() || - FR->getDecl()->getType()->isReferenceType()) && + FR->getDecl()->getType()->isReferenceType() || + FR->getDecl()->getType()->isBlockPointerType()) && "This method only checks pointer/reference objects!"); SVal V = State->getSVal(FR); Index: test/Analysis/objcpp-uninitialized-object.mm === --- test/Analysis/objcpp-uninitialized-object.mm +++ test/Analysis/objcpp-uninitialized-object.mm @@ -0,0 +1,22 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -std=c++11 -fblocks -verify %s + +typedef void (^myBlock) (); + +struct StructWithBlock { + int a; + myBlock z; // expected-note{{uninitialized field 'this->z'}} + + StructWithBlock() : a(0), z(^{}) {} + + // Miss initialization of field `z`. + StructWithBlock(int pA) : a(pA) {} // expected-warning{{1 uninitialized field at the end of the constructor call}} + +}; + +void warnOnUninitializedBlock() { + StructWithBlock a(10); +} + +void noWarningWhenInitialized() { + StructWithBlock a; +} Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -417,7 +417,7 @@ continue; } -if (T->isPointerType() || T->isReferenceType()) { +if (T->isPointerType() || T->isReferenceType() || T->isBlockPointerType()) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; @@ -478,7 +478,8 @@ const FieldRegion *FR, FieldChainInfo LocalChain) { assert((FR->getDecl()->getType()->isPointerType() || - FR->getDecl()->getType()->isReferenceType()) && + FR->getDecl()->getType()->isReferenceType() || + FR->getDecl()->getType()->isBlockPointerType()) && "This method only checks pointer/reference objects!"); SVal V = State->getSVal(FR); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. All the comments above + if anything, should use ASTMatchers. Repository: rC Clang https://reviews.llvm.org/D50488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50504: [analyzer][UninitializedObjectChecker] Refactoring p2.: Moving pointer chasing to a separate file
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Overall: great! Thank you for following this direction! However, to avoid clutter, could we put all the checker files into a separate folder, like MPI-Checker? Repository: rC Clang https://reviews.llvm.org/D50504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50506: [analyzer][UninitializedObjectChecker] Refactoring p4.: Wrap FieldRegions and reduce weight on FieldChainInfo
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. OK. I'm not a fan of inheritance hierarchies in general (especially if you only have two cases) but OK if it makes your life easier. Repository: rC Clang https://reviews.llvm.org/D50506 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50509: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: lib/StaticAnalyzer/Checkers/UninitializedPointee.cpp:78 +/// If for whatever reason dereferencing fails, returns with false. +static bool dereference(ProgramStateRef State, SVal &V, QualType &DynT); + In general, using return values is better than out-parameters. Could you instead return `Optional>` ? https://reviews.llvm.org/D50509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164 + // - sizeof(dst) + if (Append && isSizeof(LenArg, DstArg)) +return true; george.karpenkov wrote: > I am confused on what is happening in this branch and why is it bad. Could we > add a commend? Sorry I'm still confused about this branch: could you clarify? https://reviews.llvm.org/D49722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164 + // - sizeof(dst) + if (Append && isSizeof(LenArg, DstArg)) +return true; george.karpenkov wrote: > george.karpenkov wrote: > > I am confused on what is happening in this branch and why is it bad. Could > > we add a commend? > Sorry I'm still confused about this branch: could you clarify? Ah, OK, I see it needs one more byte due to null-termination. Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:199 uint64_t BufferLen = C.getTypeSize(Buffer) / 8; -if ((BufferLen - DstOff) < ILRawVal) - return true; +auto RemainingBufferLen = BufferLen - DstOff; +if (Append) { Probably better expressed as: ``` if (Append) RemainingBufferLen -= 1; ``` Then you don't need branching. https://reviews.llvm.org/D49722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50545: [analyzer] [NFC] [tests] Move plist-based diagnostics tests to separate files, use diff instead of a FileCheck
This revision was automatically updated to reflect the committed changes. Closed by commit rC339475: [analyzer] [NFC] [tests] Move plist-based diagnostics tests to separate files… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50545?vs=160139&id=160182#toc Repository: rC Clang https://reviews.llvm.org/D50545 Files: test/Analysis/ExpectedOutputs/plists/conditional-path-notes.c.plist test/Analysis/ExpectedOutputs/plists/copypaste/plist-diagnostics-notes-as-events.cpp.plist test/Analysis/ExpectedOutputs/plists/copypaste/plist-diagnostics.cpp.plist test/Analysis/ExpectedOutputs/plists/cstring-plist.c.plist test/Analysis/ExpectedOutputs/plists/cxx-for-range.cpp.plist test/Analysis/ExpectedOutputs/plists/diagnostics/deref-track-symbolic-region.c.plist test/Analysis/ExpectedOutputs/plists/diagnostics/undef-value-caller.c.plist test/Analysis/ExpectedOutputs/plists/diagnostics/undef-value-param.c.plist test/Analysis/ExpectedOutputs/plists/diagnostics/undef-value-param.m.plist test/Analysis/ExpectedOutputs/plists/edges-new.mm.plist test/Analysis/ExpectedOutputs/plists/generics.m.plist test/Analysis/ExpectedOutputs/plists/inline-plist.c.plist test/Analysis/ExpectedOutputs/plists/inline-unique-reports.c.plist test/Analysis/ExpectedOutputs/plists/inlining/eager-reclamation-path-notes.c.plist test/Analysis/ExpectedOutputs/plists/inlining/eager-reclamation-path-notes.cpp.plist test/Analysis/ExpectedOutputs/plists/inlining/path-notes.c.plist test/Analysis/ExpectedOutputs/plists/inlining/path-notes.cpp.plist test/Analysis/ExpectedOutputs/plists/inlining/path-notes.m.plist test/Analysis/ExpectedOutputs/plists/model-file.cpp.plist test/Analysis/ExpectedOutputs/plists/null-deref-path-notes.m.plist test/Analysis/ExpectedOutputs/plists/nullability-notes.m.plist test/Analysis/ExpectedOutputs/plists/objc-arc.m.plist test/Analysis/ExpectedOutputs/plists/objc-radar17039661.m.plist test/Analysis/ExpectedOutputs/plists/plist-output.m.plist test/Analysis/ExpectedOutputs/plists/retain-release-path-notes-gc.m.plist test/Analysis/ExpectedOutputs/plists/retain-release-path-notes.m.plist test/Analysis/ExpectedOutputs/plists/unix-fns.c.plist test/Analysis/ExpectedOutputs/plists/yaccignore.c.plist test/Analysis/conditional-path-notes.c test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp test/Analysis/copypaste/plist-diagnostics.cpp test/Analysis/cxx-for-range.cpp test/Analysis/diagnostics/deref-track-symbolic-region.c test/Analysis/diagnostics/undef-value-caller.c test/Analysis/diagnostics/undef-value-param.c test/Analysis/diagnostics/undef-value-param.m test/Analysis/edges-new.mm test/Analysis/generics.m test/Analysis/inline-plist.c test/Analysis/inline-unique-reports.c test/Analysis/inlining/eager-reclamation-path-notes.c test/Analysis/inlining/eager-reclamation-path-notes.cpp test/Analysis/inlining/path-notes.c test/Analysis/inlining/path-notes.cpp test/Analysis/inlining/path-notes.m test/Analysis/model-file.cpp test/Analysis/null-deref-path-notes.m test/Analysis/nullability-notes.m test/Analysis/objc-arc.m test/Analysis/plist-output.m test/Analysis/retain-release-path-notes-gc.m test/Analysis/retain-release-path-notes.m ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50124: [analyzer] Record nullability implications on getting items from NSDictionary
This revision was automatically updated to reflect the committed changes. Closed by commit rC339482: [analyzer] Record nullability implications on getting items from NSDictionary (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50124?vs=160188&id=160196#toc Repository: rC Clang https://reviews.llvm.org/D50124 Files: lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp test/Analysis/Inputs/system-header-simulator-for-nullability.h test/Analysis/trustnonnullchecker_test.m Index: test/Analysis/Inputs/system-header-simulator-for-nullability.h === --- test/Analysis/Inputs/system-header-simulator-for-nullability.h +++ test/Analysis/Inputs/system-header-simulator-for-nullability.h @@ -9,6 +9,8 @@ NS_ASSUME_NONNULL_BEGIN typedef struct _NSZone NSZone; +typedef unsigned long NSUInteger; +@class NSCoder, NSEnumerator; @protocol NSObject + (instancetype)alloc; @@ -24,6 +26,22 @@ - (id)mutableCopyWithZone:(nullable NSZone *)zone; @end +@protocol NSCoding +- (void)encodeWithCoder:(NSCoder *)aCoder; +@end + +@protocol NSSecureCoding +@required ++ (BOOL)supportsSecureCoding; +@end + +typedef struct { + unsigned long state; + id *itemsPtr; + unsigned long *mutationsPtr; + unsigned long extra[5]; +} NSFastEnumerationState; + __attribute__((objc_root_class)) @interface NSObject @@ -52,3 +70,36 @@ @end NS_ASSUME_NONNULL_END + +@interface NSDictionary : NSObject + +- (NSUInteger)count; +- (id)objectForKey:(id)aKey; +- (NSEnumerator *)keyEnumerator; +- (id)objectForKeyedSubscript:(id)aKey; + +@end + +@interface NSDictionary (NSDictionaryCreation) + ++ (id)dictionary; ++ (id)dictionaryWithObject:(id)object forKey:(id )key; ++ (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(NSUInteger)cnt; + +@end + +@interface NSMutableDictionary : NSDictionary + +- (void)removeObjectForKey:(id)aKey; +- (void)setObject:(id)anObject forKey:(id )aKey; + +@end + +@interface NSMutableDictionary (NSExtendedMutableDictionary) + +- (void)addEntriesFromDictionary:(NSDictionary *)otherDictionary; +- (void)removeAllObjects; +- (void)setDictionary:(NSDictionary *)otherDictionary; +- (void)setObject:(id)obj forKeyedSubscript:(id )key __attribute__((availability(macosx,introduced=10.8))); + +@end Index: test/Analysis/trustnonnullchecker_test.m === --- test/Analysis/trustnonnullchecker_test.m +++ test/Analysis/trustnonnullchecker_test.m @@ -1,7 +1,9 @@ -// RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-checker=core,nullability,apiModeling -verify %s +// RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-checker=core,nullability,apiModeling,debug.ExprInspection -verify %s #include "Inputs/system-header-simulator-for-nullability.h" +void clang_analyzer_warnIfReached(); + NSString* _Nonnull trust_nonnull_framework_annotation() { NSString* out = [NSString generateString]; if (out) {} @@ -67,3 +69,104 @@ return out; // expected-warning{{}} } +// If the return value is non-nil, the index is non-nil. +NSString *_Nonnull retImpliesIndex(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (obj) +return s; // no-warning + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexOtherMethod(NSString *s, + NSDictionary *dic) { + id obj = [dic objectForKey:s]; + if (s) {} + if (obj) +return s; // no-warning + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexOnRHS(NSString *s, +NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (nil != obj) +return s; // no-warning + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexReverseCheck(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (!obj) +return @"foo"; + return s; // no-warning +} + +NSString *_Nonnull retImpliesIndexReverseCheckOnRHS(NSString *s, +NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (nil == obj) +return @"foo"; + return s; // no-warning +} + +NSString *_Nonnull retImpliesIndexWrongBranch(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (!obj) +return s; // expected-warning{{}} + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexWrongBranchOnRHS(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (nil == obj) +return s; // expected-warning{{}} + return @"foo"; +} + +// The return value could still be nil for a non-nil index. +NSDictionary *_Nonnull indexDoesNotImplyRet(NSString *s, +NSDi
[PATCH] D50595: [analyzer] Fix keyboard navigation for .msgNote events
This revision was automatically updated to reflect the committed changes. Closed by commit rC339493: [analyzer] Fix keyboard navigation for .msgNote events (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50595?vs=160220&id=160223#toc Repository: rC Clang https://reviews.llvm.org/D50595 Files: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp === --- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -997,7 +997,8 @@ }; var navigateTo = function(up) { - var numItems = document.querySelectorAll(".line > .msg").length; + var numItems = document.querySelectorAll( + ".line > .msgEvent, .line > .msgControl").length; var currentSelected = findNum(); var newSelected = move(currentSelected, up, numItems); var newEl = numToId(newSelected, numItems); Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp === --- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -997,7 +997,8 @@ }; var navigateTo = function(up) { - var numItems = document.querySelectorAll(".line > .msg").length; + var numItems = document.querySelectorAll( + ".line > .msgEvent, .line > .msgControl").length; var currentSelected = findNum(); var newSelected = move(currentSelected, up, numItems); var newEl = numToId(newSelected, numItems); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50594: [analyzer] [NFC] Introduce separate targets for testing the analyzer: check-clang-analyzer and check-clang-analyzer-z3
This revision was automatically updated to reflect the committed changes. Closed by commit rC339629: [analyzer] [NFC] Introduce separate targets for testing the analyzer: check… (authored by george.karpenkov, committed by ). Changed prior to commit: https://reviews.llvm.org/D50594?vs=160388&id=160479#toc Repository: rC Clang https://reviews.llvm.org/D50594 Files: test/Analysis/analyzer_test.py test/Analysis/lit.local.cfg test/CMakeLists.txt test/lit.site.cfg.py.in Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@ -88,8 +88,15 @@ set(CLANG_TEST_PARAMS clang_site_config=${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg + USE_Z3_SOLVER=0 ) +set(ANALYZER_TEST_PARAMS + USE_Z3_SOLVER=0) + +set(ANALYZER_TEST_PARAMS_Z3 + USE_Z3_SOLVER=1) + if( NOT CLANG_BUILT_STANDALONE ) list(APPEND CLANG_TEST_DEPS llvm-config @@ -126,6 +133,24 @@ ) set_target_properties(check-clang PROPERTIES FOLDER "Clang tests") +if (CLANG_ENABLE_STATIC_ANALYZER) + add_lit_testsuite(check-clang-analyzer "Running the Clang analyzer tests" +${CMAKE_CURRENT_BINARY_DIR}/Analysis +PARAMS ${ANALYZER_TEST_PARAMS} +DEPENDS ${CLANG_TEST_DEPS}) + set_target_properties(check-clang-analyzer PROPERTIES FOLDER "Clang tests") + + + if (CLANG_ANALYZER_WITH_Z3) +add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer tests, using Z3 as a solver" + ${CMAKE_CURRENT_BINARY_DIR}/Analysis + PARAMS ${ANALYZER_TEST_PARAMS_Z3} + DEPENDS ${CLANG_TEST_DEPS}) +set_target_properties(check-clang-analyzer-z3 PROPERTIES FOLDER "Clang tests") + endif() + +endif() + add_lit_testsuites(CLANG ${CMAKE_CURRENT_SOURCE_DIR} PARAMS ${CLANG_TEST_PARAMS} DEPENDS ${CLANG_TEST_DEPS} Index: test/lit.site.cfg.py.in === --- test/lit.site.cfg.py.in +++ test/lit.site.cfg.py.in @@ -26,14 +26,16 @@ config.enable_backtrace = @ENABLE_BACKTRACES@ config.host_arch = "@HOST_ARCH@" config.python_executable = "@PYTHON_EXECUTABLE@" +config.use_z3_solver = "@USE_Z3_SOLVER@" # Support substitution of the tools and libs dirs with user parameters. This is # used when we can't determine the tool dir at configuration time. try: config.clang_tools_dir = config.clang_tools_dir % lit_config.params config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params config.llvm_shlib_dir = config.llvm_shlib_dir % lit_config.params config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params +config.use_z3_solver = lit_config.params['USE_Z3_SOLVER'] except KeyError: e = sys.exc_info()[1] key, = e.args Index: test/Analysis/lit.local.cfg === --- test/Analysis/lit.local.cfg +++ test/Analysis/lit.local.cfg @@ -7,7 +7,7 @@ site.addsitedir(os.path.dirname(__file__)) import analyzer_test config.test_format = analyzer_test.AnalyzerTest( -config.test_format.execute_external) +config.test_format.execute_external, config.use_z3_solver) if not config.root.clang_staticanalyzer: config.unsupported = True Index: test/Analysis/analyzer_test.py === --- test/Analysis/analyzer_test.py +++ test/Analysis/analyzer_test.py @@ -4,6 +4,10 @@ # Custom format class for static analyzer tests class AnalyzerTest(lit.formats.ShTest): +def __init__(self, execute_external, use_z3_solver=False): +super(AnalyzerTest, self).__init__(execute_external) +self.use_z3_solver = use_z3_solver + def execute(self, test, litConfig): results = [] @@ -19,7 +23,8 @@ return results[-1] # If z3 backend available, add an additional run line for it -if test.config.clang_staticanalyzer_z3 == '1': +if self.use_z3_solver == '1': +assert(test.config.clang_staticanalyzer_z3 == '1') results.append(self.executeWithAnalyzeSubstitution( saved_test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50673: [analyzer] Fix UninitializedObjectChecker to not crash on uninitialized "id" fields
This revision was automatically updated to reflect the committed changes. Closed by commit rC339631: [analyzer] Fix UninitializedObjectChecker to not crash on uninitialized "id"… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50673?vs=160473&id=160482#toc Repository: rC Clang https://reviews.llvm.org/D50673 Files: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp test/Analysis/objcpp-uninitialized-object.mm Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -78,7 +78,7 @@ bool FindUninitializedFields::isPointerOrReferenceUninit( const FieldRegion *FR, FieldChainInfo LocalChain) { - assert((FR->getDecl()->getType()->isPointerType() || + assert((FR->getDecl()->getType()->isAnyPointerType() || FR->getDecl()->getType()->isReferenceType() || FR->getDecl()->getType()->isBlockPointerType()) && "This method only checks pointer/reference objects!"); Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -265,7 +265,7 @@ continue; } -if (T->isPointerType() || T->isReferenceType() || T->isBlockPointerType()) { +if (T->isAnyPointerType() || T->isReferenceType() || T->isBlockPointerType()) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; Index: test/Analysis/objcpp-uninitialized-object.mm === --- test/Analysis/objcpp-uninitialized-object.mm +++ test/Analysis/objcpp-uninitialized-object.mm @@ -20,3 +20,13 @@ void noWarningWhenInitialized() { StructWithBlock a; } + +struct StructWithId { + int a; + id z; // expected-note{{uninitialized pointer 'this->z'}} + StructWithId() : a(0) {} // expected-warning{{1 uninitialized field at the end of the constructor call}} +}; + +void warnOnUninitializedId() { + StructWithId s; +} Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -78,7 +78,7 @@ bool FindUninitializedFields::isPointerOrReferenceUninit( const FieldRegion *FR, FieldChainInfo LocalChain) { - assert((FR->getDecl()->getType()->isPointerType() || + assert((FR->getDecl()->getType()->isAnyPointerType() || FR->getDecl()->getType()->isReferenceType() || FR->getDecl()->getType()->isBlockPointerType()) && "This method only checks pointer/reference objects!"); Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -265,7 +265,7 @@ continue; } -if (T->isPointerType() || T->isReferenceType() || T->isBlockPointerType()) { +if (T->isAnyPointerType() || T->isReferenceType() || T->isBlockPointerType()) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; Index: test/Analysis/objcpp-uninitialized-object.mm === --- test/Analysis/objcpp-uninitialized-object.mm +++ test/Analysis/objcpp-uninitialized-object.mm @@ -20,3 +20,13 @@ void noWarningWhenInitialized() { StructWithBlock a; } + +struct StructWithId { + int a; + id z; // expected-note{{uninitialized pointer 'this->z'}} + StructWithId() : a(0) {} // expected-warning{{1 uninitialized field at the end of the constructor call}} +}; + +void warnOnUninitializedId() { + StructWithId s; +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50594: [analyzer] [NFC] Introduce separate targets for testing the analyzer: check-clang-analyzer and check-clang-analyzer-z3
george.karpenkov added subscribers: dcoughlin, bogner, NoQ, vlad.tsyrklevich. george.karpenkov added a comment. Yes, investigating. Will rollback if not fixed in a few hours. Repository: rC Clang https://reviews.llvm.org/D50594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50594: [analyzer] [NFC] Introduce separate targets for testing the analyzer: check-clang-analyzer and check-clang-analyzer-z3
george.karpenkov added a comment. Should be fixed now. Repository: rC Clang https://reviews.llvm.org/D50594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager
george.karpenkov added a comment. @baloghadamsoftware @xazax.hun we've had very promising results with using Z3 for refutation so far, almost at no cost, see Mikhail's recent email on cfe-dev (and sometimes at a negative cost!). Do you still not want to try it first? False negatives could be addressed as well separately (e.g. by giving a checkers an option to cross-check the report with a more expensive solver before throwing), currently false positives are a far more pressing problem. https://reviews.llvm.org/D49074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50830: [analyzer] [NFC] Split up summary generation in RetainCountChecker in two methods
This revision was automatically updated to reflect the committed changes. Closed by commit rC340093: [analyzer] [NFC] Split up summary generation in RetainCountChecker in two… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50830?vs=160963&id=161334#toc Repository: rC Clang https://reviews.llvm.org/D50830 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h @@ -495,6 +495,10 @@ RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; } +private: + const RetainSummary * generateSummary(const FunctionDecl *FD, +bool &AllowAnnotations); + friend class RetainSummaryTemplate; }; Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp @@ -58,228 +58,215 @@ } const RetainSummary * -RetainSummaryManager::getFunctionSummary(const FunctionDecl *FD) { - // If we don't know what function we're calling, use our default summary. - if (!FD) +RetainSummaryManager::generateSummary(const FunctionDecl *FD, + bool &AllowAnnotations) { + // We generate "stop" summaries for implicitly defined functions. + if (FD->isImplicit()) { +return getPersistentStopSummary(); + } + + // [PR 3337] Use 'getAs' to strip away any typedefs on the + // function's type. + const FunctionType *FT = FD->getType()->getAs(); + const IdentifierInfo *II = FD->getIdentifier(); + if (!II) return getDefaultSummary(); - // Look up a summary in our cache of FunctionDecls -> Summaries. - FuncSummariesTy::iterator I = FuncSummaries.find(FD); - if (I != FuncSummaries.end()) -return I->second; + StringRef FName = II->getName(); - // No summary? Generate one. - const RetainSummary *S = nullptr; - bool AllowAnnotations = true; + // Strip away preceding '_'. Doing this here will effect all the checks + // down below. + FName = FName.substr(FName.find_first_not_of('_')); + + // Inspect the result type. + QualType RetTy = FT->getReturnType(); + std::string RetTyName = RetTy.getAsString(); - do { -// We generate "stop" summaries for implicitly defined functions. -if (FD->isImplicit()) { - S = getPersistentStopSummary(); - break; -} - -// [PR 3337] Use 'getAs' to strip away any typedefs on the -// function's type. -const FunctionType* FT = FD->getType()->getAs(); -const IdentifierInfo *II = FD->getIdentifier(); -if (!II) - break; - -StringRef FName = II->getName(); + // FIXME: This should all be refactored into a chain of "summary lookup" + // filters. + assert(ScratchArgs.isEmpty()); -// Strip away preceding '_'. Doing this here will effect all the checks -// down below. -FName = FName.substr(FName.find_first_not_of('_')); - -// Inspect the result type. -QualType RetTy = FT->getReturnType(); -std::string RetTyName = RetTy.getAsString(); - -// FIXME: This should all be refactored into a chain of "summary lookup" -// filters. -assert(ScratchArgs.isEmpty()); - -if (FName == "pthread_create" || FName == "pthread_setspecific") { - // Part of: and . - // This will be addressed better with IPA. - S = getPersistentStopSummary(); -} else if (FName == "CFPlugInInstanceCreate") { - S = getPersistentSummary(RetEffect::MakeNoRet()); -} else if (FName == "IORegistryEntrySearchCFProperty" -|| (RetTyName == "CFMutableDictionaryRef" && ( - FName == "IOBSDNameMatching" || - FName == "IOServiceMatching" || + if (FName == "pthread_create" || FName == "pthread_setspecific") { +// Part of: and . +// This will be addressed better with IPA. +return getPersistentStopSummary(); + } else if (FName == "CFPlugInInstanceCreate") { +return getPersistentSummary(RetEffect::MakeNoRet()); + } else if (FName == "IORegistryEntrySearchCFProperty" || + (RetTyName == "CFMutableDictionaryRef" && + (FName == "IOBSDNameMatching" || FName == "IOServiceMatching" || FName == "IOServiceNameMatching" || FName == "IORegistryEntryIDMatching" || - FName == "IOOpenFirmwarePathMatching" -))) { - // Part of . (IOKit) - // This should be addressed using a API table. - S = getPersi
[PATCH] D50863: [analyzer] [NFC] Move canEval function from RetainCountChecker to RetainCountSummaries
This revision was automatically updated to reflect the committed changes. Closed by commit rC340094: [analyzer] [NFC] Move canEval function from RetainCountChecker to… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50863?vs=161105&id=161335#toc Repository: rC Clang https://reviews.llvm.org/D50863 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h @@ -199,32 +199,6 @@ return Args.isEmpty(); } - - static bool isRetain(const FunctionDecl *FD, StringRef FName) { -return FName.startswith_lower("retain") || FName.endswith_lower("retain"); - } - - static bool isRelease(const FunctionDecl *FD, StringRef FName) { -return FName.startswith_lower("release") || FName.endswith_lower("release"); - } - - static bool isAutorelease(const FunctionDecl *FD, StringRef FName) { -return FName.startswith_lower("autorelease") || - FName.endswith_lower("autorelease"); - } - - static bool hasRCAnnotation(const Decl *D, StringRef rcAnnotation) { -for (const auto *Ann : D->specific_attrs()) { - if (Ann->getAnnotation() == rcAnnotation) -return true; -} -return false; - } - - static bool isTrustedReferenceCountImplementation(const FunctionDecl *FD) { -return hasRCAnnotation(FD, "rc_ownership_trusted_implementation"); - } - private: ArgEffects getArgEffects() const { return Args; } ArgEffect getDefaultArgEffect() const { return DefaultArgEffect; } @@ -375,7 +349,7 @@ void InitializeClassMethodSummaries(); void InitializeMethodSummaries(); -private: + void addNSObjectClsMethSummary(Selector S, const RetainSummary *Summ) { ObjCClassMethodSummaries[S] = Summ; } @@ -438,6 +412,12 @@ InitializeMethodSummaries(); } + bool canEval(const CallExpr *CE, + const FunctionDecl *FD, + bool &hasTrustedImplementationAnnotation); + + bool isTrustedReferenceCountImplementation(const FunctionDecl *FD); + const RetainSummary *getSummary(const CallEvent &Call, ProgramStateRef State = nullptr); Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp @@ -57,6 +57,27 @@ return Summ; } +static bool hasRCAnnotation(const Decl *D, StringRef rcAnnotation) { + for (const auto *Ann : D->specific_attrs()) { +if (Ann->getAnnotation() == rcAnnotation) + return true; + } + return false; +} + +static bool isRetain(const FunctionDecl *FD, StringRef FName) { + return FName.startswith_lower("retain") || FName.endswith_lower("retain"); +} + +static bool isRelease(const FunctionDecl *FD, StringRef FName) { + return FName.startswith_lower("release") || FName.endswith_lower("release"); +} + +static bool isAutorelease(const FunctionDecl *FD, StringRef FName) { + return FName.startswith_lower("autorelease") || + FName.endswith_lower("autorelease"); +} + const RetainSummary * RetainSummaryManager::generateSummary(const FunctionDecl *FD, bool &AllowAnnotations) { @@ -172,15 +193,15 @@ if (RetTy->isPointerType()) { // For CoreFoundation ('CF') types. if (cocoa::isRefType(RetTy, "CF", FName)) { - if (RetainSummary::isRetain(FD, FName)) { + if (isRetain(FD, FName)) { // CFRetain isn't supposed to be annotated. However, this may as well // be a user-made "safe" CFRetain function that is incorrectly // annotated as cf_returns_retained due to lack of better options. // We want to ignore such annotation. AllowAnnotations = false; return getUnarySummary(FT, cfretain); - } else if (RetainSummary::isAutorelease(FD, FName)) { + } else if (isAutorelease(FD, FName)) { // The headers use cf_consumed, but we can fully model CFAutorelease // ourselves. AllowAnnotations = false; @@ -194,7 +215,7 @@ // For CoreGraphics ('CG') and CoreVideo ('CV') types. if (cocoa::isRefType(RetTy, "CG", FName) || cocoa::isRefType(RetTy, "CV", FName)) { - if (RetainSummary::isRetain(FD, FName)) + if (isRetain(FD, FName)) return getUnarySummary(FT, cfretain); else return getCFCreateGetRuleSummary(FD); @@ -219,7 +2
[PATCH] D50869: [analyzer] [NFC] Move ObjCRetainCount to include/Analysis
This revision was automatically updated to reflect the committed changes. Closed by commit rC340096: [analyzer] [NFC] Move ObjCRetainCount to include/Analysis (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50869?vs=161117&id=161336#toc Repository: rC Clang https://reviews.llvm.org/D50869 Files: include/clang/Analysis/ObjCRetainCount.h include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h lib/ARCMigrate/ObjCMT.cpp lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h @@ -21,11 +21,11 @@ #include "../SelectorExtras.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/FoldingSet.h" +#include "clang/Analysis/ObjCRetainCount.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ParentMap.h" -#include "clang/StaticAnalyzer/Checkers/ObjCRetainCount.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -20,14 +20,14 @@ #include "../SelectorExtras.h" #include "RetainCountSummaries.h" #include "RetainCountDiagnostics.h" +#include "clang/Analysis/ObjCRetainCount.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ParentMap.h" #include "clang/Analysis/DomainSpecific/CocoaConventions.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceManager.h" -#include "clang/StaticAnalyzer/Checkers/ObjCRetainCount.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" Index: lib/ARCMigrate/ObjCMT.cpp === --- lib/ARCMigrate/ObjCMT.cpp +++ lib/ARCMigrate/ObjCMT.cpp @@ -8,6 +8,7 @@ //===--===// #include "Transforms.h" +#include "clang/Analysis/ObjCRetainCount.h" #include "clang/ARCMigrate/ARCMT.h" #include "clang/ARCMigrate/ARCMTActions.h" #include "clang/AST/ASTConsumer.h" @@ -27,7 +28,6 @@ #include "clang/Lex/PPConditionalDirectiveRecord.h" #include "clang/Lex/Preprocessor.h" #include "clang/Rewrite/Core/Rewriter.h" -#include "clang/StaticAnalyzer/Checkers/ObjCRetainCount.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Path.h" Index: include/clang/Analysis/ObjCRetainCount.h === --- include/clang/Analysis/ObjCRetainCount.h +++ include/clang/Analysis/ObjCRetainCount.h @@ -0,0 +1,228 @@ +//==-- ObjCRetainCount.h - Retain count summaries for Cocoa ---*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// This file defines the core data structures for retain count "summaries" +// for Objective-C and Core Foundation APIs. These summaries are used +// by the static analyzer to summarize the retain/release effects of +// function and method calls. This drives a path-sensitive typestate +// analysis in the static analyzer, but can also potentially be used by +// other clients. +// +//===--===// + +#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H +#define LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H + +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallVector.h" + +namespace clang { +class FunctionDecl; +class ObjCMethodDecl; + +namespace ento { namespace objc_retain { + +/// An ArgEffect summarizes the retain count behavior on an argument or receiver +/// to a function or method. +enum ArgEffect { + /// There is no effect. + DoNothing, + + /// The argument is treated as if an -autorelease message had been sent to + /// the referenced object. + Autorelease, + + /// The argument is treated as if an -dealloc message had been sent to + /// the referenced object. + Dea
[PATCH] D50879: [analyzer] [NFC] Minor refactoring of ISL-specific code in RetainCountChecker
This revision was automatically updated to reflect the committed changes. Closed by commit rC340098: [analyzer] [NFC] Minor refactoring of ISL-specific code in RetainCountChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50879?vs=161151&id=161339#toc Repository: rC Clang https://reviews.llvm.org/D50879 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp @@ -482,10 +482,10 @@ return isRetain(FD, FName) || isAutorelease(FD, FName) || isMakeCollectable(FName); -if (FD->getDefinition()) { - bool out = isTrustedReferenceCountImplementation(FD->getDefinition()); - hasTrustedImplementationAnnotation = out; - return out; +const FunctionDecl* FDD = FD->getDefinition(); +if (FDD && isTrustedReferenceCountImplementation(FDD)) { + hasTrustedImplementationAnnotation = true; + return true; } } Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -765,9 +765,7 @@ bool hasTrustedImplementationAnnotation = false; // See if it's one of the specific functions we know how to eval. - bool canEval = SmrMgr.canEval(CE, FD, hasTrustedImplementationAnnotation); - - if (!canEval) + if (!SmrMgr.canEval(CE, FD, hasTrustedImplementationAnnotation)) return false; // Bind the return value. @@ -1235,11 +1233,8 @@ return N; } -static bool isGeneralizedObjectRef(QualType Ty) { - if (Ty.getAsString().substr(0, 4) == "isl_") -return true; - else -return false; +static bool isISLObjectRef(QualType Ty) { + return StringRef(Ty.getAsString()).startswith("isl_"); } void RetainCountChecker::checkBeginFunction(CheckerContext &Ctx) const { @@ -1263,9 +1258,9 @@ QualType Ty = Param->getType(); const ArgEffect *AE = CalleeSideArgEffects.lookup(idx); -if (AE && *AE == DecRef && isGeneralizedObjectRef(Ty)) { +if (AE && *AE == DecRef && isISLObjectRef(Ty)) { state = setRefBinding(state, Sym, RefVal::makeOwned(RetEffect::ObjKind::Generalized, Ty)); -} else if (isGeneralizedObjectRef(Ty)) { +} else if (isISLObjectRef(Ty)) { state = setRefBinding( state, Sym, RefVal::makeNotOwned(RetEffect::ObjKind::Generalized, Ty)); Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp @@ -482,10 +482,10 @@ return isRetain(FD, FName) || isAutorelease(FD, FName) || isMakeCollectable(FName); -if (FD->getDefinition()) { - bool out = isTrustedReferenceCountImplementation(FD->getDefinition()); - hasTrustedImplementationAnnotation = out; - return out; +const FunctionDecl* FDD = FD->getDefinition(); +if (FDD && isTrustedReferenceCountImplementation(FDD)) { + hasTrustedImplementationAnnotation = true; + return true; } } Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -765,9 +765,7 @@ bool hasTrustedImplementationAnnotation = false; // See if it's one of the specific functions we know how to eval. - bool canEval = SmrMgr.canEval(CE, FD, hasTrustedImplementationAnnotation); - - if (!canEval) + if (!SmrMgr.canEval(CE, FD, hasTrustedImplementationAnnotation)) return false; // Bind the return value. @@ -1235,11 +1233,8 @@ return N; } -static bool isGeneralizedObjectRef(QualType Ty) { - if (Ty.getAsString().substr(0, 4) == "isl_") -return true; - else -return false; +static bool isISLObjectRef(QualType Ty) { + return StringRef(Ty.getAsString()).startswith("isl_"); } void RetainCountChecker::checkBeginFunction(CheckerContext &Ctx) const { @@ -1263,9 +1258,9 @@ QualType Ty = Param->getType(); const ArgEffect *AE = CalleeSideArgEffects.lookup(idx); -if (AE && *AE == DecRef && isGeneralizedObjectRef(Ty)) { +if (AE && *AE == DecRef && isISLObjectRef(Ty)) { sta
[PATCH] D50866: [analyzer] CFRetainReleaseChecker: Avoid checking C++ methods with the same name.
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp:537 + mutable APIMisuse BT{this, "null passed to CF memory management function"}; + CallDescription CFRetain{"CFRetain", 1}, + CFRelease{"CFRelease", 1}, I personally would prefer being less fancy, and avoiding the comma operator, but I suppose it's a matter of style. Comment at: lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp:567 + ProgramStateRef stateNonNull, stateNull; + std::tie(stateNonNull, stateNull) = state->assume(*DefArgVal); There's also DefArgVal Repository: rC Clang https://reviews.llvm.org/D50866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50855: [analyzer] pr37578: Fix lvalue/rvalue problem in field-of-temporary adjustments.
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. looks good apart from minor nits Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:744 + const Expr *InitWithAdjustments, const Expr *Result = nullptr, + const SubRegion **OutRegionWithAdjustments = nullptr); In general I would say that pair is still preferable, but I agree that with pre-c++17 codebase it gets too verbose. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:423 // correctly in case (Result == Init). - State = State->BindExpr(Result, LC, Reg); + if (Result->isGLValue()) +State = State->BindExpr(Result, LC, Reg); could we have braces here? Once we have "else" I would say we should have those. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2543 // Handle regular struct fields / member variables. - state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr); - SVal baseExprVal = state->getSVal(BaseExpr, LCtx); + const SubRegion *MR; + state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr, Let's initialize that to nullptr https://reviews.llvm.org/D50855 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51072: [analyzer] [NFC] Fix minor formatting issues in RetainCountChecker
This revision was automatically updated to reflect the committed changes. Closed by commit rC340378: [analyzer] [NFC] Fix minor formatting issues in RetainCountChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D51072?vs=161844&id=161872#toc Repository: rC Clang https://reviews.llvm.org/D51072 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -218,7 +218,6 @@ } // Comparison, profiling, and pretty-printing. - bool hasSameState(const RefVal &X) const { return getKind() == X.getKind() && Cnt == X.Cnt && ACnt == X.ACnt && getIvarAccessHistory() == X.getIvarAccessHistory(); Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -457,8 +457,7 @@ " This violates the naming convention rules" " given in the Memory Management Guide for Cocoa"; } - } - else { + } else { const FunctionDecl *FD = cast(D); os << "whose name ('" << *FD << "') does not contain 'Copy' or 'Create'. This violates the naming" Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -409,8 +409,7 @@ } // Evaluate the effect on the message receiver. - const ObjCMethodCall *MsgInvocation = dyn_cast(&CallOrMsg); - if (MsgInvocation) { + if (const auto *MsgInvocation = dyn_cast(&CallOrMsg)) { if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) { if (Summ.getReceiverEffect() == StopTrackingHard) { state = removeRefBinding(state, Sym); @@ -987,7 +986,7 @@ // does not understand. ProgramStateRef state = C.getState(); - if (Optional regionLoc = loc.getAs()) { + if (auto regionLoc = loc.getAs()) { escapes = !regionLoc->getRegion()->hasStackStorage(); if (!escapes) { @@ -1011,7 +1010,7 @@ // If we are storing the value into an auto function scope variable annotated // with (__attribute__((cleanup))), stop tracking the value to avoid leak // false positives. - if (const VarRegion *LVR = dyn_cast_or_null(loc.getAsRegion())) { + if (const auto *LVR = dyn_cast_or_null(loc.getAsRegion())) { const VarDecl *VD = LVR->getDecl(); if (VD->hasAttr()) { escapes = true; @@ -1031,8 +1030,8 @@ } ProgramStateRef RetainCountChecker::evalAssume(ProgramStateRef state, - SVal Cond, - bool Assumption) const { + SVal Cond, + bool Assumption) const { // FIXME: We may add to the interface of evalAssume the list of symbols // whose assumptions have changed. For now we just iterate through the // bindings and check if any of the tracked symbols are NULL. This isn't @@ -1253,7 +1252,8 @@ QualType Ty = Param->getType(); const ArgEffect *AE = CalleeSideArgEffects.lookup(idx); if (AE && *AE == DecRef && isISLObjectRef(Ty)) { - state = setRefBinding(state, Sym, RefVal::makeOwned(RetEffect::ObjKind::Generalized, Ty)); + state = setRefBinding( + state, Sym, RefVal::makeOwned(RetEffect::ObjKind::Generalized, Ty)); } else if (isISLObjectRef(Ty)) { state = setRefBinding( state, Sym, Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h === --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -218,7 +218,6 @@ } // Comparison, profiling, and pretty-printing. - bool hasSameState(const RefVal &X) const { return getKind() == X.getKind() && Cnt == X.Cnt && ACnt == X.ACnt && getIvarAccessHistory() == X.getIvarAccessHistory(); Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp ==
[PATCH] D51072: [analyzer] [NFC] Fix minor formatting issues in RetainCountChecker
This revision was automatically updated to reflect the committed changes. Closed by commit rL340378: [analyzer] [NFC] Fix minor formatting issues in RetainCountChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51072?vs=161844&id=161873#toc Repository: rL LLVM https://reviews.llvm.org/D51072 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h === --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -218,7 +218,6 @@ } // Comparison, profiling, and pretty-printing. - bool hasSameState(const RefVal &X) const { return getKind() == X.getKind() && Cnt == X.Cnt && ACnt == X.ACnt && getIvarAccessHistory() == X.getIvarAccessHistory(); Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -457,8 +457,7 @@ " This violates the naming convention rules" " given in the Memory Management Guide for Cocoa"; } - } - else { + } else { const FunctionDecl *FD = cast(D); os << "whose name ('" << *FD << "') does not contain 'Copy' or 'Create'. This violates the naming" Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -409,8 +409,7 @@ } // Evaluate the effect on the message receiver. - const ObjCMethodCall *MsgInvocation = dyn_cast(&CallOrMsg); - if (MsgInvocation) { + if (const auto *MsgInvocation = dyn_cast(&CallOrMsg)) { if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) { if (Summ.getReceiverEffect() == StopTrackingHard) { state = removeRefBinding(state, Sym); @@ -987,7 +986,7 @@ // does not understand. ProgramStateRef state = C.getState(); - if (Optional regionLoc = loc.getAs()) { + if (auto regionLoc = loc.getAs()) { escapes = !regionLoc->getRegion()->hasStackStorage(); if (!escapes) { @@ -1011,7 +1010,7 @@ // If we are storing the value into an auto function scope variable annotated // with (__attribute__((cleanup))), stop tracking the value to avoid leak // false positives. - if (const VarRegion *LVR = dyn_cast_or_null(loc.getAsRegion())) { + if (const auto *LVR = dyn_cast_or_null(loc.getAsRegion())) { const VarDecl *VD = LVR->getDecl(); if (VD->hasAttr()) { escapes = true; @@ -1031,8 +1030,8 @@ } ProgramStateRef RetainCountChecker::evalAssume(ProgramStateRef state, - SVal Cond, - bool Assumption) const { + SVal Cond, + bool Assumption) const { // FIXME: We may add to the interface of evalAssume the list of symbols // whose assumptions have changed. For now we just iterate through the // bindings and check if any of the tracked symbols are NULL. This isn't @@ -1253,7 +1252,8 @@ QualType Ty = Param->getType(); const ArgEffect *AE = CalleeSideArgEffects.lookup(idx); if (AE && *AE == DecRef && isISLObjectRef(Ty)) { - state = setRefBinding(state, Sym, RefVal::makeOwned(RetEffect::ObjKind::Generalized, Ty)); + state = setRefBinding( + state, Sym, RefVal::makeOwned(RetEffect::ObjKind::Generalized, Ty)); } else if (isISLObjectRef(Ty)) { state = setRefBinding( state, Sym, Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h === --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -218,7 +218,6 @@ } // Comparison, profiling, and pretty-printing. - bool hasSameState(const RefVal &X) const { return getKind() == X.getKind() && Cnt == X.Cnt && ACnt == X.ACnt && getIvarAccessHistory() == X.getIva
[PATCH] D51130: [analyzer] [NFC] Minor refactoring of BugReporterVisitors
This revision was automatically updated to reflect the committed changes. Closed by commit rC340473: [analyzer] [NFC] Minor refactoring of BugReporterVisitors (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D51130?vs=162066&id=162096#toc Repository: rC Clang https://reviews.llvm.org/D51130 Files: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp === --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -759,10 +759,14 @@ bool EnableNullFPSuppression; bool ShouldInvalidate = true; + AnalyzerOptions& Options; public: - ReturnVisitor(const StackFrameContext *Frame, bool Suppressed) - : StackFrame(Frame), EnableNullFPSuppression(Suppressed) {} + ReturnVisitor(const StackFrameContext *Frame, +bool Suppressed, +AnalyzerOptions &Options) + : StackFrame(Frame), EnableNullFPSuppression(Suppressed), +Options(Options) {} static void *getTag() { static int Tag = 0; @@ -790,10 +794,10 @@ // First, find when we processed the statement. do { - if (Optional CEE = Node->getLocationAs()) + if (auto CEE = Node->getLocationAs()) if (CEE->getCalleeContext()->getCallSite() == S) break; - if (Optional SP = Node->getLocationAs()) + if (auto SP = Node->getLocationAs()) if (SP->getStmt() == S) break; @@ -834,13 +838,8 @@ BR.markInteresting(CalleeContext); BR.addVisitor(llvm::make_unique(CalleeContext, - EnableNullFPSuppression)); - } - - /// Returns true if any counter-suppression heuristics are enabled for - /// ReturnVisitor. - static bool hasCounterSuppression(AnalyzerOptions &Options) { -return Options.shouldAvoidSuppressingNullArgumentPaths(); + EnableNullFPSuppression, + Options)); } std::shared_ptr @@ -909,8 +908,8 @@ // If we have counter-suppression enabled, make sure we keep visiting // future nodes. We want to emit a path note as well, in case // the report is resurrected as valid later on. - AnalyzerOptions &Options = BRC.getAnalyzerOptions(); - if (EnableNullFPSuppression && hasCounterSuppression(Options)) + if (EnableNullFPSuppression && + Options.shouldAvoidSuppressingNullArgumentPaths()) Mode = MaybeUnsuppress; if (RetE->getType()->isObjCObjectPointerType()) @@ -947,8 +946,7 @@ visitNodeMaybeUnsuppress(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) { #ifndef NDEBUG -AnalyzerOptions &Options = BRC.getAnalyzerOptions(); -assert(hasCounterSuppression(Options)); +assert(Options.shouldAvoidSuppressingNullArgumentPaths()); #endif // Are we at the entry node for this call? Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp === --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -759,10 +759,14 @@ bool EnableNullFPSuppression; bool ShouldInvalidate = true; + AnalyzerOptions& Options; public: - ReturnVisitor(const StackFrameContext *Frame, bool Suppressed) - : StackFrame(Frame), EnableNullFPSuppression(Suppressed) {} + ReturnVisitor(const StackFrameContext *Frame, +bool Suppressed, +AnalyzerOptions &Options) + : StackFrame(Frame), EnableNullFPSuppression(Suppressed), +Options(Options) {} static void *getTag() { static int Tag = 0; @@ -790,10 +794,10 @@ // First, find when we processed the statement. do { - if (Optional CEE = Node->getLocationAs()) + if (auto CEE = Node->getLocationAs()) if (CEE->getCalleeContext()->getCallSite() == S) break; - if (Optional SP = Node->getLocationAs()) + if (auto SP = Node->getLocationAs()) if (SP->getStmt() == S) break; @@ -834,13 +838,8 @@ BR.markInteresting(CalleeContext); BR.addVisitor(llvm::make_unique(CalleeContext, - EnableNullFPSuppression)); - } - - /// Returns true if any counter-suppression heuristics are enabled for - /// ReturnVisitor. - static bool hasCounterSuppression(AnalyzerOptions &Options) { -return Options.shouldAvoidSuppressingNullArgumentPaths(); + EnableNullFPSuppression, + Options)); } std::shared_ptr @@ -909,8 +908,8 @@ // If we have counter-suppression enabled, make sure we ke
[PATCH] D51131: [analyzer] Track non-zero values in ReturnVisitor
This revision was automatically updated to reflect the committed changes. Closed by commit rC340475: [analyzer] Track non-zero values in ReturnVisitor (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D51131?vs=162067&id=162098#toc Repository: rC Clang https://reviews.llvm.org/D51131 Files: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp test/Analysis/diagnostics/track_subexpressions.cpp test/Analysis/uninit-const.cpp Index: test/Analysis/uninit-const.cpp === --- test/Analysis/uninit-const.cpp +++ test/Analysis/uninit-const.cpp @@ -49,15 +49,17 @@ int& f6_1_sub(int &p) { - return p; + return p; // expected-note{{Returning without writing to 'p'}} +// expected-note@-1{{Returning pointer (reference to 't')}} } void f6_1(void) { int t; // expected-note{{'t' declared without an initial value}} int p = f6_1_sub(t); //expected-warning {{Assigned value is garbage or undefined}} - //expected-note@-1 {{Calling 'f6_1_sub'}} - //expected-note@-2 {{Returning from 'f6_1_sub'}} - //expected-note@-3 {{Assigned value is garbage or undefined}} + //expected-note@-1 {{Passing value via 1st parameter 'p'}} + //expected-note@-2 {{Calling 'f6_1_sub'}} + //expected-note@-3 {{Returning from 'f6_1_sub'}} + //expected-note@-4 {{Assigned value is garbage or undefined}} int q = p; doStuff6(q); } Index: test/Analysis/diagnostics/track_subexpressions.cpp === --- test/Analysis/diagnostics/track_subexpressions.cpp +++ test/Analysis/diagnostics/track_subexpressions.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_analyze_cc1 -x c++ -analyzer-checker=core -analyzer-output=text -verify %s + +typedef unsigned char uint8_t; + +#define UINT8_MAX 255 +#define TCP_MAXWIN 65535 + +uint8_t get_uint8_max() { + uint8_t rcvscale = UINT8_MAX; // expected-note{{'rcvscale' initialized to 255}} + return rcvscale; // expected-note{{Returning the value 255 (loaded from 'rcvscale')}} +} + +void shift_by_undefined_value() { + uint8_t shift_amount = get_uint8_max(); // expected-note{{'shift_amount' initialized to 255}} +// expected-note@-1{{Calling 'get_uint8_max'}} +// expected-note@-2{{Returning from 'get_uint8_max'}} + (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}} + // expected-note@-1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}} +} Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp === --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -887,37 +887,41 @@ RetE = RetE->IgnoreParenCasts(); -// If we can't prove the return value is 0, just mark it interesting, and -// make sure to track it into any further inner functions. -if (!State->isNull(V).isConstrainedTrue()) { - BR.markInteresting(V); - ReturnVisitor::addVisitorIfNecessary(N, RetE, BR, - EnableNullFPSuppression); - return nullptr; -} - // If we're returning 0, we should track where that 0 came from. bugreporter::trackNullOrUndefValue(N, RetE, BR, /*IsArg*/ false, EnableNullFPSuppression); // Build an appropriate message based on the return value. SmallString<64> Msg; llvm::raw_svector_ostream Out(Msg); -if (V.getAs()) { - // If we have counter-suppression enabled, make sure we keep visiting - // future nodes. We want to emit a path note as well, in case - // the report is resurrected as valid later on. - if (EnableNullFPSuppression && - Options.shouldAvoidSuppressingNullArgumentPaths()) -Mode = MaybeUnsuppress; +if (State->isNull(V).isConstrainedTrue()) { + if (V.getAs()) { + +// If we have counter-suppression enabled, make sure we keep visiting +// future nodes. We want to emit a path note as well, in case +// the report is resurrected as valid later on. +if (EnableNullFPSuppression && +Options.shouldAvoidSuppressingNullArgumentPaths()) + Mode = MaybeUnsuppress; + +if (RetE->getType()->isObjCObjectPointerType()) { + Out << "Returning nil"; +} else { + Out << "Returning null pointer"; +} + } else { +Out << "Returning zero"; + } - if (RetE->getType()->i
[PATCH] D51139: [analyzer] Track the problematic subexpression in UndefResultChecker
This revision was automatically updated to reflect the committed changes. Closed by commit rC340474: [analyzer] Track the problematic subexpression in UndefResultChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D51139?vs=162087&id=162097#toc Repository: rC Clang https://reviews.llvm.org/D51139 Files: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp === --- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -122,14 +122,16 @@ << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left" : "right") << " shift is undefined because the right operand is negative"; +Ex = B->getRHS(); } else if ((B->getOpcode() == BinaryOperatorKind::BO_Shl || B->getOpcode() == BinaryOperatorKind::BO_Shr) && isShiftOverflow(B, C)) { OS << "The result of the " << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left" : "right") << " shift is undefined due to shifting by "; +Ex = B->getRHS(); SValBuilder &SB = C.getSValBuilder(); const llvm::APSInt *I = @@ -147,6 +149,7 @@ C.isNegative(B->getLHS())) { OS << "The result of the left shift is undefined because the left " "operand is negative"; +Ex = B->getLHS(); } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl && isLeftShiftResultUnrepresentable(B, C)) { ProgramStateRef State = C.getState(); @@ -160,6 +163,7 @@ << "\', which is unrepresentable in the unsigned version of " << "the return type \'" << B->getLHS()->getType().getAsString() << "\'"; +Ex = B->getLHS(); } else { OS << "The result of the '" << BinaryOperator::getOpcodeStr(B->getOpcode()) Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp === --- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -122,14 +122,16 @@ << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left" : "right") << " shift is undefined because the right operand is negative"; +Ex = B->getRHS(); } else if ((B->getOpcode() == BinaryOperatorKind::BO_Shl || B->getOpcode() == BinaryOperatorKind::BO_Shr) && isShiftOverflow(B, C)) { OS << "The result of the " << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left" : "right") << " shift is undefined due to shifting by "; +Ex = B->getRHS(); SValBuilder &SB = C.getSValBuilder(); const llvm::APSInt *I = @@ -147,6 +149,7 @@ C.isNegative(B->getLHS())) { OS << "The result of the left shift is undefined because the left " "operand is negative"; +Ex = B->getLHS(); } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl && isLeftShiftResultUnrepresentable(B, C)) { ProgramStateRef State = C.getState(); @@ -160,6 +163,7 @@ << "\', which is unrepresentable in the unsigned version of " << "the return type \'" << B->getLHS()->getType().getAsString() << "\'"; +Ex = B->getLHS(); } else { OS << "The result of the '" << BinaryOperator::getOpcodeStr(B->getOpcode()) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50109: [NFC] Add tags file to .gitignore
This revision was automatically updated to reflect the committed changes. Closed by commit rC340479: [NFC] Add tags file to .gitignore (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D50109?vs=162099&id=162101#toc Repository: rC Clang https://reviews.llvm.org/D50109 Files: .gitignore Index: .gitignore === --- .gitignore +++ .gitignore @@ -24,6 +24,7 @@ #==# cscope.files cscope.out +/tags #==# # Directories to ignore (do not add trailing '/'s, they skip symlinks). Index: .gitignore === --- .gitignore +++ .gitignore @@ -24,6 +24,7 @@ #==# cscope.files cscope.out +/tags #==# # Directories to ignore (do not add trailing '/'s, they skip symlinks). ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
george.karpenkov reopened this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. This is not correct, `strlcat(dest, "mystr", sizeof(dest))` is perfectly fine, as can be seen in many examples including strlcat man page. Repository: rL LLVM https://reviews.llvm.org/D49722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46603: [Support] Print TimeRecord as CSV
george.karpenkov added a comment. @lebedev.ri LLVM already has facilities for outputting statistics in JSON, it seems that would be sufficient for your purposes? I did something similar for the static analyzer in https://reviews.llvm.org/D43131 Repository: rL LLVM https://reviews.llvm.org/D46603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46633: [analyzer] add range check for InitList lookup
george.karpenkov added a comment. Looks good, thanks! Repository: rL LLVM https://reviews.llvm.org/D46633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46772: [analyzer] Extend the ObjCAutoreleaseWriteChecker to warn on captures as well
This revision was automatically updated to reflect the committed changes. Closed by commit rC332300: [analyzer] Extend the ObjCAutoreleaseWriteChecker to warn on captures as well (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D46772 Files: lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp test/Analysis/autoreleasewritechecker_test.m Index: lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp === --- lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp +++ lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp @@ -8,7 +8,7 @@ //===--===// // // This file defines ObjCAutoreleaseWriteChecker which warns against writes -// into autoreleased out parameters which are likely to cause crashes. +// into autoreleased out parameters which cause crashes. // An example of a problematic write is a write to {@code error} in the example // below: // @@ -21,8 +21,9 @@ // return false; // } // -// Such code is very likely to crash due to the other queue autorelease pool -// begin able to free the error object. +// Such code will crash on read from `*error` due to the autorelease pool +// in `enumerateObjectsUsingBlock` implementation freeing the error object +// on exit from the function. // //===--===// @@ -41,8 +42,9 @@ namespace { const char *ProblematicWriteBind = "problematicwrite"; +const char *CapturedBind = "capturedbind"; const char *ParamBind = "parambind"; -const char *MethodBind = "methodbind"; +const char *IsMethodBind = "ismethodbind"; class ObjCAutoreleaseWriteChecker : public Checker { public: @@ -110,67 +112,87 @@ AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D); const auto *PVD = Match.getNodeAs(ParamBind); - assert(PVD); QualType Ty = PVD->getType(); if (Ty->getPointeeType().getObjCLifetime() != Qualifiers::OCL_Autoreleasing) return; - const auto *SW = Match.getNodeAs(ProblematicWriteBind); - bool IsMethod = Match.getNodeAs(MethodBind) != nullptr; + const char *WarningMsg = "Write to"; + const auto *MarkedStmt = Match.getNodeAs(ProblematicWriteBind); + + // Prefer to warn on write, but if not available, warn on capture. + if (!MarkedStmt) { +MarkedStmt = Match.getNodeAs(CapturedBind); +assert(MarkedStmt); +WarningMsg = "Capture of"; + } + + SourceRange Range = MarkedStmt->getSourceRange(); + PathDiagnosticLocation Location = PathDiagnosticLocation::createBegin( + MarkedStmt, BR.getSourceManager(), ADC); + bool IsMethod = Match.getNodeAs(IsMethodBind) != nullptr; const char *Name = IsMethod ? "method" : "function"; - assert(SW); BR.EmitBasicReport( ADC->getDecl(), Checker, - /*Name=*/"Write to autoreleasing out parameter inside autorelease pool", + /*Name=*/(llvm::Twine(WarningMsg) ++ " autoreleasing out parameter inside autorelease pool").str(), /*Category=*/"Memory", - (llvm::Twine("Write to autoreleasing out parameter inside ") + + (llvm::Twine(WarningMsg) + " autoreleasing out parameter inside " + "autorelease pool that may exit before " + Name + " returns; consider " "writing first to a strong local variable declared outside of the block") .str(), - PathDiagnosticLocation::createBegin(SW, BR.getSourceManager(), ADC), - SW->getSourceRange()); + Location, + Range); } void ObjCAutoreleaseWriteChecker::checkASTCodeBody(const Decl *D, AnalysisManager &AM, BugReporter &BR) const { + auto DoublePointerParamM = + parmVarDecl(hasType(hasCanonicalType(pointerType( + pointee(hasCanonicalType(objcObjectPointerType())) + .bind(ParamBind); + + auto ReferencedParamM = + declRefExpr(to(parmVarDecl(DoublePointerParamM))); + // Write into a binded object, e.g. *ParamBind = X. auto WritesIntoM = binaryOperator( hasLHS(unaryOperator( hasOperatorName("*"), hasUnaryOperand( - ignoringParenImpCasts( -declRefExpr(to(parmVarDecl(equalsBoundNode(ParamBind)) + ignoringParenImpCasts(ReferencedParamM)) )), hasOperatorName("=") ).bind(ProblematicWriteBind); + auto ArgumentCaptureM = hasAnyArgument( +ignoringParenImpCasts(ReferencedParamM)); + auto CapturedInParamM = stmt(anyOf( + callExpr(ArgumentCaptureM), + objcMessageExpr(ArgumentCaptureM))).bind(CapturedBind); + // WritesIntoM happens inside a block passed as an argument. - auto WritesInBlockM = hasAnyArgument(allOf( + auto WritesOrCapturesInBlockM = hasAnyArgument(allOf( hasType(hasCanonicalType(blockPointerType())), -
[PATCH] D46902: [analyzer] Make plist-html multi-file.
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM! with stable-filename option we could even avoid the regexp. Repository: rC Clang https://reviews.llvm.org/D46902 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46603: [Support] TimerGroup changes
george.karpenkov added a comment. I see four separate changes: s/.sys/mem one (can be committed without review), exposing printJSONValues and consequent adding of a lock, adding a constructor accepting a map, and fixing formatting in `printJSONValue`. All four look good to me, but probably should be reviewed separately. Comment at: lib/Support/Timer.cpp:385 + constexpr auto max_digits10 = std::numeric_limits::max_digits10; + OS << "\t\"time." << Name << '.' << R.Name << suffix + << "\": " << format("%.*e", max_digits10 - 1, Value); This change seems unrelated to the new constructor accepting map, right? Comment at: lib/Support/Timer.cpp:405 OS << delim; - printJSONValue(OS, R, ".sys", T.getMemUsed()); + printJSONValue(OS, R, ".mem", T.getMemUsed()); } That's independent from this change, right? Repository: rL LLVM https://reviews.llvm.org/D46603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46937: [Timers] TimerGroup::printJSONValue(): print doubles with no precision loss
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D46937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46603: [Support] TimerGroup changes
george.karpenkov added a comment. > Not sure yet whether i will land them right away, or wait for clang-tidy part. I think it's better to land, as otherwise you risk merge conflicts, and implicit conflicts (git reports no errors, but the resulting code is bad) can be very annoying. Repository: rL LLVM https://reviews.llvm.org/D46603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47007: [Sanitizer] CStringChecker fix for strlcpy when no bytes are copied to the dest buffer
george.karpenkov added a comment. Is it a fix for https://bugs.llvm.org/show_bug.cgi?id=37503 ? Repository: rC Clang https://reviews.llvm.org/D47007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47155: [analyzer] Reduce simplifySVal complexity threshold further.
george.karpenkov added a comment. @NoQ I'm really wary of magic numbers. - We should expose it through an analyzer-config flag. We already do so for the budget. - We should probably have both positive and negative tests. What scenarios _stop_ working after the threshold is decreased? [point 1 is required to be able to test that] - Finally, do we understand where the slowdown comes from? If it comes from attempted rearrangements due to https://reviews.llvm.org/rC329780 I think we should roll that back instead. Repository: rC Clang https://reviews.llvm.org/D47155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer][WIP] A checker for dangling string pointers in C++
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:29 + +class DanglingStringPointerChecker : public Checker { + CallDescription CStrFn; "string" is a bit ambiguous, if this checker is specifically for std::string should we change the name to reflect that? Repository: rC Clang https://reviews.llvm.org/D47135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47155: [analyzer] Reduce simplifySVal complexity threshold further.
george.karpenkov added a comment. Also we should make sure that all recursive transformations on expressions represented as DAGs should be memoized. Repository: rC Clang https://reviews.llvm.org/D47155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47303: [analyzer] NFC: Merge object construction related state traits into one.
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Definitely looks much cleaner, some nits inline. Can we protect against API misuse? Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:769 + /// that tracks objects under construction. + static ProgramStateRef finishObjectConstruction(ProgramStateRef State, + const Stmt *S, Should we somehow assert that those functions are called in the correct order? What will happen if e.g. `finishObjectConstruction` is called twice with the same statement? Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:101 -using InitializedTemporariesMap = -llvm::ImmutableMap, - const CXXTempObjectRegion *>; - -// Keeps track of whether CXXBindTemporaryExpr nodes have been evaluated. -// The StackFrameContext assures that nested calls due to inlined recursive -// functions do not interfere. -REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries, - InitializedTemporariesMap) - -using TemporaryMaterializationMap = -llvm::ImmutableMap, - const CXXTempObjectRegion *>; - -// Keeps track of temporaries that will need to be materialized later. -// The StackFrameContext assures that nested calls due to inlined recursive -// functions do not interfere. -REGISTER_TRAIT_WITH_PROGRAMSTATE(TemporaryMaterializations, - TemporaryMaterializationMap) - -using CXXNewAllocatorValuesMap = -llvm::ImmutableMap, - SVal>; - -// Keeps track of return values of various operator new() calls between -// evaluation of the inlined operator new(), through the constructor call, -// to the actual evaluation of the CXXNewExpr. -// TODO: Refactor the key for this trait into a LocationContext sub-class, -// which would be put on the stack of location contexts before operator new() -// is evaluated, and removed from the stack when the whole CXXNewExpr -// is fully evaluated. -// Probably do something similar to the previous trait as well. -REGISTER_TRAIT_WITH_PROGRAMSTATE(CXXNewAllocatorValues, - CXXNewAllocatorValuesMap) +// Keeps track of whether objects corresponding to the statement have been +// fully constructed. The comment seems incorrect, the map is not only answering the "whether" question, it also maps those pairs into --- (their regions where these objects are constructed into?) Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:377 + if (!State->contains(Key)) { +return State->set(Key, V); } nitpick: most C++ containers eliminate the need for two lookups by allowing the `get` method to return a reference. I'm not sure whether this can be done here though. Repository: rC Clang https://reviews.llvm.org/D47303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47402: [analyzer] Improve simplifySVal performance further.
george.karpenkov added a comment. @NoQ we should make sure the memory is not exploding and that we don't make the analyzer slower in other cases. Though we could commit this, and then let CI figure out potential regressions. Repository: rC Clang https://reviews.llvm.org/D47402 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] False positive refutation with Z3
george.karpenkov added a comment. Please resubmit with -U999 diff flag (or using arc) Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:362 +class FalsePositiveRefutationBRVisitor final +: public BugReporterVisitorImpl { Can we have the whole class inside the `.cpp` file? It's annoying to recompile half of the analyzer when an internal implementation detail changes Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:365 + // Flag first node as seen by the visitor. + bool FirstNodeVisited = true; + I'm really not convinced we need this boolean field Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:21 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h" +#include "llvm/ADT/ImmutableMap.h" #include "llvm/ADT/Optional.h" NB: diff should be resubmitted with -U999, as phabricator shows "context not available" Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1261 + + for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) { +SymbolRef Sym = I.getKey(); xbolva00 wrote: > for (auto I : CR)? @mikhail.ramalho yes please do fix this one https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] False positive refutation with Z3
george.karpenkov added inline comments. Comment at: test/Analysis/z3-crosscheck.c:2 +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config crosscheck-with-z3=true -verify %s +// REQUIRES: z3 +// expected-no-diagnostics Could we also have a second RUN line without Z3, and then use ifdef's to differentiate between the two in tests? https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] False positive refutation with Z3
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:3153 +if (getAnalyzerOptions().shouldCrosscheckWithZ3()) + R->addVisitor(llvm::make_unique()); + Unless I'm mistaken, visitors are run in the order they are being declared. It seems to me we would want to register our visitor first, as it does not make sense to run diagnostics-visitors if we have already deemed the path to be unfeasible. Probably `LikelyFalsePositiveSuppressionBRVisitor` should be even before that. Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2382 +// Reset the solver +RefutationMgr.reset(); + } (apologies in advance for nitpicking not on your code). Currently, this is written in a stateful way: we have a solver, at each iteration we add constraints, and at the end we reset it. To me it would make considerably more sense to write the code in a functional style: as we go, generate a vector of formulas, then once we reach the path end, create the solver object, check satisfiability, and then destroy the entire solver. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:923 + + void addRangeConstraints(ConstraintRangeTy PrevCR, ConstraintRangeTy SuccCR, + bool OnlyPurged) override; @mikhail.ramalho I know the first version was not yours, but could you write a doxygen comment explaining the semantics of all parameters? (I know we are guilty for not writing those often). I am also quite confused by the semantics of `OnlyPurged` variable. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1249 +bool Z3ConstraintManager::isModelFeasible() { + return Solver.check() != Z3_L_FALSE; +} solver can also return "unknown", what happens then? Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1259 +return; + ConstraintRangeTy CR = OnlyPurged ? PrevCR : SuccCR; + TBH I'm really confused here. Why does the method take two constraint ranges? What's `OnlyPurged`? From reading the code it seems it's set by seeing whether the program point only purges dead symbols, but at least a comment should be added as to why this affects behavior. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1264 + +if (OnlyPurged && SuccCR.contains(Sym)) + continue; I would guess that this is an optimization done in order not to re-add the constraints we already have. I think we should really not bother doing that, as Z3 will do a much better job here then we can. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1267 + +Z3Expr Constraints = Z3Expr::fromBoolean(false); + almost certainly a bug, we shouldn't default to unfeasible when the list of constraints is empty. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1278 + // a valid APSIntType. + if (RangeTy.isNull()) +continue; I'm really curious where does it happen and why. https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] False positive refutation with Z3
george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:496 std::unique_ptr ConstraintMgr; + std::unique_ptr RefutationMgr; See the comment below, I think we should not have this manager here. Just create one in the visitor constructor. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:563 + + ConstraintManager& getRefutationManager() { +return *RefutationMgr; This should be deleted as well (see the comment above) Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2382 +// Reset the solver +RefutationMgr.reset(); + } george.karpenkov wrote: > (apologies in advance for nitpicking not on your code). > > Currently, this is written in a stateful way: we have a solver, at each > iteration we add constraints, and at the end we reset it. To me it would make > considerably more sense to write the code in a functional style: as we go, > generate a vector of formulas, then once we reach the path end, create the > solver object, check satisfiability, and then destroy the entire solver. Elaborating more: we are already forced to have visitor object state, let's use that. `RefutationMgr` is essentially a wrapper around a Z3 solver object, let's just create one when visitor is constructed (directly or in unique_ptr) and then rely on the destructor to destroy it. Then no `reset` is necessary. Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:83 ConstraintMgr = (*CreateCMgr)(*this, SubEng); + AnalyzerOptions &Opts = SubEng->getAnalysisManager().getAnalyzerOptions(); + RefutationMgr = Opts.shouldCrosscheckWithZ3() This could be removed as well (see the comment above) Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:919 + void reset() override; + `reset` should be removed, see comments above. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1246 +void Z3ConstraintManager::reset() { Solver.reset(); } + I would remove this, see comments above. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1292 + Constraints = + Z3Expr::fromBinOp(Constraints, BO_LOr, SymRange, IsSignedTy); +} I'm very confused as to why are we doing disjunctions here. https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] False positive refutation with Z3
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1267 + +Z3Expr Constraints = Z3Expr::fromBoolean(false); + george.karpenkov wrote: > almost certainly a bug, we shouldn't default to unfeasible when the list of > constraints is empty. Ooops, sorry, now I see how the code is supposed to work. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1292 + Constraints = + Z3Expr::fromBinOp(Constraints, BO_LOr, SymRange, IsSignedTy); +} NoQ wrote: > george.karpenkov wrote: > > I'm very confused as to why are we doing disjunctions here. > I think this corresponds to RangeSet being a union of Ranges. Ah, thanks, right! Then my previous comment regarding `false` is wrong. https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38444: [compiler-rt] [cmake] Create dummy gtest target for stand-alone builds
george.karpenkov added a comment. > breaking stand-alone builds as a result That's a strong statement. Could you clarify? We have a lot of buildbots performing standalone builds, and they are still green. > and the likeliness of people mistakenly adding more unconditional dependencies That's a good point, however I'm not sure how your change would fix the problem. As far as I remember targets in compiler-rt had quite a few dependencies which required checking for whether it is a standalone build. In general, I'm not sure what this change would achieve, and the added complexity can always cause more bugs in the future. Repository: rL LLVM https://reviews.llvm.org/D38444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure
george.karpenkov updated this revision to Diff 117847. https://reviews.llvm.org/D38589 Files: utils/analyzer/SATestBuild.py Index: utils/analyzer/SATestBuild.py === --- utils/analyzer/SATestBuild.py +++ utils/analyzer/SATestBuild.py @@ -570,7 +570,11 @@ 0 - success if there are no crashes or analyzer failure. 1 - success if there are no difference in the number of reported bugs. 2 - success if all the bug reports are identical. + +:return success: Whether tests pass according to the Strictness +criteria. """ +TestsPassed = True TBegin = time.time() RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName) @@ -586,8 +590,6 @@ RefList.remove(RefLogDir) NewList.remove(os.path.join(NewDir, LogFolderName)) -if len(RefList) == 0 or len(NewList) == 0: -return False assert(len(RefList) == len(NewList)) # There might be more then one folder underneath - one per each scan-build @@ -624,15 +626,15 @@ (NumDiffs, DiffsPath,) if Strictness >= 2 and NumDiffs > 0: print "Error: Diffs found in strict mode (2)." -sys.exit(-1) +TestsPassed = False elif Strictness >= 1 and ReportsInRef != ReportsInNew: print "Error: The number of results are different in "\ "strict mode (1)." -sys.exit(-1) +TestsPassed = False print "Diagnostic comparison complete (time: %.2f)." % ( time.time() - TBegin) -return (NumDiffs > 0) +return TestsPassed def cleanupReferenceResults(SBOutputDir): @@ -686,6 +688,11 @@ def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0): +""" +Test a given project. +:return TestsPassed: Whether tests have passed according +to the :param Strictness: criteria. +""" print " \n\n--- Building project %s" % (ID,) TBegin = time.time() @@ -704,11 +711,13 @@ if IsReferenceBuild: cleanupReferenceResults(SBOutputDir) +TestsPassed = True else: -runCmpResults(Dir, Strictness) +TestsPassed = runCmpResults(Dir, Strictness) print "Completed tests for project %s (time: %.2f)." % \ (ID, (time.time() - TBegin)) +return TestsPassed def isCommentCSVLine(Entries): @@ -749,6 +758,7 @@ def testAll(IsReferenceBuild=False, UpdateSVN=False, Strictness=0): +TestsPassed = True with projectFileHandler() as PMapFile: validateProjectFile(PMapFile) @@ -760,7 +770,7 @@ # Test the projects. for (ProjName, ProjBuildMode) in iterateOverProjects(PMapFile): -testProject( +TestsPassed &= testProject( ProjName, int(ProjBuildMode), IsReferenceBuild, Strictness) # Re-add reference results to SVN. @@ -793,4 +803,6 @@ IsReference = True UpdateSVN = True -testAll(IsReference, UpdateSVN, Strictness) +TestsPassed = testAll(IsReference, UpdateSVN, Strictness) +if not TestsPassed: +sys.exit(-1) Index: utils/analyzer/SATestBuild.py === --- utils/analyzer/SATestBuild.py +++ utils/analyzer/SATestBuild.py @@ -570,7 +570,11 @@ 0 - success if there are no crashes or analyzer failure. 1 - success if there are no difference in the number of reported bugs. 2 - success if all the bug reports are identical. + +:return success: Whether tests pass according to the Strictness +criteria. """ +TestsPassed = True TBegin = time.time() RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName) @@ -586,8 +590,6 @@ RefList.remove(RefLogDir) NewList.remove(os.path.join(NewDir, LogFolderName)) -if len(RefList) == 0 or len(NewList) == 0: -return False assert(len(RefList) == len(NewList)) # There might be more then one folder underneath - one per each scan-build @@ -624,15 +626,15 @@ (NumDiffs, DiffsPath,) if Strictness >= 2 and NumDiffs > 0: print "Error: Diffs found in strict mode (2)." -sys.exit(-1) +TestsPassed = False elif Strictness >= 1 and ReportsInRef != ReportsInNew: print "Error: The number of results are different in "\ "strict mode (1)." -sys.exit(-1) +TestsPassed = False print "Diagnostic comparison complete (time: %.2f)." % ( time.time() - TBegin) -return (NumDiffs > 0) +return TestsPassed def cleanupReferenceResults(SBOutputDir): @@ -686,6 +688,11 @@ def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0): +""" +Test a given project. +:return TestsPassed: Whether tests have passed according +to the :param Strictness: c
[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure
george.karpenkov created this revision. Herald added subscribers: szepet, xazax.hun. This is especially important for updating reference results. https://reviews.llvm.org/D38589 Files: utils/analyzer/SATestBuild.py Index: utils/analyzer/SATestBuild.py === --- utils/analyzer/SATestBuild.py +++ utils/analyzer/SATestBuild.py @@ -46,6 +46,7 @@ import os import csv +from collections import namedtuple import sys import glob import math @@ -570,7 +571,11 @@ 0 - success if there are no crashes or analyzer failure. 1 - success if there are no difference in the number of reported bugs. 2 - success if all the bug reports are identical. + +:return success: Whether tests pass according to the Strictness +criteria. """ +TestsPassed = True TBegin = time.time() RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName) @@ -586,8 +591,6 @@ RefList.remove(RefLogDir) NewList.remove(os.path.join(NewDir, LogFolderName)) -if len(RefList) == 0 or len(NewList) == 0: -return False assert(len(RefList) == len(NewList)) # There might be more then one folder underneath - one per each scan-build @@ -624,15 +627,15 @@ (NumDiffs, DiffsPath,) if Strictness >= 2 and NumDiffs > 0: print "Error: Diffs found in strict mode (2)." -sys.exit(-1) +TestsPassed = False elif Strictness >= 1 and ReportsInRef != ReportsInNew: print "Error: The number of results are different in "\ "strict mode (1)." -sys.exit(-1) +TestsPassed = False print "Diagnostic comparison complete (time: %.2f)." % ( time.time() - TBegin) -return (NumDiffs > 0) +return TestsPassed def cleanupReferenceResults(SBOutputDir): @@ -686,6 +689,11 @@ def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0): +""" +Test a given project. +:return TestsPassed: Whether tests have passed according +to the :param Strictness: criteria. +""" print " \n\n--- Building project %s" % (ID,) TBegin = time.time() @@ -704,11 +712,13 @@ if IsReferenceBuild: cleanupReferenceResults(SBOutputDir) +TestsPassed = True else: -runCmpResults(Dir, Strictness) +TestsPassed = runCmpResults(Dir, Strictness) print "Completed tests for project %s (time: %.2f)." % \ (ID, (time.time() - TBegin)) +return TestsPassed def isCommentCSVLine(Entries): @@ -749,6 +759,7 @@ def testAll(IsReferenceBuild=False, UpdateSVN=False, Strictness=0): +TestsPassed = True with projectFileHandler() as PMapFile: validateProjectFile(PMapFile) @@ -760,7 +771,7 @@ # Test the projects. for (ProjName, ProjBuildMode) in iterateOverProjects(PMapFile): -testProject( +TestsPassed &= testProject( ProjName, int(ProjBuildMode), IsReferenceBuild, Strictness) # Re-add reference results to SVN. @@ -793,4 +804,6 @@ IsReference = True UpdateSVN = True -testAll(IsReference, UpdateSVN, Strictness) +TestsPassed = testAll(IsReference, UpdateSVN, Strictness) +if not TestsPassed: +sys.exit(-1) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure
This revision was automatically updated to reflect the committed changes. Closed by commit rL314992: [Analyzer Tests] Run static analyzer integration tests until the end, (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D38589?vs=117847&id=117848#toc Repository: rL LLVM https://reviews.llvm.org/D38589 Files: cfe/trunk/utils/analyzer/SATestBuild.py Index: cfe/trunk/utils/analyzer/SATestBuild.py === --- cfe/trunk/utils/analyzer/SATestBuild.py +++ cfe/trunk/utils/analyzer/SATestBuild.py @@ -570,7 +570,11 @@ 0 - success if there are no crashes or analyzer failure. 1 - success if there are no difference in the number of reported bugs. 2 - success if all the bug reports are identical. + +:return success: Whether tests pass according to the Strictness +criteria. """ +TestsPassed = True TBegin = time.time() RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName) @@ -586,8 +590,6 @@ RefList.remove(RefLogDir) NewList.remove(os.path.join(NewDir, LogFolderName)) -if len(RefList) == 0 or len(NewList) == 0: -return False assert(len(RefList) == len(NewList)) # There might be more then one folder underneath - one per each scan-build @@ -624,15 +626,15 @@ (NumDiffs, DiffsPath,) if Strictness >= 2 and NumDiffs > 0: print "Error: Diffs found in strict mode (2)." -sys.exit(-1) +TestsPassed = False elif Strictness >= 1 and ReportsInRef != ReportsInNew: print "Error: The number of results are different in "\ "strict mode (1)." -sys.exit(-1) +TestsPassed = False print "Diagnostic comparison complete (time: %.2f)." % ( time.time() - TBegin) -return (NumDiffs > 0) +return TestsPassed def cleanupReferenceResults(SBOutputDir): @@ -686,6 +688,11 @@ def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0): +""" +Test a given project. +:return TestsPassed: Whether tests have passed according +to the :param Strictness: criteria. +""" print " \n\n--- Building project %s" % (ID,) TBegin = time.time() @@ -704,11 +711,13 @@ if IsReferenceBuild: cleanupReferenceResults(SBOutputDir) +TestsPassed = True else: -runCmpResults(Dir, Strictness) +TestsPassed = runCmpResults(Dir, Strictness) print "Completed tests for project %s (time: %.2f)." % \ (ID, (time.time() - TBegin)) +return TestsPassed def isCommentCSVLine(Entries): @@ -749,6 +758,7 @@ def testAll(IsReferenceBuild=False, UpdateSVN=False, Strictness=0): +TestsPassed = True with projectFileHandler() as PMapFile: validateProjectFile(PMapFile) @@ -760,7 +770,7 @@ # Test the projects. for (ProjName, ProjBuildMode) in iterateOverProjects(PMapFile): -testProject( +TestsPassed &= testProject( ProjName, int(ProjBuildMode), IsReferenceBuild, Strictness) # Re-add reference results to SVN. @@ -793,4 +803,6 @@ IsReference = True UpdateSVN = True -testAll(IsReference, UpdateSVN, Strictness) +TestsPassed = testAll(IsReference, UpdateSVN, Strictness) +if not TestsPassed: +sys.exit(-1) Index: cfe/trunk/utils/analyzer/SATestBuild.py === --- cfe/trunk/utils/analyzer/SATestBuild.py +++ cfe/trunk/utils/analyzer/SATestBuild.py @@ -570,7 +570,11 @@ 0 - success if there are no crashes or analyzer failure. 1 - success if there are no difference in the number of reported bugs. 2 - success if all the bug reports are identical. + +:return success: Whether tests pass according to the Strictness +criteria. """ +TestsPassed = True TBegin = time.time() RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName) @@ -586,8 +590,6 @@ RefList.remove(RefLogDir) NewList.remove(os.path.join(NewDir, LogFolderName)) -if len(RefList) == 0 or len(NewList) == 0: -return False assert(len(RefList) == len(NewList)) # There might be more then one folder underneath - one per each scan-build @@ -624,15 +626,15 @@ (NumDiffs, DiffsPath,) if Strictness >= 2 and NumDiffs > 0: print "Error: Diffs found in strict mode (2)." -sys.exit(-1) +TestsPassed = False elif Strictness >= 1 and ReportsInRef != ReportsInNew: print "Error: The number of results are different in "\ "strict mode (1)." -sys.exit(-1) +TestsPassed = False print "Diagnostic comparison complete (time: %.2f)." % ( time.time() - TBegin) -
[PATCH] D38702: [Analyzer] Do not segfault on unexpected call_once implementation
george.karpenkov created this revision. Herald added subscribers: szepet, xazax.hun, javed.absar. Fixes https://bugs.llvm.org/show_bug.cgi?id=30565 @dcoughlin Any advice on how to handle different stdlib implementations? Can we conjure a separate symbol instead of relying on a particular struct layout? For now this implementation will simply not go inside a differently implemented `call_once`. https://reviews.llvm.org/D38702 Files: lib/Analysis/BodyFarm.cpp test/Analysis/call_once.cpp Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -231,3 +231,12 @@ int x = call_once(); clang_analyzer_eval(x == 5); // expected-warning{{TRUE}} } + +namespace std { +template +void call_once(d, e); +} +void g(); +void test_no_segfault_on_different_impl() { + std::call_once(g, false); // no-warning +} Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -362,6 +362,12 @@ /* GetNonReferenceType=*/true); CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); + if (FlagCXXDecl == nullptr) { +DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " + << "unknown std::call_once implementation." + << "Ignoring the call.\n"); +return nullptr; + } // Note: here we are assuming libc++ implementation of call_once, // which has a struct with a field `__state_`. Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -231,3 +231,12 @@ int x = call_once(); clang_analyzer_eval(x == 5); // expected-warning{{TRUE}} } + +namespace std { +template +void call_once(d, e); +} +void g(); +void test_no_segfault_on_different_impl() { + std::call_once(g, false); // no-warning +} Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -362,6 +362,12 @@ /* GetNonReferenceType=*/true); CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); + if (FlagCXXDecl == nullptr) { +DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " + << "unknown std::call_once implementation." + << "Ignoring the call.\n"); +return nullptr; + } // Note: here we are assuming libc++ implementation of call_once, // which has a struct with a field `__state_`. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38702: [Analyzer] Do not segfault on unexpected call_once implementation
george.karpenkov added a comment. Ooops, updated to https://bugs.llvm.org/show_bug.cgi?id=34869 https://reviews.llvm.org/D38702 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38702: [Analyzer] Do not segfault on unexpected call_once implementation
george.karpenkov updated this revision to Diff 118291. george.karpenkov added a comment. Review comments. https://reviews.llvm.org/D38702 Files: lib/Analysis/BodyFarm.cpp test/Analysis/call_once.cpp Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -231,3 +231,12 @@ int x = call_once(); clang_analyzer_eval(x == 5); // expected-warning{{TRUE}} } + +namespace std { +template +void call_once(d, e); +} +void g(); +void test_no_segfault_on_different_impl() { + std::call_once(g, false); // no-warning +} Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -327,6 +327,28 @@ const ParmVarDecl *Flag = D->getParamDecl(0); const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); + QualType FlagType = Flag->getType().getNonReferenceType(); + CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); + if (!FlagCXXDecl) { +DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " + << "unknown std::call_once implementation." + << "Ignoring the call.\n"); +return nullptr; + } + + // Note: here we are assuming libc++ implementation of call_once, + // which has a struct with a field `__state_`. + // Body farming might not work for other `call_once` implementations. + NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); + ValueDecl *FieldDecl; + if (FoundDecl) { +FieldDecl = dyn_cast(FoundDecl); + } else { +DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " + << "unable to synthesize call_once body, ignoring " + << "the call.\n"); +return nullptr; + } bool isLambdaCall = CallbackType->getAsCXXRecordDecl() && CallbackType->getAsCXXRecordDecl()->isLambda(); @@ -355,27 +377,11 @@ CallbackCall = create_call_once_funcptr_call(C, M, Callback, CallArgs); } - QualType FlagType = Flag->getType().getNonReferenceType(); DeclRefExpr *FlagDecl = M.makeDeclRefExpr(Flag, /* RefersToEnclosingVariableOrCapture=*/true, /* GetNonReferenceType=*/true); - CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); - - // Note: here we are assuming libc++ implementation of call_once, - // which has a struct with a field `__state_`. - // Body farming might not work for other `call_once` implementations. - NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); - ValueDecl *FieldDecl; - if (FoundDecl) { -FieldDecl = dyn_cast(FoundDecl); - } else { -DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " - << "unable to synthesize call_once body, ignoring " - << "the call.\n"); -return nullptr; - } MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FieldDecl); assert(Deref->isLValue()); Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -231,3 +231,12 @@ int x = call_once(); clang_analyzer_eval(x == 5); // expected-warning{{TRUE}} } + +namespace std { +template +void call_once(d, e); +} +void g(); +void test_no_segfault_on_different_impl() { + std::call_once(g, false); // no-warning +} Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -327,6 +327,28 @@ const ParmVarDecl *Flag = D->getParamDecl(0); const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); + QualType FlagType = Flag->getType().getNonReferenceType(); + CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); + if (!FlagCXXDecl) { +DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " + << "unknown std::call_once implementation." + << "Ignoring the call.\n"); +return nullptr; + } + + // Note: here we are assuming libc++ implementation of call_once, + // which has a struct with a field `__state_`. + // Body farming might not work for other `call_once` implementations. + NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); + ValueDecl *FieldDecl; + if (FoundDecl) { +FieldDecl = dyn_cast(FoundDecl); + } else { +DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " + << "unable to synthesize call_once body, ignoring " + << "the call.\n"); +return nullptr; + } bool isLambdaCall = CallbackType->getAsCXXRecordDecl() && Callback
[PATCH] D38702: [Analyzer] Do not segfault on unexpected call_once implementation
This revision was automatically updated to reflect the committed changes. Closed by commit rL315250: [Analyzer] Do not segfault on unexpected call_once implementation (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D38702?vs=118291&id=118294#toc Repository: rL LLVM https://reviews.llvm.org/D38702 Files: cfe/trunk/lib/Analysis/BodyFarm.cpp cfe/trunk/test/Analysis/call_once.cpp Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -327,6 +327,28 @@ const ParmVarDecl *Flag = D->getParamDecl(0); const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); + QualType FlagType = Flag->getType().getNonReferenceType(); + CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); + if (!FlagCXXDecl) { +DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " + << "unknown std::call_once implementation." + << "Ignoring the call.\n"); +return nullptr; + } + + // Note: here we are assuming libc++ implementation of call_once, + // which has a struct with a field `__state_`. + // Body farming might not work for other `call_once` implementations. + NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); + ValueDecl *FieldDecl; + if (FoundDecl) { +FieldDecl = dyn_cast(FoundDecl); + } else { +DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " + << "unable to synthesize call_once body, ignoring " + << "the call.\n"); +return nullptr; + } bool isLambdaCall = CallbackType->getAsCXXRecordDecl() && CallbackType->getAsCXXRecordDecl()->isLambda(); @@ -355,27 +377,11 @@ CallbackCall = create_call_once_funcptr_call(C, M, Callback, CallArgs); } - QualType FlagType = Flag->getType().getNonReferenceType(); DeclRefExpr *FlagDecl = M.makeDeclRefExpr(Flag, /* RefersToEnclosingVariableOrCapture=*/true, /* GetNonReferenceType=*/true); - CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); - - // Note: here we are assuming libc++ implementation of call_once, - // which has a struct with a field `__state_`. - // Body farming might not work for other `call_once` implementations. - NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); - ValueDecl *FieldDecl; - if (FoundDecl) { -FieldDecl = dyn_cast(FoundDecl); - } else { -DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " - << "unable to synthesize call_once body, ignoring " - << "the call.\n"); -return nullptr; - } MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FieldDecl); assert(Deref->isLValue()); Index: cfe/trunk/test/Analysis/call_once.cpp === --- cfe/trunk/test/Analysis/call_once.cpp +++ cfe/trunk/test/Analysis/call_once.cpp @@ -231,3 +231,12 @@ int x = call_once(); clang_analyzer_eval(x == 5); // expected-warning{{TRUE}} } + +namespace std { +template +void call_once(d, e); +} +void g(); +void test_no_segfault_on_different_impl() { + std::call_once(g, false); // no-warning +} Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -327,6 +327,28 @@ const ParmVarDecl *Flag = D->getParamDecl(0); const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); + QualType FlagType = Flag->getType().getNonReferenceType(); + CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); + if (!FlagCXXDecl) { +DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " + << "unknown std::call_once implementation." + << "Ignoring the call.\n"); +return nullptr; + } + + // Note: here we are assuming libc++ implementation of call_once, + // which has a struct with a field `__state_`. + // Body farming might not work for other `call_once` implementations. + NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); + ValueDecl *FieldDecl; + if (FoundDecl) { +FieldDecl = dyn_cast(FoundDecl); + } else { +DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " + << "unable to synthesize call_once body, ignoring " + << "the call.\n"); +return nullptr; + } bool isLambdaCall = CallbackType->getAsCXXRecordDecl() && CallbackType->getAsCXXRecordDecl()->isLambda(); @@ -355,27 +377,11 @@ CallbackCall = create_call_once_funcptr_call(C, M, Callba
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov created this revision. Herald added subscribers: szepet, xazax.hun, mgorny. https://reviews.llvm.org/D38764 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp test/Analysis/nonnil-string-constants.mm Index: test/Analysis/nonnil-string-constants.mm === --- /dev/null +++ test/Analysis/nonnil-string-constants.mm @@ -0,0 +1,80 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s + +// Nullability of const string-like globals. +void clang_analyzer_eval(bool); + +@class NSString; +typedef const struct __CFString *CFStringRef; + +// Global NSString* is non-null. +extern NSString *const StringConstGlobal; +void stringConstGlobal() { + clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}} +} + +// The logic does not apply to local variables though. +extern NSString *stringGetter(); +void stringConstLocal() { + NSString *const local = stringGetter(); + clang_analyzer_eval(local); // expected-warning{{UNKNOWN}} +} + +// Global const CFStringRef's are also assumed to be non-null. +extern const CFStringRef CFStringConstGlobal; +void cfStringCheckGlobal() { + clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}} +} + +// But only "const" ones. +extern CFStringRef CFStringNonConstGlobal; +void cfStringCheckMutableGlobal() { + clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}} +} + +// Const char* is also assumed to be non-null. +extern const char *const ConstCharStarConst; +void constCharStarCheckGlobal() { + clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}} +} + +// For char* we do not require a pointer itself to be immutable. +extern const char *CharStarConst; +void charStarCheckGlobal() { + clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}} +} + +// But the pointed data should be. +extern char *CharStar; +void charStartCheckMutableGlobal() { + clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}} +} + +// Type definitions should also work across typedefs, for pointers: +typedef const char *str; +extern str globalStr; +void charStarCheckTypedef() { + clang_analyzer_eval(globalStr); // expected-warning{{TRUE}} +} + +// And for types. +typedef NSString *const NStr; +extern NStr globalNSString; +void NSStringCheckTypedef() { + clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}} +} + +// Note that constness could be either inside +// the var declaration, or in a typedef. +typedef NSString *NStr2; +extern const NStr2 globalNSString2; +void NSStringCheckConstTypedef() { + clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}} +} + +// Nested typedefs should work as well. +typedef const CFStringRef str1; +typedef str1 str2; +extern str2 globalStr2; +void testNestedTypedefs() { + clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}} +} Index: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp === --- /dev/null +++ lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp @@ -0,0 +1,120 @@ + +// +// This checker ensures that const string globals are assumed to be non-null. +// +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" + +using namespace clang; +using namespace ento; + +namespace { + +class NonnilStringConstantsChecker : public Checker { + mutable IdentifierInfo *NSStringII; + mutable IdentifierInfo *CFStringRefII; + +public: + NonnilStringConstantsChecker(AnalyzerOptions &AO) {} + + void checkLocation(SVal l, bool isLoad, const Stmt *S, + CheckerContext &C) const; + +private: + /// Lazily initialize cache for required identifier informations. + void initIdentifierInfo(ASTContext &Ctx) const; + bool typeIsConstString(QualType type, bool isConstQualified) const; + bool isGlobalConstString(SVal val) const; +}; + +} // namespace + +void NonnilStringConstantsChecker::checkLocation(SVal location, bool isLoad, + const Stmt *S, + CheckerContext &C) const { + initIdentifierInfo(C.getASTContext()); + if (!isLoad) +return; + + ProgramStateRef State = C.getState(); + SVal V = UnknownVal(); + if (location.isValid()) { +V = State->getSVal(location.castAs()); + } + + if (isGlobalConstString(location)) { +Optional Constr = V.getAs(); + +if (Constr) { + + // Assume that the variable is non-null. + ProgramStateRef OutputState = State->assume(*Constr, true); + C.addT
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118493. https://reviews.llvm.org/D38764 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp test/Analysis/nonnil-string-constants.mm Index: test/Analysis/nonnil-string-constants.mm === --- /dev/null +++ test/Analysis/nonnil-string-constants.mm @@ -0,0 +1,80 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s + +// Nullability of const string-like globals. +void clang_analyzer_eval(bool); + +@class NSString; +typedef const struct __CFString *CFStringRef; + +// Global NSString* is non-null. +extern NSString *const StringConstGlobal; +void stringConstGlobal() { + clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}} +} + +// The logic does not apply to local variables though. +extern NSString *stringGetter(); +void stringConstLocal() { + NSString *const local = stringGetter(); + clang_analyzer_eval(local); // expected-warning{{UNKNOWN}} +} + +// Global const CFStringRef's are also assumed to be non-null. +extern const CFStringRef CFStringConstGlobal; +void cfStringCheckGlobal() { + clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}} +} + +// But only "const" ones. +extern CFStringRef CFStringNonConstGlobal; +void cfStringCheckMutableGlobal() { + clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}} +} + +// Const char* is also assumed to be non-null. +extern const char *const ConstCharStarConst; +void constCharStarCheckGlobal() { + clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}} +} + +// For char* we do not require a pointer itself to be immutable. +extern const char *CharStarConst; +void charStarCheckGlobal() { + clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}} +} + +// But the pointed data should be. +extern char *CharStar; +void charStartCheckMutableGlobal() { + clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}} +} + +// Type definitions should also work across typedefs, for pointers: +typedef const char *str; +extern str globalStr; +void charStarCheckTypedef() { + clang_analyzer_eval(globalStr); // expected-warning{{TRUE}} +} + +// And for types. +typedef NSString *const NStr; +extern NStr globalNSString; +void NSStringCheckTypedef() { + clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}} +} + +// Note that constness could be either inside +// the var declaration, or in a typedef. +typedef NSString *NStr2; +extern const NStr2 globalNSString2; +void NSStringCheckConstTypedef() { + clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}} +} + +// Nested typedefs should work as well. +typedef const CFStringRef str1; +typedef str1 str2; +extern str2 globalStr2; +void testNestedTypedefs() { + clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}} +} Index: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp === --- /dev/null +++ lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp @@ -0,0 +1,121 @@ + +// +// This checker ensures that const string globals are assumed to be non-null. +// +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" + +using namespace clang; +using namespace ento; + +namespace { + +class NonnilStringConstantsChecker : public Checker { + mutable IdentifierInfo *NSStringII = nullptr; + mutable IdentifierInfo *CFStringRefII = nullptr; + +public: + NonnilStringConstantsChecker(AnalyzerOptions &AO) {} + + void checkLocation(SVal l, bool isLoad, const Stmt *S, + CheckerContext &C) const; + +private: + /// Lazily initialize cache for required identifier informations. + void initIdentifierInfo(ASTContext &Ctx) const; + bool typeIsConstString(QualType type, bool isConstQualified) const; + bool isGlobalConstString(SVal val) const; +}; + +} // namespace + +void NonnilStringConstantsChecker::initIdentifierInfo(ASTContext &Ctx) const { + if (NSStringII) +return; + + NSStringII = &Ctx.Idents.get("NSString"); + CFStringRefII = &Ctx.Idents.get("CFStringRef"); +} + +void NonnilStringConstantsChecker::checkLocation(SVal location, bool isLoad, + const Stmt *S, + CheckerContext &C) const { + initIdentifierInfo(C.getASTContext()); + if (!isLoad) +return; + + ProgramStateRef State = C.getState(); + SVal V = UnknownVal(); + if (location.isValid()) { +V = State->getSVal(location.castAs()); + } + + if (isGlobalConstString(l
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov added a comment. Marking requests as "done". https://reviews.llvm.org/D38764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118521. george.karpenkov marked 12 inline comments as done. george.karpenkov added a comment. Adhering to comments. https://reviews.llvm.org/D38764 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp test/Analysis/nonnnull-string-constants.mm Index: test/Analysis/nonnnull-string-constants.mm === --- /dev/null +++ test/Analysis/nonnnull-string-constants.mm @@ -0,0 +1,91 @@ +// Nullability of const string-like globals. +// Relies on the checker defined in +// lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp. + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s + +void clang_analyzer_eval(bool); + +@class NSString; +typedef const struct __CFString *CFStringRef; + +// Global NSString* is non-null. +extern NSString *const StringConstGlobal; +void stringConstGlobal() { + clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}} +} + +// The logic does not apply to local variables though. +extern NSString *stringGetter(); +void stringConstLocal() { + NSString *const local = stringGetter(); + clang_analyzer_eval(local); // expected-warning{{UNKNOWN}} +} + +// Global const CFStringRef's are also assumed to be non-null. +extern const CFStringRef CFStringConstGlobal; +void cfStringCheckGlobal() { + clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}} +} + +// But only "const" ones. +extern CFStringRef CFStringNonConstGlobal; +void cfStringCheckMutableGlobal() { + clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}} +} + +// char* const is also assumed to be non-null. +extern const char *const ConstCharStarConst; +void constCharStarCheckGlobal() { + clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}} +} + +// Pointer value can be mutable. +extern char *const CharStarConst; +void charStarCheckGlobal() { + clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}} +} + +// But the pointer itself should be immutable. +extern char *CharStar; +void charStartCheckMutableGlobal() { + clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}} +} + +// Type definitions should also work across typedefs, for pointers: +typedef char *const str; +extern str globalStr; +void charStarCheckTypedef() { + clang_analyzer_eval(globalStr); // expected-warning{{TRUE}} +} + +// And for types. +typedef NSString *const NStr; +extern NStr globalNSString; +void NSStringCheckTypedef() { + clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}} +} + +// Note that constness could be either inside +// the var declaration, or in a typedef. +typedef NSString *NStr2; +extern const NStr2 globalNSString2; +void NSStringCheckConstTypedef() { + clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}} +} + +// Nested typedefs should work as well. +typedef const CFStringRef str1; +typedef str1 str2; +extern str2 globalStr2; +void testNestedTypedefs() { + clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}} +} + +// And for NSString *. +typedef NSString *const nstr1; +typedef nstr1 nstr2; +extern nstr2 nglobalStr2; +void testNestedTypedefsForNSString() { + clang_analyzer_eval(nglobalStr2); // expected-warning{{TRUE}} +} Index: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp === --- /dev/null +++ lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp @@ -0,0 +1,141 @@ +//==-- RetainCountChecker.cpp - Checks for leaks and other issues -*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Class definition for NonnullStringConstantsChecker. +// This checker adds an assumption that constant string-like globals are +// non-null, as otherwise they generally do not convey any useful information. +// The assumption is useful, as many framework use such global const strings, +// and the analyzer might not be able to infer the global value if the +// definition is in a separate translation unit. +// The following types (and their typedef aliases) are considered string-like: +// - `char* const` +// - `const CFStringRef` from CoreFoundation +// - `NSString* const` from Foundation +// +// Checker uses are defined in the test file: +// - test/Analysis/nonnull-string-constants.mm +// +//===--===// + +// +// This checker ensures that const string globals are assumed to be non-null. +// +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/S
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118522. george.karpenkov added a comment. Typo fix. https://reviews.llvm.org/D38764 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp test/Analysis/nonnull-string-constants.mm Index: test/Analysis/nonnull-string-constants.mm === --- /dev/null +++ test/Analysis/nonnull-string-constants.mm @@ -0,0 +1,91 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s + +// Nullability of const string-like globals. +// Relies on the checker defined in +// lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp. + +void clang_analyzer_eval(bool); + +@class NSString; +typedef const struct __CFString *CFStringRef; + +// Global NSString* is non-null. +extern NSString *const StringConstGlobal; +void stringConstGlobal() { + clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}} +} + +// The logic does not apply to local variables though. +extern NSString *stringGetter(); +void stringConstLocal() { + NSString *const local = stringGetter(); + clang_analyzer_eval(local); // expected-warning{{UNKNOWN}} +} + +// Global const CFStringRef's are also assumed to be non-null. +extern const CFStringRef CFStringConstGlobal; +void cfStringCheckGlobal() { + clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}} +} + +// But only "const" ones. +extern CFStringRef CFStringNonConstGlobal; +void cfStringCheckMutableGlobal() { + clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}} +} + +// char* const is also assumed to be non-null. +extern const char *const ConstCharStarConst; +void constCharStarCheckGlobal() { + clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}} +} + +// Pointer value can be mutable. +extern char *const CharStarConst; +void charStarCheckGlobal() { + clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}} +} + +// But the pointer itself should be immutable. +extern char *CharStar; +void charStartCheckMutableGlobal() { + clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}} +} + +// Type definitions should also work across typedefs, for pointers: +typedef char *const str; +extern str globalStr; +void charStarCheckTypedef() { + clang_analyzer_eval(globalStr); // expected-warning{{TRUE}} +} + +// And for types. +typedef NSString *const NStr; +extern NStr globalNSString; +void NSStringCheckTypedef() { + clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}} +} + +// Note that constness could be either inside +// the var declaration, or in a typedef. +typedef NSString *NStr2; +extern const NStr2 globalNSString2; +void NSStringCheckConstTypedef() { + clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}} +} + +// Nested typedefs should work as well. +typedef const CFStringRef str1; +typedef str1 str2; +extern str2 globalStr2; +void testNestedTypedefs() { + clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}} +} + +// And for NSString *. +typedef NSString *const nstr1; +typedef nstr1 nstr2; +extern nstr2 nglobalStr2; +void testNestedTypedefsForNSString() { + clang_analyzer_eval(nglobalStr2); // expected-warning{{TRUE}} +} Index: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp === --- /dev/null +++ lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp @@ -0,0 +1,141 @@ +//==-- RetainCountChecker.cpp - Checks for leaks and other issues -*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Class definition for NonnullStringConstantsChecker. +// This checker adds an assumption that constant string-like globals are +// non-null, as otherwise they generally do not convey any useful information. +// The assumption is useful, as many framework use such global const strings, +// and the analyzer might not be able to infer the global value if the +// definition is in a separate translation unit. +// The following types (and their typedef aliases) are considered string-like: +// - `char* const` +// - `const CFStringRef` from CoreFoundation +// - `NSString* const` from Foundation +// +// Checker uses are defined in the test file: +// - test/Analysis/nonnull-string-constants.mm +// +//===--===// + +// +// This checker ensures that const string globals are assumed to be non-null. +// +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/
[PATCH] D38810: [Analyzer] Support bodyfarming std::call_once for libstdc++
george.karpenkov created this revision. Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, aemerson. https://reviews.llvm.org/D38810 Files: lib/Analysis/BodyFarm.cpp test/Analysis/call_once.cpp Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -1,15 +1,24 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -w -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBSTDCPP -verify %s void clang_analyzer_eval(bool); // Faking std::std::call_once implementation. namespace std { + +#ifndef EMULATE_LIBSTDCPP typedef struct once_flag_s { unsigned long __state_ = 0; } once_flag; +#else +typedef struct once_flag_s { + int _M_once = 0; +} once_flag; +#endif template void call_once(once_flag &o, Callable func, Args... args) {}; + } // namespace std // Check with Lambdas. Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -108,7 +108,7 @@ /// Returns a *first* member field of a record declaration with a given name. /// \return an nullptr if no member with such a name exists. - NamedDecl *findMemberField(const CXXRecordDecl *RD, StringRef Name); + NamedDecl *findMemberField(const RecordDecl *RD, StringRef Name); private: ASTContext &C; @@ -234,7 +234,7 @@ OK_Ordinary); } -NamedDecl *ASTMaker::findMemberField(const CXXRecordDecl *RD, StringRef Name) { +NamedDecl *ASTMaker::findMemberField(const RecordDecl *RD, StringRef Name) { CXXBasePaths Paths( /* FindAmbiguities=*/false, @@ -328,28 +328,35 @@ const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); QualType FlagType = Flag->getType().getNonReferenceType(); - CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); - if (!FlagCXXDecl) { -DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " - << "unknown std::call_once implementation." - << "Ignoring the call.\n"); + auto *FlagRecordDecl = dyn_cast_or_null(FlagType->getAsTagDecl()); + + if (!FlagRecordDecl) { +DEBUG(llvm::dbgs() << "Flag field is not a record: " + << "unknown std::call_once implementation, " + << "ignoring the call.\n"); return nullptr; } - // Note: here we are assuming libc++ implementation of call_once, - // which has a struct with a field `__state_`. - // Body farming might not work for other `call_once` implementations. - NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); - ValueDecl *FieldDecl; - if (FoundDecl) { -FieldDecl = dyn_cast(FoundDecl); - } else { + // We initially assume libc++ implementation of call_once, + // where the once_flag struct has a field `__state_`. + NamedDecl *FoundDecl = M.findMemberField(FlagRecordDecl, "__state_"); + + // Otherwise, try libstdc++ implementation, with a field + // `_M_once` + if (!FoundDecl) { DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " - << "unable to synthesize call_once body, ignoring " - << "the call.\n"); + << "trying libstdc++ implementation\n"); +FoundDecl = M.findMemberField(FlagRecordDecl, "_M_once"); + } + + if (!FoundDecl) { +DEBUG(llvm::dbgs() << "No field _M_once found on std::once flag struct: " + << "unknown std::call_once implementation, " + << "ignoring the call"); return nullptr; } + ValueDecl *FieldDecl = dyn_cast(FoundDecl); bool isLambdaCall = CallbackType->getAsCXXRecordDecl() && CallbackType->getAsCXXRecordDecl()->isLambda(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov marked 9 inline comments as done. george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:22 +// Checker uses are defined in the test file: +// - test/Analysis/nonnull-string-constants.mm +// dcoughlin wrote: > We generally don't do this kind of cross-reference in comments since they > tend to get stale fast when things get moved around. There is no tool support > to keep them in sync. :( I guess I'm too spoiled by Javadoc. Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:53 +private: + /// Lazily initialize cache for required identifier informations. + void initIdentifierInfo(ASTContext &Ctx) const; dcoughlin wrote: > We usually put the oxygen comments on checkers on the implementation and not > the interface since they aren't API and people reading the code generally > look at the implementations. Can you move them to the implementations? OK. I assume there's no good universal solution here. https://reviews.llvm.org/D38764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118657. george.karpenkov marked 2 inline comments as done. https://reviews.llvm.org/D38764 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp test/Analysis/nonnull-string-constants.mm Index: test/Analysis/nonnull-string-constants.mm === --- /dev/null +++ test/Analysis/nonnull-string-constants.mm @@ -0,0 +1,90 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s + +// Nullability of const string-like globals, testing +// NonnullStringConstantsChecker. + +void clang_analyzer_eval(bool); + +@class NSString; +typedef const struct __CFString *CFStringRef; + +// Global NSString* is non-null. +extern NSString *const StringConstGlobal; +void stringConstGlobal() { + clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}} +} + +// The logic does not apply to local variables though. +extern NSString *stringGetter(); +void stringConstLocal() { + NSString *const local = stringGetter(); + clang_analyzer_eval(local); // expected-warning{{UNKNOWN}} +} + +// Global const CFStringRef's are also assumed to be non-null. +extern const CFStringRef CFStringConstGlobal; +void cfStringCheckGlobal() { + clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}} +} + +// But only "const" ones. +extern CFStringRef CFStringNonConstGlobal; +void cfStringCheckMutableGlobal() { + clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}} +} + +// char* const is also assumed to be non-null. +extern const char *const ConstCharStarConst; +void constCharStarCheckGlobal() { + clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}} +} + +// Pointer value can be mutable. +extern char *const CharStarConst; +void charStarCheckGlobal() { + clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}} +} + +// But the pointer itself should be immutable. +extern char *CharStar; +void charStartCheckMutableGlobal() { + clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}} +} + +// Type definitions should also work across typedefs, for pointers: +typedef char *const str; +extern str globalStr; +void charStarCheckTypedef() { + clang_analyzer_eval(globalStr); // expected-warning{{TRUE}} +} + +// And for types. +typedef NSString *const NStr; +extern NStr globalNSString; +void NSStringCheckTypedef() { + clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}} +} + +// Note that constness could be either inside +// the var declaration, or in a typedef. +typedef NSString *NStr2; +extern const NStr2 globalNSString2; +void NSStringCheckConstTypedef() { + clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}} +} + +// Nested typedefs should work as well. +typedef const CFStringRef str1; +typedef str1 str2; +extern str2 globalStr2; +void testNestedTypedefs() { + clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}} +} + +// And for NSString *. +typedef NSString *const nstr1; +typedef nstr1 nstr2; +extern nstr2 nglobalStr2; +void testNestedTypedefsForNSString() { + clang_analyzer_eval(nglobalStr2); // expected-warning{{TRUE}} +} Index: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp === --- /dev/null +++ lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp @@ -0,0 +1,134 @@ +//==- NonnullStringConstantsChecker.cpp ---*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// This checker adds an assumption that constant string-like globals are +// non-null, as otherwise they generally do not convey any useful information. +// The assumption is useful, as many framework use such global const strings, +// and the analyzer might not be able to infer the global value if the +// definition is in a separate translation unit. +// The following types (and their typedef aliases) are considered string-like: +// - `char* const` +// - `const CFStringRef` from CoreFoundation +// - `NSString* const` from Foundation +// +//===--===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" + +using namespace clang; +using namespace ento; + +namespace { + +class NonnullStringConstantsChecker : public Checker { + mutable IdentifierInfo *N
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
This revision was automatically updated to reflect the committed changes. Closed by commit rL315488: [Analyzer] Assume that string-like const globals are non-nil. (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D38764?vs=118657&id=118660#toc Repository: rL LLVM https://reviews.llvm.org/D38764 Files: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp cfe/trunk/test/Analysis/nonnull-string-constants.mm Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt === --- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -57,6 +57,7 @@ NSErrorChecker.cpp NoReturnFunctionChecker.cpp NonNullParamChecker.cpp + NonnullStringConstantsChecker.cpp NullabilityChecker.cpp NumberObjectConversionChecker.cpp ObjCAtSyncChecker.cpp Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp @@ -0,0 +1,134 @@ +//==- NonnullStringConstantsChecker.cpp ---*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// This checker adds an assumption that constant string-like globals are +// non-null, as otherwise they generally do not convey any useful information. +// The assumption is useful, as many framework use such global const strings, +// and the analyzer might not be able to infer the global value if the +// definition is in a separate translation unit. +// The following types (and their typedef aliases) are considered string-like: +// - `char* const` +// - `const CFStringRef` from CoreFoundation +// - `NSString* const` from Foundation +// +//===--===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" + +using namespace clang; +using namespace ento; + +namespace { + +class NonnullStringConstantsChecker : public Checker { + mutable IdentifierInfo *NSStringII = nullptr; + mutable IdentifierInfo *CFStringRefII = nullptr; + +public: + NonnullStringConstantsChecker() {} + + void checkLocation(SVal l, bool isLoad, const Stmt *S, + CheckerContext &C) const; + +private: + void initIdentifierInfo(ASTContext &Ctx) const; + + bool isGlobalConstString(SVal V) const; + + bool isStringlike(QualType Ty) const; +}; + +} // namespace + +/// Lazily initialize cache for required identifier informations. +void NonnullStringConstantsChecker::initIdentifierInfo(ASTContext &Ctx) const { + if (NSStringII) +return; + + NSStringII = &Ctx.Idents.get("NSString"); + CFStringRefII = &Ctx.Idents.get("CFStringRef"); +} + +/// Add an assumption that const string-like globals are non-null. +void NonnullStringConstantsChecker::checkLocation(SVal location, bool isLoad, + const Stmt *S, + CheckerContext &C) const { + initIdentifierInfo(C.getASTContext()); + if (!isLoad || !location.isValid()) +return; + + ProgramStateRef State = C.getState(); + SVal V = State->getSVal(location.castAs()); + + if (isGlobalConstString(location)) { +Optional Constr = V.getAs(); + +if (Constr) { + + // Assume that the variable is non-null. + ProgramStateRef OutputState = State->assume(*Constr, true); + C.addTransition(OutputState); +} + } +} + +/// \param V loaded lvalue. +/// \return whether {@code val} is a string-like const global. +bool NonnullStringConstantsChecker::isGlobalConstString(SVal V) const { + Optional RegionVal = V.getAs(); + if (!RegionVal) +return false; + auto *Region = dyn_cast(RegionVal->getAsRegion()); + if (!Region) +return false; + const VarDecl *Decl = Region->getDecl(); + + if (!Decl->hasGlobalStorage()) +return false; + + QualType Ty = Decl->getType(); + bool HasConst = Ty.isConstQualified(); + if (isStringlike(Ty) && HasConst) +return true; + + // Look through the typedefs. + while (auto *T = dyn_cast(Ty)) { +Ty = T->getDecl()->getUnderlyingType(); + +// It is sufficient for any intermediate typ
[PATCH] D38810: [Analyzer] Support bodyfarming std::call_once for libstdc++
This revision was automatically updated to reflect the committed changes. Closed by commit rL315508: [Analyzer] Support bodyfarming libstdc++ implementation of std::call_once. (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D38810?vs=118644&id=118686#toc Repository: rL LLVM https://reviews.llvm.org/D38810 Files: cfe/trunk/lib/Analysis/BodyFarm.cpp cfe/trunk/test/Analysis/call_once.cpp Index: cfe/trunk/test/Analysis/call_once.cpp === --- cfe/trunk/test/Analysis/call_once.cpp +++ cfe/trunk/test/Analysis/call_once.cpp @@ -1,15 +1,24 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -w -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBSTDCPP -verify %s void clang_analyzer_eval(bool); // Faking std::std::call_once implementation. namespace std { + +#ifndef EMULATE_LIBSTDCPP typedef struct once_flag_s { unsigned long __state_ = 0; } once_flag; +#else +typedef struct once_flag_s { + int _M_once = 0; +} once_flag; +#endif template void call_once(once_flag &o, Callable func, Args... args) {}; + } // namespace std // Check with Lambdas. Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -108,7 +108,7 @@ /// Returns a *first* member field of a record declaration with a given name. /// \return an nullptr if no member with such a name exists. - NamedDecl *findMemberField(const CXXRecordDecl *RD, StringRef Name); + ValueDecl *findMemberField(const RecordDecl *RD, StringRef Name); private: ASTContext &C; @@ -234,7 +234,7 @@ OK_Ordinary); } -NamedDecl *ASTMaker::findMemberField(const CXXRecordDecl *RD, StringRef Name) { +ValueDecl *ASTMaker::findMemberField(const RecordDecl *RD, StringRef Name) { CXXBasePaths Paths( /* FindAmbiguities=*/false, @@ -246,7 +246,7 @@ DeclContextLookupResult Decls = RD->lookup(DeclName); for (NamedDecl *FoundDecl : Decls) if (!FoundDecl->getDeclContext()->isFunctionOrMethod()) - return FoundDecl; + return cast(FoundDecl); return nullptr; } @@ -328,25 +328,31 @@ const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); QualType FlagType = Flag->getType().getNonReferenceType(); - CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl(); - if (!FlagCXXDecl) { -DEBUG(llvm::dbgs() << "Flag field is not a CXX record: " - << "unknown std::call_once implementation." - << "Ignoring the call.\n"); + auto *FlagRecordDecl = dyn_cast_or_null(FlagType->getAsTagDecl()); + + if (!FlagRecordDecl) { +DEBUG(llvm::dbgs() << "Flag field is not a record: " + << "unknown std::call_once implementation, " + << "ignoring the call.\n"); return nullptr; } - // Note: here we are assuming libc++ implementation of call_once, - // which has a struct with a field `__state_`. - // Body farming might not work for other `call_once` implementations. - NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_"); - ValueDecl *FieldDecl; - if (FoundDecl) { -FieldDecl = dyn_cast(FoundDecl); - } else { + // We initially assume libc++ implementation of call_once, + // where the once_flag struct has a field `__state_`. + ValueDecl *FlagFieldDecl = M.findMemberField(FlagRecordDecl, "__state_"); + + // Otherwise, try libstdc++ implementation, with a field + // `_M_once` + if (!FlagFieldDecl) { DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, " - << "unable to synthesize call_once body, ignoring " - << "the call.\n"); + << "assuming libstdc++ implementation\n"); +FlagFieldDecl = M.findMemberField(FlagRecordDecl, "_M_once"); + } + + if (!FlagFieldDecl) { +DEBUG(llvm::dbgs() << "No field _M_once found on std::once flag struct: " + << "unknown std::call_once implementation, " + << "ignoring the call"); return nullptr; } @@ -383,7 +389,7 @@ /* GetNonReferenceType=*/true); - MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FieldDecl); + MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FlagFieldDecl); assert(Deref->isLValue()); QualType DerefType = Deref->getType(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38444: [compiler-rt] [cmake] Create dummy gtest target for stand-alone builds
george.karpenkov added a comment. @mgorny I've replied via email, but the message didn't seem to appear here. From my (maybe limited) understanding, running tests on standalone compiler-rt builds was never something which was supported, as that required a fresh compiler. I've just tried running them, and for me even `check-*` targets don't exist. How do you create the standalone build? I've checked out `compiler-rt` separately, and ran cmake -GNinja -DLLVM_CONFIG=/Users/george/code/llvm/release-build/bin/llvm-config ../. but the resulting build directory does not have any `check-*` targets. In the refactoring I did try to make the dependence conditional: all compilation goes through `sanitizer_test_compile`, but looking into the `sanitizer_test_compile` macro there is an obvious bug. Would it be better to fix that instead? Repository: rL LLVM https://reviews.llvm.org/D38444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38839: [compiler-rt] [cmake] [interception] Remove duplicate gtest from test COMPILE_DEPS
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Yep, thanks! This was probably the root cause of the failure. https://reviews.llvm.org/D38839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38838: [compiler-rt] [cmake] Fix skipping DEPS (typo) in sanitizer_test_compile()
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Yep, thanks! Good to go provided tests run. Looks like we haven't spotted it due to other bug you have fixed cancelling this one out. https://reviews.llvm.org/D38838 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38840: [compiler-rt] [cmake] [asan] Reuse generate_asan_tests for dynamic tests
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. @mgorny so this one actually changes semantics. The previous one reused generated object files, and with the change it will start recompiling all the tests again. (and actually being able to export object files has added quite a bit of complexity to all these compiler-rt macros). The current code also looks weird, because it depends on `ASAN_INST_TEST_OBJECTS`, but will actually recompile and export them. I think a better fix would be to change `add_compiler_rt_test` to accept two kinds of dependencies (probably again `DEPS` and `COMPILE_DEPS`? I know the name is not very descriptive, but I've kept it from the previous version). It already checks that variable to conditionally add `clang` to dependencies. https://reviews.llvm.org/D38840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38840: [compiler-rt] [cmake] [asan] Remove unnecessary gtest dep from dynamic tests
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM, provided tests pass both in standalone and "normal" modes. https://reviews.llvm.org/D38840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38904: Allow building libFuzzer in two-stage compiler-rt build
george.karpenkov created this revision. Herald added subscribers: mgorny, dberris. When `LLVM_BUILD_EXTERNAL_COMPILER_RT` option is set to true, all of projects in compiler-rt are built with a freshly-built compiler using a recursive CMake invocation. (e.g. that's how compiler-rt is used in Swift) Just now I have noticed that `libFuzzer` binaries were missing in such a case, and `ninja fuzzer` returned "no such target", while `ninja asan` worked just fine. To my surprise, the list of allowed targets was actually hardcoded in Clang! While the current setup is clearly suboptimal, for the lack of a better fix I'm just adding `fuzzer` to a list of compiler-rt targets. https://reviews.llvm.org/D38904 Files: runtime/CMakeLists.txt Index: runtime/CMakeLists.txt === --- runtime/CMakeLists.txt +++ runtime/CMakeLists.txt @@ -109,7 +109,7 @@ USES_TERMINAL) # Add top-level targets that build specific compiler-rt runtimes. - set(COMPILER_RT_RUNTIMES asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) + set(COMPILER_RT_RUNTIMES fuzzer asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) foreach(runtime ${COMPILER_RT_RUNTIMES}) get_ext_project_build_command(build_runtime_cmd ${runtime}) add_custom_target(${runtime} Index: runtime/CMakeLists.txt === --- runtime/CMakeLists.txt +++ runtime/CMakeLists.txt @@ -109,7 +109,7 @@ USES_TERMINAL) # Add top-level targets that build specific compiler-rt runtimes. - set(COMPILER_RT_RUNTIMES asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) + set(COMPILER_RT_RUNTIMES fuzzer asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) foreach(runtime ${COMPILER_RT_RUNTIMES}) get_ext_project_build_command(build_runtime_cmd ${runtime}) add_custom_target(${runtime} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38904: Allow building libFuzzer in two-stage compiler-rt build
This revision was automatically updated to reflect the committed changes. Closed by commit rL315771: Allow building libFuzzer in two-stage compiler-rt build (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D38904?vs=118977&id=118979#toc Repository: rL LLVM https://reviews.llvm.org/D38904 Files: cfe/trunk/runtime/CMakeLists.txt Index: cfe/trunk/runtime/CMakeLists.txt === --- cfe/trunk/runtime/CMakeLists.txt +++ cfe/trunk/runtime/CMakeLists.txt @@ -109,7 +109,7 @@ USES_TERMINAL) # Add top-level targets that build specific compiler-rt runtimes. - set(COMPILER_RT_RUNTIMES asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) + set(COMPILER_RT_RUNTIMES fuzzer asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) foreach(runtime ${COMPILER_RT_RUNTIMES}) get_ext_project_build_command(build_runtime_cmd ${runtime}) add_custom_target(${runtime} Index: cfe/trunk/runtime/CMakeLists.txt === --- cfe/trunk/runtime/CMakeLists.txt +++ cfe/trunk/runtime/CMakeLists.txt @@ -109,7 +109,7 @@ USES_TERMINAL) # Add top-level targets that build specific compiler-rt runtimes. - set(COMPILER_RT_RUNTIMES asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) + set(COMPILER_RT_RUNTIMES fuzzer asan builtins dfsan lsan msan profile tsan ubsan ubsan-minimal) foreach(runtime ${COMPILER_RT_RUNTIMES}) get_ext_project_build_command(build_runtime_cmd ${runtime}) add_custom_target(${runtime} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38986: [Analyzer] Better unreachable message in enumeration
george.karpenkov created this revision. Herald added subscribers: szepet, xazax.hun. @dcoughlin I'm curious whether you'd like such a change: in general, I think it is much better when the assert failure tells the developer _what_ value is failing, rather than saying "oops we are dead". I would personally even go further with providing a templated function from dump-able object to a string, which could be used to avoid such a cumbersome construction [which I have seen in a few places in Clang] https://reviews.llvm.org/D38986 Files: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp === --- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp +++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp @@ -64,8 +64,12 @@ } switch (Cond.getSubKind()) { - default: -llvm_unreachable("'Assume' not implemented for this NonLoc"); + default: { +SmallString<64> buf; +llvm::raw_svector_ostream out(buf); +out << "'Assume' not implemented for this NonLoc: " << Cond << "\n"; +llvm_unreachable(out.str().data()); + } case nonloc::SymbolValKind: { nonloc::SymbolVal SV = Cond.castAs(); Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp === --- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp +++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp @@ -64,8 +64,12 @@ } switch (Cond.getSubKind()) { - default: -llvm_unreachable("'Assume' not implemented for this NonLoc"); + default: { +SmallString<64> buf; +llvm::raw_svector_ostream out(buf); +out << "'Assume' not implemented for this NonLoc: " << Cond << "\n"; +llvm_unreachable(out.str().data()); + } case nonloc::SymbolValKind: { nonloc::SymbolVal SV = Cond.castAs(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash
george.karpenkov created this revision. Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, aemerson. Remove an option to use a reference type (on by default!) since a non-reference type is always needed for creating expressions, functions with multiple boolean parameters are very hard to use, and in general it was just a booby trap for further crashes. Furthermore, generalize `call_once` test case to fix some of the crashes mentioned https://bugs.llvm.org/show_bug.cgi?id=34869 https://reviews.llvm.org/D39015 Files: lib/Analysis/BodyFarm.cpp test/Analysis/call_once.cpp Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -17,7 +17,7 @@ #endif template -void call_once(once_flag &o, Callable func, Args... args) {}; +void call_once(once_flag &o, Callable&& func, Args&&... args) {}; } // namespace std Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -63,8 +63,7 @@ /// Create a new DeclRefExpr for the referenced variable. DeclRefExpr *makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonReferenceType = false); + bool RefersToEnclosingVariableOrCapture = false); /// Create a new UnaryOperator representing a dereference. UnaryOperator *makeDereference(const Expr *Arg, QualType Ty); @@ -82,8 +81,7 @@ /// DeclRefExpr in the process. ImplicitCastExpr * makeLvalueToRvalue(const VarDecl *Decl, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonReferenceType = false); + bool RefersToEnclosingVariableOrCapture = false); /// Create an implicit cast of the given type. ImplicitCastExpr *makeImplicitCast(const Expr *Arg, QualType Ty, @@ -138,12 +136,10 @@ return new (C) CompoundStmt(C, Stmts, SourceLocation(), SourceLocation()); } -DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture, - bool GetNonReferenceType) { - auto Type = D->getType(); - if (GetNonReferenceType) -Type = Type.getNonReferenceType(); +DeclRefExpr *ASTMaker::makeDeclRefExpr( +const VarDecl *D, +bool RefersToEnclosingVariableOrCapture) { + QualType Type = D->getType().getNonReferenceType(); DeclRefExpr *DR = DeclRefExpr::Create( C, NestedNameSpecifierLoc(), SourceLocation(), const_cast(D), @@ -162,14 +158,10 @@ ImplicitCastExpr * ASTMaker::makeLvalueToRvalue(const VarDecl *Arg, - bool RefersToEnclosingVariableOrCapture, - bool GetNonReferenceType) { - auto Type = Arg->getType(); - if (GetNonReferenceType) -Type = Type.getNonReferenceType(); + bool RefersToEnclosingVariableOrCapture) { + QualType Type = Arg->getType().getNonReferenceType(); return makeLvalueToRvalue(makeDeclRefExpr(Arg, -RefersToEnclosingVariableOrCapture, -GetNonReferenceType), +RefersToEnclosingVariableOrCapture), Type); } @@ -263,7 +255,7 @@ return new (C) CallExpr( /*ASTContext=*/C, - /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback), + /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback), // TODO: this is a crashling line w/ reference type /*args=*/CallArgs, /*QualType=*/C.VoidTy, /*ExprValueType=*/VK_RValue, @@ -365,12 +357,13 @@ // Lambda requires callback itself inserted as a first parameter. CallArgs.push_back( M.makeDeclRefExpr(Callback, - /* RefersToEnclosingVariableOrCapture= */ true, - /* GetNonReferenceType= */ true)); + /* RefersToEnclosingVariableOrCapture= */ true)); // All arguments past first two ones are passed to the callback. for (unsigned int i = 2; i < D->getNumParams(); i++) -CallArgs.push_back(M.makeLvalueToRvalue(D->getParamDecl(i))); +CallArgs.push_back( +M.makeLvalueToRvalue(D->getParamDecl(i), + /* RefersToEnclosingVariableOrCapture= */ false)); CallExpr *CallbackCall; if (isLambdaCall) { @@ -385,8 +378,7 @@ DeclRefExpr *FlagDecl = M.makeDeclRefExpr(Flag, -/* RefersToEnclosingVariableOrCapture=*/true, -/* GetNonReferenceType=*/true); +/* RefersToEnclosingVariableOrCapture=*/true); MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FlagFieldDecl);
[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash
george.karpenkov updated this revision to Diff 119360. https://reviews.llvm.org/D39015 Files: lib/Analysis/BodyFarm.cpp test/Analysis/call_once.cpp Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -17,7 +17,7 @@ #endif template -void call_once(once_flag &o, Callable func, Args... args) {}; +void call_once(once_flag &o, Callable&& func, Args&&... args) {}; } // namespace std Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -63,8 +63,7 @@ /// Create a new DeclRefExpr for the referenced variable. DeclRefExpr *makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonReferenceType = false); + bool RefersToEnclosingVariableOrCapture = false); /// Create a new UnaryOperator representing a dereference. UnaryOperator *makeDereference(const Expr *Arg, QualType Ty); @@ -82,8 +81,7 @@ /// DeclRefExpr in the process. ImplicitCastExpr * makeLvalueToRvalue(const VarDecl *Decl, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonReferenceType = false); + bool RefersToEnclosingVariableOrCapture = false); /// Create an implicit cast of the given type. ImplicitCastExpr *makeImplicitCast(const Expr *Arg, QualType Ty, @@ -138,12 +136,10 @@ return new (C) CompoundStmt(C, Stmts, SourceLocation(), SourceLocation()); } -DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture, - bool GetNonReferenceType) { - auto Type = D->getType(); - if (GetNonReferenceType) -Type = Type.getNonReferenceType(); +DeclRefExpr *ASTMaker::makeDeclRefExpr( +const VarDecl *D, +bool RefersToEnclosingVariableOrCapture) { + QualType Type = D->getType().getNonReferenceType(); DeclRefExpr *DR = DeclRefExpr::Create( C, NestedNameSpecifierLoc(), SourceLocation(), const_cast(D), @@ -162,14 +158,10 @@ ImplicitCastExpr * ASTMaker::makeLvalueToRvalue(const VarDecl *Arg, - bool RefersToEnclosingVariableOrCapture, - bool GetNonReferenceType) { - auto Type = Arg->getType(); - if (GetNonReferenceType) -Type = Type.getNonReferenceType(); + bool RefersToEnclosingVariableOrCapture) { + QualType Type = Arg->getType().getNonReferenceType(); return makeLvalueToRvalue(makeDeclRefExpr(Arg, -RefersToEnclosingVariableOrCapture, -GetNonReferenceType), + RefersToEnclosingVariableOrCapture), Type); } @@ -365,12 +357,13 @@ // Lambda requires callback itself inserted as a first parameter. CallArgs.push_back( M.makeDeclRefExpr(Callback, - /* RefersToEnclosingVariableOrCapture= */ true, - /* GetNonReferenceType= */ true)); + /* RefersToEnclosingVariableOrCapture= */ true)); // All arguments past first two ones are passed to the callback. for (unsigned int i = 2; i < D->getNumParams(); i++) -CallArgs.push_back(M.makeLvalueToRvalue(D->getParamDecl(i))); +CallArgs.push_back( +M.makeLvalueToRvalue(D->getParamDecl(i), + /* RefersToEnclosingVariableOrCapture= */ false)); CallExpr *CallbackCall; if (isLambdaCall) { @@ -385,8 +378,7 @@ DeclRefExpr *FlagDecl = M.makeDeclRefExpr(Flag, -/* RefersToEnclosingVariableOrCapture=*/true, -/* GetNonReferenceType=*/true); +/* RefersToEnclosingVariableOrCapture=*/true); MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FlagFieldDecl); Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -17,7 +17,7 @@ #endif template -void call_once(once_flag &o, Callable func, Args... args) {}; +void call_once(once_flag &o, Callable&& func, Args&&... args) {}; } // namespace std Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -63,8 +63,7 @@ /// Create a new DeclRefExpr for the referenced variable. DeclRefExpr *makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonReferenceType = fals
[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash
This revision was automatically updated to reflect the committed changes. Closed by commit rL316041: [Analyzer] Always use non-reference types when creating expressions in BodyFarm. (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D39015?vs=119360&id=119392#toc Repository: rL LLVM https://reviews.llvm.org/D39015 Files: cfe/trunk/lib/Analysis/BodyFarm.cpp cfe/trunk/test/Analysis/call_once.cpp Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -63,8 +63,7 @@ /// Create a new DeclRefExpr for the referenced variable. DeclRefExpr *makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonReferenceType = false); + bool RefersToEnclosingVariableOrCapture = false); /// Create a new UnaryOperator representing a dereference. UnaryOperator *makeDereference(const Expr *Arg, QualType Ty); @@ -82,8 +81,7 @@ /// DeclRefExpr in the process. ImplicitCastExpr * makeLvalueToRvalue(const VarDecl *Decl, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonReferenceType = false); + bool RefersToEnclosingVariableOrCapture = false); /// Create an implicit cast of the given type. ImplicitCastExpr *makeImplicitCast(const Expr *Arg, QualType Ty, @@ -138,12 +136,10 @@ return new (C) CompoundStmt(C, Stmts, SourceLocation(), SourceLocation()); } -DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture, - bool GetNonReferenceType) { - auto Type = D->getType(); - if (GetNonReferenceType) -Type = Type.getNonReferenceType(); +DeclRefExpr *ASTMaker::makeDeclRefExpr( +const VarDecl *D, +bool RefersToEnclosingVariableOrCapture) { + QualType Type = D->getType().getNonReferenceType(); DeclRefExpr *DR = DeclRefExpr::Create( C, NestedNameSpecifierLoc(), SourceLocation(), const_cast(D), @@ -162,14 +158,10 @@ ImplicitCastExpr * ASTMaker::makeLvalueToRvalue(const VarDecl *Arg, - bool RefersToEnclosingVariableOrCapture, - bool GetNonReferenceType) { - auto Type = Arg->getType(); - if (GetNonReferenceType) -Type = Type.getNonReferenceType(); + bool RefersToEnclosingVariableOrCapture) { + QualType Type = Arg->getType().getNonReferenceType(); return makeLvalueToRvalue(makeDeclRefExpr(Arg, -RefersToEnclosingVariableOrCapture, -GetNonReferenceType), + RefersToEnclosingVariableOrCapture), Type); } @@ -365,12 +357,13 @@ // Lambda requires callback itself inserted as a first parameter. CallArgs.push_back( M.makeDeclRefExpr(Callback, - /* RefersToEnclosingVariableOrCapture= */ true, - /* GetNonReferenceType= */ true)); + /* RefersToEnclosingVariableOrCapture= */ true)); // All arguments past first two ones are passed to the callback. for (unsigned int i = 2; i < D->getNumParams(); i++) -CallArgs.push_back(M.makeLvalueToRvalue(D->getParamDecl(i))); +CallArgs.push_back( +M.makeLvalueToRvalue(D->getParamDecl(i), + /* RefersToEnclosingVariableOrCapture= */ false)); CallExpr *CallbackCall; if (isLambdaCall) { @@ -385,8 +378,7 @@ DeclRefExpr *FlagDecl = M.makeDeclRefExpr(Flag, -/* RefersToEnclosingVariableOrCapture=*/true, -/* GetNonReferenceType=*/true); +/* RefersToEnclosingVariableOrCapture=*/true); MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FlagFieldDecl); Index: cfe/trunk/test/Analysis/call_once.cpp === --- cfe/trunk/test/Analysis/call_once.cpp +++ cfe/trunk/test/Analysis/call_once.cpp @@ -17,7 +17,7 @@ #endif template -void call_once(once_flag &o, Callable func, Args... args) {}; +void call_once(once_flag &o, Callable&& func, Args&&... args) {}; } // namespace std Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -63,8 +63,7 @@ /// Create a new DeclRefExpr for the referenced variable. DeclRefExpr *makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture = false, - bool GetNonRef
[PATCH] D39031: [Analyzer] Correctly handle parameters passed by reference when bodyfarming std::call_once
george.karpenkov created this revision. Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, aemerson. Also explicitly do not support functors for now, since that entails figuring out which call operator corresponds to the passed parameters. Resolution: we really should not do body farming for templated C++ functions, even in seemingly trivial cases! https://reviews.llvm.org/D39031 Files: lib/Analysis/BodyFarm.cpp test/Analysis/call_once.cpp Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -249,3 +249,44 @@ void test_no_segfault_on_different_impl() { std::call_once(g, false); // no-warning } + +void test_lambda_refcapture() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, [&](int &a) { a = 42; }, a); + clang_analyzer_eval(a == 42); // expected-warning{{TRUE}} +} + +void test_lambda_refcapture2() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, [=](int &a) { a = 42; }, a); + clang_analyzer_eval(a == 42); // expected-warning{{TRUE}} +} + +void test_lambda_fail_refcapture() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, [=](int a) { a = 42; }, a); + clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} +} + +void mutator(int ¶m) { + param = 42; +} +void test_reftypes_funcptr() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, &mutator, a); + clang_analyzer_eval(a == 42); // expected-warning{{TRUE}} +} + +void fail_mutator(int param) { + param = 42; +} +void test_mutator_noref() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, &fail_mutator, a); + clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} +} Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -264,11 +264,8 @@ static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, const ParmVarDecl *Callback, - QualType CallbackType, + CXXRecordDecl *CallbackDecl, ArrayRef CallArgs) { - - CXXRecordDecl *CallbackDecl = CallbackType->getAsCXXRecordDecl(); - assert(CallbackDecl != nullptr); assert(CallbackDecl->isLambda()); FunctionDecl *callOperatorDecl = CallbackDecl->getLambdaCallOperator(); @@ -319,6 +316,9 @@ const ParmVarDecl *Flag = D->getParamDecl(0); const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); + + // Nullable pointer, non-null iff function is a CXXRecordDecl. + CXXRecordDecl *CallbackRecordDecl = CallbackType->getAsCXXRecordDecl(); QualType FlagType = Flag->getType().getNonReferenceType(); auto *FlagRecordDecl = dyn_cast_or_null(FlagType->getAsTagDecl()); @@ -348,28 +348,59 @@ return nullptr; } - bool isLambdaCall = CallbackType->getAsCXXRecordDecl() && - CallbackType->getAsCXXRecordDecl()->isLambda(); + bool isLambdaCall = CallbackRecordDecl && CallbackRecordDecl->isLambda(); + if (CallbackRecordDecl && !isLambdaCall) { +DEBUG(llvm::dbgs() << "Not supported: synthesizing body for functors when " + << "body farming std::call_once, ignoring the call."); +return nullptr; + } SmallVector CallArgs; + const FunctionProtoType *CallbackFunctionType; + if (isLambdaCall) { - if (isLambdaCall) // Lambda requires callback itself inserted as a first parameter. CallArgs.push_back( M.makeDeclRefExpr(Callback, /* RefersToEnclosingVariableOrCapture= */ true)); +CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator() + ->getType() + ->getAs(); + } else { +CallbackFunctionType = +CallbackType->getPointeeType()->getAs(); + } - // All arguments past first two ones are passed to the callback. - for (unsigned int i = 2; i < D->getNumParams(); i++) -CallArgs.push_back( -M.makeLvalueToRvalue(D->getParamDecl(i), - /* RefersToEnclosingVariableOrCapture= */ false)); + if (!CallbackFunctionType) +return nullptr; + + // First two arguments are used for the flag and for the callback. + if (D->getNumParams() != CallbackFunctionType->getNumParams() + 2) { +DEBUG(llvm::dbgs() << "Number of params of the callback does not match " + << "the number of params passed to std::call_once, " + << "ignoring the call"); +return nullptr; + } + + // All arguments past first two ones are passed to the callback, + // and we turn lvalues into rvalues if the argument is not passed by + // reference. + for
[PATCH] D39031: [Analyzer] Correctly handle parameters passed by reference when bodyfarming std::call_once
This revision was automatically updated to reflect the committed changes. Closed by commit rL316249: [Analyzer] Correctly handle parameters passed by reference when bodyfarming std… (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D39031?vs=119415&id=119724#toc Repository: rL LLVM https://reviews.llvm.org/D39031 Files: cfe/trunk/lib/Analysis/BodyFarm.cpp cfe/trunk/test/Analysis/call_once.cpp Index: cfe/trunk/test/Analysis/call_once.cpp === --- cfe/trunk/test/Analysis/call_once.cpp +++ cfe/trunk/test/Analysis/call_once.cpp @@ -249,3 +249,44 @@ void test_no_segfault_on_different_impl() { std::call_once(g, false); // no-warning } + +void test_lambda_refcapture() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, [&](int &a) { a = 42; }, a); + clang_analyzer_eval(a == 42); // expected-warning{{TRUE}} +} + +void test_lambda_refcapture2() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, [=](int &a) { a = 42; }, a); + clang_analyzer_eval(a == 42); // expected-warning{{TRUE}} +} + +void test_lambda_fail_refcapture() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, [=](int a) { a = 42; }, a); + clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} +} + +void mutator(int ¶m) { + param = 42; +} +void test_reftypes_funcptr() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, &mutator, a); + clang_analyzer_eval(a == 42); // expected-warning{{TRUE}} +} + +void fail_mutator(int param) { + param = 42; +} +void test_mutator_noref() { + static std::once_flag flag; + int a = 6; + std::call_once(flag, &fail_mutator, a); + clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} +} Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -264,11 +264,8 @@ static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, const ParmVarDecl *Callback, - QualType CallbackType, + CXXRecordDecl *CallbackDecl, ArrayRef CallArgs) { - - CXXRecordDecl *CallbackDecl = CallbackType->getAsCXXRecordDecl(); - assert(CallbackDecl != nullptr); assert(CallbackDecl->isLambda()); FunctionDecl *callOperatorDecl = CallbackDecl->getLambdaCallOperator(); @@ -319,6 +316,9 @@ const ParmVarDecl *Flag = D->getParamDecl(0); const ParmVarDecl *Callback = D->getParamDecl(1); QualType CallbackType = Callback->getType().getNonReferenceType(); + + // Nullable pointer, non-null iff function is a CXXRecordDecl. + CXXRecordDecl *CallbackRecordDecl = CallbackType->getAsCXXRecordDecl(); QualType FlagType = Flag->getType().getNonReferenceType(); auto *FlagRecordDecl = dyn_cast_or_null(FlagType->getAsTagDecl()); @@ -348,28 +348,58 @@ return nullptr; } - bool isLambdaCall = CallbackType->getAsCXXRecordDecl() && - CallbackType->getAsCXXRecordDecl()->isLambda(); + bool isLambdaCall = CallbackRecordDecl && CallbackRecordDecl->isLambda(); + if (CallbackRecordDecl && !isLambdaCall) { +DEBUG(llvm::dbgs() << "Not supported: synthesizing body for functors when " + << "body farming std::call_once, ignoring the call."); +return nullptr; + } SmallVector CallArgs; + const FunctionProtoType *CallbackFunctionType; + if (isLambdaCall) { - if (isLambdaCall) // Lambda requires callback itself inserted as a first parameter. CallArgs.push_back( M.makeDeclRefExpr(Callback, /* RefersToEnclosingVariableOrCapture= */ true)); +CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator() + ->getType() + ->getAs(); + } else { +CallbackFunctionType = +CallbackType->getPointeeType()->getAs(); + } - // All arguments past first two ones are passed to the callback. - for (unsigned int i = 2; i < D->getNumParams(); i++) -CallArgs.push_back( -M.makeLvalueToRvalue(D->getParamDecl(i), - /* RefersToEnclosingVariableOrCapture= */ false)); + if (!CallbackFunctionType) +return nullptr; + + // First two arguments are used for the flag and for the callback. + if (D->getNumParams() != CallbackFunctionType->getNumParams() + 2) { +DEBUG(llvm::dbgs() << "Number of params of the callback does not match " + << "the number of params passed to std::call_once, " + << "ignoring the call"); +return nullptr; + } + + // All arguments past first two ones are passed to the callback, + // and we turn lvalues into rvalues if the argument is not passed by
[PATCH] D39201: [Analyzer] Handle implicit function reference in bodyfarming std::call_once
george.karpenkov created this revision. Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, aemerson. https://reviews.llvm.org/D39201 Files: lib/Analysis/BodyFarm.cpp test/Analysis/call_once.cpp Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -290,3 +290,16 @@ std::call_once(flag, &fail_mutator, a); clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} } + +// Function is implicitly treated as a function pointer +// even when an ampersand is not explicitly set. +void callbackn(int ¶m) { + param = 42; +}; +void test_implicit_funcptr() { + int x = 0; + static std::once_flag flagn; + + std::call_once(flagn, callbackn, x); + clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} +} Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -253,13 +253,23 @@ const ParmVarDecl *Callback, ArrayRef CallArgs) { - return new (C) CallExpr( - /*ASTContext=*/C, - /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback), - /*args=*/CallArgs, - /*QualType=*/C.VoidTy, - /*ExprValueType=*/VK_RValue, - /*SourceLocation=*/SourceLocation()); + QualType Ty = Callback->getType(); + DeclRefExpr *Call = M.makeDeclRefExpr(Callback); + CastKind CK; + if (Ty->isRValueReferenceType()) { +CK = CK_LValueToRValue; + } else { +assert(Ty->isLValueReferenceType()); +CK = CK_FunctionToPointerDecay; +Ty = C.getPointerType(Ty.getNonReferenceType()); + } + + return new (C) + CallExpr(C, M.makeImplicitCast(Call, Ty.getNonReferenceType(), CK), + /*args=*/CallArgs, + /*QualType=*/C.VoidTy, + /*ExprValueType=*/VK_RValue, + /*SourceLocation=*/SourceLocation()); } static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, @@ -366,9 +376,11 @@ CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator() ->getType() ->getAs(); - } else { + } else if (!CallbackType->getPointeeType().isNull()) { CallbackFunctionType = CallbackType->getPointeeType()->getAs(); + } else { +CallbackFunctionType = CallbackType->getAs(); } if (!CallbackFunctionType) Index: test/Analysis/call_once.cpp === --- test/Analysis/call_once.cpp +++ test/Analysis/call_once.cpp @@ -290,3 +290,16 @@ std::call_once(flag, &fail_mutator, a); clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} } + +// Function is implicitly treated as a function pointer +// even when an ampersand is not explicitly set. +void callbackn(int ¶m) { + param = 42; +}; +void test_implicit_funcptr() { + int x = 0; + static std::once_flag flagn; + + std::call_once(flagn, callbackn, x); + clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} +} Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -253,13 +253,23 @@ const ParmVarDecl *Callback, ArrayRef CallArgs) { - return new (C) CallExpr( - /*ASTContext=*/C, - /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback), - /*args=*/CallArgs, - /*QualType=*/C.VoidTy, - /*ExprValueType=*/VK_RValue, - /*SourceLocation=*/SourceLocation()); + QualType Ty = Callback->getType(); + DeclRefExpr *Call = M.makeDeclRefExpr(Callback); + CastKind CK; + if (Ty->isRValueReferenceType()) { +CK = CK_LValueToRValue; + } else { +assert(Ty->isLValueReferenceType()); +CK = CK_FunctionToPointerDecay; +Ty = C.getPointerType(Ty.getNonReferenceType()); + } + + return new (C) + CallExpr(C, M.makeImplicitCast(Call, Ty.getNonReferenceType(), CK), + /*args=*/CallArgs, + /*QualType=*/C.VoidTy, + /*ExprValueType=*/VK_RValue, + /*SourceLocation=*/SourceLocation()); } static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, @@ -366,9 +376,11 @@ CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator() ->getType() ->getAs(); - } else { + } else if (!CallbackType->getPointeeType().isNull()) { CallbackFunctionType = CallbackType->getPointeeType()->getAs(); + } else { +CallbackFunctionType = CallbackType->getAs(); } if (!CallbackFunctionType) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list
[PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp
george.karpenkov created this revision. Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, aemerson. https://reviews.llvm.org/D39208 Files: include/clang/Analysis/AnalysisDeclContext.h include/clang/Analysis/BodyFarm.h lib/Analysis/AnalysisDeclContext.cpp lib/Analysis/BodyFarm.cpp lib/Analysis/BodyFarm.h lib/StaticAnalyzer/Core/AnalysisManager.cpp Index: lib/StaticAnalyzer/Core/AnalysisManager.cpp === --- lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -14,33 +14,25 @@ void AnalysisManager::anchor() { } -AnalysisManager::AnalysisManager(ASTContext &ctx, DiagnosticsEngine &diags, - const LangOptions &lang, - const PathDiagnosticConsumers &PDC, - StoreManagerCreator storemgr, - ConstraintManagerCreator constraintmgr, - CheckerManager *checkerMgr, - AnalyzerOptions &Options, - CodeInjector *injector) - : AnaCtxMgr(Options.UnoptimizedCFG, - Options.includeImplicitDtorsInCFG(), - /*AddInitializers=*/true, - Options.includeTemporaryDtorsInCFG(), - Options.includeLifetimeInCFG(), - // Adding LoopExit elements to the CFG is a requirement for loop - // unrolling. - Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(), - Options.shouldSynthesizeBodies(), - Options.shouldConditionalizeStaticInitializers(), - /*addCXXNewAllocator=*/true, - injector), -Ctx(ctx), -Diags(diags), -LangOpts(lang), -PathConsumers(PDC), -CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr), -CheckerMgr(checkerMgr), -options(Options) { +AnalysisManager::AnalysisManager( +ASTContext &ASTCtx, DiagnosticsEngine &diags, const LangOptions &lang, +const PathDiagnosticConsumers &PDC, StoreManagerCreator storemgr, +ConstraintManagerCreator constraintmgr, CheckerManager *checkerMgr, +AnalyzerOptions &Options, CodeInjector *injector) +: AnaCtxMgr(ASTCtx, Options.UnoptimizedCFG, +Options.includeImplicitDtorsInCFG(), +/*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(), +Options.includeLifetimeInCFG(), +// Adding LoopExit elements to the CFG is a requirement for loop +// unrolling. +Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(), +Options.shouldSynthesizeBodies(), +Options.shouldConditionalizeStaticInitializers(), +/*addCXXNewAllocator=*/true, +injector), + Ctx(ASTCtx), Diags(diags), LangOpts(lang), PathConsumers(PDC), + CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr), + CheckerMgr(checkerMgr), options(Options) { AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd(); } Index: lib/Analysis/BodyFarm.cpp === --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -12,7 +12,7 @@ // //===--===// -#include "BodyFarm.h" +#include "clang/Analysis/BodyFarm.h" #include "clang/AST/ASTContext.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/Decl.h" Index: lib/Analysis/AnalysisDeclContext.cpp === --- lib/Analysis/AnalysisDeclContext.cpp +++ lib/Analysis/AnalysisDeclContext.cpp @@ -13,7 +13,6 @@ //===--===// #include "clang/Analysis/AnalysisDeclContext.h" -#include "BodyFarm.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclObjC.h" @@ -23,6 +22,7 @@ #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" #include "clang/Analysis/Analyses/LiveVariables.h" #include "clang/Analysis/Analyses/PseudoConstantAnalysis.h" +#include "clang/Analysis/BodyFarm.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/CFGStmtMap.h" #include "clang/Analysis/Support/BumpVector.h" @@ -63,18 +63,12 @@ cfgBuildOptions.forcedBlkExprs = &forcedBlkExprs; } -AnalysisDeclContextManager::AnalysisDeclContextManager(bool useUnoptimizedCFG, - bool addImplicitDtors, - bool addInitializers, - bool addTemporaryDtors, - bool addLifetime, - bool addLoopExit, -
[PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp
george.karpenkov added a comment. @dcoughlin that's not me, that's clang-format. If we agree on always running it, I think the changes should stay there. https://reviews.llvm.org/D39208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp
george.karpenkov added a comment. @dcoughlin the context I was thinking about is that if everyone consistently runs `clang-format` (if we want that), then we never would have discussion. The alternative is that every run of `clang-format` would be followed by manually reverting changes which were introduced by mistake (in this case, because the file was moved). https://reviews.llvm.org/D39208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp
This revision was automatically updated to reflect the committed changes. Closed by commit rL316400: [Analyzer] Do not use static storage to for implementations created in BodyFarm. (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D39208?vs=119949&id=119974#toc Repository: rL LLVM https://reviews.llvm.org/D39208 Files: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h cfe/trunk/include/clang/Analysis/BodyFarm.h cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp cfe/trunk/lib/Analysis/BodyFarm.cpp cfe/trunk/lib/Analysis/BodyFarm.h cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp Index: cfe/trunk/include/clang/Analysis/BodyFarm.h === --- cfe/trunk/include/clang/Analysis/BodyFarm.h +++ cfe/trunk/include/clang/Analysis/BodyFarm.h @@ -0,0 +1,51 @@ +//== BodyFarm.h - Factory for conjuring up fake bodies -*- C++ -*-// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// BodyFarm is a factory for creating faux implementations for functions/methods +// for analysis purposes. +// +//===--===// + +#ifndef LLVM_CLANG_LIB_ANALYSIS_BODYFARM_H +#define LLVM_CLANG_LIB_ANALYSIS_BODYFARM_H + +#include "clang/AST/DeclBase.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/Optional.h" + +namespace clang { + +class ASTContext; +class FunctionDecl; +class ObjCMethodDecl; +class ObjCPropertyDecl; +class Stmt; +class CodeInjector; + +class BodyFarm { +public: + BodyFarm(ASTContext &C, CodeInjector *injector) : C(C), Injector(injector) {} + + /// Factory method for creating bodies for ordinary functions. + Stmt *getBody(const FunctionDecl *D); + + /// Factory method for creating bodies for Objective-C properties. + Stmt *getBody(const ObjCMethodDecl *D); + +private: + typedef llvm::DenseMap> BodyMap; + + ASTContext &C; + BodyMap Bodies; + CodeInjector *Injector; +}; +} // namespace clang + +#endif Index: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h === --- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h +++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSISDECLCONTEXT_H #include "clang/AST/Decl.h" +#include "clang/Analysis/BodyFarm.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/CodeInjector.h" #include "llvm/ADT/DenseMap.h" @@ -409,29 +410,33 @@ typedef llvm::DenseMap> ContextMap; + ASTContext &ASTCtx; ContextMap Contexts; LocationContextManager LocContexts; CFG::BuildOptions cfgBuildOptions; /// Pointer to an interface that can provide function bodies for /// declarations from external source. std::unique_ptr Injector; - + + /// Pointer to a factory for creating and caching implementations for common + /// methods during the analysis. + BodyFarm *BdyFrm = nullptr; + /// Flag to indicate whether or not bodies should be synthesized /// for well-known functions. bool SynthesizeBodies; public: - AnalysisDeclContextManager(bool useUnoptimizedCFG = false, + AnalysisDeclContextManager(ASTContext &ASTCtx, bool useUnoptimizedCFG = false, bool addImplicitDtors = false, bool addInitializers = false, bool addTemporaryDtors = false, - bool addLifetime = false, - bool addLoopExit = false, + bool addLifetime = false, bool addLoopExit = false, bool synthesizeBodies = false, bool addStaticInitBranches = false, bool addCXXNewAllocator = true, - CodeInjector* injector = nullptr); + CodeInjector *injector = nullptr); ~AnalysisDeclContextManager(); @@ -472,6 +477,9 @@ return LocContexts.getStackFrame(getContext(D), Parent, S, Blk, Idx); } + /// Get and lazily create a {@code BodyFarm} instance. + BodyFarm *getBodyFarm(); + /// Discard all previously created AnalysisDeclContexts. void clear(); Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -14,33 +14,25 @@ void AnalysisManager::anchor() { } -AnalysisManager::AnalysisManager(ASTContext &ctx, DiagnosticsEngine &diags, - const LangOptions &lang, -
[PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp
george.karpenkov added a comment. @dcoughlin OK sorry, I haven't considered the point that codebase predates the requirement, and the master format-all commit was never done. I've committed this one already, but I will be more careful with applying clang-format to future changes. Repository: rL LLVM https://reviews.llvm.org/D39208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39201: [Analyzer] Handle implicit function reference in bodyfarming std::call_once
This revision was automatically updated to reflect the committed changes. Closed by commit rL316402: [Analyzer] Handle implicit function reference in bodyfarming std::call_once (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D39201?vs=119911&id=119975#toc Repository: rL LLVM https://reviews.llvm.org/D39201 Files: cfe/trunk/lib/Analysis/BodyFarm.cpp cfe/trunk/test/Analysis/call_once.cpp Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -253,13 +253,23 @@ const ParmVarDecl *Callback, ArrayRef CallArgs) { - return new (C) CallExpr( - /*ASTContext=*/C, - /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback), - /*args=*/CallArgs, - /*QualType=*/C.VoidTy, - /*ExprValueType=*/VK_RValue, - /*SourceLocation=*/SourceLocation()); + QualType Ty = Callback->getType(); + DeclRefExpr *Call = M.makeDeclRefExpr(Callback); + CastKind CK; + if (Ty->isRValueReferenceType()) { +CK = CK_LValueToRValue; + } else { +assert(Ty->isLValueReferenceType()); +CK = CK_FunctionToPointerDecay; +Ty = C.getPointerType(Ty.getNonReferenceType()); + } + + return new (C) + CallExpr(C, M.makeImplicitCast(Call, Ty.getNonReferenceType(), CK), + /*args=*/CallArgs, + /*QualType=*/C.VoidTy, + /*ExprValueType=*/VK_RValue, + /*SourceLocation=*/SourceLocation()); } static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, @@ -366,9 +376,11 @@ CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator() ->getType() ->getAs(); - } else { + } else if (!CallbackType->getPointeeType().isNull()) { CallbackFunctionType = CallbackType->getPointeeType()->getAs(); + } else { +CallbackFunctionType = CallbackType->getAs(); } if (!CallbackFunctionType) Index: cfe/trunk/test/Analysis/call_once.cpp === --- cfe/trunk/test/Analysis/call_once.cpp +++ cfe/trunk/test/Analysis/call_once.cpp @@ -290,3 +290,16 @@ std::call_once(flag, &fail_mutator, a); clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} } + +// Function is implicitly treated as a function pointer +// even when an ampersand is not explicitly set. +void callbackn(int ¶m) { + param = 42; +}; +void test_implicit_funcptr() { + int x = 0; + static std::once_flag flagn; + + std::call_once(flagn, callbackn, x); + clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} +} Index: cfe/trunk/lib/Analysis/BodyFarm.cpp === --- cfe/trunk/lib/Analysis/BodyFarm.cpp +++ cfe/trunk/lib/Analysis/BodyFarm.cpp @@ -253,13 +253,23 @@ const ParmVarDecl *Callback, ArrayRef CallArgs) { - return new (C) CallExpr( - /*ASTContext=*/C, - /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback), - /*args=*/CallArgs, - /*QualType=*/C.VoidTy, - /*ExprValueType=*/VK_RValue, - /*SourceLocation=*/SourceLocation()); + QualType Ty = Callback->getType(); + DeclRefExpr *Call = M.makeDeclRefExpr(Callback); + CastKind CK; + if (Ty->isRValueReferenceType()) { +CK = CK_LValueToRValue; + } else { +assert(Ty->isLValueReferenceType()); +CK = CK_FunctionToPointerDecay; +Ty = C.getPointerType(Ty.getNonReferenceType()); + } + + return new (C) + CallExpr(C, M.makeImplicitCast(Call, Ty.getNonReferenceType(), CK), + /*args=*/CallArgs, + /*QualType=*/C.VoidTy, + /*ExprValueType=*/VK_RValue, + /*SourceLocation=*/SourceLocation()); } static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, @@ -366,9 +376,11 @@ CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator() ->getType() ->getAs(); - } else { + } else if (!CallbackType->getPointeeType().isNull()) { CallbackFunctionType = CallbackType->getPointeeType()->getAs(); + } else { +CallbackFunctionType = CallbackType->getAs(); } if (!CallbackFunctionType) Index: cfe/trunk/test/Analysis/call_once.cpp === --- cfe/trunk/test/Analysis/call_once.cpp +++ cfe/trunk/test/Analysis/call_once.cpp @@ -290,3 +290,16 @@ std::call_once(flag, &fail_mutator, a); clang_analyzer_eval(a == 42); // expected-warning{{FALSE}} } + +// Function is implicitly treated as a function pointer +// even when an ampersand is not explicitly set. +void callbackn(int ¶m) { + param = 4
[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr
george.karpenkov created this revision. Herald added subscribers: szepet, kristof.beyls, xazax.hun, aemerson. While by using `.get()` method we don't get the full protection offered by `std::unique_ptr`, given that two bugs were already encountered (http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/9065 and omitted nullptr assignment), using `std::unique_ptr` seems strictly better than raw pointer for all practical purposes. https://reviews.llvm.org/D39220 Files: include/clang/Analysis/AnalysisDeclContext.h lib/Analysis/AnalysisDeclContext.cpp Index: lib/Analysis/AnalysisDeclContext.cpp === --- lib/Analysis/AnalysisDeclContext.cpp +++ lib/Analysis/AnalysisDeclContext.cpp @@ -306,8 +306,8 @@ BodyFarm *AnalysisDeclContextManager::getBodyFarm() { if (!BdyFrm) -BdyFrm = new BodyFarm(ASTCtx, Injector.get()); - return BdyFrm; +BdyFrm = llvm::make_unique(ASTCtx, Injector.get()); + return BdyFrm.get(); } const StackFrameContext * @@ -603,10 +603,7 @@ } } -AnalysisDeclContextManager::~AnalysisDeclContextManager() { - if (BdyFrm) -delete BdyFrm; -} +AnalysisDeclContextManager::~AnalysisDeclContextManager() {} LocationContext::~LocationContext() {} Index: include/clang/Analysis/AnalysisDeclContext.h === --- include/clang/Analysis/AnalysisDeclContext.h +++ include/clang/Analysis/AnalysisDeclContext.h @@ -421,7 +421,7 @@ /// Pointer to a factory for creating and caching implementations for common /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; /// Flag to indicate whether or not bodies should be synthesized /// for well-known functions. Index: lib/Analysis/AnalysisDeclContext.cpp === --- lib/Analysis/AnalysisDeclContext.cpp +++ lib/Analysis/AnalysisDeclContext.cpp @@ -306,8 +306,8 @@ BodyFarm *AnalysisDeclContextManager::getBodyFarm() { if (!BdyFrm) -BdyFrm = new BodyFarm(ASTCtx, Injector.get()); - return BdyFrm; +BdyFrm = llvm::make_unique(ASTCtx, Injector.get()); + return BdyFrm.get(); } const StackFrameContext * @@ -603,10 +603,7 @@ } } -AnalysisDeclContextManager::~AnalysisDeclContextManager() { - if (BdyFrm) -delete BdyFrm; -} +AnalysisDeclContextManager::~AnalysisDeclContextManager() {} LocationContext::~LocationContext() {} Index: include/clang/Analysis/AnalysisDeclContext.h === --- include/clang/Analysis/AnalysisDeclContext.h +++ include/clang/Analysis/AnalysisDeclContext.h @@ -421,7 +421,7 @@ /// Pointer to a factory for creating and caching implementations for common /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; /// Flag to indicate whether or not bodies should be synthesized /// for well-known functions. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38986: [Analyzer] Better unreachable message in enumeration
george.karpenkov added a comment. @dcoughlin > Is it when an end-user is running a build with assertions and can't provide a > reproducer but can provide the console output? Yes, or just for developer staring at the crash for the first time, or for the crashers in CI. > Does llvm_unreachable() guarantee that the string construction code is > completely removed from release builds? I am not sure what do you mean, the string construction code would only be encountered when we are in the `default` branch, which would mean that we are crashing already. Or do you worry about the code size being slightly larger? https://reviews.llvm.org/D38986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr
george.karpenkov updated this revision to Diff 120109. https://reviews.llvm.org/D39220 Files: include/clang/Analysis/AnalysisDeclContext.h lib/Analysis/AnalysisDeclContext.cpp Index: lib/Analysis/AnalysisDeclContext.cpp === --- lib/Analysis/AnalysisDeclContext.cpp +++ lib/Analysis/AnalysisDeclContext.cpp @@ -306,8 +306,8 @@ BodyFarm *AnalysisDeclContextManager::getBodyFarm() { if (!BdyFrm) -BdyFrm = new BodyFarm(ASTCtx, Injector.get()); - return BdyFrm; +BdyFrm = llvm::make_unique(ASTCtx, Injector.get()); + return BdyFrm.get(); } const StackFrameContext * @@ -603,11 +603,6 @@ } } -AnalysisDeclContextManager::~AnalysisDeclContextManager() { - if (BdyFrm) -delete BdyFrm; -} - LocationContext::~LocationContext() {} LocationContextManager::~LocationContextManager() { Index: include/clang/Analysis/AnalysisDeclContext.h === --- include/clang/Analysis/AnalysisDeclContext.h +++ include/clang/Analysis/AnalysisDeclContext.h @@ -421,7 +421,7 @@ /// Pointer to a factory for creating and caching implementations for common /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; /// Flag to indicate whether or not bodies should be synthesized /// for well-known functions. @@ -438,8 +438,6 @@ bool addCXXNewAllocator = true, CodeInjector *injector = nullptr); - ~AnalysisDeclContextManager(); - AnalysisDeclContext *getContext(const Decl *D); bool getUseUnoptimizedCFG() const { Index: lib/Analysis/AnalysisDeclContext.cpp === --- lib/Analysis/AnalysisDeclContext.cpp +++ lib/Analysis/AnalysisDeclContext.cpp @@ -306,8 +306,8 @@ BodyFarm *AnalysisDeclContextManager::getBodyFarm() { if (!BdyFrm) -BdyFrm = new BodyFarm(ASTCtx, Injector.get()); - return BdyFrm; +BdyFrm = llvm::make_unique(ASTCtx, Injector.get()); + return BdyFrm.get(); } const StackFrameContext * @@ -603,11 +603,6 @@ } } -AnalysisDeclContextManager::~AnalysisDeclContextManager() { - if (BdyFrm) -delete BdyFrm; -} - LocationContext::~LocationContext() {} LocationContextManager::~LocationContextManager() { Index: include/clang/Analysis/AnalysisDeclContext.h === --- include/clang/Analysis/AnalysisDeclContext.h +++ include/clang/Analysis/AnalysisDeclContext.h @@ -421,7 +421,7 @@ /// Pointer to a factory for creating and caching implementations for common /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; /// Flag to indicate whether or not bodies should be synthesized /// for well-known functions. @@ -438,8 +438,6 @@ bool addCXXNewAllocator = true, CodeInjector *injector = nullptr); - ~AnalysisDeclContextManager(); - AnalysisDeclContext *getContext(const Decl *D); bool getUseUnoptimizedCFG() const { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp
george.karpenkov added inline comments. Comment at: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp:607 +AnalysisDeclContextManager::~AnalysisDeclContextManager() { + if (!BdyFrm) +delete BdyFrm; alexfh wrote: > This is an almost guaranteed memory leak. I think, you meant `if (BdyFrm) > delete BdyFrm;`. But `delete` deals well with `nullptr`, so just delete the > pointer unconditionally. But first consider making `BdyFrm` a > `std::unique_ptr`. From a cursory look I don't see any reasons not to. @alexfh yes, this was fixed. Also, yes, https://reviews.llvm.org/D39220 Repository: rL LLVM https://reviews.llvm.org/D39208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr
george.karpenkov added inline comments. Comment at: include/clang/Analysis/AnalysisDeclContext.h:424 /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; alexfh wrote: > BdyFrm is somewhat cryptic. Maybe Farm, Bodies or something else that is not > so weirdly abbreviated? But that's just forced on us by the naming convention, isn't it? I think other names are worse though: they don't tell us which class we can look at to figure out what is it doing. https://reviews.llvm.org/D39220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr
This revision was automatically updated to reflect the committed changes. Closed by commit rL316536: [Analyzer] Store BodyFarm in std::unique_ptr (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D39220?vs=120109&id=120151#toc Repository: rL LLVM https://reviews.llvm.org/D39220 Files: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Index: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h === --- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h +++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h @@ -421,7 +421,7 @@ /// Pointer to a factory for creating and caching implementations for common /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; /// Flag to indicate whether or not bodies should be synthesized /// for well-known functions. @@ -438,8 +438,6 @@ bool addCXXNewAllocator = true, CodeInjector *injector = nullptr); - ~AnalysisDeclContextManager(); - AnalysisDeclContext *getContext(const Decl *D); bool getUseUnoptimizedCFG() const { Index: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp === --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp @@ -306,8 +306,8 @@ BodyFarm *AnalysisDeclContextManager::getBodyFarm() { if (!BdyFrm) -BdyFrm = new BodyFarm(ASTCtx, Injector.get()); - return BdyFrm; +BdyFrm = llvm::make_unique(ASTCtx, Injector.get()); + return BdyFrm.get(); } const StackFrameContext * @@ -603,11 +603,6 @@ } } -AnalysisDeclContextManager::~AnalysisDeclContextManager() { - if (BdyFrm) -delete BdyFrm; -} - LocationContext::~LocationContext() {} LocationContextManager::~LocationContextManager() { Index: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h === --- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h +++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h @@ -421,7 +421,7 @@ /// Pointer to a factory for creating and caching implementations for common /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; /// Flag to indicate whether or not bodies should be synthesized /// for well-known functions. @@ -438,8 +438,6 @@ bool addCXXNewAllocator = true, CodeInjector *injector = nullptr); - ~AnalysisDeclContextManager(); - AnalysisDeclContext *getContext(const Decl *D); bool getUseUnoptimizedCFG() const { Index: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp === --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp @@ -306,8 +306,8 @@ BodyFarm *AnalysisDeclContextManager::getBodyFarm() { if (!BdyFrm) -BdyFrm = new BodyFarm(ASTCtx, Injector.get()); - return BdyFrm; +BdyFrm = llvm::make_unique(ASTCtx, Injector.get()); + return BdyFrm.get(); } const StackFrameContext * @@ -603,11 +603,6 @@ } } -AnalysisDeclContextManager::~AnalysisDeclContextManager() { - if (BdyFrm) -delete BdyFrm; -} - LocationContext::~LocationContext() {} LocationContextManager::~LocationContextManager() { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash
george.karpenkov added inline comments. Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:139 -DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture, - bool GetNonReferenceType) { - auto Type = D->getType(); - if (GetNonReferenceType) -Type = Type.getNonReferenceType(); +DeclRefExpr *ASTMaker::makeDeclRefExpr( +const VarDecl *D, danielmarjamaki wrote: > this looks strange, did clang-format do this? yes i believe so. Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:366 +M.makeLvalueToRvalue(D->getParamDecl(i), + /* RefersToEnclosingVariableOrCapture= */ false)); alexfh wrote: > Remove the spaces inside the comment. `/*ParamName=*/value` is the format > that is understood by clang-format and the > https://clang.llvm.org/extra/clang-tidy/checks/misc-argument-comment.html > clang-tidy check. Same elsewhere in this patch. yeah, i can do that. can we also consider changing `clang-tidy` to ignore those spaces? Repository: rL LLVM https://reviews.llvm.org/D39015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39269: [Analyzer] [Tests] Do not discard output from CmpRuns.py when running integration tests
george.karpenkov created this revision. Herald added subscribers: szepet, xazax.hun. Contrary to the deleted comment, in most cases CmpRuns.py produces a fairly small amount of output, which is useful to see straight away to see what has changed when executing the integration tests. https://reviews.llvm.org/D39269 Files: utils/analyzer/SATestBuild.py utils/analyzer/SATestUtils.py Index: utils/analyzer/SATestUtils.py === --- utils/analyzer/SATestUtils.py +++ utils/analyzer/SATestUtils.py @@ -93,14 +93,6 @@ sys.exit(-1) -class Discarder(object): -""" -Auxiliary object to discard stdout. -""" -def write(self, text): -pass # do nothing - - def isCommentCSVLine(Entries): """ Treat CSV lines starting with a '#' as a comment. Index: utils/analyzer/SATestBuild.py === --- utils/analyzer/SATestBuild.py +++ utils/analyzer/SATestBuild.py @@ -526,14 +526,9 @@ DiffsPath = os.path.join(NewDir, DiffsSummaryFileName) PatchedSourceDirPath = os.path.join(Dir, PatchedSourceDirName) Opts = CmpRuns.CmpOptions(DiffsPath, "", PatchedSourceDirPath) -# Discard everything coming out of stdout -# (CmpRun produces a lot of them). -OLD_STDOUT = sys.stdout -sys.stdout = SATestUtils.Discarder() # Scan the results, delete empty plist files. NumDiffs, ReportsInRef, ReportsInNew = \ CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False) -sys.stdout = OLD_STDOUT if (NumDiffs > 0): print "Warning: %r differences in diagnostics. See %s" % \ (NumDiffs, DiffsPath,) Index: utils/analyzer/SATestUtils.py === --- utils/analyzer/SATestUtils.py +++ utils/analyzer/SATestUtils.py @@ -93,14 +93,6 @@ sys.exit(-1) -class Discarder(object): -""" -Auxiliary object to discard stdout. -""" -def write(self, text): -pass # do nothing - - def isCommentCSVLine(Entries): """ Treat CSV lines starting with a '#' as a comment. Index: utils/analyzer/SATestBuild.py === --- utils/analyzer/SATestBuild.py +++ utils/analyzer/SATestBuild.py @@ -526,14 +526,9 @@ DiffsPath = os.path.join(NewDir, DiffsSummaryFileName) PatchedSourceDirPath = os.path.join(Dir, PatchedSourceDirName) Opts = CmpRuns.CmpOptions(DiffsPath, "", PatchedSourceDirPath) -# Discard everything coming out of stdout -# (CmpRun produces a lot of them). -OLD_STDOUT = sys.stdout -sys.stdout = SATestUtils.Discarder() # Scan the results, delete empty plist files. NumDiffs, ReportsInRef, ReportsInNew = \ CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False) -sys.stdout = OLD_STDOUT if (NumDiffs > 0): print "Warning: %r differences in diagnostics. See %s" % \ (NumDiffs, DiffsPath,) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator
george.karpenkov created this revision. george.karpenkov added reviewers: NoQ, bogner, chandlerc, zturner, aprantl. Herald added a subscriber: mgrang. There are many "dumping" actions available from the compiler: Clang dumps AST, Clang static analyzer dumps generated nodes in the "exploded graph" it generates. However, in many of those dumps raw pointer values are printed for the objects of interest, which can considerably complicate debugging due to those values being non-reproducible. This patch adds a method for converting a pointer generated through an allocator to a deterministic (up to an architecture) pair of integers, which can make the debugging experience much more pleasant. (E.g. compare hunting for a value "0xa9273f732" which changes every run to hunting for "(1, 23)"). Discussion started at http://lists.llvm.org/pipermail/cfe-dev/2018-August/059178.html Code in collaboration with @NoQ https://reviews.llvm.org/D51393 Files: llvm/include/llvm/Support/Allocator.h Index: llvm/include/llvm/Support/Allocator.h === --- llvm/include/llvm/Support/Allocator.h +++ llvm/include/llvm/Support/Allocator.h @@ -21,6 +21,7 @@ #ifndef LLVM_SUPPORT_ALLOCATOR_H #define LLVM_SUPPORT_ALLOCATOR_H +#include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" @@ -283,6 +284,28 @@ size_t GetNumSlabs() const { return Slabs.size() + CustomSizedSlabs.size(); } + /// \return A reproducible pair of objects, uniquely identifying + /// an input pointer \p Ptr in the given allocator. + /// Returns an empty optional if the pointer is not found in the allocator. + llvm::Optional> identifyObject(const void *Ptr) { +const char *P = reinterpret_cast(Ptr); +for (size_t Idx=0; Idx < Slabs.size(); Idx++) { + const char *S = reinterpret_cast(Slabs[Idx]); + if (P >= S && P < S + computeSlabSize(Idx)) +return std::make_pair(Idx, P - S); +} +for (size_t Idx=0; Idx < CustomSizedSlabs.size(); Idx++) { + const char *S = + reinterpret_cast(CustomSizedSlabs[Idx].first); + size_t Size = CustomSizedSlabs[Idx].second; + + // Use negative index to denote custom sized slabs. + if (P >= S && P < S + Size) +return std::make_pair(-Idx - 1, P - S); +} +return None; + } + size_t getTotalMemory() const { size_t TotalMemory = 0; for (auto I = Slabs.begin(), E = Slabs.end(); I != E; ++I) Index: llvm/include/llvm/Support/Allocator.h === --- llvm/include/llvm/Support/Allocator.h +++ llvm/include/llvm/Support/Allocator.h @@ -21,6 +21,7 @@ #ifndef LLVM_SUPPORT_ALLOCATOR_H #define LLVM_SUPPORT_ALLOCATOR_H +#include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" @@ -283,6 +284,28 @@ size_t GetNumSlabs() const { return Slabs.size() + CustomSizedSlabs.size(); } + /// \return A reproducible pair of objects, uniquely identifying + /// an input pointer \p Ptr in the given allocator. + /// Returns an empty optional if the pointer is not found in the allocator. + llvm::Optional> identifyObject(const void *Ptr) { +const char *P = reinterpret_cast(Ptr); +for (size_t Idx=0; Idx < Slabs.size(); Idx++) { + const char *S = reinterpret_cast(Slabs[Idx]); + if (P >= S && P < S + computeSlabSize(Idx)) +return std::make_pair(Idx, P - S); +} +for (size_t Idx=0; Idx < CustomSizedSlabs.size(); Idx++) { + const char *S = + reinterpret_cast(CustomSizedSlabs[Idx].first); + size_t Size = CustomSizedSlabs[Idx].second; + + // Use negative index to denote custom sized slabs. + if (P >= S && P < S + Size) +return std::make_pair(-Idx - 1, P - S); +} +return None; + } + size_t getTotalMemory() const { size_t TotalMemory = 0; for (auto I = Slabs.begin(), E = Slabs.end(); I != E; ++I) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51395: [analyzer] Dump a reproducible, deterministic ID of program state to exploded graph
george.karpenkov created this revision. george.karpenkov added reviewers: dcoughlin, NoQ. Herald added subscribers: Szelethus, mikhail.ramalho, a.sidorin, mgrang, szepet, baloghadamsoftware, xazax.hun. https://reviews.llvm.org/D51395 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/lib/StaticAnalyzer/Core/ProgramState.cpp Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp === --- clang/lib/StaticAnalyzer/Core/ProgramState.cpp +++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp @@ -69,6 +69,17 @@ stateMgr->getStoreManager().decrementReferenceCount(store); } +std::pair ProgramState::getID() const { + auto Out = getStateManager().Alloc.identifyObject(this); + return *Out; +} + +void ProgramState::dumpID(llvm::raw_ostream &Out) const { + auto P = getID(); + assert(P.second % alignof(ProgramState) == 0 && "Unexpected offset"); + Out << "(" << P.first << ", " << (P.second / alignof(ProgramState)) << ")"; +} + ProgramStateManager::ProgramStateManager(ASTContext &Ctx, StoreManagerCreator CreateSMgr, ConstraintManagerCreator CreateCMgr, Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp === --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -3129,8 +3129,9 @@ } ProgramStateRef state = N->getState(); -Out << "\\|StateID: " << (const void*) state.get() -<< " NodeID: " << (const void*) N << "\\|"; +Out << "\\|StateID: "; +state->dumpID(Out); +Out << " NodeID: " << (const void*) N << "\\|"; state->printDOT(Out, N->getLocationContext()); Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h === --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h @@ -107,6 +107,12 @@ ~ProgramState(); + /// Get a pair of numbers uniquely identifying the state. + std::pair getID() const; + + /// Dump a unique ID to a given stream. + void dumpID(llvm::raw_ostream &Out) const; + /// Return the ProgramStateManager associated with this state. ProgramStateManager &getStateManager() const { return *stateMgr; Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp === --- clang/lib/StaticAnalyzer/Core/ProgramState.cpp +++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp @@ -69,6 +69,17 @@ stateMgr->getStoreManager().decrementReferenceCount(store); } +std::pair ProgramState::getID() const { + auto Out = getStateManager().Alloc.identifyObject(this); + return *Out; +} + +void ProgramState::dumpID(llvm::raw_ostream &Out) const { + auto P = getID(); + assert(P.second % alignof(ProgramState) == 0 && "Unexpected offset"); + Out << "(" << P.first << ", " << (P.second / alignof(ProgramState)) << ")"; +} + ProgramStateManager::ProgramStateManager(ASTContext &Ctx, StoreManagerCreator CreateSMgr, ConstraintManagerCreator CreateCMgr, Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp === --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -3129,8 +3129,9 @@ } ProgramStateRef state = N->getState(); -Out << "\\|StateID: " << (const void*) state.get() -<< " NodeID: " << (const void*) N << "\\|"; +Out << "\\|StateID: "; +state->dumpID(Out); +Out << " NodeID: " << (const void*) N << "\\|"; state->printDOT(Out, N->getLocationContext()); Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h === --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h @@ -107,6 +107,12 @@ ~ProgramState(); + /// Get a pair of numbers uniquely identifying the state. + std::pair getID() const; + + /// Dump a unique ID to a given stream. + void dumpID(llvm::raw_ostream &Out) const; + /// Return the ProgramStateManager associated with this state. ProgramStateManager &getStateManager() const { return *stateMgr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits