Summary: We were conflating reads/writes with container reads/writes that created false positives.
Reviewed By: sblackshear
Differential Revision: D6232768
fbshipit-source-id: 39159cb
Summary: Better error message for the direct dereference of nullable method without intermediate variable.
Reviewed By: sblackshear
Differential Revision: D6244494
fbshipit-source-id: 2ca2d22
Summary: This is a hack to removes most of the false positives of this checker in Objective C.
Reviewed By: sblackshear
Differential Revision: D6239914
fbshipit-source-id: 1cf05de
Summary:
Update plugin to take into account that some fields of VarDecl were unused by
infer. Also, use a boolean holding `hasExternalStorage` instead of comparing to
the fragile (and probably not entirely accurate) `"extern"` string.
update-submodule: facebook-clang-plugins
Reviewed By: mbouaziz
Differential Revision: D6231836
fbshipit-source-id: 3c0a75b
Summary:
This confuses the SIOF checker and causes false positives. This dummy deref is
generated for constructors of classes that are modeled as being pointer types
instead of the actual class in infer, typically for smart pointers. I do not
understand how this works.
The biabduction also analyses this code, so might now get confused itself.
Reviewed By: jberdine
Differential Revision: D6221817
fbshipit-source-id: 050c5a9
Summary:
The issue is with classes defining static data members:
```
$ cat foo.h
struct A {
static int foo;
};
$ cat foo.cpp
#include "foo.h"
int A::foo = 12;
int f() { return A::foo; // should see A::foo as defined in this translation unit
$ cat bar.cpp
#include "foo.h"
void g() { return A::foo; // should see A::foo defined externally
```
Previously, both foo.cpp and bar.cpp would see `A::foo` as defined within their
translation unit, because it comes from the header. This is wrong, and static
data members should be treated as extern unless they're defined in the same
file.
This doesn't change much except for frontend tests. SIOF FP fix in the next diff.
update-submodule: facebook-clang-plugins
Reviewed By: da319
Differential Revision: D6221744
fbshipit-source-id: bef88fd
Summary: This only works for Java at the moment but we can re-organise the code later to add the Objective C equivalent of these assertion methods.
Reviewed By: mbouaziz
Differential Revision: D6230588
fbshipit-source-id: 46ee98e
Summary:
When C++ functions are translated to SIL procedures, their type is C rather then C++. In RacerD, we want to treat C++ functions the same as C++ methods.
Added a function to check if the procedure is Objc/Objc++/C/C++.
Reviewed By: sblackshear
Differential Revision: D6209523
fbshipit-source-id: 293f938
Summary:
Seems it should have been done there all along.
The analyzer does not currently understand the implementation of
atomicity in folly::AtomicStruct.
The analyzer does not currently understand when std::atomic operations
are are used correctly versus incorrectly.
The analyzer does not currently understand that the representation of
folly::ThreadLocal is, ah, thread-local, leading to false alarms.
The analyzer does not currently understand the control flow /
scheduling constraints imposed by the implementation of Future.
It seems that the implementation of folly::Optional is more C++
template magic than the analyzer can currently understand.
The model of std::vector contains bogus memory accesses, leading to
false alarms.
Reviewed By: sblackshear
Differential Revision: D6226199
fbshipit-source-id: 8cb083b
Summary:
Destructors usually do not race with other methods.
We do not want to analyze or report on destructors.
Reviewed By: sblackshear
Differential Revision: D6222145
fbshipit-source-id: 5266622
Summary:
:
As we want to model many C++ methods, using a lot of matchers with `if / else if` will be tiring.
This diff introduces a dispatcher which is a nicer way to write the same thing.
No new model for now, just a refactoring.
Ideally we'd need a parser generator for C++ names...
Reviewed By: jvillard
Differential Revision: D6209234
fbshipit-source-id: 49fae5e
Summary: The checker should not report nullable violations on repeated calls
Reviewed By: sblackshear
Differential Revision: D6195471
fbshipit-source-id: 16ff76d
Summary: The Java bytecode does not contain information about the location of abstract of interface methods. Before this diff, the analysis trace was tuncated and the file where the abstract or interface method was not included in the trace, which makes it harder to understand the Infer report, especially when the method is on a generated file that is not checked in the repository.
Reviewed By: sblackshear
Differential Revision: D6223612
fbshipit-source-id: c80c6f2
Summary: A source can belong to more than one target. In this case, we should keep only one of the report.
Reviewed By: sblackshear
Differential Revision: D6200058
fbshipit-source-id: 4eced42
Summary:
The SIOF checker relies on the header models to detect whether `<iostream>` has
been included in source files.
Reviewed By: mbouaziz
Differential Revision: D6209904
fbshipit-source-id: a48855b
Summary: More general version of the fix in D6138749. This diff moves RacerD's lock modeling into a separate module and uses the module in the HIL translation to check when a function has lock/unlock semantics.
Reviewed By: jberdine, da319
Differential Revision: D6191886
fbshipit-source-id: 6e1fdc3
Summary:
This diff takes the first step toward a more general filtering
system. This step is concerned only with filtering at the reporting
stage, filtering for the capture and analysis stages is left for
later.
This diff adds a new command line / config option
```
--filter-report +string
Specify a filter for issues to report. If multiple filters are
specified, they are applied in the order in which they are
specified. Each filter is applied to each issue detected, and only
issues which are accepted by all filters are reported. Each filter
is of the form:
`<issue_type_regex>:<filename_regex>:<reason_string>`. The first
two components are OCaml Str regular expressions, with an optional
`!` character prefix. If a regex has a `!` prefix, the polarity is
inverted, and the filter becomes a "blacklist" instead of a
"whitelist". Each filter is interpreted as an implication: an issue
matches if it does not match the `issue_type_regex` or if it does
match the `filename_regex`. The filenames that are tested by the
regex are relative to the `--project-root` directory. The
`<reason_string>` is a non-empty string used to explain why the
issue was filtered.
See also infer-report(1) and infer-run(1).
```
Reviewed By: jvillard
Differential Revision: D6182486
fbshipit-source-id: 9d3922b
Summary: Functions that do not belong to a class or a struct are translated to c-style functions even in the context of cpp. We need to add ownership to locals for c-style functions too.
Reviewed By: sblackshear
Differential Revision: D6196882
fbshipit-source-id: 715f129
Summary:
vector::data returns a pointer to the first value of the vector.
- The size of the (array) pointer should be the same with the vector.
- The pointer should point to the same abstract value with the vector.
Reviewed By: mbouaziz
Differential Revision: D6196592
fbshipit-source-id: cc17096
Summary: `std::unique_lock` constructor allows to create a unique lock without locking the mutex. `std::unique_lock::try_lock` returns true if mutex has been acquired successfully, and false otherwise. It could be that an exception is being thrown while trying to acquire mutex, which is not modeled.
Reviewed By: jberdine
Differential Revision: D6185568
fbshipit-source-id: 192bf10
Summary:
The concurrency analyzer often does not understand object lifetimes
well enough to realize that destructors are usually not called in
parallel with any other methods. This leads to false alarms. This diff
suppresses these by simply skipping destructors in the concurrency
analysis.
Reviewed By: sblackshear
Differential Revision: D6182646
fbshipit-source-id: e9d1cac
Summary:
The clang frontend translates static locals incorrectly, in the sense
that the initializer is executed many times instead of once. This
leads to false alarms in the concurrency analysis. This diff
suppresses these by ignoring accesses to static locals.
Reviewed By: sblackshear
Differential Revision: D6182644
fbshipit-source-id: d8ca4c0
Summary:
Code often uses std::unique_lock::owns_lock to test if a deferred lock
using the 2-arg std::unique_lock constructor actually acquired the
lock.
Reviewed By: sblackshear
Differential Revision: D6181631
fbshipit-source-id: 11e9df2
Summary:
Use a distinct issue type for the Java and C++ concurrency analyses,
as the properties they are checking are significantly different.
Reviewed By: sblackshear
Differential Revision: D6151682
fbshipit-source-id: 00e00eb
Summary:
In a summary, you never want to see a trace where non-footprint sources flow to a sink.
Such a trace is useless because nothing the caller does can make more data flow into that sink.
Reviewed By: jeremydubreil
Differential Revision: D5779983
fbshipit-source-id: d06778a
Summary:
Due to limitations in our Buck integration, the thread-safety analysis cannot create a trace that bottoms out in a Buck target that is not a direct dependency of the current target.
These truncated traces are confusing and tough to act on.
Until we can address these limitations, let's avoid reporting on truncated traces.
Reviewed By: jeremydubreil
Differential Revision: D5969840
fbshipit-source-id: 877b9de
Dulma Churchill
Dominic Cooney
Daiva Naudziuniene
Jeremy Dubreil
Sam Blackshear
Jules Villard
Josh Berdine
Dino Distefano
Mehdi Bouaziz
Martino Luca
Sungkeun Cho
Darío Hereñú