zaks.anna added a comment.
Sounds good!
http://reviews.llvm.org/D14014
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added inline comments.
Comment at: www/analyzer/installation.html:103
@@ -102,3 +102,1 @@
-Currently these are not installed using make install, and
-are located in $(SRCDIR)/tools/clang/tools/scan-build and
jroelofs wrote:
> @zaks.anna Do you know if
zaks.anna added a subscriber: zaks.anna.
zaks.anna added a comment.
Removed myself as a reviewer and added Argyrios instead.
http://reviews.llvm.org/D14506
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
zaks.anna added a comment.
Please, move the conversation on how to configure/use CodeChecker in the
particular setting elsewhere. I think it could be useful to the clang dev list;
however, this patch review is not the proper place for it.
http://reviews.llvm.org/D12906
_
zaks.anna added a comment.
> This makes forwards compatibility difficult, since there is no way to predict
> the names of future hashes
> (As far as I understand).
Can you describe what you are trying to achieve?
We can agree that all issue hashes start with "issue_hash" prefix. If you find
Author: zaks
Date: Tue Nov 10 18:49:22 2015
New Revision: 252679
URL: http://llvm.org/viewvc/llvm-project?rev=252679&view=rev
Log:
[static analyzer] Don't flag nil storage into NSMutableDictionary.
This is now allowed and has the behavior of removing the mapping.
Modified:
cfe/trunk/lib/Stat
zaks.anna added a comment.
> Just for the sake of explaining, lets say in 3 subsequent Analyzer releases
> the hashes are called “hash_1”, “hash_2” and “hash_3”.
> In the first release the suppression tool will record hash_1 to suppress a
> warning. Some developers will upgrade to the second
zaks.anna added a comment.
Thanks! See the comments inline.
Comment at: lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp:182
@@ +181,3 @@
+ if (rhs->isEvaluatable(Context))
+eraseAssign = true;
+ // Erase if the multiplicand was assigned a value,
-
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thank you! LGTM.
I'll commit this tomorrow.
http://reviews.llvm.org/D11572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
zaks.anna added a comment.
Committed in r244389.
Thank you!
http://reviews.llvm.org/D11572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: zaks
Date: Fri Aug 7 23:53:04 2015
New Revision: 244394
URL: http://llvm.org/viewvc/llvm-project?rev=244394&view=rev
Log:
Revert "[analyzer] Add checkers for OS X / iOS localizability issues"
This reverts commit fc885033a30b6e30ccf82398ae7c30e646727b10.
Revert all localization checker c
/24169
> <http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24169>
>
> On Fri, Aug 7, 2015 at 6:50 PM Anna Zaks via cfe-commits
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: zaks
> Date: Fri Aug 7 20:49:26 2015
> New Revision: 244389
>
> UR
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D22671
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
zaks.anna added a comment.
Great!
How does this work when a path spans multiple files, specifically, when the
definitions from a header file are inlined? We could add file names but only in
cases the file name does not match the main file.
> and hard to navigate (especially when using a specia
zaks.anna added inline comments.
Comment at: test/Analysis/PR12421.c:11
@@ +10,2 @@
+
+// CHECK: warning: Path diagnostic report is not generated. HTMLDiagnostics
does not support diagnostics that cross file boundaries.
We should use the name of the diagnostic co
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
> Yeah, just since we're all here anyway... By the way, -analyzer-config
> prune-paths=false is also very handy.
I think you should add these to the checker writer manual in the debuggi
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:191
@@ +190,3 @@
+// impossible to verify this precisely, but at least
+// this check avoids potential crashes.
+bool matchesCall(const CallExpr *CE) const;
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
A couple of nits. Otherwise, LGTM. Thanks!
Comment at: www/analyzer/checker_dev_manual.html:534
@@ +533,3 @@
+itself to the child process (for example, in gcc you can
+s
zaks.anna added inline comments.
Comment at: test/Analysis/PR12421.c:11
@@ +10,2 @@
+
+// CHECK: warning: Path diagnostic report is not generated. HTMLDiagnostics
does not support diagnostics that cross file boundaries.
ayartsev wrote:
> zaks.anna wrote:
> > We s
zaks.anna added a comment.
The report you are referencing does not seem to have "Description" as a column
name. Also, the titles row is not gray all the way to the right...
https://reviews.llvm.org/D22810
___
cfe-commits mailing list
cfe-commits@li
zaks.anna added a comment.
I am not sure it's the right way of fixing this problem and it introduces a
regression. The bug is probably specific to "&&".
+ Devin as we might have seen something similar.
Comment at: test/Analysis/misc-ps-region-store.m:332
@@ -330,3 +331,3 @@
zaks.anna added a comment.
We'd definitely want the same routine in both: printing the name in
-analyzer-display-progress and be accepted by -analyze-function.
Looks like what ND->getQualifiedNameAsString() returns is not proper ObjC
syntax, but maybe it's fine for the debug feature? Even thoug
zaks.anna added inline comments.
Comment at: test/Analysis/diagnostics/Inputs/include/Something.h:1
@@ +1,2 @@
+void clang_analyzer_warnIfReached();
+
Please, choose better file names. Every test that adds something cannot add a
header called something:) It won't
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:216
@@ +215,3 @@
+llvm::raw_svector_ostream warning(buf);
+warning << "warning: Path diagnostic report is not generated. Current "
+<< "output format does not support diagno
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Ok. LGTM.
Thank you!
Anna.
https://reviews.llvm.org/D22810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM
Thank you!
https://reviews.llvm.org/D22926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mail
zaks.anna added a subscriber: Alexander_Droste.
zaks.anna added a comment.
> Even though there are some doxygen-style comments in the checkers, i’ve never
> seen doxygen actually generate any docs for checker classes.
> Are they useful for IDE quick-hints only?
I think it's useful to have co
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:206
@@ +205,3 @@
+: Call.getArgExpr(ArgNo)->getType().getCanonicalType();
+ }
+ static QualType getArgType(const CallExpr *CE, ArgNoTy ArgNo) {
---
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Since this is an improvement, it looks good to me. (Improving printing of ObjC
methods is a good add on, but not blocking..)
https://reviews.llvm.org/D22856
_
zaks.anna added a comment.
https://reviews.llvm.org/D22090
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: zaks
Date: Mon Feb 1 20:04:48 2016
New Revision: 259453
URL: http://llvm.org/viewvc/llvm-project?rev=259453&view=rev
Log:
[asan] Add iOS support for Address Sanitizer
Differential Revision: http://reviews.llvm.org/D15624
Modified:
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/li
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thank you!
http://reviews.llvm.org/D16804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
zaks.anna added inline comments.
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td:75
@@ -74,1 +74,3 @@
+def MPI : Package<"mpi">;
+
Alexander_Droste wrote:
> zaks.anna wrote:
> > This should probably go under the 'option' package. What do you thin
zaks.anna added a comment.
Looks good, below are some comments which are mostly nits.
What's the plan for bringing this out of alpha? Is it pending evaluation on
real code?
Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:11
@@ +10,3 @@
+// This defines ObjC
zaks.anna requested changes to this revision.
zaks.anna added a comment.
This revision now requires changes to proceed.
Not sure how you got these changes, but some of them seem wrong and some seem
inconsistently applied.
Has this been tested?
Comment at: C:/LLVM/llvm/tools/cl
zaks.anna added a comment.
Please, test these changes.
Thanks!
Anna.
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/CmpRuns.py:31
@@ -30,3 +30,3 @@
import plistlib
-import CmpRuns
+import CmpRuns # ?
ariccio wrote:
> This file imports itself?
>
> I wa
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM with a request for a tiny other improvement in the documentation.
Thanks!
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h:111
@@ -103,3 +110
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:89
@@ -109,7 +88,3 @@
-static bool synthesizedPropertyRequiresRelease(const ObjCPropertyDecl *PD) {
- // A synthesized property must be released if and only if the kind of setter
- //
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:165
@@ +164,3 @@
+ if (Desc.empty())
+Desc = "use of 'self' after it is freed with call to [super dealloc]";
+
Does "has been freed" sound better?
==
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:300-301
@@ -299,3 +299,4 @@
+ assert(SM && "SourceManager is NULL, cannot iterate through the
diagnostics");
for (std::vector::iterator DI = Diags.begin(),
LGTM
ht
zaks.anna added a comment.
You conditionally defined when you build ON Windows machine, not when you build
FOR Windows. You should be able to query the compiler to check which targets
it's building for.
http://reviews.llvm.org/D18073
___
cfe-commi
zaks.anna added a comment.
Thanks for fixing!
Devin, what do you think about the BugType wording?
Comment at:
llvm/tools/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:229
@@ -228,3 +228,3 @@
BT_stackleak.reset(
-new BuiltinBug(this, "Stack address s
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:686
@@ -685,3 +685,3 @@
const Expr *ArgExpr = nullptr;
if (Idx < Call.getNumArgs())
ArgExpr = Call.getArgExpr(Idx);
This conditional will assert if:
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM.
http://reviews.llvm.org/D18210
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
zaks.anna added a comment.
Once tested, this can land.
http://reviews.llvm.org/D17253
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
> > Also, we should not duplicate all of our tests. Instead, we should add a
> > single test per added API checking that it is modeling the operation we
> > think it
>
> > should model.
>
>
> So should I dump the _dbg versions from the tests?
No but do not
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:653
@@ +652,3 @@
+// name by calling 'getDescriptiveName' recursively.
+else {
+ std::string Idx = ER->getDescriptiveName(false);
xazax.hun wrote:
> Alexander_Droste wr
zaks.anna added a comment.
It's a bug in the checker. dyn_cast should not be called on a null pointer. You
could either check for nun or call dyn_cast_or_null.
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
zaks.anna added a comment.
Can you add one test per new function, which tests that the function if modeled
correctly. Please, do not duplicate all tests that contain a function with it's
windows variants.
Hope this is more clear.
http://reviews.llvm.org/D18073
_
zaks.anna added a comment.
> So for _wcsdup_dbg, I should leave only testWinWcsdupDbg?
Yes.
Also, since there will be many of these "alternate" functions, could you create
a separate test file for them?
http://reviews.llvm.org/D18073
___
cfe-com
zaks.anna added a comment.
You will have to add one test function to smoke test that the newly added API
is modeled correctly. We also have a lot of existing tests that verify that
each of the original APIs (malloc. free, new) function correctly in all
possible settings. What is the contradicti
zaks.anna added a comment.
Gabor,
Can other checkers use this function? I am Ok with this being committed with
limited coverage.
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
zaks.anna added a comment.
Alexander,
This patch is in a pretty good shape. I am fine with committing it and
iterating with smaller updates in tree if it is more convenient for you.
One task that I would like to very strongly encourage is running this on a lot
of code. You will find a lot of i
zaks.anna added a comment.
"Since we are adding support for so many new APIs that are only available on
Windows, could you please condition checking them only when we build for
Windows. You probably can look and Language Options to figure that out."
malloc-uses.c -> "alternative-malloc-api.c" ?
zaks.anna added a comment.
You do not have to supply these. Just declaring and not defining the functions
in the main test file should be sufficient.
http://reviews.llvm.org/D18595
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://list
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Awesome!
Thank you for investing SO MUCH time into improving the checker and addressing
the review comments.
Do you have commit access?
Nit: Please, use lower case letters for test name
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Checkers/CMakeLists.txt:3
@@ +2,3 @@
+ -I ${CMAKE_CURRENT_SOURCE_DIR}/../../../
+ SOURCE ../../../../lib/StaticAnalyzer/Checkers/Checkers.td
+ TARGET ClangSACheckers)
srhines wrote:
> Ann
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:696
@@ -695,1 +695,3 @@
+assert(ArgExpr && "cannot get the type of a NULL expression");
+
LGTM
http://reviews.llvm.org/D19278
___
zaks.anna added a comment.
Would it be possible to generate the diff that shows that the file moved as
opposed to being deleted and added?
http://reviews.llvm.org/D19393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
zaks.anna added a comment.
"Since we are adding support for so many new APIs that are only available on
Windows, could you please condition checking them only when we build for
Windows. You probably can look and Language Options to figure that out."
By this, I was suggesting that we should be c
zaks.anna added a comment.
Thanks for working on this!
The main unfinished task here is to investigate ways of reducing the
performance hit. See more info below.
> The patch was tested on Android open-source platform source code.
Just to double check, have you compare the pre and after result
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM!
One thing to be aware here is that a const pointer could be deleted, so we
should be able to delete a parent object without a warning. (I think that
should work with this patch si
zaks.anna added a comment.
Does FoldingSet already have reserve? (I do not see it.)
My understanding is that you propose to allocate all the nodes that would be
required to store the maximum number of nodes we allow, lowering the malloc
traffic. The current implementation just doubles the size.
zaks.anna added a comment.
Ben,
By the way, thanks for the patch! It's a clever idea.
> The implementation of FoldingSet has a vector of bucket pointers. Reserve
> preallocates that vector of
> bucket pointers to the correct size to avoid rehashing. The allocation of
> the nodes themselves
zaks.anna added a comment.
> Specifically, I suspect that C (and maybe ObjC?) won't hit the analysis limit
> often, but that C++ will hit the
> limit frequently because of the large number of inline functions.
It might be valuable to know this. Maybe we should changer the default for C++?
>
zaks.anna added a comment.
> For the pile of LLVM projects that I am currently building (llvm, clang,
> libcxx, libcxxabi), 18.9% of all analyzed
> functions hit the maximum step count. For the previously discussed large .C
> file, 37% of the analyzed functions hit the maximum step count.
Th
zaks.anna added a comment.
> If the underlying allocator that does a poor job at reusing freed memory,
> then trivial
> functions will use about 1 MB more than before, then free the memory
> immediately.
You could probably flag some of those functions, especially the ones that do
not conta
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks!
http://reviews.llvm.org/D20933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
zaks.anna added a comment.
Ah, right, please, add a comment explaining what we are doing and why.
http://reviews.llvm.org/D20933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
Please, pull out the refactor into a separate commit and ask someone who often
reviews SemaChecking review it.
Comment at: lib/AST/CMakeLists.txt:10
@@ -9,2 +9,3 @@
ASTImporter.cpp
+ ASTStructure.cpp
ASTTypeTraits.cpp
Do we wan
zaks.anna added a comment.
Thanks for the patch!
Have you run this on a large codebase?
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:406
@@ +405,3 @@
+def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
+ HelpText<"Check for blocks in critic
zaks.anna added a comment.
Thanks for the patch! Here are the comments, most of which are nits.
Could you add the high level description of what we are doing somewhere or
maybe just describe the meaning of FunctionSpec since it encodes how functions
are modeled.
Also, we should explain why we
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Please, measure that this does not regress memory consumption just to be
sure.
Thanks!
Anna.
http://reviews.llvm.org/D21229
___
cfe
zaks.anna added a comment.
Thanks!!!
http://reviews.llvm.org/D21682
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
LGTM.
Any other comments to this?
http://reviews.llvm.org/D15225
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
I missed that! Sorry about that.
http://reviews.llvm.org/D15225
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM
http://reviews.llvm.org/D22048
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
zaks.anna accepted this revision.
This revision is now accepted and ready to land.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2075
@@ +2074,3 @@
+void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
+ ExplodedNodeSet &Dst) {
+ Exp
zaks.anna added a comment.
Hi,
Thank you for working on this!
Here are several comments from a **partial** review.
Can you talk a bit about how do you envision to generalize this to copy and
paste detection, where the pasted code is only partially modified? Do you think
this could be generali
zaks.anna added a subscriber: zaks.anna.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h:81
@@ -78,2 +80,3 @@
unsigned computeComplexity() const;
+ const MemRegion *getOriginRegion() const;
};
Please, add a doxygen style comment.
http:
zaks.anna added a comment.
The two underscores in the names are hard to read. Maybe we should just prefix
them with 'win'?
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
zaks.anna added a comment.
Nit: Could you change the prefix from "win" to "win_"?
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
I suggest to update the malloc regression test with these.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
Of cause, we have regression tests for (almost) everything:
ls ./clang/test/Analysis/malloc*
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks!
I can commit this in your behalf.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
zaks.anna added a comment.
Generate a preprocessed file and keep copying the definitions from there until
you reach the types compiler knows about.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
zaks.anna added a comment.
Please, do not submit patches for review before they pass all tests.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
The second: you should not update the diff. When a patch is uploaded, the
assumption is that all tests pass:)
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
zaks.anna added a comment.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
> Side note: Has anybody ever considered just treating fopen and fclose like
> malloc and free?
Yes, there are a few checkers that follow the same open/close rule and could in
theory be merged together.
Comment at: cfe/trunk/lib/StaticAnalyzer/Chec
Author: zaks
Date: Mon Mar 7 19:21:51 2016
New Revision: 262894
URL: http://llvm.org/viewvc/llvm-project?rev=262894&view=rev
Log:
[analyzer] Fix missed leak from MSVC specific allocation functions
Add the wide character strdup variants (wcsdup, _wcsdup) and the MSVC
version of alloca (_alloca) a
zaks.anna added a comment.
You should be able to use the switch like in Darwin::getOSLibraryNameSuffix(),
right? That's much better.
http://reviews.llvm.org/D17947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bi
zaks.anna added a comment.
I thunk the closest we have is region-store* tests, which is not quite the
same. Feel free to add a new test.
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
zaks.anna added a comment.
Since we are adding support for so many new APIs that are only available on
Windows, could you please condition checking them only when we build for
Windows. You probably can look and Language Options to figure that out.
Also, we should not duplicate all of our tests.
zaks.anna added a comment.
Some comments inline. I still have to do an overall pass over this checker, but
it looks much better than the first version!
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:47
@@ +46,3 @@
+BReporter->reportDoubleNonblocking(PreC
zaks.anna added a comment.
I'd be fine if we test this function with the usual regression tests by
observing the output of the MPI checker. We could update that test with more
checks once the function is updated.
With that approach, you'd be committing both patches at the same time.
http://re
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:136
@@ +135,3 @@
+ auto NodeIt = G.eop_begin();
+ const auto NodeEndIt = G.eop_end();
+
The analyzer does not do a good job tracking global variables. You might g
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:136
@@ +135,3 @@
+ auto NodeIt = G.eop_begin();
+ const auto NodeEndIt = G.eop_end();
+
Alexander_Droste wrote:
> zaks.anna wrote:
> > The analyzer does not do a
zaks.anna added a comment.
Why is there such a large jump in the number of warnings reported in the last
patch iteration?
It went from "1678 projects where scanned. In total I got 124 warnings" to "In
2215 projects it found 875 warnings." Did the number of warnings in the initial
1678 projects
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84
@@ +83,3 @@
+// Can E value be greater or equal than Val?
+static bool canBeGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val) {
---
201 - 300 of 363 matches
Mail list logo
