Summary: @public
First diff to give better language information in the frontend.
This information is necessary to understand when 'self' is objc keyword,
when 'this' is C++ keyword and when they are not.
Reviewed By: @ddino, @dulmarod
Differential Revision: D2489252
Summary: @public Infer previously did not work correctly when a function returns the result of a skip function:
```
retUndef() {
x = undefined();
return x;
}
derefUndef() {
y = retUndef();
y.doSomething(); // Symexec_memory_error here, prevents spec inference
}
```
The problem is that angelic mode did not know to add the return value of `retUndef()` to the footprint.
This diff fixes the problem by adding return values marked with the `Aundef` attribute to the footprint.
This is done lazily (e.g., a value only gets added to the footprint when you try to deref it).
Reviewed By: @jvillard
Differential Revision: D2444929
Summary:
frontend and backend assume that Sil.Struct
doesn't have methods nor inheritance. In order to plug C++
classes we probably need them to be Sil.Class
Summary:
In the new clang the parameters to these functions have notnull annotations, because of that infer tests fail. More concretely, the tests say there would be a memory leak. In the symbolic execution of those functions though, an inconsistency is created, because the parameter was nil, and the constraint argument should not be nil was also there, which leads to an error in the execution and no object is created, hence, no memory leak.
Summary:
Added two annotations @TrueOnNull and @FalseOnNull to be used for boolean functions to specify what value is returned when the argument is null.
Added model for TextUtils.isEmpty, which corresponds to the annotation
@TrueOnNull
static boolean isEmpty(@Nullable java.lang.CharSequence s)
Summary:
Pass inheritance information to the backend
It also changes some functions in cTypes_decl and we are using type and decl maps to resolve these types
Summary:
This is the second of 3 stack diffs to deal with replacing the parser of types.
This diff is about changes to translate record types, as well as class types and enum
types. For class types and enum types we store the declaration pointer in the map of
types to find the type easier later.
For record declarations, we change the way we build record names.
Moreover, we don't translate typedefs anymore, because when we have a pointer to a typedef,
we can find the actual type it points to.
Summary:
System.getProperty can return null when the property is not found, and expects a non-null argument.
Add models for Infer and Eradicate to reflect that.
Summary:
Fix the issue with Linux, introduced in 721cc1957c and temporarily fixed in aef13134bf
Now clang gets the `x86_64-apple-darwin14` target only when running on ObjCXX source code with a specific `isysroot` flag.
Summary:
each procedure has a different scope, so we can restart the fresh name generator and have more stable instructions in the cfg, that don't change when other procedures are changed
Summary:
Errors arising from overriding methods defined in other files were not reported, because during parallel analysis the clusters did not have access to overridden methods, so could not load their annotation.
Changed cluster generation to add location information for the methods overridden by the procedures defined in the current cluster.
Summary:
When someone runs --changed-only mode, there is a risk of corrupting the results
for future analyses. The problem is that changed-only mode does not analyze the callers of changed
procedures. If a subsequent analysis relies on the specs of one of these callers, they will be stale
and may give the wrong results. To be concrete, let's say we know `Parent.foo()` calls `Child.bar()` and we do the following rounds of analysis:
Analysis round 1: Analyze all files, including `Parent` and `Child`
Analysis round 2: Analyze `Child.bar()` only with `--changed-only flag`. `Parent.foo()` is now stale.
Analysis round 3: Add procedure `Parent.baz()` that calls `Parent.foo()`, analyze in (any) incremental mode.
The analysis will only analyze `Parent.baz()`. However, the specs for `Parent.foo()` are stale and may give us bad results for `Parent.baz()`. We want the analysis to re-analyze `Parent.baz()`, but before this diff it will not.
This diff fixes this problem by adding a `STALE` status bit to procedure summaries. In `--changed-only` mode,
the callers of a changed procedures are not re-analyzed, but their summaries are marked as stale. For both
`--changed-only` and regular incremental mode, callees of changed procedures that are marked as stale are
re-analyzed even if they have not changed. This is better than a more obvious solution like deleting stale
procedure summaries, since that would force the next analysis to re-analyze all stale procedures even if it
does not need the results for whatever analysis it is doing. This scheme implemented in this diff ensures
that each analysis only does the work that it needs to compute reliable results for its changed procedures.
Summary: Handler.postDelayed keeps a persistent reference to its Runnable argument that may cause a memory leak if an Activity is reachable from the Runnable.
Summary: The Nullable checker reported FP's when a Nullable field/param was reassigned to a non-Nullable value in the footprint. This diff fixes the problem.
Summary:
The @NonNull annotation, with camel case, can now be used to inform Eradicate that some fields that are not initialized by the constructor can be initialized by other means, e.g. via dependency injection.
Summary:
This test was actually testing: "at least one Field not initialized error is found" where we actualy want to test "exactly one Field not initialized error is found". The case of @Inject was also missing from the tests.
Summary:
Handle C++ method declarations and create cfgs for them.
Doesn't do:
Method calls (CXXMethodCall)
Using `this` expression in methods (including implicit ones)
Summary:
Add basic translation for C++ `new` keyword.
Currently, it's modeled as simple `malloc` call.
Following constructs are still not working properly:
- array new `new [size_expr]`
- run initializer attached to `new` (such as `new int(5)`)
- `delete[]`
Summary:
When detecting a resource leak, Infer used to raise an Leak exception and then prevent the specs to be computed for the paths containing a leak. This diff prevents resource leak to stop the analysis.
Summary:
Creating a persistent reference to an Activity leads to a nasty form of memory leaks (see http://android-developers.blogspot.com/2009/01/avoiding-memory-leaks.html, https://corner.squareup.com/2015/05/leak-canary.html). There are many ways to create a bad persistent reference to an Activity, but the most obvious one is via a static field.
This diff implements a very simple form of Activity leak checking by inspecting postconditions to see if a subtype of Activity is reachable from a static field (and it reports an error if so). This is a very simple and limited form of leak checking that does not understand the Android lifecycle at all. In particular, if one creates a persistent reference to an Activity and then nulls it out in `onDestroy` (a reasonably common pattern), this approach will wrongly report a bug.
Summary:
The methods in objc can have the same name in the same class, but one be instance and the other class,
so that we need to take the instance flag into account when defining unique names for ObjC methods.
Summary:
The symbolic execution was not stopping in case an unitialized dangling pointer was
passed to a function and then dereferenced inside the callee.
What would happen is that a wrong footprint would be added to the unititialized pointer
at the end of the function call in the caller proposition.
This checks that if we do:
frame * new_footprint
checks that we do not add heap predicates to the frame into uninitialized local variables.
If we can identify the variable then we raise a danglind pointer dereference. If instead
we cannot give a good explanation we give an internal error.
The latter case should be temporary. We should find a general way to raise dangling pointer
deref instead of the internal error.
I also fixed the model of getc that was the way I found the problem.
Summary:
This adds a sentinel check every time a function carrying a sentinel attribute
is called, regardless of whether we have a definition for that function or not.
Summary:
In objC we already prefix field names with classes.
It's better to make it consistent since it'll allow
us to share more code between C++ and objC
Summary:
Add a partial copy of TextUtils from Android source for commonly used TextUtils.isEmpty method.
Fixes#141
Closes https://github.com/facebook/infer/pull/143
Github Author: Deniz Türkoglu <deniz@spotify.com>
Summary:
The models for InputStreamReader and OutputStreamWriter are taking into consideration the charset passed as parameter in order to follow the exception branch when the charset is not valid. However, the previsous models were only considering encoding literals with uppercase letters. This diff adds the lowercase encoding names to the list.
Closes https://github.com/facebook/infer/issues/127
Summary:
Treat `arrayWithObjects` as a special case of a sentinel attribute check. This
will make it easier to extend to other variadic functions that use a sentinel
attribute.
This also removes the need for the `Sil.Avariadic_function_argument` attribute,
which will be removed in a subsequent diff.
Summary:
This reverts commit 306f5b71c24042c89f71848898402cbc9269c543.
Turns out that developers think that this bugs should be fixed. So leaving it in for now until I gather more information.
Summary:
@public
There are many FPs of the form init method that contains
if ((self = [super initWithFrame:frame])) {
...
}
return self;
then an object being initialised with that constructor and added to an array or dictionary.
There we flag NPE and very likely that won't be a bug. So I'm removing the option for self
to be nil in the constructor, which should solve the problem.
Test Plan: Changed the relevant test.
Summary:
@public
Previously, if the close() method was throwing an exception, then code overriding the file attribute with a mem attribute would be skipped, resulting in reporting a wrong resource leak. This diff fixes this.
Test Plan: Added new end-to-end tests which would previously have been failing
Summary:
@public
Add support for default function arguments.
As a side change - always create cmethod_signature for a function
Test Plan:
1. Call function with default parameter and confirm that it gets parsed and reports null dereference (B5 but still). It didn't before.
2. Created a test case
Summary: @publicThe first argument of builtin calls in C gets translated twice, which is bad if the argument is a side-effecting expression like a function call.
Test Plan: Attached test previously reported a memory leak because the translation introduces an extra call to malloc(), now reports nothing.
Summary:
@public
Translate CXXStaticCastExpr
Test Plan:
Add test, confirm that it gets translated.
Also create example to see that infer reports null dereference with this change:
struct X { int a; };
int main() {
X *x = static_cast<X*>(nullptr); // <- reports now
//X *x = (X*)nullptr; // <- reported before
return x->a;
}
Summary:
@public
Make c frontend understand CXXNullPtrLiteralExpr.
Note that the implementation differs from GNUNullExpr
Test Plan:
Create function that returns nullptr:
int* getPtr() {return nullptr;}
look at specs:
InferPrint infer-out/specs/getPtr\{831F\}.specs
Procedure: getPtr
int *getPtr()
Timestamp: 1
Status: INACTIVE
Phase: RE_EXECUTION
Dependency_map:
TIME:0.002853 s TIMEOUT:N SYMOPS:10 CALLS:1,0
ERRORS:
--------------------------- 1 of 1 [nvisited: 1] ---------------------------
PRE:
POST 1 of 1:
return = null:
----------------------------------------------------------------
Add test for it
Summary:
@public
Using InferBuiltins.assume previously caused an assertion failure in the analyzer. Fixed this, and fixed the implementation of the assume builtin to block when the assumed condition cannot hold.
Test Plan: Added several new tests.
Summary:
@public
Modeling bypasses the Closeable as resource assumption for `java.io.StringReader`, `java.io.ByteArrayInputStream` and `java.io.ByteArrayOutputStream`.
Test Plan: Infer CI. Some resource leak should also disappear on Instagram.
Summary:
@public
Sorting the fields in structs and classes. Was needed in the backend and forgotten.
Fixes the github issue https://github.com/facebook/infer/issues/90.
Test Plan: Added a new test that shows that we now get a spec for the example from the github issue.
Summary:
@public
The models for Java no longer require to keep the original fields since we now make the union of the fields from the models and the fields from the code to analyze.
Test Plan: Infer CI. No functional change intended.
Summary:
@public
Adds a small example of a Ant project in order to test that there is not regresssion when modifying the toplevel scripts
Test Plan:
cd infer/tests/codetoanalyze/java/infer
ant clean && infer -- ant compile
Summary:
@public
Attaching the resource attribute to the object allows to more easily remove this attribute during the symbolic execution when the resource is passed as a argument, e.g. with `res.close()` or when this resource is passed around via a skipped function.
Test Plan: Infer CI.
Summary:
@public
The clang location information is described in an incremental way: each location information is a delta with respect to the previous one in the AST. This is based on a the visit of the AST nodes which corresponds to the order in which the lines are printed with the standard clang AST dump:
clang -cc1 -ast-dump filename.c
This diff adds a preprocessing phase to the front-end so that location information is composed during a visit, and explicit location information is used instead.
In the case of include files, we report the last known location before including the file.
The current file for a function is the file where it is defined. So if a function is entirely defined in a .h file, then the location information will consistently be about the .h file. If instead a function is defined in the source file being analyzed, and some AST nodes come from macro expansion, line information will refer to the original file.
The front-end tests reveal that the location information was incorrect in a few dot files.
Test Plan: arc unit, after having fixed the wrong location in the existing .dot files
jrm
Andrzej Kotulski
Sam Blackshear
Dulma Rodriguez
Cristiano Calcagno
martinoluca
Rohan Jacob-Rao
Jules Villard
Dino Distefano
Deniz Türkoglu