[analyzer] Enable the new edge algorithm by default.

...but don't yet migrate over the existing plist tests. Some of these
would be trivial to migrate; others could use a bit of inspection first.
In

[analyzer] Enable the new edge algorithm by default.

...but don't yet migrate over the existing plist tests. Some of these
would be trivial to migrate; others could use a bit of inspection first.
In any case, though, the new edge algorithm seems to have proven itself,
and we'd like more coverage (and more usage) of it going forwards.

llvm-svn: 183165

show more ...

[analyzer] Refactor BugReport::getLocation and PathDiagnosticLocation::createEndOfPath for greater code reuse

The 2 functions were computing the same location using different logic (each one had edg

[analyzer] Refactor BugReport::getLocation and 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.

llvm-svn: 180165

show more ...

[analyzer] Gain more precision retrieving the right SVal by specifying the type of the expression.

Thanks to Jordan for suggesting the fix.

llvm-svn: 179732

[analyzer] Address code review for r179395

Mostly refactoring + handle the nested fields by printing the innermost field only.

llvm-svn: 179572

[analyzer]Print field region even when the base region is not printable

llvm-svn: 179395

[analyzer] Show "Returning from ..." note at caller's depth, not callee's.

Before:
1. Calling 'foo'
2. Doing something interesting
3. Returning from 'foo'
4. Some kind of error here

Aft

[analyzer] Show "Returning from ..." note at caller's depth, not callee's.

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>

llvm-svn: 179351

show more ...

[analyzer] Don't emit extra context arrow after returning from an inlined call.

In this code

int getZero() {
return 0;
}

void test() {
int problem = 1 / getZero(); // expected-warnin

[analyzer] Don't emit extra context arrow after returning from an inlined call.

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>

llvm-svn: 179350

show more ...

[analyzer] Simple inline defensive checks suppression

Inlining brought a few "null pointer use" false positives, which occur because
the callee defensively checks if a pointer is NULL, whereas the c

[analyzer] Simple inline defensive checks suppression

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.

llvm-svn: 176402

show more ...

[analyzer] Use 'MemRegion::printPretty()' instead of assuming the region is a VarRegion.

Fixes PR15358 and <rdar://problem/13295437>.

Along the way, shorten path diagnostics that say "Variable 'x'"

[analyzer] Use 'MemRegion::printPretty()' instead of assuming the region is 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.

llvm-svn: 176115

show more ...

[analyzer] Plist: change the type of issue_hash from int to string.

This gives more flexibility to what could be stored as issue_hash.

llvm-svn: 171824

TrackConstraintBRVisitor and ConditionBRVisitor can emit similar
path notes for cases where a value may be assumed to be null, etc.
Instead of having redundant diagnostics, do a pass over the generat

TrackConstraintBRVisitor and ConditionBRVisitor can emit similar
path notes for cases where a value may be assumed to be null, etc.
Instead of having redundant diagnostics, do a pass over the generated
PathDiagnostic pieces and remove notes from TrackConstraintBRVisitor
that are already covered by ConditionBRVisitor, whose notes tend
to be better.

Fixes <rdar://problem/12252783>

llvm-svn: 166728

show more ...

[analyzer] Suppress bugs whose paths go through the return of a null pointer.

This is a heuristic intended to greatly reduce the number of false
positives resulting from inlining, particularly inlin

[analyzer] Suppress bugs whose paths go through the return of a null pointer.

This is a heuristic intended to greatly reduce the number of false
positives resulting from inlining, particularly inlining of generic,
defensive C++ methods that live in header files. The suppression is
triggered in the cases where we ask to track where a null pointer came
from, and it turns out that the source of the null pointer was an inlined
function call.

This change brings the number of bug reports in LLVM from ~1500 down to
around ~300, a much more manageable number. Yes, some true positives may
be hidden as well, but from what I looked at the vast majority of silenced
reports are false positives, and many of the true issues found by the
analyzer are still reported.

I'm hoping to improve this heuristic further by adding some exceptions
next week (cases in which a bug should still be reported).

llvm-svn: 164449

show more ...

[analyzer] Track a null value back through FindLastStoreBRVisitor.

Also, tidy up the other tracking visitors so that they mark the right
things as interesting and don't do extra work.

llvm-svn: 164

[analyzer] Track a null value back through FindLastStoreBRVisitor.

Also, tidy up the other tracking visitors so that they mark the right
things as interesting and don't do extra work.

llvm-svn: 164448

show more ...

Attempt (again) to stabilize the order of the emission of diagnostics
of the analyzer by using the FullProfile() of a PathDiagnostic
for ordering them.

llvm-svn: 163455

[analyzer] If the last store into a region came from a function, step into it.

Previously, if we were tracking stores to a variable 'x', and came across this:

x = foo();

...we would simply emit a

[analyzer] If the last store into a region came from a function, step into it.

Previously, if we were tracking stores to a variable 'x', and came across this:

x = foo();

...we would simply emit a note here and stop. Now, we'll step into 'foo' and
continue tracking the returned value from there.

<rdar://problem/12114689>

llvm-svn: 162718

show more ...

[analyzer] Look through casts when trying to track a null pointer dereference.

Also, add comments to addTrackNullOrUndefValueVisitor.

Thanks for the review, Anna!

llvm-svn: 162695

[analyzer] If we dereference a NULL that came from a function, show the return.

More generally, any time we try to track where a null value came from, we
should show if it came from a function. This

[analyzer] If we dereference a NULL that came from a function, show the return.

More generally, any time we try to track where a null value came from, we
should show if it came from a function. This usually isn't necessary if
the value is symbolic, but if the value is just a constant we previously
just ignored its origin entirely. Now, we'll step into the function and
recursively add a visitor to the returned expression.

<rdar://problem/12114609>

llvm-svn: 162563

show more ...

[analyzer] -analyzer-ipa=inlining is now the default. Remove it from tests.

The actual change here is a little more complicated than the summary above.
What we want to do is have our generic inlinin

[analyzer] -analyzer-ipa=inlining is now the default. Remove it from tests.

The actual change here is a little more complicated than the summary above.
What we want to do is have our generic inlining tests run under whatever
mode is the default. However, there are some tests that depend on the
presence of C++ inlining, which still has some rough edges. These tests have
been explicitly marked as -analyzer-ipa=inlining in preparation for a new
mode that limits inlining to C functions and blocks. This will be the
default until the false positives for C++ have been brought down to
manageable levels.

llvm-svn: 162317

show more ...

[analyzer] Add plist output checks for all four "path notes" tests.

No functionality change, but from now on, any new path notes should be
tested both with plain-text output (for ease of human audit

[analyzer] Add plist output checks for all four "path notes" tests.

No functionality change, but from now on, any new path notes should be
tested both with plain-text output (for ease of human auditing) and with
plist output (to ensure control flow and events are being correctly
represented in Xcode).

llvm-svn: 161351

show more ...

[analyzer] When a symbol is null, we should track its constraints.

Because of this, we would previously emit NO path notes when a parameter
is constrained to null (because there are no stores). Now

[analyzer] When a symbol is null, we should track its constraints.

Because of this, we would previously emit NO path notes when a parameter
is constrained to null (because there are no stores). Now we show where we
made the assumption, which is much more useful.

llvm-svn: 161280

show more ...

[analyzer] Flatten path diagnostics for text output like we do for HTML.

llvm-svn: 161279