Commit Graph

101 Commits

Author SHA1 Message Date
Jordan Rose 656fdd55dd [analyzer] Warn about double-delete in C++ at the second delete...
...rather somewhere in the destructor when we try to access something and
realize the object has already been deleted. This is necessary because
the destructor is processed before the 'delete' itself.

Patch by Karthik Bhat!

llvm-svn: 198779
2014-01-08 18:46:55 +00:00
Jordan Rose 60bd88d341 [analyzer] Extend IdenticalExprChecker to check ternary operator results.
Warn if both result expressions of a ternary operator (? :) are the same.
Because only one of them will be executed, this warning will fire even if
the expressions have side effects.

Patch by Anders Rönnholm and Per Viberg!

llvm-svn: 196937
2013-12-10 18:18:06 +00:00
Jordan Rose 1417a7b174 [analyzer] Don't crash when a path goes through a 'delete' destructor call.
This was just left unimplemnted from r191381; the fix is to report this call
location as the location of the 'delete' expr.

PR17746

llvm-svn: 193783
2013-10-31 18:41:15 +00:00
Jordan Rose d2f4079db9 Add an implicit dtor CFG node just before C++ 'delete' expressions.
This paves the way for adding support for modeling the destructor of a
region before it is deleted. The statement "delete <expr>" now generates
this series of CFG elements:

  1. <expr>
  2. [B1.1]->~Foo() (Implicit destructor)
  3. delete [B1.1]

Patch by Karthik Bhat!

llvm-svn: 189828
2013-09-03 17:00:57 +00:00
Robert Wilhelm 25284cc95b Use pop_back_val() instead of both back() and pop_back().
No functionality change intended.

llvm-svn: 189112
2013-08-23 16:11:15 +00:00
Eli Friedman 5ba37d5282 Split isFromMainFile into two functions.
Basically, isInMainFile considers line markers, and isWrittenInMainFile
doesn't.  Distinguishing between the two is useful when dealing with
files which are preprocessed files or rewritten with -frewrite-includes
(so we don't, for example, print useless warnings).

llvm-svn: 188968
2013-08-22 00:27:10 +00:00
Jordan Rose 56138268b0 [analyzer] Treat analyzer-synthesized function bodies like implicit bodies.
When generating path notes, implicit function bodies are shown at the call
site, so that, say, copying a POD type in C++ doesn't jump you to a header
file. This is especially important when the synthesized function itself
calls another function (or block), in which case we should try to jump the
user around as little as possible.

By checking whether a called function has a body in the AST, we can tell
if the analyzer synthesized the body, and if we should therefore collapse
the call down to the call site like a true implicitly-defined function.

<rdar://problem/13978414>

llvm-svn: 182677
2013-05-24 21:43:11 +00:00
Anna Zaks 2f74ff1b3c [analyzer] Do not assert on reports ending in calls within macros.
The crash is triggered by the newly added option (-analyzer-config report-in-main-source-file=true) introduced in r182058.

Note, ideally, we’d like to report the issue within the main source file here as well.
For now, just do not crash.

llvm-svn: 182445
2013-05-22 01:54:34 +00:00
Anna Zaks 6334579623 [analyzer] Address Jordan's review comments for r182058
llvm-svn: 182156
2013-05-17 20:51:16 +00:00
Anna Zaks c5e2eca042 [analyzer] Add an option to use the last location in the main source file as the report location.
Previously, we’ve used the last location of the analyzer issue path as the location of the
report. This might not provide the best user experience, when one analyzer a source
file and the issue appears in the header. Introduce an option to use the last location
of the path that is in the main source file as the report location.

New option can be enabled with -analyzer-config report-in-main-source-file=true.

llvm-svn: 182058
2013-05-16 22:30:45 +00:00
Ted Kremenek eba09facff Revert "[analyzer] Change PathPieces to be a wrapper around an ilist of (through indirection) PathDiagnosticPieces."
Jordan rightly pointed out that we can do the same with std::list.

llvm-svn: 180746
2013-04-29 23:12:59 +00:00
Ted Kremenek 03ae57b5af [analyzer] Change PathPieces to be a wrapper around an ilist of (through indirection) PathDiagnosticPieces.
Much of this patch outside of PathDiagnostics.h are just minor
syntactic changes due to the return type for operator* and the like
changing for the iterator, so the real focus should be on
PathPieces itself.

This change is motivated so that we can do efficient insertion
and removal of individual pieces from within a PathPiece, just like
this was a kind of "IR" for static analyzer diagnostics.  We
currently implement path transformations by iterating over an
entire PathPiece and making a copy.  This isn't very natural for
some algorithms.

We use an ilist here instead of std::list because we want operations
to rip out/insert nodes in place, just like IR manipulation.  This
isn't being used yet, but opens the door for more powerful
transformation algorithms on diagnostic paths.

llvm-svn: 180741
2013-04-29 22:38:26 +00:00
Ted Kremenek 518e781256 [analyzer] Remove comparePath's dependency on subscript operator.
llvm-svn: 180740
2013-04-29 22:38:22 +00:00
Anna Zaks 4e16b29c13 [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
2013-04-23 23:57:43 +00:00
Anna Zaks ece622ab46 [analyzer] Show path diagnostic for C++ initializers
Also had to modify the PostInitializer ProgramLocation to contain the field region.

llvm-svn: 178826
2013-04-05 00:59:33 +00:00
David Blaikie 00be69ab5c Remove the CFGElement "Invalid" state.
Use Optional<CFG*> where invalid states were needed previously. In the one case
where that's not possible (beginAutomaticObjDtorsInsert) just use a dummy
CFGAutomaticObjDtor.

Thanks for the help from Jordan Rose & discussion/feedback from Ted Kremenek
and Doug Gregor.

Post commit code review feedback on r175796 by Ted Kremenek.

llvm-svn: 175938
2013-02-23 00:29:34 +00:00
David Blaikie b169f55961 Simplify code to use castAs rather than getAs + assert.
Post commit review feedback on r175812 from Jordan Rose.

llvm-svn: 175826
2013-02-21 23:35:06 +00:00
David Blaikie 87396b9b08 Replace ProgramPoint llvm::cast support to be well-defined.
See r175462 for another example/more details.

llvm-svn: 175812
2013-02-21 22:23:56 +00:00
David Blaikie 2a01f5d426 Replace CFGElement llvm::cast support to be well-defined.
See r175462 for another example/more details.

llvm-svn: 175796
2013-02-21 20:58:29 +00:00
David Blaikie 7a30dc53c5 Use None rather than Optional<T>() where possible.
llvm-svn: 175705
2013-02-21 01:47:18 +00:00
David Blaikie 05785d1622 Include llvm::Optional in clang/Basic/LLVM.h
Post-commit CR feedback from Jordan Rose regarding r175594.

llvm-svn: 175679
2013-02-20 22:23:23 +00:00
David Blaikie 2fdacbc5b0 Replace SVal llvm::cast support to be well-defined.
See r175462 for another example/more details.

llvm-svn: 175594
2013-02-20 05:52:05 +00:00
Jordan Rose d8876a7450 [analyzer] Don't show "Entered 'foo'" if 'foo' is implicit.
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.

llvm-svn: 172915
2013-01-19 19:52:57 +00:00
Jordan Rose 1dc3940383 [analyzer] Special path notes for C++ special member functions.
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

llvm-svn: 172833
2013-01-18 18:27:21 +00:00
Jordan Rose fe856d58a3 [analyzer] Do a better job describing C++ member functions in the call stack.
Examples:
  Calling constructor for 'Foo'
  Entered call from 'Foo::create'

llvm-svn: 172832
2013-01-18 18:27:14 +00:00
Dmitri Gribenko f857950d39 Remove useless 'llvm::' qualifier from names like StringRef and others that are
brought into 'clang' namespace by clang/Basic/LLVM.h

llvm-svn: 172323
2013-01-12 19:30:44 +00:00
Anna Zaks a043d0cef2 [analyzer] Include the bug uniqueing location in the issue_hash.
The issue here is that if we have 2 leaks reported at the same line for
which we cannot print the corresponding region info, they will get
treated as the same by issue_hash+description. We need to AUGMENT the
issue_hash with the allocation info to differentiate the two issues.

Add the "hash" (offset from the beginning of a function) representing
allocation site to solve the issue.

We might want to generalize solution in the future when we decide to
track more than just the 2 locations from the diagnostics.

llvm-svn: 171825
2013-01-08 00:25:29 +00:00
Chandler Carruth 3a02247dc9 Sort all of Clang's files under 'lib', and fix up the broken headers
uncovered.

This required manually correcting all of the incorrect main-module
headers I could find, and running the new llvm/utils/sort_includes.py
script over the files.

I also manually added quite a few missing headers that were uncovered by
shuffling the order or moving headers up to be main-module-headers.

llvm-svn: 169237
2012-12-04 09:13:33 +00:00
Benjamin Kramer 444a1304ad Include pruning and general cleanup.
llvm-svn: 169095
2012-12-01 17:12:56 +00:00
Jordan Rose e37ab50a6e [analyzer] Report leaks at the closing brace of a function body.
This fixes a few cases where we'd emit path notes like this:

  +---+
 1|   v
  p = malloc(len);
  ^   |2
  +---+

In general this should make path notes more consistent and more correct,
especially in cases where the leak happens on the false branch of an if
that jumps directly to the end of the function. There are a couple places
where the leak is reported farther away from the cause; these are usually
cases where there are several levels of nested braces before the end of
the function. This still matches our current behavior for when there /is/
a statement after all the braces, though.

llvm-svn: 168070
2012-11-15 19:11:43 +00:00
Jordan Rose 2d98b97e10 [analyzer] Make sure calls in synthesized functions have valid path locations.
We do this by using the "most recent" good location: if a synthesized
function 'A' calls another function 'B', the path notes for the call to 'B'
will be placed at the same location as the path note for calling 'A'.

Similarly, the call to 'A' will have a note saying "Entered call from...",
and now we just don't emit that (since the user doesn't have a body to look
at anyway).

Previously, we were doing this for the "Calling..." notes, but not for the
"Entered call from..." or "Returning to caller". This caused a crash when
the path entered and then exiting a call within a synthesized body.

<rdar://problem/12657843>

llvm-svn: 168019
2012-11-15 02:07:23 +00:00
Jordan Rose ec44ac6a59 [analyzer] New option to not suppress null return paths if an argument is null.
Our one basic suppression heuristic is to assume that functions do not
usually return NULL. However, when one of the arguments is NULL it is
suddenly much more likely that NULL is a valid return value. In this case,
we don't suppress the report here, but we do attach /another/ visitor to
go find out if this NULL argument also comes from an inlined function's
error path.

This new behavior, controlled by the 'avoid-suppressing-null-argument-paths'
analyzer-config option, is turned off by default. Turning it on produced
two false positives and no new true positives when running over LLVM/Clang.

This is one of the possible refinements to our suppression heuristics.
<rdar://problem/12350829>

llvm-svn: 166941
2012-10-29 17:31:59 +00:00
Jordan Rose c8a78a37bb [analyzer] Handle implicit statements used for end-of-path nodes' source locs.
Some implicit statements, such as the implicit 'self' inserted for "free"
Objective-C ivar access, have invalid source locations. If one of these
statements is the location where an issue is reported, we'll now look at
the enclosing statements for a valid source location.

<rdar://problem/12446776>

llvm-svn: 165354
2012-10-06 01:19:30 +00:00
Jordan Rose c102b35b44 Use llvm::getOrdinalSuffix to print ordinal numbers in diagnostics.
Just a refactoring of common infrastructure. No intended functionality change.

llvm-svn: 164443
2012-09-22 01:24:42 +00:00
Ted Kremenek a0fa5d6564 Fix another case where we should be using isBeforeInTranslationUnit().
llvm-svn: 163533
2012-09-10 19:07:56 +00:00
Ted Kremenek 54fd079265 Add a few more cases where we should be using isBeforeInTranslationUnit().
llvm-svn: 163531
2012-09-10 19:02:33 +00:00
Ted Kremenek 6c7a5eae6d Revert "Revert Ted's r163489 and r163490, due to breakage."
I need to see how this breaks on other platforms when I fix the issue
that Benjamin Kramer pointed out.

This includes r163489 and r163490, plus a two line change.

llvm-svn: 163512
2012-09-10 14:50:55 +00:00
NAKAMURA Takumi 6eb1399088 Revert Ted's r163489 and r163490, due to breakage.
r163489, "Take another crack at stabilizing the emission order of analyzer"
r163490, "Use isBeforeInTranslationUnitThan() instead of operator<."

llvm-svn: 163497
2012-09-10 09:17:27 +00:00
Ted Kremenek f1fc8ce65d Use isBeforeInTranslationUnitThan() instead of operator<.
llvm-svn: 163490
2012-09-10 06:56:07 +00:00
Ted Kremenek 3d92699d3c Take another crack at stabilizing the emission order of analyzer
diagnostics without using FoldingSetNodeIDs.  This is done
by doing a complete recursive comparison of the PathDiagnostics.

Note that the previous method of comparing FoldingSetNodeIDs did
not end up relying on unstable things such as pointer addresses, so
I suspect this may still have some issues on various buildbots because
I'm not sure if the true source of non-determinism has been eliminated.
The tests pass for me, so the only way to know is to commit this change
and see what happens.

llvm-svn: 163489
2012-09-10 06:20:06 +00:00
Ted Kremenek b0d1c70258 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
2012-09-08 04:26:37 +00:00
Ted Kremenek 1fdcfcdf13 Revert "Attempt to make the PathDiagnostic emission order more deterministic by"
llvm-svn: 163446
2012-09-08 01:25:00 +00:00
Ted Kremenek af4cc7eab1 Revert "Further tweaks to hopefully make the PathDiagnostic emission more deterministic."
llvm-svn: 163445
2012-09-08 01:24:53 +00:00
Ted Kremenek a11a741e2f Further tweaks to hopefully make the PathDiagnostic emission more deterministic.
llvm-svn: 163430
2012-09-07 23:13:11 +00:00
Ted Kremenek 334ad6ac13 Attempt to make the PathDiagnostic emission order more deterministic by
looking at PathPieces.

llvm-svn: 163427
2012-09-07 22:24:24 +00:00
Jordan Rose cc0b1bfa56 [analyzer] Ensure that PathDiagnostics profile the same regardless of path.
PathDiagnostics are actually profiled and uniqued independently of the
path on which the bug occurred. This is used to merge diagnostics that
refer to the same issue along different paths, as well as by the plist
diagnostics to reference files created by the HTML diagnostics.

However, there are two problems with the current implementation:

1) The bug description is included in the profile, but some
   PathDiagnosticConsumers prefer abbreviated descriptions and some
   prefer verbose descriptions. Fixed by including both descriptions in
   the PathDiagnostic objects and always using the verbose one in the profile.

2) The "minimal" path generation scheme provides extra information about
   which events came from macros that the "extensive" scheme does not.
   This resulted not only in different locations for the plist and HTML
   diagnostics, but also in diagnostics being uniqued in the plist output
   but not in the HTML output. Fixed by storing the "end path" location
   explicitly in the PathDiagnostic object, rather than trying to find the
   last piece of the path when the diagnostic is requested.

This should hopefully finish unsticking our internal buildbot.

llvm-svn: 162965
2012-08-31 00:36:26 +00:00
Ted Kremenek 5bc38bad73 Rework how PathDiagnosticConsumers pass knowledge of what files they
generated for a given diagnostic to another.  Because PathDiagnostics
are specific to a give PathDiagnosticConsumer, store in
a FoldingSet a unique hash for a PathDiagnostic (that will be the same
for the same bug for different PathDiagnosticConsumers) that
stores a list of files generated.  This can then be read by the
other PathDiagnosticConsumers.

This fixes breakage in the PLIST-HTML output.

llvm-svn: 162580
2012-08-24 19:35:19 +00:00
Ted Kremenek 1e60273eed Remove "range_iterator" from PathDiagnosticPiece and just use ArrayRef<SourceRange> for ranges. This
removes conceptual clutter, and can allow us to easy migrate to C++11 style for-range loops if we
ever move to using C++11 in Clang.

llvm-svn: 162029
2012-08-16 17:45:29 +00:00
Ted Kremenek 9bf9af92a4 Allow multiple PathDiagnosticConsumers to be used with a BugReporter at the same time.
This fixes several issues:

- removes egregious hack where PlistDiagnosticConsumer would forward to HTMLDiagnosticConsumer,
but diagnostics wouldn't be generated consistently in the same way if PlistDiagnosticConsumer
was used by itself.

- emitting diagnostics to the terminal (using clang's diagnostic machinery) is no longer a special
case, just another PathDiagnosticConsumer.  This also magically resolved some duplicate warnings,
as we now use PathDiagnosticConsumer's diagnostic pruning, which has scope for the entire translation
unit, not just the scope of a BugReporter (which is limited to a particular ExprEngine).

As an interesting side-effect, diagnostics emitted to the terminal also have their trailing "." stripped,
just like with diagnostics emitted to plists and HTML.  This required some tests to be updated, but now
the tests have higher fidelity with what users will see.

There are some inefficiencies in this patch.  We currently generate the report graph (from the ExplodedGraph)
once per PathDiagnosticConsumer, which is a bit wasteful, but that could be pulled up higher in the
logic stack.  There is some intended duplication, however, as we now generate different PathDiagnostics (for the same issue)
for different PathDiagnosticConsumers.  This is necessary to produce the diagnostics that a particular
consumer expects.

llvm-svn: 162028
2012-08-16 17:45:23 +00:00
Jordan Rose 3eb3cd45b8 [analyzer] Flatten path diagnostics for text output like we do for HTML.
llvm-svn: 161279
2012-08-03 23:08:54 +00:00