Summary:
Previously, we would correctly be silent on code like `x = new T(); x.f = ...`, but would wrongly warn on code like `x = makeT(); x.f = ...`.
The reason is that we only allowed ownership through direct allocation.
This diff adds a boolean that specifies whether the return value is owned as part of the summary.
This allows us to correctly handle many common cases of (transitively) returning a freshly allocated object, but still won't work for understanding that ownership is maintained in examples like
`x = new T(); y = id(x); y.f = ...`.
Reviewed By: jvillard
Differential Revision: D4456864
fbshipit-source-id: b5eec02
Summary:
Eradicate currently considers a field initialized if it's simply accessed (not written to),
or initialized with another initialized field.
This fixes the issue.
Reviewed By: jvillard
Differential Revision: D4449541
fbshipit-source-id: 06265a8
Summary:
If we have code like
```
o.setF(source())
sink(o)
```
and `setF` is an unknown method, we probably want to report.
Reviewed By: jeremydubreil, mburman
Differential Revision: D4438896
fbshipit-source-id: 5edd204
Summary:
In code like
```
foo(o) {
iWriteToF(o)
}
```
, the condtional write to `f` in `iWriteToF` should become a conditional write for `foo`.
Reviewed By: peterogithub
Differential Revision: D4429160
fbshipit-source-id: f111ac4
Summary:
In code like
```
foo() {
Object local = new Object();
iWriteToAField(local);
}
```
, we don't want to warn because the object pointed to by `local` is owned by the caller, then ownership is transferred to the callee.
This diff supports this by introducing a notion of "conditional" and "unconditional" writes.
Conditional writes are writes that are rooted in a formal of the current procedure, and they are safe only if the actual bound to that formal is owned at the call site (as in the `foo` example above).
Unconditional writes are rooted in a local, and they are only safe if a lock is held in the caller.
Reviewed By: peterogithub
Differential Revision: D4429131
fbshipit-source-id: 2c6112b
Summary:
Races on volatile fields are less concerning than races on non-volatile fields because at least the read/write won't result in garbage.
For now, let's de-prioritize these writes by ignoring them.
Reviewed By: peterogithub
Differential Revision: D4434023
fbshipit-source-id: 05043ba
Summary:
Also make sure we don't introduce deprecated options in our repo, eg when
calling infer from infer.
Reviewed By: jeremydubreil
Differential Revision: D4430379
fbshipit-source-id: 77ea7fd
Summary: Just cleanup; gives us slightly less test code to maintain.
Reviewed By: jeremydubreil
Differential Revision: D4429265
fbshipit-source-id: d43c308
Summary:
Similar to marking classes ThreadConfined, we want to support marking fields as well.
The intended semantics are: don't warn on writes to the marked field outside of syncrhonization, but continue to warn on accesses to subfields.
Reviewed By: peterogithub
Differential Revision: D4406890
fbshipit-source-id: af8a114
Summary:
Adding models that allow us to warn on unguarded accesses to subclasses of `Map`, but not on accesses of threadsafe containers like `ConcurrentMap`.
Lots more containers to model later, but stopping at `Map`s for now to make sure the approach looks ok.
Reviewed By: jvillard
Differential Revision: D4385306
fbshipit-source-id: d791eee
Summary: Need to upgrade in order to specify some taint properties on a more recent `WebView` API.
Reviewed By: cristianoc
Differential Revision: D4382590
fbshipit-source-id: 0925742
Summary: These methods should only be called from other methods that also run on the UI thread, and they should not be starting new threads.
Reviewed By: peterogithub
Differential Revision: D4383133
fbshipit-source-id: 6cb2e40
Summary: Use the lazy dynamic dispatch by default in prod for the Java analysis
Reviewed By: sblackshear
Differential Revision: D4356872
fbshipit-source-id: 491e92e
Summary:
Without this it's not always obvious which test fails. It also makes it easier
to mass-patch test failures from the CI jobs to replace expected outputs with
actual outputs (eg, when debugging osx frontend tests from linux).
Reviewed By: jberdine
Differential Revision: D4352205
fbshipit-source-id: 8887d7b
Summary:
We currently can only model the return values of functions as sources.
In order to model inputs of endpoints as sources, we need the capability to treat the formals of certain functions as sources too.
This diff adds that capability by adding a function for getting the tainted sources to the source module, then using that info in the analysis.
Reviewed By: jeremydubreil
Differential Revision: D4314738
fbshipit-source-id: dd7d423
Summary:
Previously, summaries worked by flattening the access tree representing the post of the procedure into (in essence) a list of functions from caller input traces to callee output traces.
This is inefficient in many ways, and is also much more complex than just using the original access tree as the summary.
One big inefficiency of the old way is this: calling `Trace.append` is slow, and we want to do it as few times as possible.
Under the old summary system, we would do it at most once for each "function" in the summary list.
Now, we'll do it at most once for each node in the access tree summary.
This will be a smaller number of calls, since each node can summarize many input/output relationships.
Reviewed By: jeremydubreil
Differential Revision: D4271579
fbshipit-source-id: 34e407a
Summary: Don't warn on NotThreadSafe class, particularly when super is ThreadSafe
Reviewed By: sblackshear
Differential Revision: D4334417
fbshipit-source-id: 0df3b9d
Summary:
SuppressWarnings annotations are hardly used and add considerable
complexity due to requiring recompilation with an annotation processor.
Reviewed By: jvillard
Differential Revision: D4312193
fbshipit-source-id: c4fc07e
Summary:
If these collections don't encapsulate their state properly, there are bigger problems than thread safety issues :).
Plus, these warnings are less-than-actionable for non-Guava maintainers.
Reviewed By: peterogithub
Differential Revision: D4324277
fbshipit-source-id: cacfbf0
Summary:
Maintain an "ownership" set of access paths that hold locally allocated memory that has not escaped.
This memory is owned by the current procedure, so modifying it outside of synchronization is safe.
If an owned access path does escape to another procedure, we remove it from the ownership set.
Reviewed By: peterogithub
Differential Revision: D4320034
fbshipit-source-id: 64f9169
Summary: Rename the intermediate .exp.test files to .exp.test.noreplace so that they don't match the regexp used by `make test-replace`. Otherwise they can accidentally become .exp files that will show up in `git status`.
Reviewed By: cristianoc
Differential Revision: D4319436
fbshipit-source-id: df2ef21
Summary:
Before the diff, the code was considering as Nullable any annotation ending with `...Nullable`, including `SuppressParameterNotNullable`.
Closes#533
Reviewed By: jberdine
Differential Revision: D4317356
fbshipit-source-id: 6091c0f
Summary: Adding Buck `DEFS` macros for generating Infer genrules. The generated genrules can be used to run the analysis on any existing `java_library` targets.
Reviewed By: sblackshear
Differential Revision: D4291234
fbshipit-source-id: 6430e2e
Summary:
The Java frontend translates exceptions by assigning them to the return value.
This leads to weird behavior when the return type of the function is void.
Already handled one case of this in Quandary (ignoring assignments of exceptions to return value), but was missing the case where null is assigned to the return value.
The frontend does this to "clear" the value of previously assigned exceptions.
Reviewed By: jeremydubreil
Differential Revision: D4294060
fbshipit-source-id: 6bef5ef
Sam Blackshear
Cristiano Calcagno
Jules Villard
Peter O'Hearn
Josh Berdine
Jeremy Dubreil