Summary:
Races in Nullsafe classes can undermine NPE safety despite the class passing the type checks.
This diff adds to the report text of THREAD_SAFETY_VIOLATION and GUARDEDBY_VIOLATION the following trailer:
> Data races in `Nullsafe` classes may still cause NPEs.
This only happens if the race is directly on a non-primitively-typed member field of the class.
It also uses distinct bug types (adds the suffix _NULLSAFE to the bug types above) for easier accounting.
Reviewed By: ezgicicek
Differential Revision: D26403274
fbshipit-source-id: 3cd6ca082
Summary: As there are no dependencies between procedure and file analyses in RacerD, split them into separate modules.
Reviewed By: ezgicicek
Differential Revision: D26198874
fbshipit-source-id: 032aad9d8
Summary:
The `--pulse-model-return-nonnull` config option currently works for C++. Now we
will be using it also for Java. Changing type from string list to regexp to
make it more general.
Reviewed By: ezgicicek
Differential Revision: D26367888
fbshipit-source-id: 9a06b9b32
Summary:
Modeling Java instanceof operator in Pulse. This
implementation does not yet provide the proper semantics for instanceof.
For now, it will always return true. This is temporary and should reduce the false positive rate.
Reviewed By: da319
Differential Revision: D26317089
fbshipit-source-id: 494e3dec5
Summary: D25952894 (1bce54aaf3) changes translation of struct assignments. This diff adopts to this change for loads from global struct arrays.
Reviewed By: skcho
Differential Revision: D26398627
fbshipit-source-id: cc1fb47ab
Summary:
Before this diff:
```
// Summary of const global
// { global -> v }
n$0 =* global
// n$0 -> {global}
x *= n$0
// x -> {global}
```
However, this is incorrect because we expect `x` have `v` instead of the abstract location of `global`.
To fix the issue, this diff lookups the initializer summary when `global` is evaluated as RHS of load statement.
After this diff:
```
// Summary of const global
// { global -> v }
n$0 =* global
// n$0 -> v
x *= n$0
// x -> v
```
Reviewed By: ezgicicek
Differential Revision: D26369645
fbshipit-source-id: 98b1ed085
Summary:
Sometimes purity running failed because it couldn't find inferbo mem. Let's make it print a warning
message, instead of raising an exception.
Reviewed By: ezgicicek
Differential Revision: D26367275
fbshipit-source-id: d2350e855
Summary:
`SettableFuture.set` invokes callbacks registered prior to the call, which may also try to acquire extra locks. If the called of `set` already holds a lock this creates lock dependencies which may lead to deadlocks.
Here we warn whenever `set` is called under a lock taken in a different source file. This avoids reporting when a class internally manages locks and calls `set`, reasoning that developers will be aware this is happening.
Reviewed By: jvillard
Differential Revision: D25562190
fbshipit-source-id: d1b5cb69c
Summary:
Currently there is a direct cycle of dependencies Var -> Trm ->
Var. Inside Trm, these two modules are defined as mutually recursive
modules, so at some level this is ok. Due to internals of how
recursive modules are compiled, recursive modules that are
"unsafe" (that is, export some value of non-function type) are
compiled more efficiently. This diff moves Var from being defined
recursively with Trm to being a submodule of Trm. This eliminates that
Var -> Trm -> Var cycle and enables making Trm an "unsafe"
module. (Trm is still mutually recursive with Arith (and Arith0), but
they are "safe".)
The difference between how "safe" and "unsafe" recursive modules are
compiled, in this context, is that the safe ones are first initialized
to a block containing dummy function closures that immediately raise,
then the unsafe ones are initialized as normal modules, and then the
safe ones are back-patched. A consequence of this is that calls to
functions in safe recursive modules are "unknown" calls, and they are
implemented as loading from a pointer to a closure and calling the
generic caml_apply function. In contrast, calls to functions in normal
or "unsafe" modules can be known, and get compiled to direct assembly
calls to the statically resolved callee. The second case is
faster. Additionally, in the indirect case, the callee is unknown, and
so some register spilling and restoring is also potentially
involved. Normally (I guess), this difference in performance is not
significant, but it can be significant for e.g. compare or hash
functions of modules used as container keys / elements, where the
container data structure is very hot.
Reviewed By: ngorogiannis
Differential Revision: D26250517
fbshipit-source-id: ecef49b32
Summary:
Variable ids are currently unique non-negative integers, and register
ids are unique positive integers, so using register ids negated as
variable ids yields a situation where all variable ids are unique.
Reviewed By: jvillard
Differential Revision: D26250544
fbshipit-source-id: 9e47e9776
Summary:
The names of instructions that produce void results, other than Call
instructions, are not used. So do not consume ids for them.
There are actually very many of these, so the saved work during
translation can be significant.
Reviewed By: jvillard
Differential Revision: D26250529
fbshipit-source-id: f9eea5684
Summary:
The source block name is normally visible in the LLAIR code, while the
name of e.g. a branch instruction is some internal number. This change
only makes the output LLAIR code slightly easier to read.
Reviewed By: jvillard
Differential Revision: D26250521
fbshipit-source-id: 4723a58f7
Summary:
Sometimes symbols are added to the symbol table multiple times during
translation from LLVM to LLAIR. For example, this happens when a
`llvm.dbg.declare` instruction is encountered that attaches a debug
location to a symbol. Currently when this happens, the symbol name is
regenerated unnecessarily. This is not economical, and since counters
are used in some cases to avoid clashes, this can cause visible
changes to names.
This diff fixes this, and also makes the location update more robust
by not relying on the location added last being the best.
Reviewed By: jvillard
Differential Revision: D26250542
fbshipit-source-id: 5d52ce193
Summary:
`Sh.and_ b q` normalizes `b` using the equality context of `q` and
then conjoins the result to `q`. This is incorrect in case normalizing
`b` results in expressing it using existentials of `q`, which takes
the existentials out of their scope. So this diff changes from
essentially
`(∃x.Q) ∧ B = (∃x.Q) ∧ (∃x.Bρ)`
to
`(∃x.Q) ∧ B = (∃x'.Q[x'/x] ∧ Bρ)`
where `ρ` is the substitution that normalizes with respect to the
equality context.
Reviewed By: jvillard
Differential Revision: D26250536
fbshipit-source-id: 05f5c48c0
Summary:
Add assertion to detect if the variable a function call modifies
appears in the actual parameters. In this case, the implementation is
incomplete.
Reviewed By: jvillard
Differential Revision: D26250527
fbshipit-source-id: d2e81a467
Summary:
Instructions and terminators are not themselves unique, there can be
two instructions that compare equal in distinct blocks, and likewise
for terminators. Such clashes make the coverage statistics report
incorrectly low coverage. This diff fixes this by also considering the
parent blocks.
Reviewed By: jvillard
Differential Revision: D26250514
fbshipit-source-id: 93f916eab
Summary:
Instead of passing the Var.strength across interfaces, pass the Trm.pp
partially applied to the Var.strength. This hides the type of
Var.strength, and thereby removes the need for `Arithmetic.S.var`.
Reviewed By: jvillard
Differential Revision: D26250516
fbshipit-source-id: 64dbd3870
Summary:
This diff resets the id generator before generating ObjC getter/setter, so parsed results are the
same without regard to the generation order. Note that the order may change when we change the type
of Procname.t since their hash values are used for the hash set of procnames.
Reviewed By: ngorogiannis
Differential Revision: D26277348
fbshipit-source-id: a66d77845
Summary:
We are getting lots of FPs due to modeling `Provider.get` as expensive. This is coming from Dependency Injection and Infer cannot statically determine the type of the provider and determine whether that provider is expensive (requires a global analysis and instrumentation).
Instead, we are downgrading this method to the default constant cost.
Reviewed By: skcho
Differential Revision: D26223978
fbshipit-source-id: 79f81c997
Summary:
Dear Infer team,
To contribute to Infer community, I would like to integrate infer#'s language agnostic layer into Infer.
Please help to review, discuss and consider to merge this feature.
Thanks,
Xiaoyu
Pull Request resolved: https://github.com/facebook/infer/pull/1361
Reviewed By: skcho
Differential Revision: D25928458
Pulled By: jvillard
fbshipit-source-id: 7726150b8
Summary: Added some basic examples for Objective-C we want to address next in pulse nullptr dereference analysis. In particular, we should not get a `nil` dereference error when we call a method on `nil`, except if the method returns a non-POD (Plain Old Data) type.
Reviewed By: ezgicicek
Differential Revision: D26053402
fbshipit-source-id: 66f4600c3
Summary: The wrapper functions do not get inlined enough to avoid excess allocations.
Reviewed By: jvillard
Differential Revision: D25946110
fbshipit-source-id: 8dc3d8259
Summary:
This diff replaces Fml.fold_dnf, which eagerly enumerates a
disjunctive-normal form expansion, with iter_dnf which can be iterated
lazily. Fml.iter_dnf is then used in Context.dnf. This means that the
full DNF expansion does not need to be allocated by
e.g. Smtlib.check_sat, which yields huge peak memory savings.
Reviewed By: jvillard
Differential Revision: D25946111
fbshipit-source-id: e6d179e9f
Summary:
Currently detection and propagation of consequences of newly added
equality constraints is implemented using some operations that
traverse the entire representation of the equality relation. This
implementation strategy makes it relatively easy and robust to ensure
that the consequences of constraints are treated in a sound and
complete way. It does have the drawback that adding a constraint
involves work that is proportional (nonlinearly even) to the number of
all constraints. This can become a performance bottleneck.
This diff reimplements the core operations using an additional data
structure that essentially tracks the "super-term" relation between
terms, which can be used to look up the existing constraints that
might be involved in deriving new consequences when adding
equalities. This means that adding new equalities no longer takes time
proportional to the size of the entire equality context
representation, but only to the number of existing constraints that
might interact with the new constraint. Also, rewriting terms
into canonical form no longer takes time proportional to size of the
whole context, but only performs table lookups on the subterms of the
term to canonize.
Reviewed By: jvillard
Differential Revision: D25883705
fbshipit-source-id: b3f266533
Summary:
This diff changes Context.extend to not only add terms and their
subterms to the carrier, but to simultaneously normalize the added
term. The normalized form is currently ignored, but it will be needed
when adding use lists.
Reviewed By: jvillard
Differential Revision: D25883709
fbshipit-source-id: bfe06f2a8
Nikos Gorogiannis
Gabriela Cunha Sampaio
Ezgi Çiçek
Joshua Selbo
Loc Le
Sungkeun Cho
Josh Berdine
Jules Villard
Xiaoyu Liu
Daiva Naudziuniene