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
|
|
This goes with r178516, which instructed the analyzer not to inline the
constructors and destructors of C++ container classes. This goes a step
further and does the same thing for iterators, so that the analyzer won't
falsely decide we're trying to construct an iterator pointing to a
nonexistent element.
The heuristic for determining whether something is an iterator is the
presence of an 'iterator_category' member. This is controlled under the
same -analyzer-config option as container constructor/destructor inlining:
'c++-container-inlining'.
<rdar://problem/13770187>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180890 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
PathDiagnosticLocation::createEndOfPath for greater code reuse
The 2 functions were computing the same location using different logic (each one had edge case bugs that the other
one did not). Refactor them to rely on the same logic.
The location of the warning reported in text/command line output format will now match that of the plist file.
There is one change in the plist output as well. When reporting an error on a BinaryOperator, we use the location of the
operator instead of the beginning of the BinaryOperator expression. This matches our output on command line and
looks better in most cases.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180165 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
This improves our handling of dynamic_cast and devirtualization for
objects allocated by 'new'.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180051 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
|
|
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179914 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
|
|
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179573 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Mostly refactoring + handle the nested fields by printing the innermost field only.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179572 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
|
|
Before:
1. Calling 'foo'
2. Doing something interesting
3. Returning from 'foo'
4. Some kind of error here
After:
1. Calling 'foo'
2. Doing something interesting
3. Returning from 'foo'
4. Some kind of error here
The location of the note is already in the caller, not the callee, so this
just brings the "depth" attribute in line with that.
This only affects plist diagnostic consumers (i.e. Xcode). It's necessary
for Xcode to associate the control flow arrows with the right stack frame.
<rdar://problem/13634363>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179351 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
In this code
int getZero() {
return 0;
}
void test() {
int problem = 1 / getZero(); // expected-warning {{Division by zero}}
}
we generate these arrows:
+-----------------+
| v
int problem = 1 / getZero();
^ |
+---+
where the top one represents the control flow up to the first call, and the
bottom one represents the flow to the division.* It turns out, however, that
we were generating the top arrow twice, as if attempting to "set up context"
after we had already returned from the call. This resulted in poor
highlighting in Xcode.
* Arguably the best location for the division is the '/', but that's a
different problem.
<rdar://problem/13326040>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179350 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 a heuristic to make up for the fact that the analyzer doesn't
model C++ containers very well. One example is modeling that
'std::distance(I, E) == 0' implies 'I == E'. In the future, it would be
nice to model this explicitly, but for now it just results in a lot of
false positives.
The actual heuristic checks if the base type has a member named 'begin' or
'iterator'. If so, we treat the constructors and destructors of that type
as opaque, rather than inlining them.
This is intended to drastically reduce the number of false positives
reported with experimental destructor support turned on. We can tweak the
heuristic in the future, but we'd rather err on the side of false negatives
for now.
<rdar://problem/13497258>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178516 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
|
|
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
|
|
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177763 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
|
|
Silences a few false positives in LLVM.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177186 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
|
|
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
|
|
Officially in the C++ standard, a null reference cannot exist. However,
it's still very easy to create one:
int &getNullRef() {
int *p = 0;
return *p;
}
We already check that binds to reference regions don't create null references.
This patch checks that we don't create null references by returning, either.
<rdar://problem/13364378>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176601 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
with CompoundLiteralExpr
This allows us to trigger the IDC visitor in the added test case.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176577 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
|
|
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
|
|
type.
This addresses a case when we inline a wrong method due to incorrect
dynamic type inference. Specifically, when user code contains a method from init
family, which creates an instance of another class.
Use hasRelatedResultType() to find out if our inference rules should be triggered.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176054 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
The expression 'a->b.c()' contains a call to the 'c' method of 'a->b'.
We emit an error if 'a' is NULL, but previously didn't actually track
the null value back through the 'a->b' expression, which caused us to
miss important false-positive-suppression cases, including
<rdar://problem/12676053>.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@173547 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
The idea is to eventually place all analyzer options under
"analyzer-config". In addition, this lays the ground for introduction of
a high-level analyzer mode option, which will influence the
default setting for IPAMode.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@173385 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Note to self: don't remove comments /after/ updating the line-sensitive
part of a test.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@173070 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Before:
struct Wrapper { <-- 2. Calling default constructor for 'NonTrivial'.
NonTrivial m;
};
Wrapper w; <-- 1. Calling implicit default constructor for 'Wrapper'.
After:
struct Wrapper {
NonTrivial m;
};
Wrapper w; <-- 1. Calling implicit default constructor for 'Wrapper'.
^-- 2. Calling default constructor for 'NonTrivial'.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@173067 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Before:
Calling implicit default constructor for 'Foo' (where Foo is constructed)
Entered call from 'test' (at "=default" or 'Foo' declaration)
Calling default constructor for 'Bar' (at "=default" or 'Foo' declaration)
After:
Calling implicit default constructor for 'Foo' (where Foo is constructed)
Calling default constructor for 'Bar' (at "=default" or 'Foo' declaration)
This only affects the plist diagnostics; this note is never shown in the
other diagnostics.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@172915 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Examples:
Calling implicit default constructor for Foo
Calling defaulted move constructor for Foo
Calling copy constructor for Foo
Calling implicit destructor for Foo
Calling defaulted move assignment operator for Foo
Calling copy assignment operator for Foo
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@172833 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
Examples:
Calling constructor for 'Foo'
Entered call from 'Foo::create'
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@172832 91177308-0d34-0410-b5e6-96231b3b80d8
|
|
This gives more flexibility to what could be stored as issue_hash.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@171824 91177308-0d34-0410-b5e6-96231b3b80d8
|