Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-24 Thread Devin Coughlin via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL248516: [analyzer] When memcpy'ing into a fixed-size array, do not invalidate entireā€¦ (authored by dcoughlin). Changed prior to commit: http://reviews.llvm.org/D12571?vs=35505&id=35644#toc Repository:

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-24 Thread pierre gousseau via cfe-commits
pgousseau added a comment. In http://reviews.llvm.org/D12571#252366, @dcoughlin wrote: > Looks good to me! Thanks Pierre! I will commit. Much appreciated thanks ! http://reviews.llvm.org/D12571 ___ cfe-commits mailing list cfe-commits@lists.llvm.

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-23 Thread Devin Coughlin via cfe-commits
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. Looks good to me! Thanks Pierre! I will commit. http://reviews.llvm.org/D12571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-23 Thread pierre gousseau via cfe-commits
pgousseau updated this revision to Diff 35505. pgousseau added a comment. Following Devin's review: Fix a comment in 'IsFirstBufInBound'. Remove incorrect assertion expecting 'RegionOffset::getOffset' to only return positives values. Add test cases for regions having negatives offsets. Updated

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-18 Thread pierre gousseau via cfe-commits
pgousseau added a comment. In http://reviews.llvm.org/D12571#248464, @dcoughlin wrote: > Here is a reduced test case: Very useful thanks ! Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:198 @@ +197,3 @@ + + // Return true if the destination buffer of the copy fun

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-17 Thread Devin Coughlin via cfe-commits
dcoughlin added a comment. Here is a reduced test case: class B { public: B &operator= (const B &v) { return *this; } }; class A { int a[1]; B b; }; typedef long int ptrdiff_t; void copyInto(A *first, A *last, A *result) { ptrdiff_t n; for

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-17 Thread Devin Coughlin via cfe-commits
dcoughlin added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:198 @@ +197,3 @@ + + // Return true if the destination buffer of the copy function must/may be in + // bound. Since this returns true on unknown, it should be "may". ===

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-17 Thread pierre gousseau via cfe-commits
pgousseau added a comment. Ping ! http://reviews.llvm.org/D12571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-09 Thread pierre gousseau via cfe-commits
pgousseau updated the summary for this revision. pgousseau updated this revision to Diff 34359. pgousseau added a comment. Following Devin's review: Correct test for unknown length and unknown destination buffer. Add comment to 'IsFirstBufInBound' behavior regarding unknown states. Remove comments

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-09 Thread pierre gousseau via cfe-commits
pgousseau added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:843 @@ +842,3 @@ + if (!Length) +return true; + dcoughlin wrote: > There doesn't seem to be a test that cares about this returning true (as > compared to false). Will

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-08 Thread Devin Coughlin via cfe-commits
dcoughlin added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:843 @@ +842,3 @@ + if (!Length) +return true; + There doesn't seem to be a test that cares about this returning true (as compared to false). Comment