| Age | Commit message (Collapse) | Author |
|---|
|
FindLastStoreBRVisitor is responsible for finding where a particular region
gets its value; if the region is a VarRegion, it's possible that value was
assigned at initialization, i.e. at its DeclStmt. However, if a function is
called recursively, the same DeclStmt may be evaluated multiple times in
multiple stack frames. FindLastStoreBRVisitor was not taking this into
account and just picking the first one it saw.
<rdar://problem/13787723>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180997 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
There were actually two bugs here:
- if we decided to look for an interesting lvalue or call expression, we
wouldn't go find its node if we also knew we were at a (different) call.
- if we looked through one message send with a nil receiver, we thought we
were still looking at an argument to the original call.
Put together, this kept us from being able to track the right values, which
means sub-par diagnostics and worse false-positive suppression.
Noticed by inspection.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180996 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Introduce a new helper function, which computes the first symbolic region in
the base region chain. The corresponding symbol has been used for assuming that
a pointer is null. Now, it will also be used for checking if it is null.
This ensures that we are tracking a null pointer correctly in the BugReporter.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179916 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
method not called.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179776 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
In the committed example, we now see a note that tells us when the pointer
was assumed to be null.
This is the only case in which getDerefExpr returned null (failed to get
the dereferenced expr) throughout our regression tests. (There were multiple
occurrences of this one.)
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179736 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
dot and non-reference base
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179734 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
type of the expression.
Thanks to Jordan for suggesting the fix.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179732 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
tracking is not live in the last node of the path
We always register the visitor on a node in which the value we are tracking is live and constrained. However,
the visitation can restart at a node, later on the path, in which the value is under constrained because
it is no longer live. Previously, we just silently stopped tracking in that case.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179731 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Jordan's code review for r179396
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179571 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
There are few cases where we can track the region, but cannot print the note,
which makes the testing limited. (Though, I’ve tested this manually by making
all regions non-printable.) Even though the applicability is limited now, the enhancement
will be more relevant as we start tracking more regions.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179396 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179395 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Previously, the analyzer used isIntegerType() everywhere, which uses the C
definition of "integer". The C++ predicate with the same behavior is
isIntegerOrUnscopedEnumerationType().
However, the analyzer is /really/ using this to ask if it's some sort of
"integrally representable" type, i.e. it should include C++11 scoped
enumerations as well. hasIntegerRepresentation() sounds like the right
predicate, but that includes vectors, which the analyzer represents by its
elements.
This commit audits all uses of isIntegerType() and replaces them with the
general isIntegerOrEnumerationType(), except in some specific cases where
it makes sense to exclude scoped enumerations, or any enumerations. These
cases now use isIntegerOrUnscopedEnumerationType() and getAs<BuiltinType>()
plus BuiltinType::isInteger().
isIntegerType() is hereby banned in the analyzer - lib/StaticAnalysis and
include/clang/StaticAnalysis. :-)
Fixes real assertion failures. PR15703 / <rdar://problem/12350701>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179081 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178934 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
state and removing redundant code.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178933 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Also had to modify the PostInitializer ProgramLocation to contain the field region.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178826 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
when condition is unknown
Improvement of r178684 and r178685.
Jordan has pointed out that I should not rely on the value of the condition to know which expression branch
has been taken. It will not work in cases the branch condition is an unknown value (ex: we do not track the constraints for floats).
The better way of doing this would be to find out if the current node is the right or left successor of the node
that has the ternary operator as a terminator (which is how this is done in other places, like ConditionBRVisitor).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178701 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
select Exprs
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178685 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
1) Look for the node where the condition expression is live when checking if
it is constrained to true or false.
2) Fix a bug in ProgramState::isNull, which was masking the problem. When
the expression is not a symbol (,which is the case when it is Unknown) return
unconstrained value, instead of value constrained to “false”!
(Thankfully other callers of isNull have not been effected by the bug.)
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178684 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
This is controlled by the 'suppress-c++-stdlib' analyzer-config flag.
It is currently off by default.
This is more suppression than we'd like to do, since obviously there can
be user-caused issues within 'std', but it gives us the option to wield
a large hammer to suppress false positives the user likely can't work
around.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178513 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
visitor for nil receiver
We can check if the receiver is nil in the node that corresponds to the StmtPoint of the message send.
At that point, the receiver is guaranteed to be live. We will find at least one unreclaimed node due to
my previous commit (look for StmtPoint instead of PostStmt) and the fact that the nil receiver nodes are tagged.
+ a couple of extra tests.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178381 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
trackNullOrUndefValue.
trackNullOrUndefValue tries to find the first node that matches the statement it is tracking.
Since we collect PostStmt nodes (in node reclamation), none of those might be on the
current path, so relax the search to look for any StmtPoint.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178380 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
participates in the computation of the nil we warn about.
We should only suppress a bug report if the IDCed or null returned nil value is directly related to the value we are warning about. This was
not the case for nil receivers - we would suppress a bug report that had an IDCed nil receiver on the path regardless of how it’s
related to the warning.
1) Thread EnableNullFPSuppression parameter through the visitors to differentiate between tracking the value which
is directly responsible for the bug and other values that visitors are tracking (ex: general tracking of nil receivers).
2) in trackNullOrUndef specifically address the case when a value of the message send is nil due to the receiver being nil.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178309 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
reclaimed
The visitor should look for the PreStmt node as the receiver is nil in the PreStmt and this is the node. Also, tag the nil
receiver nodes with a special tag for consistency.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178152 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Register the nil tracking visitors with the region and refactor trackNullOrUndefValue a bit.
Also adds the cast and paren stripping before checking if the value is an OpaqueValueExpr
or ExprWithCleanups.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178093 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Also, replace a std::string with a SmallString.
No functionality change.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177352 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
wrapped in a cast.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177205 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177204 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Silences a few false positives in LLVM.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177186 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177138 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Allows the suppression visitors trigger more often.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177137 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
sure it attaches in the given edge case
In the test case below, the value V is not constrained to 0 in ErrorNode but it is in node N.
So we used to fail to register the Suppression visitor.
We also need to change the way we determine that the Visitor should kick in because the node N belongs to
the ExplodedGraph and might not be on the BugReporter path that the visitor sees. Instead of trying to match the node,
turn on the visitor when we see the last node in which the symbol is ‘0’.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177121 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
When BugReporter tracks C++ references involved in a null pointer violation, we
want to differentiate between a null reference and a reference to a null pointer. In the
first case, we want to track the region for the reference location; in the second, we want
to track the null pointer.
In addition, the core creates CXXTempObjectRegion to represent the location of the
C++ reference, so teach FindLastStoreBRVisitor about it.
This helps null pointer suppression to kick in.
(Patch by Anna and Jordan.)
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176969 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
r176737 fixed bugreporter::trackNullOrUndefValue to find nodes for an lvalue
even if the rvalue node had already been collected. This commit extends that
to call statement nodes as well, so that if a call is contained within
implicit casts we can still track the return value.
No test case because node reclamation is extremely finicky (dependent on
how the AST and CFG are built, and then on our current reclamation rules,
and /then/ on how many nodes were generated by the analyzer core and the
current set of checkers). I consider this a low-risk change, though, and
it will only happen in cases of reclamation when the rvalue node isn't
available.
<rdar://problem/13340764>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176829 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
same node it was registered at
The visitor used to assume that the value it’s tracking is null in the first node it examines. This is not true.
If we are registering the Suppress Inlined Defensive checks visitor while traversing in another visitor
(such as FindlastStoreVisitor). When we restart with the IDC visitor, the invariance of the visitor does
not hold since the symbol we are tracking no longer exists at that point.
I had to pass the ErrorNode when creating the IDC visitor, because, in some cases, node N is
neither the error node nor will be visible along the path (we had not finalized the path at that point
and are dealing with ExplodedGraph.)
We should revisit the other visitors which might not be aware that they might get nodes, which are
later in path than the trigger point.
This suppresses a number of inline defensive checks in JavaScriptCore.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176756 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
r176010 introduced the notion of "interesting" lvalue expressions, whose
nodes are guaranteed never to be reclaimed by the ExplodedGraph. This was
used in bugreporter::trackNullOrUndefValue to find the region that contains
the null or undef value being tracked.
However, the /rvalue/ nodes (i.e. the loads from these lvalues that produce
a null or undef value) /are/ still being reclaimed, and if we couldn't
find the node for the rvalue, we just give up. This patch changes that so
that we look for the node for either the rvalue or the lvalue -- preferring
the former, since it lets us fall back to value-only tracking in cases
where we can't get a region, but allowing the latter as well.
<rdar://problem/13342842>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176737 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Previously, ReturnVisitor waited to suppress a null return path until it
had found the inlined "return" statement. Now, it checks up front whether
the return value was NULL, and suppresses the warning right away if so.
We still have to wait until generating the path notes to invalidate the bug
report, or counter-suppression will never be triggered. (Counter-suppression
happens while generating path notes, but the generation won't happen for
reports already marked invalid.)
This isn't actually an issue today because we never reclaim nodes for
top-level statements (like return statements), but it could be an issue
some day in the future. (But, no expected behavioral change and no new
test case.)
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176736 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
node” rather than last “non-null”.
The second modification does not lead to any visible result, but, theoretically, is what we should
have been looking at to begin with since we are checking if the node was assumed to be null in
an inlined function.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176576 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Inlining brought a few "null pointer use" false positives, which occur because
the callee defensively checks if a pointer is NULL, whereas the caller knows
that the pointer cannot be NULL in the context of the given call.
This is a first attempt to silence these warnings by tracking the symbolic value
along the execution path in the BugReporter. The new visitor finds the node
in which the symbol was first constrained to NULL. If the node belongs to
a function on the active stack, the warning is reported, otherwise, it is
suppressed.
There are several areas for follow up work, for example:
- How do we differentiate the cases where the first check is followed by
another one, which does happen on the active stack?
Also, this only silences a fraction of null pointer use warnings. For example, it
does not do anything for the cases where NULL was assigned inside a callee.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176402 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Most map types have an operator[] that inserts a new element if the key
isn't found, then returns a reference to the value slot so that you can
assign into it. However, if the value type is a pointer, it will be
initialized to null. This is usually no problem.
However, if the user /knows/ the map contains a value for a particular key,
they may just use it immediately:
// From ClangSACheckersEmitter.cpp
recordGroupMap[group]->Checkers
In this case the analyzer reports a null dereference on the path where the
key is not in the map, even though the user knows that path is impossible
here. They could silence the warning by adding an assertion, but that means
splitting up the expression and introducing a local variable. (Note that
the analyzer has no way of knowing that recordGroupMap[group] will return
the same reference if called twice in a row!)
We already have logic that says a null dereference has a high chance of
being a false positive if the null came from an inlined function. This
patch simply extends that to references whose rvalues are null as well,
silencing several false positives in LLVM.
<rdar://problem/13239854>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176371 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Consider this case:
int *p = 0;
p = getPointerThatMayBeNull();
*p = 1;
If we inline 'getPointerThatMayBeNull', we might know that the value of 'p'
is NULL, and thus emit a null pointer dereference report. However, we
usually want to suppress such warnings as error paths, and we do so by using
FindLastStoreBRVisitor to see where the NULL came from. In this case, though,
because 'p' was NULL both before and after the assignment, the visitor
would decide that the "last store" was the initialization, not the
re-assignment.
This commit changes FindLastStoreBRVisitor to consider all PostStore nodes
that assign to this region. This still won't catches changes made directly
by checkers if they re-assign the same value, but it does handle the common
case in user-written code and will trigger ReturnVisitor's suppression
machinery as expected.
<rdar://problem/13299738>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176201 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
This is essentially the same problem as r174031: a lazy binding for the first
field of a struct may stomp on an existing default binding for the
entire struct. Because of the way RegionStore is set up, we can't help
but lose the top-level binding, but then we need to make sure that accessing
one of the other fields doesn't come back as Undefined.
In this case, RegionStore is now correctly detecting that the lazy binding
we have isn't the right type, but then failing to follow through on the
implications of that: we don't know anything about the other fields in the
aggregate. This fix adds a test when searching for other kinds of default
values to see if there's a lazy binding we rejected, and if so returns
a symbolic value instead of Undefined.
The long-term fix for this is probably a new Store model; see
<rdar://problem/12701038>.
Fixes <rdar://problem/13292559>.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176144 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
a VarRegion.
Fixes PR15358 and <rdar://problem/13295437>.
Along the way, shorten path diagnostics that say "Variable 'x'" to just
be "'x'". By the context, it is obvious that we have a variable,
and so this just consumes text space.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176115 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
looking for is always a VarRegion.
This was triggering assertion failures when analyzing the LLVM codebase. This
is fallout from r175988.
I've got delta chewing away on a test case, but I wanted the fix to go
in now.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176011 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
ExplodedNode pruning.
r175988 modified the ExplodedGraph trimming algorithm to retain all
nodes for "lvalue" expressions. This patch refines that notion to
only "interesting" expressions that would be used for diagnostics.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176010 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
fields.
This required more changes than I originally expected:
- ObjCIvarRegion implements "canPrintPretty" et al
- DereferenceChecker indicates the null pointer source is an ivar
- bugreporter::trackNullOrUndefValue() uses an alternate algorithm
to compute the location region to track by scouring the ExplodedGraph.
This allows us to get the actual MemRegion for variables, ivars,
fields, etc. We only hand construct a VarRegion for C++ references.
- ExplodedGraph no longer drops nodes for expressions that are marked
'lvalue'. This is to facilitate the logic in the previous bullet.
This may lead to a slight increase in size in the ExplodedGraph,
which I have not measured, but it is likely not to be a big deal.
I have validated each of the changed plist output.
Fixes <rdar://problem/12114812>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@175988 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
This provides a few sundry cleanups, and allows us to provide
a compile-time check for a case that was a runtime assertion.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@175987 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
See r175462 for another example/more details.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@175812 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Post-commit CR feedback from Jordan Rose regarding r175594.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@175679 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
See r175462 for another example/more details.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@175594 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
This is a more natural order of evaluation, and it is very important
for visualization in the static analyzer. Within Xcode, the arrows
will not jump from right to left, which looks very visually jarring.
It also provides a more natural location for dataflow-based diagnostics.
Along the way, we found a case in the analyzer diagnostics where we
needed to indicate that a variable was "captured" by a block.
-fsyntax-only timings on sqlite3.c show no visible performance change,
although this is just one test case.
Fixes <rdar://problem/13016513>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174447 91177308-0d34-0410-b5e6-96231b3b80d8
|
