Summary:
This call was producing confusing false positives when deleted object was possible to be null.
Changing frontend to add that check is not trivial so I turned it off for now (we don't handle
destructors in other cases anyway)
Reviewed By: dulmarod
Differential Revision: D3509354
fbshipit-source-id: c23dc81
Summary:
When clang instantiates template function with argument pack, it will
give the same name to all parameters coming from the pack. To avoid
name collisions, always add index of argument's position to mangled part
of the variable.
Seemingly unrelated changes are to make existing tests pass (don't use
simple variable name where it matters)
Reviewed By: dulmarod
Differential Revision: D3503608
fbshipit-source-id: 794093a
Summary: Those functions have simple enough implementations for infer to understand them
Reviewed By: jvillard
Differential Revision: D3463084
fbshipit-source-id: f84160f
Summary:
Assume that std::vector::resize will always create nonempty vector. While this is clearly
wrong for resize(0), it removes many FPs for `resize(n)` calls, where value of `n` is unknown.
Without it, infer was thinking that `n` could be 0 and reported empty vector access.
Reviewed By: jvillard
Differential Revision: D3424355
fbshipit-source-id: cb476de
Summary:
The extra dereference in stmtexpr was wrong. When a dereference is needed, we have a cast.
This was causing one dereference too many, and creating wrong results.
Reviewed By: akotulski
Differential Revision: D3393294
fbshipit-source-id: 7a1ec8e
Summary:
Now all code in tests is reachable by the analyzer which increases
test quality.
Reviewed By: dulmarod
Differential Revision: D3358591
fbshipit-source-id: d54877e
Summary:
Pass object by reference every time struct object is passed by value
in C++. Do it only for C++/objC++ where we have guarantee that the
object which is passed will be temporary one (created by copy constructor).
Reviewed By: jberdine
Differential Revision: D3346271
fbshipit-source-id: d3e5daa
Summary:
Make analyzer find out when null dereference comes from std::vector method.
If it does, it means that it's really empty vector access (due to the
way infer models std::vector)
Reviewed By: sblackshear
Differential Revision: D3327933
fbshipit-source-id: b9e11d6
Summary:
Turns out, analyzer was getting confused with complicated
model and it was reporting empty access in places it
shouldn't. Fixing backend is not trivial (tracing mode is the answer),
but the model can be simplified.
It introduces the problem that get() method doesn't return fresh value
every time, but we should be able to change backend later to deal with it.
Reviewed By: sblackshear
Differential Revision: D3328228
fbshipit-source-id: dddbaf8
Summary:
Part of the migration of .inferconfig-specific options into options accepted
both by .inferconfig and the CLI.
Reviewed By: jberdine
Differential Revision: D3304783
fbshipit-source-id: 4a7ee6f
Summary:
Create model of C++ std::vector to find occurrences when vector which might be empty is accessed. Do it by triggering null dereference every time empty vector access is performed.
Note: model will be used only when c++11 (or c++14) are used.
Reviewed By: sblackshear
Differential Revision: D3276203
fbshipit-source-id: 420a95a
Summary:
The philosophy of the tracing mode reporting is to not report the errors in a method if reaching this error does depend on information that can be false at call site. Typically with:
void foo(Object obj, int x) {
if (x == 3) {
obj.toString();
}
}
it may be that we always call `foo` with a non-null parameter or `x != 3`.
Thechnically, the reporting code matches the pairs of the form (precondition, error) and filtering out the cases where the precondtions was not imposing constraints on the calling context, and report the other cases. So the NPE could be reported in the following case:
void bar() {
foo(null, 3);
}
However, we were missing the case where there was anyway no way to call a method in a safe way, i.e. all the preconditions were of the form: (precondition, error), for example:
void baz(boolean b) {
if (b) {
foo(null, 3);
} else {
foo(null, 3);
}
}
In that case, the summary is of the form
PRE (1): b = false
POST: NullPointerException
PRE (2): b = true
POST: NullPointerException
In which case it is legit to report `NullPointerException` in `baz`.
Reviewed By: sblackshear, jberdine
Differential Revision: D3220501
fb-gh-sync-id: 7fb7d70
fbshipit-source-id: 7fb7d70
Summary: Example of dynamic dispatch with interfaces were already working. Adding some tests now so that we don't break this.
Reviewed By: sblackshear
Differential Revision: D3220360
fb-gh-sync-id: 11395dd
fbshipit-source-id: 11395dd
Summary:Local variable created by conditional operator translation is now declared in scope of whole
procedure. Semantically there is no difference, hopefuly backend will not complain about this
change. Also, nullifying that variable is deferred to preanalysis instead of calling it manually
Reviewed By: jvillard
Differential Revision: D3155733
fb-gh-sync-id: 6cec8fc
fbshipit-source-id: 6cec8fc
Summary: For performance critical sections of the code, this checker detects memory allocations or calls to methods annotated as expensive. However, such cases of memory allocations or expensive calls are acceptable is occuring in rare cases. This diff adds supports for the "unlikely" branch prediction method and does not track expensive calls in unlikely branches.
Reviewed By: sblackshear
Differential Revision: D3193473
fb-gh-sync-id: ea87e49
fbshipit-source-id: ea87e49
Summary:BinaryConditionalOperator should evaluate condition expression once, but we used to evaluate it twice.
Fix translation to account for it.
Reviewed By: dulmarod
Differential Revision: D3179803
fb-gh-sync-id: a801a7e
fbshipit-source-id: a801a7e
Summary:This diff translate cpp lambdas. For the moment it does not take care of
captured variables. Captured variables will come in the next diff.
Reviewed By: dulmarod
Differential Revision: D3114790
fb-gh-sync-id: bf36450
fbshipit-source-id: bf36450
Summary:public
When a conditional is the last instruction, there will be a join node leading directly to the exit node.
Some instructions, such as nullification of dead variables, and abstraction, are added to the control flow graph automatically. But, join nodes cannot contain instructions. So when a procedure ends with a conditional, there might be no place to store these instructions.
This diff adds one extra node between the join and the exit node in that situation.
Reviewed By: jvillard
Differential Revision: D3179056
fb-gh-sync-id: 2b9cd7e
fbshipit-source-id: 2b9cd7e
Summary:public
The code:
DataInputStream in = new DataInputStream(new BufferedInputStream(new FileInputStream(file)));
creates a resource with `FileInputStream()` and wraps it twice as a field of `BufferedInputStream` and then as a field of `DataInputStream`. Then calling:
in.close();
needs to go down the wrappers hierachy: `DataInputStream.close()` -> `FilterInputStream.close()` which then calls `BufferedInputStream.close()` -> `FilterInputStream.close()` -> `FileInputStream.close()`.
Going down the wrapper was not working before because `FilterInputStream.close()` was only going further when the type of field `in` was `FileInputStream` wheras it should also continue when the type of the field is any subtype of `FilterInputStream`, e.g. `DataInputStream` and `BufferedInputStream` like in the test example. This diff fixes this last aspect.
Reviewed By: sblackshear
Differential Revision: D3174822
fb-gh-sync-id: 3adbb7e
fbshipit-source-id: 3adbb7e
Summary:public
Instead of translating code from headers blindly, translate only gets transitively referenced from source code.
It won't translate functions from system headers, but in the future we could do that as well
since most of them aren't used and it shouldn't add much overhead.
For now this functionality is hidden behind --cxx-experimental flag
Reviewed By: dulmarod
Differential Revision: D3163519
fb-gh-sync-id: 0c53b10
fbshipit-source-id: 0c53b10
Summary:public
Instead of using location of init_stmt, use location of variable when translating initialization.
Most of the time it change anything with some exceptions:
// example1 - C/C++/objC
int x = // now: assignment happens in this line
3; // past: assignment happens in this line
// example2: valid in C++11 only
struct X {
int x = 0; // now: one assignment here
int y = 2; // now: one assigmnent here
X() = default; // before: 2 assignments in this line
};
Reviewed By: dulmarod
Differential Revision: D3155870
fb-gh-sync-id: f38c78c
fbshipit-source-id: f38c78c
Summary:public
Add modeling of methods that can raise exceptions when parameters are
nil, and that return nil when passed nil.
Reviewed By: dulmarod
Differential Revision: D3101739
fb-gh-sync-id: 76af5a2
fbshipit-source-id: 76af5a2
Summary:public
Create a model of std::unique_ptr in similar fashion to what was done to std::shared_ptr.
For now, we are modeling it as container of raw pointer (no ownership concept).
This time unique_ptr is not derived from std__unique_ptr (unlike shared_ptr, it was easier to not do that) and so we need to provide implementations for all non-member functions per C++ reference:
http://en.cppreference.com/w/cpp/memory/unique_ptr
Reviewed By: dulmarod
Differential Revision: D3048209
fb-gh-sync-id: a9a6455
shipit-source-id: a9a6455
Summary:public
Before this diff, the Java frontend was not adding the definition of the inherited interfaces to the type environment, thus failing to answer questions like "does type X implements Closeable". Infer was therefore missing to detect resource leaks when the resource was indirectly implementing Closeable via an intermediate interface.
Reviewed By: sblackshear
Differential Revision: D3067555
fb-gh-sync-id: 86d0760
shipit-source-id: 86d0760
Summary:public
Implementation of std::move is straightforward and infer understands it without
any problems. To use it, we translate it even though it's coming from system headers.
Reviewed By: jvillard
Differential Revision: D3064019
fb-gh-sync-id: 823ae75
shipit-source-id: 823ae75
Summary:This pull request adds the SuppressViewNullability annotation.
The reasoning behind this is that in libraries, one cannot use Butterknife for view binding, which forces you to do it manually. Basically, this makes a new annotation that infer treats the same way as Bind/InjectView
Closes https://github.com/facebook/infer/pull/301
Reviewed By: jvillard
Differential Revision: D3047235
Pulled By: cristianoc
fb-gh-sync-id: 6286d2b
shipit-source-id: 6286d2b
Summary:public
Use the configuration file .inferconfig to model the library method that are considered expensive
Reviewed By: cristianoc
Differential Revision: D3045288
fb-gh-sync-id: e58d85c
shipit-source-id: e58d85c
Summary:public
Create initial model of C++ std::shared_ptr. This means that infer will replace implementation of
shared_ptr and the resulting binary will change. Make sure no one will run it by crashing any binary that includes that code.
Reviewed By: jvillard
Differential Revision: D2999948
fb-gh-sync-id: 5753559
shipit-source-id: 5753559
Summary:public This also required a refactoring of InitListExpr.
The idea is that ImplicitValueInitExpr can stand for initialising a whole struct,
so we translating as a list of zero expressions, according to the struct's fields,
which is then paired with a list of field expressions, such that one get a list of
assignment instructions.
Reviewed By: ddino
Differential Revision: D2999875
fb-gh-sync-id: 7f609a0
shipit-source-id: 7f609a0
Summary:public
Lazy dynamic dispatch handling works as follows:
Assuming a call of the form:
foo(a);
where the static type of `a` is `A`. If during the symbolic execution, the dynamic type of the variable `a` is `B` where `B <: A`, then we create on-demand a copy `foo(B)` of `foo(A)` where all the uses of the typed parameter `a` are replaced with a parameter of type `B`. Especially, if `foo` contains virtual call, say `get` where `a` is the receiver, then the call gets redirected to the overridden method in `B`, which simulates the runtime behavior of Java.
This lazy dynamic dispatch mode is only turn on for the tracing mode for now in order to avoid conflicts with sblackshear's approach for sound dynamic dispatch.
Reviewed By: sblackshear
Differential Revision: D2888922
fb-gh-sync-id: 3250c9e
shipit-source-id: 3250c9e
Summary:public
Deprecate the incremental mode.
Several parts of the back-end can be removed.
The options for incremental analysis -i at the python level are now deprecated, and re-routed to --reactive.
The main difference with --reactive is that it does not produce an analysis of the whole project, but is limited to what is reachable via reactive propagation starting from the changed files.
Reviewed By: sblackshear
Differential Revision: D2960078
fb-gh-sync-id: 6e8b46b
shipit-source-id: 6e8b46b
Summary:public
An observer object that registered to a notification center needs to be
unregistered before it is deallocated.
If not, the notification center may send a notification to a gost object.
This diff introduce a checker for this problem.
Reviewed By: dulmarod
Differential Revision: D2949692
fb-gh-sync-id: 1653cec
shipit-source-id: 1653cec
Summary:public
Improved/simplified framework for fronend checkers.
Now we have a unique hook from cTrans to run checkers on statements and a unique
hook from cFrontent to run checkers on declarations.
So now when adding a checker we don't have to modify cTrans/cFrontend.
Moreover made more sistematic the way checkers are invoked. This simplify the definition
of checkers and the way we use them.
Code is now simpler.
Reviewed By: jvillard
Differential Revision: D2976589
fb-gh-sync-id: fbe22d4
shipit-source-id: fbe22d4
Summary:public
The NoAllocation checker should not report on the creation of exceptions
Reviewed By: sblackshear
Differential Revision: D2969719
fb-gh-sync-id: 4a8ffc8
shipit-source-id: 4a8ffc8
Summary:public
Add extra dereference when accessing fields that have T& type. It is similar
to what is done when accessing variables of T& type.
The only difference is that we need to handle constructor initializer list
separately (this is the only place where the field can be initialized)
Reviewed By: ddino
Differential Revision: D2965887
fb-gh-sync-id: 1b8708b
shipit-source-id: 1b8708b
Summary:public
Do same thing we do to CXXDefaultArgExpr
Reviewed By: dulmarod
Differential Revision: D2954128
fb-gh-sync-id: 2c92c16
shipit-source-id: 2c92c16
Summary:public
Running clang-format on symbolic link replaced it with another file.
This is second diff out of two to fix that problem.
Reviewed By: jvillard
Differential Revision: D2960206
fb-gh-sync-id: 0f21a8f
shipit-source-id: 0f21a8f
Summary:public
Running clang-format on symbolic link replaced it with another file.
This is first diff out of two to fix that problem.
Reviewed By: jvillard
Differential Revision: D2960204
fb-gh-sync-id: aaa3231
shipit-source-id: aaa3231
Summary:public
Create separate specs for C models compiled in C++. It will allow us to tweak behavior/names of certain
functions based on the compilation language (such as adding `std::` namespace in C++).
Reviewed By: jvillard
Differential Revision: D2938992
fb-gh-sync-id: 73902f8
shipit-source-id: 73902f8
Summary:public
Add to the code to detect violation of the `NoAllocation` annotation. This diff adds the code to detect such issue based on the code of the `PerformanceCritical` checker. In the next diff, I will refine the list of acceptable allocations, like new exceptions, etc, and add the list of corresponding tests.
Reviewed By: sblackshear
Differential Revision: D2938641
fb-gh-sync-id: 9a047dd
shipit-source-id: 9a047dd
Summary:public
Before this diff, the checker was collecting in a bottom-up fashion all possible call trees from `PerforamanceCritical`-annotated methods to `Expensive`-annotated ones. With this diff, we just collect the names of the direct transitively expensive callees and compute the expensive call stacks when reporting errors only.
Reviewed By: sblackshear
Differential Revision: D2938635
fb-gh-sync-id: dcdd13c
shipit-source-id: dcdd13c
Summary:public We model it as the builtin __instanceof which models the instanceof construct of Java.
The behaviour is the same.
Reviewed By: jvillard
Differential Revision: D2938969
fb-gh-sync-id: 2258de3
shipit-source-id: 2258de3
Summary:public
Translate headers every time they are included provided that they are located inside project_root directory.
While this is suboptimal (we might end up translating same header many times), doing it exactly once
is hard due to parallel compilation and template instantiations
Reviewed By: dulmarod
Differential Revision: D2916799
fb-gh-sync-id: 93b72c4
shipit-source-id: 93b72c4
Summary:public
Is seems that automatically inheriting annotations like `PerformanceCritical` or `NoAllocation` is the right thing to do in general. Otherwise, we need to enforce sub-typing rules which in the best case just adds a little bit of documentation, but could miss important issues when the code is not fully annotated. I am simplifying this part to avoid adding boilerplate code for the `NoAllocation` case.
Reviewed By: sblackshear
Differential Revision: D2938627
fb-gh-sync-id: ddb668b
shipit-source-id: ddb668b
Summary:
public
Avoid problems of overwriting good type information with incomplete information
when type declaration happens after its complete definition.
The solution is that we will only time we *update* type information is
when struct declaration has definition as well (which should happen once)
Reviewed By: cristianoc, sblackshear
Differential Revision: D2921811
fb-gh-sync-id: 16baba3
shipit-source-id: 16baba3
Summary:
public
The inductive list predicate was not firing during abstraction because of a type mismatch between C and Java. In Java, the second parameter of the `Sil.Sizeof` constructor is always `Sil.Subtype.exact` in C but is `Sil.Subtype.subtypes` in Java. This diff fixes the confution by comparing the `Sil` types only instead of the type expressions.
Reviewed By: jberdine
Differential Revision: D2912493
fb-gh-sync-id: 3f712a8
shipit-source-id: 3f712a8
Summary:
public
This expression is used to value-initialize non-class types. Per definition of value initialization for non-class types:
1. If it's an array, value-initialize each of its elements
2. Otherwise, zero-initialize it
http://en.cppreference.com/w/cpp/language/value_initialization
I was unable to reproduce (1) in a way that produced CXXScalarValueInitExpr and so this diff
deals with case (2)
Reviewed By: jvillard
Differential Revision: D2901311
fb-gh-sync-id: beeafa2
Summary:
public
Add type of return parameter to the context. It allows for better translation
of returnStmt and will be necessary for easy implementation of constructor init lists
Reviewed By: cristianoc
Differential Revision: D2890838
fb-gh-sync-id: e791c3d
Summary:
public
xvalues is concept introduced in C++11. While they are not same as lvalues, they have one common trait:
They have identity which means that:
> it's possible to determine whether the expression refers to the same entity as another expression, such as by comparing addresses of the objects or the functions they identify (obtained directly or indirectly);
It means that as far as backend is concerned, they should be treated in same way. Right now there is no concept of "move" in the backend and so we don't have
to differentiate between them.
Reference:
http://en.cppreference.com/w/cpp/language/value_category
Reviewed By: cristianoc
Differential Revision: D2895593
fb-gh-sync-id: 5101e28
Summary:
public
C++ allows for parameters with empty names (unused/default copy constructors). Make backend happy by assigning a non-empty
name to these variables
Reviewed By: jvillard
Differential Revision: D2895550
fb-gh-sync-id: b466397
Sam Blackshear
Andrzej Kotulski
Dulma Churchill
Lázaro Clapp Jiménez Labora
Peter O'Hearn
Jeremy Dubreil
Dino Distefano
Cristiano Calcagno
Jules Villard
Josh Berdine
Nick Firmani