NoQ added a comment.
Hmm, curious, having a look. A couple of blind guesses before i actually
understand what's going on:
(1) The `simplifySVal()` code has its own complexity threshold:
1060 SVal VisitNonLocSymbolVal(nonloc::SymbolVal V) {
1061 // Simplification is much more costl
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Thanks, this was very useful, please commit!
Repository:
rL LLVM
https://reviews.llvm.org/D31868
___
cfe-commits mailing list
cfe-commits@lists.llvm
NoQ added a comment.
I'm sorry, i'd try to get back to this and unblock your progress as soon as
possible.
One thing i notice is that you manipulate symbolic expressions manually,
however many of the things that you need, eg stuff in your `compose()` method,
seem to be already available in `SV
NoQ added a comment.
Hmm, should there be new tests that demonstrate that we cover the new APIs?
Repository:
rL LLVM
https://reviews.llvm.org/D34266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
NoQ added a comment.
Gabor makes such a good point. Maybe we should commit the zombie symbols patch
as well (:
https://reviews.llvm.org/D34277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
NoQ added a comment.
In https://reviews.llvm.org/D34102#783161, @zaks.anna wrote:
> > 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
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Hmm, yeah, right :)
Repository:
rL LLVM
https://reviews.llvm.org/D34502
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.or
NoQ added a comment.
Regarding serializing vs not serializing and now vs later.
1. I think we eventually need to provide a reasonable default approach
presented to the user. This approach shouldn't be hurting the user dramatically
in any sense. Because //serializing// hurts the user's disk spac
NoQ added a comment.
In https://reviews.llvm.org/D32642#787913, @baloghadamsoftware wrote:
> I tried `SValBuilder::evalBinOp()` first but it did not help too much. It
> could decide only if I compared the same conjured symbols or different ones,
> but nothing more. It always gave me `UnknownVal
NoQ added a comment.
In https://reviews.llvm.org/D32642#788822, @baloghadamsoftware wrote:
> > I'm sure that simplification `(($x + N) + M) ~> ($x + (M + N))` is already
> > working in `SValBuilder`.
>
> No, it is unfortunately not working: I tried to increase `${conj_X}` by `1`,
> then again b
NoQ added a comment.
For example,
**`$ cat test.c`**
void clang_analyzer_dump(int);
int bar();
void foo() {
int x = bar();
clang_analyzer_dump(x);
++x;
clang_analyzer_dump(x);
++x;
clang_analyzer_dump(x);
}
**`$ ~/debug/bin/clang -cc1 -analyze -analyzer-che
NoQ added a comment.
Maxim, totally thanks for picking this up!
Could you explain the idea behind `shouldDeferScopeEnd`, maybe in a code
comment before the function?
In https://reviews.llvm.org/D16403#788926, @m.ostapenko wrote:
> Current patch should support basic {If, While, For, Compound, S
NoQ added a comment.
In https://reviews.llvm.org/D32642#789004, @baloghadamsoftware wrote:
> Now I can improve `SValBuilder` to compare `{conj_X}+n` to `conj_X}+m`, but I
> am not sure if it helps to simplify `compare()` much. How to handle cases
> where I have to compare `{conj_X}+n` to `{conj
NoQ added a comment.
In https://reviews.llvm.org/D40809#954858, @dcoughlin wrote:
> One possibility is to turn this into a debug checker similar to
> debug.ViewExplodedGraph. That checker registers for a checkEndAnalysis()
> callback and traverses the node graph (see DebugCheckers.cpp). Can you
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs, eraman.
C++ overridable `operator new()` has the following prototype:
void *operator new(size_t size, user-defined arguments...);
The retu
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs, eraman.
`bugreporter::trackNullOrUndefValue()` checker API function extends a bug
report with a recursive family of bug report visitors (`Ret
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
This addresses the regression in https://reviews.llvm.org/D41254 in
`inlining/path-notes.cpp` by adding a new straightforward mechanism that
NoQ added a comment.
In https://reviews.llvm.org/D40809#955929, @dcoughlin wrote:
> As it stands, this is a debugging tool not a way to communicate error paths
> to end users. (I think that, too, would be very helpful but I'd like to see
> this as a debugging aid first.) The workflow for debugg
NoQ added inline comments.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:605
+ if (Pos && !Pos->isValid()) {
+// If I do not put a tag here, some invalidation tests will fail
+static CheckerProgramPoint
NoQ added a comment.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
This patch would be in a good shape once we settle the rearrangement stuff. I
had a look at all follow-up patches and identified other, hopefully smaller,
places where i have overall design concerns; otherwise, the rest
NoQ added inline comments.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:340-358
+const auto *Templ = Func->getPrimaryTemplate();
+if (!Templ)
+ return;
+
+const auto *TParams = Templ->getTempla
NoQ added inline comments.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1489-1511
+SymbolRef replaceSymbol(SymbolManager &SymMgr, SymbolRef OrigExpr,
+SymbolRef OldExpr, SymbolRef NewSym
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
This looks clear to me.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:554-555
+
+ verifyMatch(C, State->getSVa
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
This looks clear to me.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1449-1464
+const CXXRecordDecl *getCXXRec
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
This looks clear to me.
https://reviews.llvm.org/D32904
___
cfe-commits mailing list
cfe-commits
NoQ added inline comments.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1584-1588
+ auto stateFound = state->BindExpr(CE, LCtx, RetVal);
+ auto stateNotFound = state->BindExpr(CE, LCtx, SecondParam);
+
+ C.a
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
I really hope that i'd be able to do something about pass-by-copy in C++,
because there's a lot of mess there. I approve this trick unless i act
NoQ added a comment.
Herald added a subscriber: rnkovacs.
In https://reviews.llvm.org/D35109#943558, @baloghadamsoftware wrote:
> In https://reviews.llvm.org/D35109#937763, @NoQ wrote:
>
> > For the type extension approach, somebody simply needs to do the math and
> > prove that it works soundly
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Default global `operator new()`, like `malloc()`, should return heap pointers,
which in the analyzer are represented by `SymbolicRegion`s wit
NoQ updated this revision to Diff 127202.
NoQ added a comment.
`VisitCXXNewExpr` is too late. We need to perform cast before calling the
constructor. Otherwise bad things happen, for instance `performTrivialCopy`
would construct another void region :)
Move the cast to `pushCXXNewAllocatorValue(
NoQ updated this revision to Diff 127420.
NoQ added a comment.
- Fix pop from empty stack.
- Add recursive operator new tests.
- Disable argument invalidation when the allocator was inlined (needed for
those tests to work)
In https://reviews.llvm.org/D40560#957653, @xazax.hun wrote:
> I think
NoQ added a comment.
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:522
+symVal = peekCXXNewAllocatorValue(State);
+ State = popCXXNewAllocatorValue(State);
+
a.sidorin wrote:
> Should this be under 'if' as well?
Whops!! Thanks!
Apparently
NoQ added a comment.
I think this commit starts to make sense, so i removed the "WIP" marker. I
guess it's better to leave todos 2 and 4 to follow-up commits, and 1 and 3 are
already addressed.
https://reviews.llvm.org/D40560
___
cfe-commits maili
NoQ updated this revision to Diff 127436.
NoQ added a comment.
- Actually fix the comment about why we need to act on null or undefined values.
- Fix `for(:)` whitespace.
https://reviews.llvm.org/D40560
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/C
NoQ updated this revision to Diff 127437.
NoQ added a comment.
- Fix more `for(:)` whitespace.
https://reviews.llvm.org/D40560
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticA
NoQ marked 2 inline comments as done.
NoQ added a comment.
Sorry for the spam.
https://reviews.llvm.org/D40560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ updated this revision to Diff 127548.
NoQ added a comment.
- Fix comments as suggested by Devin.
- Point out that arithmetic on void pointers is a GNU extension.
https://reviews.llvm.org/D40939
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
lib/StaticAnalyzer/Core/Ex
NoQ added a subscriber: lebedev.ri.
NoQ added a comment.
@lebedev.ri wrote:
> No tests?
This patch adds an assertion => Multiple existing tests immediately starts
crashing => This patch fixes the crashes with the new functionality.
So in my understanding the new assertion is all the tests we
NoQ updated this revision to Diff 127549.
NoQ added a comment.
Rebase.
> I also noticed that `evalCast` from `void *` to `T *` is uncomfortable to use
> because sometimes it transforms `&SymRegion{$x}` into `&element{T, 0S32b,
> SymRegion{$x}}` even when `$x` is already of type `T *`. The form
NoQ updated this revision to Diff 127560.
NoQ added a comment.
Rebase.
Remove the redundant cast that is done in `c++-allocator-inlining` mode when
modeling array new. After rebase it started causing two identical element
regions top appear on top of each other.
https://reviews.llvm.org/D4126
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added a subscriber: rnkovacs.
This patch is roughly based on the discussion we've had in
http://lists.llvm.org/pipermail/cfe-dev/2017-December/056314.html about how our
support for C
NoQ updated this revision to Diff 127579.
NoQ added a comment.
- Actually call the new callback when the allocator call is inlined.
- Update checker documentation :)
https://reviews.llvm.org/D41406
Files:
include/clang/StaticAnalyzer/Core/Checker.h
include/clang/StaticAnalyzer/Core/CheckerM
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Fun C++ fact: definition
void *operator new(std::size_t size, std::nothrow_t ¬hrow) throw() { ... }
does not override the global nothrow o
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
When operator new() is inlined, diagnostic pieces may appear within it. They'd
be surrounded by `Calling 'operator new'` and `Returning from
NoQ added a comment.
I guess i'd commit it together with https://reviews.llvm.org/D41250 in a single
commit, so that there obviously were tests.
https://reviews.llvm.org/D40939
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.ll
NoQ added a comment.
TODOs for the future commits:
- Constructor shouldn't cause pointer escape of the newly allocated pointer
immediately after the NewAllocator callback, otherwise we ain't gonna find no
leaks. Without `c++-allocator-inlining`, we only started tracking the pointer
after the c
NoQ added inline comments.
Comment at: test/Analysis/NewDelete-custom.cpp:7
-#if !LEAKS
+#if !(LEAKS && !ALLOCATOR_INLINING)
// expected-no-diagnostics
a.sidorin wrote:
> Double negation can be simplified a bit: `#if !LEAKS || ALLOCATOR_INLINING`
The rest of t
NoQ added a comment.
In https://reviews.llvm.org/D41406#960824, @xazax.hun wrote:
> Maybe `debug.AnalysisOrder` could be used to test the callback order
> explicitly. This way the test could also serve as a documentation for the
> callback order.
Yep, totally, will do.
https://reviews.llvm.
NoQ added a comment.
1. Devin suggested a fantastic idea: it may be great to have a new
`LocationContext` for whatever happens within the big-new-expression. This
would help immensely with CFG look-aheads and look-behinds and reduce parent
map usage - not only for operator new, but for other co
NoQ added a comment.
A slower explanation of the approach in '3.' in the previous message:
(1) Evaluate operator new() aka the allocator call as usual.
(2) Take the return value of (1) as usual.
(3) Take `CXXConstructExpr` which is the child of the `CXXNewExpr` that
triggered the allocator call
NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.
https://developer.apple.com/library/content/releasenotes/ObjectiveC/RN-TransitioningToARC/Introduction/Introduction.html#//apple_ref/doc/uid/TP40011226-
NoQ added a comment.
So, essentially, `LoopContext` is per-loop, while `ScopeContext` is
per-iteration?
Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:28-46
struct LoopState {
private:
enum Kind { Normal, Unrolled } K;
- const Stmt *LoopStmt;
- const LocationCont
NoQ added a comment.
Or not. Loop counter has its own whole-loop scope.
I guess `LoopContext` can be treated as a sub-class of `ScopeContext`. And i
don't mind having `ScopeContext` be split into small distinct sub-classes.
Because we're stuck in cornercases for covering all possible scopes, wh
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yep, so it still doesn't answer if `removePiecesWithInvalidLocations()` was
dead code to begin with (considering that the assertion in
`adjustCallLocations()` says that everything but call locations
NoQ added a comment.
Because it wasn't immediately obvious to me how to apply both this patch and
https://reviews.llvm.org/D39398 (there are a couple of minor conflicts between
them), i wanted to post a few pictures for the reference, because
`debug.ViewCFG` graphs are much easier to comprehend
NoQ added a comment.
(edited my comment: fixed the suggested change to mention the right block)
https://reviews.llvm.org/D41150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ added a comment.
Herald added subscribers: dkrupp, a.sidorin, rnkovacs.
These CFGs make perfect sense to me so far, and i guess this is a must-have.
Thank you!
> Note: In case of IndirectGotoStmt, it could happen that we generate LoopExit
> elements even for loops which is not exited by tha
NoQ added a comment.
//*asked stuff in https://reviews.llvm.org/D39398 regarding how indirect gotos
are supported*//
https://reviews.llvm.org/D41151
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
NoQ added a comment.
I'm still not quite sure what's the whole point of having `BugType` without a
checker. We can still easily write anything we want in the "category" and
"name" fields anyways, so we can easily produce bugs that are indistinguishable
to the user from different checkers, while
NoQ added a comment.
This patch makes a totally valid point :)
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:513
+ int ArraySize = -1, StrLen = -1;
+ const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
You might want to use a wider i
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.
Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h:51
StringRef getCategory() const { return Category; }
- StringRef getCheckName() const { return Check.ge
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs, mehdi_amini.
Similarly to how we allow (since https://reviews.llvm.org/D40560) inlining the
constructor after `operator new` which isn't `ope
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:692-693
+ if (const Stmt *DtorExpr = Dtor.getOriginExpr())
+if (const Stmt *ParentExpr =
+CurLC->getParentMap().getParent(DtorExpr))
+ if (const CXXDel
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov.
Herald added subscribers: cfe-commits, rnkovacs, szepet.
This continues the series of fine-tuning of how everything behaves in
`-analyzer-config c++-allocator-inlining=true` mode with respects to ca
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
This one's easy. Under `-analyzer-config c++-allocator-inlining=true`, since
https://reviews.llvm.org/D41406, we've teached `MallocChecker` t
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Make use of the new callback introduced in https://reviews.llvm.org/D41406 for
tracking values allocated by `operator new()` in `-analyzer-co
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
This addresses a TODO from https://reviews.llvm.org/D41406. I re-used
`PostImplicitCall` program point when calling the new callback, but it
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2906-2907
Out << "\\lPostLValue\\l";
+else if (Loc.getAs())
+ Out << "\\lPostAllocatorCall\\l";
{F5743196}
Repository:
rC Clang
https://reviews.llvm.o
NoQ added a comment.
In https://reviews.llvm.org/D35109#968314, @baloghadamsoftware wrote:
> But how to add a flag for this? Is it a flag enabled by the user or is it
> automatically enabled if the checker is enabled?
I guess it'd be an `-analyzer-config` flag. You can add it to the
`Analyzer
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
This makes sense. @dcoughlin: does this harmonize with your original intent for
adding this suppression in the first place?
https://reviews.llvm.org/D41749
__
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Neat, thank you!
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:992-993
+/// Walk through nodes until we get one that matches the statement exactly.
+/// Alternately,
NoQ added a comment.
In https://reviews.llvm.org/D41406#970985, @xazax.hun wrote:
> Also, I was wondering if it would make sense to only have the PostCall
> callback return the casted memory region in these specific cases instead of
> introducing a new callback.
Yep, i actually think it's a v
NoQ added a comment.
> which would re-evaluate the cast and return the casted object
Rather not. Because i'm changing my mind again about avoiding the redundant
cast in `&element{T, HeapSymRegion{conj}}` - this time by not calling
`evalCast` after a conservatively evaluated operator new call (t
NoQ added a comment.
> Like, make a new `CallEvent` sub-class called, say, `CXXNewAllocatorCall`
Oh, we already have it (`CXXAllocatorCall`).
https://reviews.llvm.org/D41406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.
NoQ added a comment.
But still, i guess, it is also easier for checker authors to discover a checker
callback (it's right there in the `CheckerDocumentation`, which is short enough
to read fully) than to discover a `CallEvent` sub-class (which was so hard that
i never discovered it until like n
NoQ updated this revision to Diff 129210.
NoQ added a comment.
That thing didn't immediately work, because there are a lot of other places
where we need to put the value, not just the Store, before entering the
constructor - such as our constructor call events for checker callbacks. It'd
be har
NoQ updated this revision to Diff 129211.
NoQ added a comment.
Rebase.
https://reviews.llvm.org/D40939
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
Index: lib/StaticAnalyzer/Core/S
NoQ updated this revision to Diff 129212.
NoQ added a comment.
In https://reviews.llvm.org/D41250#959755, @NoQ wrote:
> > I also noticed that `evalCast` from `void *` to `T *` is uncomfortable to
> > use because sometimes it transforms `&SymRegion{$x}` into `&element{T,
> > 0S32b, SymRegion{$x}
NoQ updated this revision to Diff 129213.
NoQ added a comment.
Rebase. Address the comment.
In https://reviews.llvm.org/D41266#959763, @NoQ wrote:
> Remove the redundant cast that is done in `c++-allocator-inlining` mode when
> modeling array new. After rebase it started causing two identical e
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:477
+bool ExprEngine::isStandardGlobalOperatorNewFunction(const CXXNewExpr *CNE) {
+ const FunctionDecl *FD = CNE->getOperatorNew();
+ if (FD && !isa(FD) && !FD->isVariadic()) {
dc
NoQ added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:311
+ ExprEngine &Eng,
+ bool wasInlined = false);
+
dcoughlin wrote:
> Does 'wasInlined' really need to ha
NoQ updated this revision to Diff 129214.
NoQ marked 6 inline comments as done.
NoQ added a comment.
Address review comments. Stick to the callback approach for now.
https://reviews.llvm.org/D41406
Files:
include/clang/StaticAnalyzer/Core/Checker.h
include/clang/StaticAnalyzer/Core/CheckerM
NoQ updated this revision to Diff 129215.
NoQ marked an inline comment as done.
NoQ added a comment.
Fix the comment.
https://reviews.llvm.org/D41800
Files:
include/clang/Analysis/ProgramPoint.h
lib/StaticAnalyzer/Core/CheckerManager.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
lib/Static
NoQ added inline comments.
Comment at: include/clang/Analysis/ProgramPoint.h:592
+ friend class ProgramPoint;
+ PostAllocatorCall() {}
+ static bool isKind(const ProgramPoint &Location) {
xazax.hun wrote:
> Maybe `= default` is getting more canonical within LL
NoQ abandoned this revision.
NoQ added a comment.
> because the new behavior of operator new is to return an ElementRegion
> surrounding the void pointer
The new behavior was reverted in https://reviews.llvm.org/D41250#971888 so this
patch is no longer useful.
Repository:
rC Clang
https://
NoQ abandoned this revision.
NoQ added a comment.
> is not quite intended (but not addressed yet)
https://reviews.llvm.org/D41250#971888 addresses the issue.
> However, if the operator is inlined
It isn't. Because if it is inlined, `MallocChecker` bails out immediately and
doesn't try to model
NoQ planned changes to this revision.
NoQ added a comment.
https://reviews.llvm.org/D41250#971888 reverts the change in how casts work,
and the checker doesn't really need the casted value, because it only tracks
the pointer.
This still leaves us with the problem of `PostStmt` being called
twi
NoQ added a comment.
In https://reviews.llvm.org/D41406#970985, @xazax.hun wrote:
> Do you have a plan for the new false negatives when `c++-allocator-inlining`
> is on? Should the user mark allocation functions with attributes?
Not immediately - the immediate plan is to simply believe that we
NoQ added a comment.
In https://reviews.llvm.org/D35109#969712, @baloghadamsoftware wrote:
> In https://reviews.llvm.org/D35109#969109, @NoQ wrote:
>
> > I guess it'd be an `-analyzer-config` flag. You can add it to the
> > `AnalyzerOptions` object, which has access to these flags and can be
>
NoQ added a comment.
In https://reviews.llvm.org/D35110#969782, @baloghadamsoftware wrote:
> Strange, but modifying the tests from `m n` to `m - n
> 0` does not help. The statement `if (m - n 0)` does not store a
> range for `m - n` in the constraint manager. With the other patch which
> a
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks great. @dcoughlin: would you approve the warning message text?
Maybe actually we could print out the exact numbers that cause the bit shift to
overflow, since we do have them when we check.
NoQ updated this revision to Diff 129362.
NoQ added a comment.
Rename getters and setters for the new state trait (i.e. "push" -> "set", etc.,
because we no longer have a stack).
Also make them static, so that it was potentially possible to access them from
elsewhere, eg. from `CallEvent`, if d
NoQ updated this revision to Diff 129365.
NoQ added a comment.
Rebase.
Add a FIXME to bring back the cast in the conservative case.
https://reviews.llvm.org/D41250
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/new-ctor-
NoQ updated this revision to Diff 129367.
NoQ added a comment.
Rebase (fix minor conflict).
https://reviews.llvm.org/D41266
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/NewDelete-checker-test.cpp
test/Analysis/new-cto
NoQ updated this revision to Diff 129369.
NoQ added a comment.
Rebase (getter rename).
https://reviews.llvm.org/D41406
Files:
include/clang/StaticAnalyzer/Core/Checker.h
include/clang/StaticAnalyzer/Core/CheckerManager.h
lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
lib/StaticAna
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs, eraman.
Even if we later change how these callbacks work (as in
http://lists.llvm.org/pipermail/cfe-dev/2017-December/056314.html), i wanted
NoQ added inline comments.
Comment at: test/Analysis/cxxnewexpr-callback-inline.cpp:31-32
+// CHECK-NEXT: PostStmt
+// CHECK-NEXT: PreCall (foo)
+// CHECK-NEXT: PostCall (foo)
This ensures that there are no other callbacks after `PostStmt`, in
particular that th
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.
As suggested by Gabor in https://reviews.llvm.org/D41800, replace `{}` with `=
default` for `ProgramPoint` default constructors.
Repository:
rC Clang
http
NoQ added a comment.
Had a fresh look on the C++ part, it is super clean now, i'm very impressed :)
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:373-374
- return RuntimeDefinition();
+ auto Engine = static_cast(
+ getState()->getStateManager().getOwningEngine());
NoQ added inline comments.
Comment at: test/Analysis/NewDelete-path-notes.cpp:44
// CHECK-NEXT:
-// CHECK-NEXT:line6
+// CHECK-NEXT:line7
// CHECK-NEXT:col3
a.sidorin wrote:
> Not even a minor concern for this patc
301 - 400 of 3498 matches
Mail list logo
