[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing

2018-08-06 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-06 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-06 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-06 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-07 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-08-07 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-09 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-13 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-13 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-14 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-14 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-14 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-17 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-17 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-17 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-17 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-08-20 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-08-21 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-21 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-21 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-22 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-22 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-22 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-22 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-22 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-08 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-09 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-14 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-05-15 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-15 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-16 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-16 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-17 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-05-21 Thread George Karpenkov via Phabricator via cfe-commits
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++

2018-05-21 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-05-21 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-05-23 Thread George Karpenkov via Phabricator via cfe-commits
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.

2018-05-25 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-28 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-29 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-29 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-29 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-05-29 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-02 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-05 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-05 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-05 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-09 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-09 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-09 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-09 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-10 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-10 Thread George Karpenkov via Phabricator via cfe-commits
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++

2017-10-11 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-11 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-11 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-11 Thread George Karpenkov via Phabricator via cfe-commits
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++

2017-10-11 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-11 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-12 Thread George Karpenkov via Phabricator via cfe-commits
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()

2017-10-12 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-12 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-12 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-13 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-13 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-16 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-17 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-17 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-17 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-17 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-20 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-24 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-24 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-24 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-24 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-24 Thread George Karpenkov via Phabricator via cfe-commits
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

2017-10-24 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-28 Thread George Karpenkov via Phabricator via cfe-commits
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

2018-08-28 Thread George Karpenkov via Phabricator via cfe-commits
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


  1   2   3   4   5   6   >