Summary:
This diff converts the Eradicate and Checkers tests to the new direct test format, which does not rely on buck or junit.
A self-contained Makefile is used to compile and analyze the test files, including all the dependencies, and a special option in InferPrint is used to produce a file of expected results `issues.exp`, which is checked into the repository.
Having an explicit Makefile makes it easy to edit and compile one set of test files in isolation, to investigate test failures, do debugging, etc.
A bunch of boilerplate code is removed. For example, the single file of expected results `issues.exp` replaces the 1.5K LOC in `endtoend/java/eradicate`.
Reviewed By: jvillard
Differential Revision: D3764632
fbshipit-source-id: 6c68ab8
Summary:
Array types where the length is not statically known were represented
using fresh variables. This diff:
- Makes array type length optional, reducing the amount of work needed
for renaming, substitution, and normalization.
- Revises uses of array length so that the length component of a
Tarray type represents only the statically determined constant
length of an array type, and the length component of a Sizeof
expression represents the dynamically determined length of an array
value.
- Restricts the type of static lengths from a general expression
(Sil.exp) to an integer (Sil.Int.t), enforcing that static types are
constant. This in particular ensures that types contain no
variables, and so are invariant under operations such as renaming
and substitution.
- Removes the type substitution and renaming functions typ_sub,
typ_normalize, and typ_captured_ren. Now that array type lengths
are constant integers, all of these functions are the identity.
Reviewed By: cristianoc
Differential Revision: D3387343
fbshipit-source-id: b5db768
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: 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: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: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
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
This just simplifies the end-to-end tests for Dividde By Zero to make the debugging easier when this test fails.
Reviewed By: cristianoc
Differential Revision: D2966296
fb-gh-sync-id: 5eaa1b5
shipit-source-id: 5eaa1b5
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
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
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 diff fixes a race condition where errors found in a procedure by one checker could be overwritten by running on demand the analysis of the same procedure with another checker.
Reviewed By: cristianoc
Differential Revision: D2847308
fb-gh-sync-id: 4f0c78e
Summary:
public
The contravariant subtyping rule for the PerformanceCritial annotation was meant to document the code but can be very too verbose on exisiting project. It is also not necessary as we can get this annotation from the supertypes. I am disabling it for now, but keep the code in case we want to revive it at some point in the future.
Reviewed By: sblackshear
Differential Revision: D2750212
fb-gh-sync-id: 2424281
Summary:
public
Lines other than the first of multi-line comments in non-ocaml files
were flush right instead of aligned.
Reviewed By: jvillard
Differential Revision: D2739752
fb-gh-sync-id: c85f56e
Summary:
public
It is possible to return null according to
http://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#getResource(java.lang.String).
Also, getResource throws NPE if passed null:
$ cat -n TestClassGetResourceArgument.java
1 import java.net.URL;
2
3 public class TestClassGetResourceArgument {
4
5 static URL testClassGetResourceArgument(Class cls) {
6 return cls.getResource(null);
7 }
8
9 public static void main(String[] args) {
10 System.out.println(testClassGetResourceArgument("".getClass()).toString());
11 }
12
13 }
$ javac TestClassGetResourceArgument.java && java TestClassGetResourceArgument
Exception in thread "main" java.lang.NullPointerException
at sun.misc.MetaIndex.mayContain(MetaIndex.java:243)
at sun.misc.URLClassPath$JarLoader.getResource(URLClassPath.java:830)
at sun.misc.URLClassPath.getResource(URLClassPath.java:199)
at sun.misc.URLClassPath.getResource(URLClassPath.java:251)
at java.lang.ClassLoader.getBootstrapResource(ClassLoader.java:1305)
at java.lang.ClassLoader.getResource(ClassLoader.java:1144)
at java.lang.ClassLoader.getResource(ClassLoader.java:1142)
at java.lang.ClassLoader.getSystemResource(ClassLoader.java:1267)
at java.lang.Class.getResource(Class.java:2145)
at TestClassGetResourceArgument.testClassGetResourceArgument(TestClassGetResourceArgument.java:6)
at TestClassGetResourceArgument.main(TestClassGetResourceArgument.java:10)
Reviewed By: cristianoc
Differential Revision: D2752301
fb-gh-sync-id: 888baf1
Summary:
public
Remove double negation in test check as per jrm's comment on D2695548.
Reviewed By: jeremydubreil
Differential Revision: D2743862
fb-gh-sync-id: d7cc0d0