Summary:
The Java models for resources are way to complex. The main issue I am facing with these models is that small changes in the analysis can affect the generation of the models in some weird ways. For instance, I get different specs for some of the models between my devserver and my devvm, which seems to be mostly related with the backend treatment of `instanceof`.
The objective here is to simplify the models as much as possible in order to:
1) make debugging regressions easier
2) get simpler specs and less modeled methods shipped in `models.jar`
Reviewed By: sblackshear
Differential Revision: D4536115
fbshipit-source-id: 577183a
Summary: Better documentation, and could perhaps be checked instead of trusted later if the analysis understands threads better.
Reviewed By: jaegs
Differential Revision: D4537463
fbshipit-source-id: 4323c78
Summary: In some cases where a function is called directly on a formal (e.g, `def foo(o) { callSomething(o) }`, we were failing to propagate the footprint trace to the caller.
Reviewed By: jeremydubreil
Differential Revision: D4502404
fbshipit-source-id: d4d632f
Summary: This case was already working but there was no tests for it
Reviewed By: sblackshear
Differential Revision: D4529473
fbshipit-source-id: ca3ff02
Summary: This will be important for maintaining ownership of `View`'s, which involve a lot of casting.
Reviewed By: peterogithub
Differential Revision: D4520441
fbshipit-source-id: fdef226
Summary:
Previously, we would lose track of ownership in code like
```
Obj owned = new Obj();
Obj stillOwned = id(owned); // would lose ownership here
stillOwned.f = ... // would report false alarm here
```
This diff partially addresses the problem by adding a notion of "unconditional" (always owned) or "conditional" (owned if some formal at index i is owned) ownership.
Now we can handle simple examples like the one above.
I say "partially" because we still can't handle cases where there are different reasons for conditional ownership, such as
```
oneOrTwo(Obj o1, Obj o2) { if (*) return o1; else return o2; } // we won't understand that this maintains ownership if both formals are owned
Obj stillOwned = oneOrTwo(owned1, owned2);
stillOwned.f = ... // we'll report a false alarm here
```
This can be addressed in the future, but will require slightly more work
Reviewed By: peterogithub
Differential Revision: D4520069
fbshipit-source-id: 99c7418
Summary: This will make it a cinch to track new "attributes" of memory locations, and to propagate more complex attributes such as conditional ownership (coming in a future diff).
Reviewed By: peterogithub
Differential Revision: D4523143
fbshipit-source-id: 57aa133
Summary: The diff remove the no-op model for `Cursor.close()` by the frontend-based `Closeable` as resources mechanism where every call of the form `object.close()` removes the file attribute on `object` when `object` is of type `Closeable`.
Reviewed By: sblackshear
Differential Revision: D4519386
fbshipit-source-id: 83633d4
Summary: This fixes a wrong level of indirection when performing the type substitution.
Reviewed By: sblackshear
Differential Revision: D4521008
fbshipit-source-id: 7324ea6
Summary: Those should be treated angelically during the analysis with the same end results
Reviewed By: sblackshear
Differential Revision: D4518930
fbshipit-source-id: ee5bae8
Summary: Being forced to separately define `pp_element`/`pp_key` is uneccessary and makes it more cumbersome to create a set/map from an existing module that already defines `pp`.
Reviewed By: jeremydubreil
Differential Revision: D4517308
fbshipit-source-id: 9b17c9c
Summary: Not clear why we need to disable this case and in which case is Infer creating too many disjunctions.
Reviewed By: sblackshear
Differential Revision: D4509394
fbshipit-source-id: fbc106d
Summary: This method can return `null` if the parameter is not a supported system service. However, since this method tends to be called with a constant value as parameter, it does seem to be returning null often in practice.
Reviewed By: sblackshear
Differential Revision: D4509185
fbshipit-source-id: 4cb80ce
Summary:
At one point I thought we'd want to have lots of different schedulers for things like exploring loops in different orders, but that hasn't materialized.
Let's make the common use-case simpler by hiding the `Scheduler` parameter inside the `AbstractInterpreter` module.
We can always expose `MakeWithScheduler` later if we want to.
Reviewed By: jberdine
Differential Revision: D4508095
fbshipit-source-id: 726e051
Summary:
This fixes false positives we had in fields written by callees of a constructor (see new E2E test).
This is also a bit cleaner than what we did before; instead of special-casing constructors, we just use the existing ownership concept.
Reviewed By: peterogithub
Differential Revision: D4505161
fbshipit-source-id: a739ebc
Summary:
Constants are always "owned" in the sense that no one can mutate them.
In code like
```
Obj getX(boolean b) {
if (b) {
return null;
}
return new Obj();
}
```
, we need to understand this in order to infer that the returned value is owned.
This should fix a few FP's that I've seen.
Reviewed By: peterogithub
Differential Revision: D4485452
fbshipit-source-id: beae15b
Summary:
Propagating the arguments read in .inferconfig shouldn't be necessary as they
are parsed by each executable. There are corner cases where I think this diff
could change the behaviour of infer (eg, .inferconfig redefines project-root
and subsequent exes read a different .inferconfig thanks to the project-root in
INFER_ARGS), but I don't think there are good use cases like that.
Reviewed By: jberdine
Differential Revision: D4475092
fbshipit-source-id: 5c020d0
Summary:
Clients should use `Config.parse_action` instead to figure in what mode they
are operating.
In particular, the biggest change is in logging. Take the `parse_action` into
account instead of the exe, and change the log/ subdirectories to be "capture",
"driver", "analyze", and "print", corresponding to the various phases of an
infer run.
Reviewed By: jberdine
Differential Revision: D4474943
fbshipit-source-id: 6d33ad3
Summary:
Support several parsing modes: Infer, Javac, NoParse
The "Infer" mode specifies a list of sections, ie the parts of infer that are affected by an option (corresponds to the old notion of "exes"):
analysis, clang frontend, print, ...
- .inferconfig and INFER_ARGS always parsed
- outside .inferconfig and INFER_ARGS, do not parse subcommand arguments before the subcommand has been activated
- command-line is parsed or not based on the subcommand/executable selected
- executable dictates subcommand, so almost nothing depends on the executable outside of Config. Another diff will restrict the API around exes to reflect this.
Reviewed By: jberdine
Differential Revision: D4474886
fbshipit-source-id: 442dfef
Summary: Just to make it a bit more interesting (and better indented).
Reviewed By: martinoluca
Differential Revision: D4455400
fbshipit-source-id: f8e29ee
Summary:
Some classes may have deleted new operator for them. To fix it, run global `new` operator instead
```
struct X {
void* operator new(size_t) = delete;
};
X *p = new X; // compilation error
X *p = ::new X; // no compilation error
```
This change is following same strategy standard headers follow.
Reviewed By: jvillard
Differential Revision: D4500977
fbshipit-source-id: 20babfa
Summary:
Infer used to report null dereference when field was accessed later:
```
vector<int> v;
int& a = v[0]; // should be EMPTY_VECTOR_ACCESS here, but it wasn't reported
int b = a; // was NULL_DEREFERENCE here
```
To avoid this problem, model all accesses to vector as dereference of its internal `beginPtr` field.
Reviewed By: jberdine
Differential Revision: D4481942
fbshipit-source-id: 2142894
Summary: This should fix the issue with broken invariants when the method specialization on pointer ends up doing a substitution on non pointer types
Reviewed By: sblackshear
Differential Revision: D4487232
fbshipit-source-id: f3fce84
Summary:
When the receiver type and return type of an unknown call are the same, propagate taint to both the receiver and the return type.
This does the right thing for common "builder-style" methods that both update and return the receiver.
We already had custom models for a few such methods (e.g., `StringBuilder.append`), but we can remove them now.
Reviewed By: jeremydubreil
Differential Revision: D4490071
fbshipit-source-id: 325ea88
Summary: The Buck integration assumes that the output jar file of all the dependency targets are available locally in order to retrieve the analysis from these targets. However, this is not guaranteed to be true when there is a cache hit on some targets. Running `buck build` with the option `--deep` forces this property.
Reviewed By: jvillard
Differential Revision: D4474036
fbshipit-source-id: accabfa
Jeremy Dubreil
Sam Blackshear
Jules Villard
Dulma Churchill
Dino Distefano
Cristiano Calcagno
Andrzej Kotulski
Kihong Heo