zaks.anna added a comment.
Thanks for fixing this!
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:39
+if (IncludeExperimental ||
+(!CheckName.startswith("debug.") && !CheckName.startswith("alpha.")))
+ Result.push_back(CheckName);
Debu
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D26311
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Looks good. Thank you!
https://reviews.llvm.org/D26310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bi
zaks.anna added a comment.
> Added a python script to merge gcov files.
When the patches that are being reviewed are growing new functionality, it's
much more difficult to review them. Please, submit the python script as a
separate patch.
Comment at: lib/StaticAnalyzer/Core/
zaks.anna marked 4 inline comments as done.
zaks.anna added inline comments.
Comment at: test/CodeGen/sanitize-thread-no-checking-at-run-time.m:1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -x objective-c++ -emit-llvm -o
- %s | FileCheck -check-prefix=WITHOUT %s
+// RUN: %cl
zaks.anna updated this revision to Diff 77435.
zaks.anna marked an inline comment as done.
https://reviews.llvm.org/D25857
Files:
lib/CodeGen/CodeGenFunction.cpp
test/CodeGen/sanitize-thread-no-checking-at-run-time.m
Index: test/CodeGen/sanitize-thread-no-checking-at-run-time.m
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:262
+
+static void dumpCoverageInfo(llvm::SmallVectorImpl &Path,
+ SourceManager &SM) {
xazax.hun wrote:
> zaks.anna wrote:
> > xazax.hun wrote:
> > >
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:219
+
+ assert(LCtx->getKind() == LocationContext::StackFrame &&
+ "Function does not begin with stack frame context");
a.sidorin wrote:
> `isa(LCtx)`?
>
Author: zaks
Date: Fri Nov 11 17:22:44 2016
New Revision: 286672
URL: http://llvm.org/viewvc/llvm-project?rev=286672&view=rev
Log:
[tsan][clang] Introduce a function attribute to disable TSan checking at run
time
This introduces a function annotation that disables TSan checking for the
function
zaks.anna accepted this revision.
This revision is now accepted and ready to land.
Comment at: lib/StaticAnalyzer/Checkers/ObjCGenericsChecker.cpp:226
@@ +225,3 @@
+/// Inputs:
+/// There is a static lower bound (SL)
+/// There is a static upper bound (SL <: SU)
--
zaks.anna added a comment.
honggyu.kim,
Uniquing HTML reports is out of the scope of this patch and should be discussed
elsewhere (either send a design idea to cfe-dev, send a patch for review, or
file a bugzilla request).
I agree that this patch is a definite improvement to issue identificati
zaks.anna added a comment.
Let's make the test more explicit about what is being tested; for example,
void negativeIndex(char *str) {
char *ptr = str + 1;
*ptr = 'a';
clang_analyzer_eval(*ptr == 'a'); // expected-warning{{TRUE}}
ptr = str - 1;
clang_analyzer_eval(*ptr == 'a'); // expect
zaks.anna added a comment.
> > If a' might overflow, then in this case we can emit warning stating that
> > the overflow is caused because a' might overflow.
>
>
> I see your point now! I think we should improve the diagnostic that is
> produced in this case!
How was the following comment
zaks.anna added a comment.
Have you tested this on a large codebase that uses lambdas? When do you think
we should turn this on by default?
Please, add test cases that demonstrate what happens when an issue is reported
within a lambda and to check if inlined defensive checks work.
(As a follow
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Please, do turn on by default.
LGTM
http://reviews.llvm.org/D12652
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:229
@@ -228,2 +228,3 @@
+ /// checkers should use generateErrorNode() instead.
ExplodedNode *generateSink(ProgramStateRef State = nullptr,
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/DynamicTypeMap.cpp:10
@@ +9,3 @@
+//
+// This file defines APIs that are related to track and query dynamic type
+// information. This information can be used to devirtualize calls during the
"
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:244
@@ +243,3 @@
+ const ProgramPointTag *Tag = nullptr) {
+return generateSink(State, /*Pred=*/nullptr,
+ (Tag
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:321
@@ +320,3 @@
+// sink, we assume that a client requesting a transition to a state that is
+// the same as the predecessor state has made a mistake. We return t
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Please, commit after the comments are addressed!
Thank you,
Anna.
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:39
@@ -26,1 +38,3 @@
+// ProgramS
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Please, explain in code why you are doing this. Otherwise, LGTM.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:139
@@ -138,2 +138,3 @@
NullabilityChe
zaks.anna added inline comments.
Comment at: test/Analysis/free.c:1
@@ -1,2 +1,2 @@
-// RUN: %clang_cc1 -analyze -analyzer-store=region
-analyzer-checker=core,unix.Malloc -fblocks -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-store=region
-analyzer-checker=core,unix.Malloc -
zaks.anna requested changes to this revision.
zaks.anna added a comment.
This revision now requires changes to proceed.
Thanks for working on this!
Comments inline.
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td:524
@@ +523,3 @@
+def MPIChecker : Checker<"MPI-C
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:672
@@ +671,3 @@
+
+ if (IsDeclaredAsInstanceType)
+return SelfType;
If you swap this 'if' with the 'if' above, you do not need to check for
IsDeclaredAsInsta
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:680
@@ +679,3 @@
+return nullptr;
+
+ QualType ResultType = Method->getReturnType().substObjCTypeArgs(
From above:
QualType StaticResultType = Method->getRetu
zaks.anna added a comment.
Thanks! Will do.
http://reviews.llvm.org/D10305
___
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/DynamicTypePropagation.cpp:871
@@ +870,3 @@
+ if (RetRegion) {
+State = setDynamicTypeInfo(State, RetRegion, ResultType,
+ /*CanBeSubclass=*/true);
Would it
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:871
@@ +870,3 @@
+ if (RetRegion && !State->get(RetRegion)) {
+// TODO: we have duplicated information in DynamicTypeMap and
+// MostSpecializedTypeArgsMap. We should only
zaks.anna added a comment.
> > It's more user friendly to report this issue at the last point where the
> > request is available rather than the last line of the function.
>
> > This looks similar to leak report checking. Is it?
>
>
> Yes, that's not very common but possible.
> Does th
zaks.anna added a comment.
This new patch does not seem to build on top of http://reviews.llvm.org/D10305
but is an alternative way of generating the hash that reuses a lot of the
building blocks from the other patch. What is the reason for that?
(It also addresses your comment to this patch an
zaks.anna added a comment.
I see, Maybe we should add a new test file to test this instead of adding it to
an existing test.
http://reviews.llvm.org/D12119
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Otherwise, LGTM.
http://reviews.llvm.org/D12119
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
zaks.anna accepted this revision.
This revision is now accepted and ready to land.
Comment at: test/Analysis/generics.m:239
@@ -238,3 +238,3 @@
NSArray *b = a;
- NSString *str = [b getObjAtIndex: 0]; // expected-warning {{Conversion}}
+ NSString *str = [b getObjAtIndex: 0]; /
zaks.anna added a comment.
Thanks for the fix! We'll commit shortly.
Repository:
rL LLVM
http://reviews.llvm.org/D12119
___
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 agree with Gabor and do not think we should have 2 entries in the plist file
for bug identification. This is confusing and I do not see a need for this.
Any comparison script can check for 2 fields in a plist file and concatenate
them. We've already discussed this d
zaks.anna added a comment.
Hi Babati,
As far as I can see, the following comments from June 15th have not been
addressed. It would be good if you could address them in the latest revision.
"I would be interested in either replacing "issue_hash" or adding
"issue_hash_bug_line_content" (or somet
zaks.anna added a comment.
> Sorry, I should have been more precise. The check tests if a request is in
> use by a nonblocking
> call at the end of a function. But the request can still be alive after
> return. You can think of it as
> a pointer for which the memory must be freed in the fun
zaks.anna added a comment.
nit: Please, use proper punctuation in the comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1616
@@ +1615,3 @@
+builder.isFeasible(false) && !StFalse &&
+BldCtx.blockCount() == AMgr.options.maxBlockVisitOnPath - 1) {
+
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
See the suggestion for an improved comment. Otherwise, LGTM!
Thanks!
Anna.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:902
@@ +901,3 @@
+} else {
+
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1616
@@ +1615,3 @@
+builder.isFeasible(false) && !StFalse &&
+BldCtx.blockCount() == AMgr.options.maxBlockVisitOnPath - 1) {
+
seaneveson wrote:
> zaks.anna wrote:
zaks.anna added a comment.
The analyzer has a notion of ConstPointerEscape, see checkConstPointerEscape
callback.
All the pointers to const parameters are escaped this way. The implementation
for that is in CallEvent::invalidateRegions, right below the code you've added:
for (unsigned Idx = 0,
zaks.anna added a comment.
Looks like this patch is causing regressions:
https://llvm.org/bugs/show_bug.cgi?id=24914
Repository:
rL LLVM
http://reviews.llvm.org/D12652
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
zaks.anna added a comment.
> I think I understand, but to clarify:
> The fields that shouldn't be invalidated should still be added to
> ValuesToInvalidate, but with
> RegionAndSymbolInvalidationTraits::TK_PreserveContents set. This will result
> in
> checkConstPointerEscape being called
zaks.anna added a comment.
We gave a talk about building checkers a while back; even though it does not
talk about the bug reporter visitors, it might be worth watching if you haven't
already seen it.
http://llvm.org/devmtg/2012-11/
Building a Checker in 24 hours
Comment at:
zaks.anna requested changes to this revision.
zaks.anna added a comment.
This revision now requires changes to proceed.
I find it very confusing to have 2 different fields for bug identification -
issue_hash and BugId. Why do we need to treat the HTML reports differently from
plist files?
http
zaks.anna added inline comments.
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h:67
@@ +66,3 @@
+ // Every time a request is 'set' a new 'RequestId' gets created.
+ // Therefore, the 'UserKind' does not need to be profiled.
+ const int RequestId{Reque
zaks.anna added a comment.
Could you address this:
Could you ask on the open source list if people are using "issue_hash" and if
they are Ok with us renaming it.
I'd suggest to suffix each issue hash field with the description of that hash.
For example, we would remove "issue_hash" and replace
zaks.anna added a comment.
This checker produces a lot of warnings! Have you analyzed how many are false
positives? Have you tried reporting these warnings?
It's hard to make use of the results you posted from the debian packages. For
most of them, I cannot tell if they are valid reports or fal
zaks.anna added a comment.
> Generating mangled names requires ASTContext which is not available during
> the error reporting. BugReporter does have the ASTContext, so it would not
> be a big change to add it to the DiagnosticConsumers though. And I think the
> mangled name might contain too
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thanks for working on this!
LGTM, Anna.
http://reviews.llvm.org/D10305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://list
zaks.anna created this revision.
zaks.anna added a reviewer: dcoughlin.
zaks.anna added subscribers: xazax.hun, cfe-commits.
Herald added a subscriber: aemerson.
The analyzer assumes that system functions will not free memory or modify the
arguments in other ways, so we assume that arguments do n
zaks.anna added a comment.
We need a way to test this functionality. One way of testing this would be to
write unit tests. Gabor has already added such tests for the static analyzer.
http://reviews.llvm.org/D10305
___
cfe-commits mailing list
cfe-c
zaks.anna added a comment.
honggyu.kim,
You are right, CmpRuns.py does not work with HTML files.
The list of HTML reports is produced by scan-build and there is no facility
there to search for newly generated reports. It is also not clear that there
should be one HTML file per report. This is
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1098
@@ +1097,3 @@
+ if (!NumElements)
+return;
+ QualType ElementTy = AT->getElementType();
What happens on early returns? Here and the one below. Are there tes
Author: zaks
Date: Fri Aug 14 15:22:22 2015
New Revision: 245093
URL: http://llvm.org/viewvc/llvm-project?rev=245093&view=rev
Log:
[analyzer] Add checkers for OS X / iOS localizability issues
Add checkers that detect code-level localizability issues for OS X / iOS:
- A path sensitive checker th
zaks.anna added a comment.
Committed earlier today in r245093.
Thank you for contributing the checker!
http://reviews.llvm.org/D11572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna requested changes to this revision.
zaks.anna added a comment.
This revision now requires changes to proceed.
This patch needs tests.
We should treat the improvements to HTML uniquing separately from this patch.
> But I think it's not a critical issue as of now and it can be considered
zaks.anna requested changes to this revision.
zaks.anna added a comment.
This revision now requires changes to proceed.
I believe we saw a regression in diagnostics with this modification. Gabor, do
you recall what it was?
http://reviews.llvm.org/D11433
__
zaks.anna added a comment.
> x = a/b; where n < b
> malloc (x*n); Then x*n will not overflow
I am not convinced that the new rule is strong enough. 'a' can be any
expression. For example, maybe you have (b-1)*a/b and the denominator cancels
out something unrelated to 'n' in the numerator? Ma
zaks.anna added a comment.
> Maybe, I should add a check that `a, b, n' are positive.
> So, in this case static analyzer can choose to be strict and reject false
> positives.
What would this buy us? Does the checker warn on underflow?
> If a' might overflow, then in this case we can emit war
zaks.anna added a comment.
I have some minor nits but looks good otherwise. Thanks for fixing this!
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:290
@@ -289,2 +289,3 @@
const ProgramPointTag *Tag = nullptr) {
-
zaks.anna added a comment.
Please, commit this after committing the nullability checker so that this could
have tests. Two tests need to be added:
1. the reference case
2. the attribute nonnull case
Also, I'd suggest adding a field to ImplicitNullDerefEvent instead of creating
a new event.
h
zaks.anna added a comment.
Partial review...
Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:137
@@ +136,3 @@
+def NullPassedToNonnullChecker : Checker<"NullPassedToNonnull">,
+ HelpText<"Warns when a null pointer is passed to a nonnull pointer.">,
+ DescFile<"NullabilityC
zaks.anna added a comment.
Another partial review.
Thanks!
Anna.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:312
@@ +311,3 @@
+ CheckerContext &C) const {
+ auto RetExpr = S->getRetValue();
+ if (!RetExpr)
--
zaks.anna added a comment.
Gabor, there is a lot of the same steps that all callbacks go through. I think
refactoring those into subroutines will help with readability of the checker.
Looking forward to seeing a new version soon!
Anna.
Comment at: lib/StaticAnalyzer/Checkers/N
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Looks good overall. I might have additional post-commit comments.
http://reviews.llvm.org/D11468
___
cfe-commits mailing list
cfe-commits@l
zaks.anna requested changes to this revision.
zaks.anna added a comment.
This revision now requires changes to proceed.
I agree that the way the static analyzer handles loops with known bounds is
quite bad and we loose coverage because of it, so this is definitely an
important problem to solve!
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thanks!
http://reviews.llvm.org/D11433
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
zaks.anna added a comment.
Partial review in-line.
Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:456
@@ -455,2 +455,3 @@
def ObjCGenericsChecker : Checker<"ObjCGenerics">,
HelpText<"Check for incorrect usages of parameterized types.">,
+ DescFile<"DynamicTypePropagati
zaks.anna added a comment.
> I accept that my current patch is not a comprehensive solution to the problem
> and that it may introduce > false positives, however I do think it is an
> improvement, where it is preferable to have false positives
> over doing no analysis after the loop.
We try
zaks.anna added a comment.
Please, make sure to include a very specific commit title/message.
http://reviews.llvm.org/D6551
___
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/NullabilityChecker.cpp:166
@@ +165,3 @@
+ // nullability precondition is violated it will not report any bugs at all.
+ void reportBug(ErrorKind Error, ExplodedNode *N, const MemRegion *Region,
+
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:166
@@ +165,3 @@
+ /// report anything and turn off the check.
+ ///
+ /// When \p SuppressPath is set to true, no more bugs will be reported on
this
It is still no
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:523
@@ -510,2 +522,3 @@
REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair)
+REGISTER_MAP_WITH_PROGRAMSTATE(ReallocSizeZeroFlag, SymbolRef, ReallocSizeZero)
-
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks!
Anna.
http://reviews.llvm.org/D12445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
zaks.anna added a comment.
I do not understand why there should be a cycle here in the Exploded Graph.
Most likely this assertion uncovers a bug in cfg-temporary-doors, which is a
disabled by default feature (it has not been completed yet). You can think of
"ProgramState" as the state of the pr
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:99
@@ +98,3 @@
+ "Nullable pointer is assigned to a "
+ "pointer which has _Nonnull type",
+
zaks.anna added a comment.
> Again I'm not sure this is worth checking for, based on the assumption that a
> reasonable const method won't modify the relevant object. What do people
> think?
These are heuristics. I think dealing with this case should not be a
pre-requisite for the patch. We c
zaks.anna added a comment.
I agree with Gabor. We should investigate how we can model the overflow on a
cast correctly.
http://reviews.llvm.org/D12901
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
zaks.anna added a comment.
> Even if you do not deal with this case I think it would be great to have an
> XFAIL test with a fixme that describes this limitation.
Yes! Every time we have a test case that shows known limitations, we should add
it to the regression tests with a TODO explaining t
zaks.anna added a comment.
Now that we have a way to test symbol reaper, please, add more coverage to the
symbol-reaper.c test, including the test that Jordan mentioned. Even if it is
not fixed, it's good to include it with a FIXME note.
What is the performance impact of this change?
The chang
zaks.anna added a comment.
> I don't notice any significant performance hit with this
> patch (+/- 2% on the whole Android codebase runs).
What did you test it on?
> we're covering all three places that were patched; maybe i could move these
> tests
> to symbol-reaper.c as well, but i gue
zaks.anna accepted this revision.
This revision is now accepted and ready to land.
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:419
@@ +418,3 @@
+const Expr *Ex = getCXXThisExpr();
+while (isa(Ex)) {
+ Ex = cast(Ex)->getSubExpr();
Use IgnoreParen
zaks.anna added a comment.
> so it'd take more time for me to conduct a deeper audit of the reaping code
> to answer this question
That would be great, thanks!
> And i guess it should be large and varied enough(?)
Yes.
http://reviews.llvm.org/D12726
_
zaks.anna added a comment.
> Should we require the generation of old hashes once a change is introduced,
> or should we expect users
> who rely on old hash to maintain the old hash generation as an out of tree
> patch?
We should maintain the old hashes, at least for a while, if there are peo
zaks.anna added a comment.
Should this be closed?
http://reviews.llvm.org/D12906
___
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/Analysis/BodyFarm.h:43
@@ +42,3 @@
+ /// \brief Merge the attributes found in model files.
+ /// Note, this adds all attributes found in the model file without any sanity
+ /// checks.
Why do we not have sanity c
zaks.anna added a comment.
> > In http://reviews.llvm.org/D10305#224956, @zaks.anna wrote:
>
> > For example, you could keep the information about the reports in the plist
> > files and use those to
>
> > render the reports in HTML.
>
>
> If you're okay with adding HTML file name in
zaks.anna added a comment.
Hi Daniel,
Have you considered contributing this work to clang/llvm?
http://reviews.llvm.org/D12906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: zaks
Date: Tue Oct 27 15:19:45 2015
New Revision: 251449
URL: http://llvm.org/viewvc/llvm-project?rev=251449&view=rev
Log:
[analyzer] Assume escape is possible through system functions taking void*
The analyzer assumes that system functions will not free memory or modify the
arguments in o
Author: zaks
Date: Tue Oct 27 15:19:38 2015
New Revision: 251448
URL: http://llvm.org/viewvc/llvm-project?rev=251448&view=rev
Log:
[analyzer] Enhance FAQ with instructions on handing unused variables.
Modified:
cfe/trunk/www/analyzer/faq.html
Modified: cfe/trunk/www/analyzer/faq.html
URL:
h
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:692-693
@@ -691,4 +691,2 @@
CXXBasePaths Paths(false, false, false);
- if (RD->lookupInBases(
- [DeclName](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) {
--
zaks.anna added a comment.
Thanks for the patch!
Some minor comments inline.
What happens when this checker and the security.insecureAPI.vfork are enabled
at the same time?
Did you run this checker on a large body of code? Did it find any issues? What
is needed to turn this into a non-alpha c
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:45
@@ +44,3 @@
+ CheckerContext &C) {
+ const Expr *CE = Call.getOriginExpr();
+
ygribov wrote:
> It seems that other checkers do more or l
zaks.anna added a comment.
Thanks for addressing all the comments. A couple of additional ones below.
Comment at: test/Analysis/vfork.c:60
@@ +59,3 @@
+// Ensure that returning from function is prohibited.
+return 0;
+ }
Shouldn't the warning appear her
zaks.anna added a comment.
This needs a test case. You can either extend ./test/Analysis/malloc.cpp or add
another QT specific test file.
Thanks!
Anna.
http://reviews.llvm.org/D14170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
zaks.anna added a comment.
See the comment below, otherwise, LGTM.
Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:23
@@ +22,3 @@
+
+def OptIn : Package<"optin">;
+
Please, add a comment describing for how this package should be used as you do
in the commit
zaks.anna added a comment.
Test cases need to be small and self contained.
You might be having a problem with reproducing with a simple test case, where
you define QCoreApplication::postEvent in the test file because it is not
considered to be in the system header. You can use special pragmas
zaks.anna added a comment.
> > What is needed to turn this into a non-alpha checker?
>
>
> I guess that's question for maintainers) We have to consider that vfork is
> used rarely enough.
Please, make this into a non-alpha, turned on by default.
Thank you!
Anna.
http://reviews.llvm.org/
zaks.anna added a comment.
Ok. Thanks!
I'll commit shortly.
http://reviews.llvm.org/D14014
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
101 - 200 of 363 matches
Mail list logo
