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) {
---
zaks.anna added a comment.
Could you add the reduced false positives to the tests file?
> As far as I see the diagnostics are showing the proper path now..
What do you mean? Does this refer to supplying more information the the path
about why the error occurs?
Comment at: li
zaks.anna added inline comments.
Comment at: test/Analysis/MPIChecker.cpp:98
@@ +97,3 @@
+
+ MPI_Request req;
+ MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
&req); // expected-note{{Request is previously used by nonblocking call here.}}
---
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:95
@@ +94,3 @@
+ if (Optional SP = N->getLocation().getAs()) {
+if (const CallExpr *CE = clang::dyn_cast(SP->getStmt())) {
+
Would it be possible to identi
zaks.anna added a comment.
Anton,
Have you investigated if we can reuse code from clang-tidy?
Also, the hope is that the python rewrite of scan-build will replace the
current scan-build in the near future. Of cause, it still needs to be tested on
Windows. I would really appreciate if you could
zaks.anna accepted this revision.
This revision is now accepted and ready to land.
Comment at: lib/Driver/Tools.cpp:3609
@@ -3602,2 +3608,3 @@
// Enable the following experimental checkers for testing.
+ if (!IsPS4CPU) {
These are no longer experimen
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def:31
@@ +30,3 @@
+// is both instantiated and derived from.
+// Additionally, its kind is not its name with "Kind" suffix,
+// unlike all other regions.
I'd rath
Author: zaks
Date: Tue Jan 5 18:32:52 2016
New Revision: 256886
URL: http://llvm.org/viewvc/llvm-project?rev=256886&view=rev
Log:
[analyzer] Suppress reports coming from std::__independent_bits_engine
The analyzer reports a shift by a negative value in the constructor. The bug can
be easily trig
Author: zaks
Date: Tue Jan 5 18:32:49 2016
New Revision: 256885
URL: http://llvm.org/viewvc/llvm-project?rev=256885&view=rev
Log:
[analyzer] Don't report null dereferences on address_space annotated memory
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
cfe/trunk/t
Author: zaks
Date: Tue Jan 5 18:32:56 2016
New Revision: 256887
URL: http://llvm.org/viewvc/llvm-project?rev=256887&view=rev
Log:
[analyzer] Fix false warning about memory leak for QApplication::postEvent
According to Qt documentation Qt takes care of memory allocated for QEvent:
http://doc.qt.i
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:31
@@ +30,3 @@
+struct VAListAcceptingFunc {
+ mutable IdentifierInfo *II;
+ StringRef FuncName;
It does not support ObjC methods.
I think this is most useful to checker
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:309
@@ -286,1 +308,3 @@
+ /// \brief Returns true if the CallEvent is call to a function that matches
+ /// the CallDescription.
"is call" -> "is a ca
zaks.anna added a comment.
> This patch also fixes a bug in 'RangeSet::pin' causing single value ranges to
> not be considered conventionally ordered.
Can that fix be submitted as a separate patch? Is there a test for it?
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:35
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def:31
@@ +30,3 @@
+// is both instantiated and derived from.
+// Additionally, its kind is not its name with "Kind" suffix,
+// unlike all other regions.
NoQ wrot
zaks.anna added a comment.
What was the last outstanding issue with this patch (I believe it was related
to either make or cmake changes)?
Has it been addressed?
Repository:
rL LLVM
http://reviews.llvm.org/D15611
___
cfe-commits mailing list
cf
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Sounds good. Please, split this into 2 patches (each fixing the separate
problem) and commit.
Thanks!
Anna.
http://reviews.llvm.org/D12901
__
zaks.anna added a comment.
> MemSpaceRegion is now an abstract base
What prevents it from being instantiated?
http://reviews.llvm.org/D16062
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312
@@ +311,3 @@
+ /// calls.
+ bool isCalled(const CallEvent &Call, const CallDescription &CD);
+
The API is a bit awkward. Maybe it would be better if
zaks.anna accepted this revision.
zaks.anna added a comment.
Laszlo,
I am very excited about having the new and much improved scan-build in tree! It
will serve as a solid foundation for moving forward.
Thank you for all your hard work!
Anna.
http://reviews.llvm.org/D9600
__
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Thank you!
http://reviews.llvm.org/D15448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
zaks.anna added a comment.
> I see, so essentially you want to use a different approach for determining
> sanitizer availability (on OS X for now): if the library is present, then we
> support
> sanitizer, otherwise we don't: i.e. the binary distribution is the source of
> truth, not the lis
zaks.anna updated this revision to Diff 45437.
zaks.anna added a comment.
Refactor the code to address Samsonov's review.
http://reviews.llvm.org/D15624
Files:
lib/Driver/ToolChains.cpp
lib/Driver/ToolChains.h
test/Driver/fsanitize.c
Index: test/Driver/fsanitize.c
===
zaks.anna marked an inline comment as done.
zaks.anna added a comment.
I've introduced the helper function. Looks like addProfileRTLibs might be able
to use it in the future (after support for autoconf is dropped) as well.
http://reviews.llvm.org/D15624
__
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM
http://reviews.llvm.org/D15921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
zaks.anna added a comment.
> I don't know, is there a way to install runtime components for ASan if your
> distribution doesn't happen to have one (that must be tricky, as the version
> of ASan should match the version of the compiler).
Correct, there is no recommended way of installing the
zaks.anna updated this revision to Diff 45720.
zaks.anna added a comment.
Thanks for spotting the bug! The bug is fixed and the tests are added in this
revision.
http://reviews.llvm.org/D15624
Files:
lib/Driver/ToolChains.cpp
lib/Driver/ToolChains.h
test/Driver/darwin-sanitizer-ld.c
te
Author: zaks
Date: Fri Jan 22 18:45:37 2016
New Revision: 258591
URL: http://llvm.org/viewvc/llvm-project?rev=258591&view=rev
Log:
[analyzer] Fixup r258572 Utility to match function calls.
Initialize the IdentifierInfo pointer. Hope this fixes the buildbot breakage.
Modified:
cfe/trunk/inclu
zaks.anna added a comment.
Looks like the 'II' pointer wasn't initialized. Should be fixed by r258591.
Repository:
rL LLVM
http://reviews.llvm.org/D15921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
Author: zaks
Date: Fri Jan 29 12:43:15 2016
New Revision: 259221
URL: http://llvm.org/viewvc/llvm-project?rev=259221&view=rev
Log:
[analyzer] Improve Nullability checker diagnostics
- Include the position of the argument on which the nullability is violated
- Differentiate between a 'method' and
zaks.anna added a comment.
Hi Alexander,
Sorry for the delay!
The patch should be rebased from the clang repo; for example, you could run
"svn diff" from the clang directory. More comments inline.
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td:75
@@ -74,1 +74
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Looks like all of Gabor's comments were addressed. LGTM. Thank you!
http://reviews.llvm.org/D16063
___
cfe-commits mailing list
cfe-commits
zaks.anna closed this revision.
zaks.anna added a comment.
Committed in r251449.
http://reviews.llvm.org/D13488
___
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: test/Analysis/inline.cpp:308
@@ +307,3 @@
+clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}}
+clang_analyzer_eval(0 != ((char *)void_string)[1]); //
expected-warning{{TRUE}}
+ }
Why are we che
zaks.anna added a comment.
> I may be mistaken, but this check looks more appropriate for Clang-tidy.
This is a syntactic check. Both clang-tidy as well as the clang static analyzer
contain this type of checks. If we move all syntactic checks to clang-tidy, the
users that use the analyzer but
Author: zaks
Date: Wed Nov 18 19:25:28 2015
New Revision: 253532
URL: http://llvm.org/viewvc/llvm-project?rev=253532&view=rev
Log:
[analyzer] Improve modeling of static initializers.
Conversions between unrelated pointer types (e.g. char * and void *) involve
bitcasts which were not properly mode
zaks.anna added a comment.
Fixed and committed.
Comment at: test/Analysis/inline.cpp:308
@@ +307,3 @@
+clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}}
+clang_analyzer_eval(0 != ((char *)void_string)[1]); //
expected-warning{{TRUE}}
+ }
--
zaks.anna added a comment.
> > Also, how is this different from -analyzer-config?
>
>
> -analyzer-config is used to transfers a number of options to the analyzer,
> while configuration file is
> used to customize scan-build, ccc-analyzer and c++-analyzer scripts.
Could you elaborate on
zaks.anna added inline comments.
Comment at: test/Analysis/qt_malloc.cpp:11
@@ +10,3 @@
+ QCoreApplication::postEvent(obj, event);
+ QApplication::postEvent(obj, event);
+}
Should the leak be reported when the object is passed to
QApplication::postEvent?
In
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thanks!
LGTM. I'll commit.
http://reviews.llvm.org/D14170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi
zaks.anna added a comment.
This is a partial review. I did not look at the padding calculations closely.
Have you run this over codebases other than clang? Are there any false
positives?
> Even with the default of 8, this checker is too noisy to justify turning on
> by default. Clang+LLVM has
zaks.anna added a comment.
> I have seen plenty of structures where the specific layout was important and
> couldn't be changed.
Can you give specific examples of these? Can we develop heuristics for them?
> These generally felt like noisy reports unless I had more specific
> justification fo
zaks.anna added a comment.
> can be suppressed without breaking ABI by adding explicit padding members.
We should convey the suppression mechanism as part of the diagnostic.
http://reviews.llvm.org/D14779
___
cfe-commits mailing list
cfe-commits@l
zaks.anna added a comment.
> And I really like how the clang-tidy guys were doing it.
It would be great if the clang static analyzer config file would be similar to
the clang-tidy one. We would avoid user confusion if they are consistent. Is
some reuse possible? Currently, when clang-tidy call
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM.
http://reviews.llvm.org/D14919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
zaks.anna added a comment.
> An analysis of llvm+clang+compiler-rt now only generates 16 excessive padding
> reports in index.html.
Should those be fixed or are they all false positives?
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:48
@@ +47,3 @@
+// The cal
zaks.anna added a comment.
Pierre,
Have you seen the post about API Notes?
http://llvm.cc/t/cfe-dev-clang-and-swift/331
I believe using API notes would be a better approach for adding annotations. By
the time the static analyzer sees the AST, the annotations would already be
there. The API Not
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM.
(Feel free to add comments to the existing code!)
> So the real question is whether (or rather how) the Store should maintain
> correct region liveness information
> after comp
zaks.anna added a comment.
I've been mostly looking at the path sensitive checks. Maybe clang-tidy team
would be interested in reviewing the syntactic checks.
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Container.h:31
@@ +30,2 @@
+
+#endif
A
zaks.anna added a comment.
This patch contains a mix of checks, some are path sensitive and some are AST
checks. See MPICheckerAST.cpp for the latter.
http://reviews.llvm.org/D12761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lis
zaks.anna added a comment.
Hi Laszlo,
Here are some comments from me.
Should we be worried about the name conflicts (between old scan-build and this
tool) during rollout? I think it would be beneficial to rename the tools, but
let's discuss the names later. (If we integrate Codecheck, that wil
zaks.anna added a comment.
Hi Laszlo,
Comment at: tools/scan-build-py/libear/ear.c:281
@@ +280,3 @@
+
+DLSYM(func, fp, "execve");
+
This is not the recommended way of interposing on Darwin. All you need to do is
provide your function, which can call the fun
zaks.anna added inline comments.
Comment at: tools/scan-build-py/bin/analyze-c++:2
@@ +1,3 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# The LLVM Compiler Infrastructure
Where/How is analyze-c++ used?
Comment at: tools
zaks.anna added inline comments.
Comment at: tools/scan-build-py/README.md:86
@@ +85,3 @@
+The 2. mode is available only on FreeBSD, Linux and OSX. Where library preload
+is available from the dynamic loader. On OSX System Integrity Protection
security
+feature enabled prevents l
zaks.anna added inline comments.
Comment at: test/Analysis/inlining/analysis-order.c:13
@@ +12,3 @@
+
+// CHECK: analysis-order.c test2
+// CHECK: analysis-order.c test1
Can you use CHECK-NEXT instead?
Repository:
rL LLVM
http://reviews.llvm.org/D15410
__
zaks.anna added a comment.
Overall, looks good!
Comment at: tools/scan-view/bin/scan-view:94
@@ +93,3 @@
+import argparse
+parser = argparse.ArgumentParser()
+parser.add_argument("root", metavar="", type=str)
Please, add the tool description, like "T
zaks.anna added a comment.
With respect to the issues this checker found, I suggest submitting patches for
the clang issues that can be fixed. Can the x-macro case be suppressed with the
recommended suppression? I'd also submit a patch to gtest. Submitting patches
with the fixes provides a good
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:49
@@ -48,1 +48,3 @@
+def Performance : Package<"performance">;
+
zaks.anna wrote:
> I think Performance should be in the OptIn package.
What do you think about this one?
http
zaks.anna added a comment.
I wonder if we can refactor the code so that it is less error prone..
shouldSkipFunction(D, Visited, VisitedAsTopLevel) works with two sets. I assume
that you have not updated Decls coming from VisitedAsTopLevel because they come
from the CFG and should already be can
zaks.anna added a comment.
Can/Should something like this be used when dumping SVals (during debugging)?
(Possibly in addition to the debug checker.)
What are the advantages of implementing this using visitors? Can this be
implemented similarly to SVal::dumpToStream? Do you envision other use ca
zaks.anna added a comment.
Sorry, I forgot to read the description before commenting; I see it is intended
to be used not only for debugging purposes:)
http://reviews.llvm.org/D15448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li
zaks.anna added a comment.
Looks good overall; comments below.
Please, provide more information on real world code evaluation. Which codebases
this has been tested on? What was the false positive rate? How many real bugs
were found/fixed?
What is the criteria for taking it out of alpha?
Pleas
zaks.anna added a comment.
Are you saying that we need to rename "SymbolValKind" to "SymbolKind"? That
would probably be a tiny change.
http://reviews.llvm.org/D15448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
zaks.anna created this revision.
zaks.anna added reviewers: kcc, cfe-commits.
Change the clang driver to accept ASan on iOS/watchOS/tvOS.
This change along with the corresponding changes in llvm and compiler-rt
complete ASan support for iOS/watchOS/tvOS.
http://reviews.llvm.org/D15624
Files:
zaks.anna added inline comments.
Comment at: lib/Driver/ToolChains.cpp:368
@@ +367,3 @@
+ StringRef OS = "";
+ if (isTargetMacOS())
+OS = "osx";
samsonov wrote:
> Wait, this looks horrible. Can we teach toolchain to give us OS name?
These are not OS names; t
zaks.anna added a comment.
Sorry for the wait!
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
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: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:134
+else if (I->isUnsigned())
+ OS << I->getZExtValue() << ", which is";
+else
Please print single quotes around the value.
===
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thanks!
https://reviews.llvm.org/D36737
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
zaks.anna accepted this revision.
zaks.anna added a comment.
Once the comments by @paquette are addressed, LGTM. Thanks!
Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:138
+
+OS << " larger or equal to the width of type '"
+ << B->getLHS()->get
zaks.anna added a comment.
Looks like the need to have the checker name in BugType along with the checker
names not being initialized early enough, leads to worse checker-writer
experience. Is there a way to ensure that the checker names are set at
construction?
Comment at:
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:651-652
+ } else if (StoreSite->getLocation().getAs()) {
+os << "Reach the max loop limit.";
+os << " Assigning a conjured symbol";
+if (R->canPrintPretty()) {
--
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:651-652
+ } else if (StoreSite->getLocation().getAs()) {
+os << "Reach the max loop limit.";
+os << " Assigning a conjured symbol";
+if (R->canPrintPretty()) {
--
zaks.anna accepted this revision.
zaks.anna added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:1674
const Decl *D = CalleeLC->getDecl();
-addEdgeToPath(PD.getActivePath(), PrevLoc,
-
zaks.anna added a comment.
These are great additions!
Going back to my comment about adding these to CheckerContext, I think we
should be adding helper functions as methods on CheckerContext as it is **the
primary place where checker writers look for helpers**. Two of the three
methods added t
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:454
+def UnixAPIPortabilityChecker : Checker<"API">,
+ HelpText<"Finds implementation-defined behavior in UNIX/Posix functions">,
+ DescFile<"UnixAPIChecker.cpp">;
Do
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:454
+def UnixAPIPortabilityChecker : Checker<"API">,
+ HelpText<"Finds implementation-defined behavior in UNIX/Posix functions">,
+ DescFile<"UnixAPIChecker.cpp">;
No
zaks.anna added a comment.
This generally sounds good. Definitely do submit these changes in small pieces!
See http://llvm.org/docs/DeveloperPolicy.html#incremental-development for
rationale.
https://reviews.llvm.org/D27753
___
cfe-commits mailing
zaks.anna added a comment.
Great cleanup! Some comments below.
Comment at: www/analyzer/alpha_checks.html:91
+(C, C++)
+Check for logical errors for function calls and Objective-C message
+expressions (e.g., uninitialized arguments, null function pointers,
fo
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Looks good with a nit!
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:325
+ Loc makeNull() { return loc::ConcreteInt(BasicVals.getZeroWith
zaks.anna added a comment.
Should this revision be "abandoned" or is the plan to iterate on it?
Repository:
rL LLVM
https://reviews.llvm.org/D31320
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:356
QualType ResultTy) {
- if (!State->isTainted(RHS) && !State->isTainted(LHS))
-return UnknownVal();
I am concerned that removing the g
zaks.anna added a comment.
> -(Anna) Scan-build-py integration of the functionality is nearly finished
> (see https://github.com/rizsotto/scan-build/issues/83) (--ctu switch performs
> both analysis phases at once). This I think could go in a different patch,
> but until we could keep the ctu-b
zaks.anna added inline comments.
Comment at: www/analyzer/alpha_checks.html:91
+(C, C++)
+Check for logical errors for function calls and Objective-C message
+expressions (e.g., uninitialized arguments, null function pointers,
szdominik wrote:
> zaks.anna wrote
zaks.anna added a comment.
> Maybe we should introduce another UMK_* for deeper analysis instead?
The difference here is not substantial enough to warrant a new level. The
general motivation for bumping these numbers is that we've set the timeouts
many years ago and the hardware improved quite
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Once Artem gives more details about the codebase he tested on, I think it would
be fine to commit this. We can revert/address concerns later if @a.sidorin or
anyone else raises concerns
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thanks! Do you have commit access?
Repository:
rL LLVM
https://reviews.llvm.org/D34266
___
cfe-commits mailing list
cfe-commits@lists.ll
zaks.anna added a comment.
> eg. checkers for portability across linux/bsd should be off on windows by
> default, checkers for non-portable C++ APIs should be off in plain C code, etc
Is the checker you are moving to portability off and not useful on Windows?
https://reviews.llvm.org/D34102
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Thank you!
(Not sure if @NoQ wants to do a final review.)
Do you have commit access or should we commit on your behalf?
https://reviews.llvm.org/D30406
___
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:356
QualType ResultTy) {
- if (!State->isTainted(RHS) && !State->isTainted(LHS))
-return UnknownVal();
ddcc wrote:
> zaks.anna wrote:
> >
zaks.anna added a comment.
> In particular, there are patches to generate more symbolic expressions, that
> could also degrade the performance (but fix some fixmes along the way).
The patch you are talking about might be promising, but needs much more
investigation and tuning for performance vs
zaks.anna added a comment.
> Hmm, should there be new tests that demonstrate that we cover the new APIs?
Unless we use a new registration mechanism for some of these APIs, I'd be fine
without adding a test for every API that has localization constraints.
Repository:
rL LLVM
https://reviews.
zaks.anna added a comment.
This just supports the statement that this particular check should not go under
unix. I understand that it will be inconsistent with the name of the malloc
checker, which we probably should not change as people might be relying on the
package names. I think it's bette
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
if (symLHS && symRHS &&
- (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
+ (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
re
zaks.anna added a comment.
I think we should have these is .rst format as this is what the rest of the
documentation predominantly uses. (Note, the formatting can be very basic, it's
the format that I care about.)
Otherwise, great to see this addition!
https://reviews.llvm.org/D36737
_
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:190
+ /* Default = */ false);
+ return shouldUnrollLoops() || explicitlyIncludeLoopExit;
}
I would rather keep this method
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
if (symLHS && symRHS &&
- (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
+ (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
re
zaks.anna added a comment.
Is this blocked on the same reasons as what was raised in
https://reviews.llvm.org/D35109?
https://reviews.llvm.org/D35110
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
zaks.anna added a comment.
> But I've never used the taint tracking mode, so I don't know what would be a
> reasonable default for MaxComp.
that one is very experimental anyway. I'd just keep the functional changes to
tain out of this patch and use the current default that taint uses.
https:/
zaks.anna added a comment.
Thanks for addressing the non-determinism!!!
The ExplodedNodeSet is predominantly used to hold very small sets and it is
used quite a bit in the analyzer. Maybe we could we use SmallSetVector here
instead?
https://reviews.llvm.org/D37400
_
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thank you!
Anna
https://reviews.llvm.org/D37400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
zaks.anna added a comment.
How about committing the refactor of the code without test modifications. And
committing changes to the test separately?
https://reviews.llvm.org/D37809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
301 - 400 of 526 matches
Mail list logo
