In an expression like "new (a, b) Foo(x, y)", two things happen:
- Memory is allocated by calling a function named 'operator new'.
- The memory is initialized using the constructor for 'Foo'.
Currently the analyzer only models the second event, though it has special
cases for both the default and placement forms of operator new. This patch
is the first step towards properly modeling both events: it changes the CFG
so that the above expression now generates the following elements.
1. a
2. b
3. (CFGNewAllocator)
4. x
5. y
6. Foo::Foo
The analyzer currently ignores the CFGNewAllocator element, but the next
step is to treat that as a call like any other.
The CFGNewAllocator element is not added to the CFG for analysis-based
warnings, since none of them take advantage of it yet.
llvm-svn: 199123
...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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
r163489, "Take another crack at stabilizing the emission order of analyzer"
r163490, "Use isBeforeInTranslationUnitThan() instead of operator<."
llvm-svn: 163497
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
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
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
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
Jordan Rose