In the wild, many cases of null pointer dereference, or uninitialized
value read occur because the value was meant to be initialized by the
inlined function, but did not, most often due to error condition in the
inlined function.
This change highlights the return branch taken by the inlined function,
in order to help user understand the error report and see why the value
was uninitialized.
rdar://36287652
Differential Revision: https://reviews.llvm.org/D41848
llvm-svn: 325976
We already suppress such reports for inlined functions, we should then
get the same behavior for macros.
The underlying reason is that the same macro, can be called from many
different contexts, and nullability can only be expected in _some_ of
them.
Assuming that the macro can return null in _all_ of them sometimes leads
to a large number of false positives.
E.g. consider the test case for the dynamic cast implementation in
macro: in such cases, the bug report is unwanted.
Tracked in rdar://36304776
Differential Revision: https://reviews.llvm.org/D42404
llvm-svn: 324161
In order to provide more test coverage for inlined operator new(), add more
run-lines to existing test cases, which would trigger our fake header
to provide a body for operator new(). Most of the code should still behave
reasonably. When behavior intentionally changes, #ifs are provided.
Differential Revision: https://reviews.llvm.org/D42221
llvm-svn: 323376
This change adds support for cross-file diagnostic paths in html output. If the
diagnostic path is not cross-file, there is no change in the output.
Patch by Vlad Tsyrklevich!
Differential Revision: https://reviews.llvm.org/D30406
llvm-svn: 309968
The new checker currently contains the very core infrastructure for tracking
the state of iterator-type objects in the analyzer: relating iterators to
their containers, tracking symbolic begin and end iterator values for
containers, and solving simple equality-type constraints over iterators.
A single specific check over this infrastructure is capable of finding usage of
out-of-range iterators in some simple cases.
Patch by Ádám Balogh!
Differential revision: https://reviews.llvm.org/D32592
llvm-svn: 304160
It was written as "Memory Error" in most places and as "Memory error" in a few
other places, however it is the latter that is more consistent with
other categories (such as "Logic error").
rdar://problem/31718115
Differential Revision: https://reviews.llvm.org/D32702
llvm-svn: 302016
We have several reports of false positives coming from libc++. For example,
there are reports of false positives in std::regex, std::wcout, and also
a bunch of issues are reported in https://reviews.llvm.org/D30593. In many
cases, the analyzer trips over the complex libc++ code invariants. Let's turn
off the reports coming from these headers until we can re-evalate the support.
We can turn this back on once we individually suppress all known false
positives and perform deeper evaluation on large codebases that use libc++.
We'd also need to commit to doing these evaluations regularly as libc++
headers change.
Differential Revision: https://reviews.llvm.org/D30798
llvm-svn: 297429
When a macro expending to a literal is used in a comparison, use the macro name
in the diagnostic rather than the literal. This improves readability of path
notes.
Added tests for various macro literals that could occur. Only BOOl, Int, and
NULL tests have changed behavior with this patch.
Differential Revision: https://reviews.llvm.org/D27726
llvm-svn: 289884
Currently if the path diagnostic consumer (e.g HTMLDiagnostics and PlistDiagnostics) do not support cross file diagnostics then the path diagnostic report is silently omitted in the case of cross file diagnostics. The patch adds a little verbosity to Clang in this case.
The patch also adds help entry for the "--analyzer-output" driver option.
llvm-svn: 283499
Some FileIDs that may be used by PlistDiagnostics were not added while building
a list of pieces. This caused assertion violation in GetFID() function.
This patch adds some missing FileIDs to avoid the assertion. It also contains
small refactoring of PlistDiagnostics::FlushDiagnosticsImpl().
Patch by Aleksei Sidorin, Ilya Palachev.
Differential Revision: https://reviews.llvm.org/D22090
llvm-svn: 280360
The analyzer does not model C++ temporary destructors completely and so
reports false alarms about leaks of memory allocated by the internals of
shared_ptr:
std::shared_ptr<int> p(new int(1));
p = nullptr; // 'Potential leak of memory pointed to by field __cntrl_'
This patch suppresses all diagnostics where the end of the path is inside
a method in std::shared_ptr.
It also reorganizes the tests for suppressions in the C++ standard library
to use a separate simulated header for library functions with bugs
that were deliberately inserted to test suppression. This will prevent
other tests from using these as models.
rdar://problem/23652766
llvm-svn: 274691
Now that the libcpp implementations of these methods has a branch that doesn't call
memmove(), the analyzer needs to invalidate the destination for these methods explicitly.
rdar://problem/23575656
llvm-svn: 260043
This patch adds hashes to the plist and html output to be able to identfy bugs
for suppressing false positives or diff results against a baseline. This hash
aims to be resilient for code evolution and is usable to identify bugs in two
different snapshots of the same software. One missing piece however is a
permanent unique identifier of the checker that produces the warning. Once that
issue is resolved, the hashes generated are going to change. Until that point
this feature is marked experimental, but it is suitable for early adoption.
Differential Revision: http://reviews.llvm.org/D10305
Original patch by: Bence Babati!
llvm-svn: 251011
to the plist output. This check_name field does not guaranteed to be the
same as the name of the checker in the future.
Reviewer: Anna Zaks
Differential Revision: http://reviews.llvm.org/D6841
llvm-svn: 228624
This once again restores notes to following their associated warnings
in -analyzer-output=text mode. (This is still only intended for use as a
debugging aid.)
One twist is that the warning locations in "regular" analysis output modes
(plist, multi-file-plist, html, and plist-html) are reported at a different
location on the command line than in the output file, since the command
line has no path context. This commit makes -analyzer-output=text behave
like a normal output format, which means that the *command line output
will be different* in -analyzer-text mode. Again, since -analyzer-text is
a debugging aid and lo-fi stand-in for a regular output mode, this change
makes sense.
Along the way, remove a few pieces of stale code related to the path
diagnostic consumers.
llvm-svn: 188514
Previously, the use of a std::initializer_list (actually, a
CXXStdInitializerListExpr) would cause the analyzer to give up on the rest
of the path. Now, it just uses an opaque symbolic value for the
initializer_list and continues on.
At some point in the future we can add proper support for initializer_list,
with access to the elements in the InitListExpr.
<rdar://problem/14340207>
llvm-svn: 186519
...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
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
In most cases it is, by just looking at the name. Also, this check prevents the heuristic from working in strange user settings.
radar://13839692
llvm-svn: 181615
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
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.
llvm-svn: 179731
VerifyDiagnosticConsumer previously would not check that the diagnostic and
its matching directive referenced the same source file. Common practice was
to create directives that referenced other files but only by line number,
and this led to problems such as when the file containing the directive
didn't have enough lines to match the location of the diagnostic in the
other file, leading to bizarre file formatting and other oddities.
This patch causes VerifyDiagnosticConsumer to match source files as well as
line numbers. Therefore, a new syntax is made available for directives, for
example:
// expected-error@file:line {{diagnostic message}}
This extends the @line feature where "file" is the file where the diagnostic
is generated. The @line syntax is still available and uses the current file
for the diagnostic. "file" can be specified either as a relative or absolute
path - although the latter has less usefulness, I think! The #include search
paths will be used to locate the file and if it is not found an error will be
generated.
The new check is not optional: if the directive is in a different file to the
diagnostic, the file must be specified. Therefore, a number of test-cases
have been updated with regard to this.
This closes out PR15613.
llvm-svn: 179677
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
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>
llvm-svn: 178516
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.
llvm-svn: 178513
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.)
llvm-svn: 176969
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
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>
llvm-svn: 176201
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
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>
llvm-svn: 175988
George Karpenkov