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:
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:
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:
Remove the remaining uses of polymorphic equality `=`.
In case of basic types, this is replaced by String.equal or Int.equal.
In case of `= []`, this is replaced by `List.is_empty`.
In case of `= None`, this is replaced by `is_none`.
In case of a datatype definition such as `type a = A | B`,
a `compare_a` function is defined by adding `type a = A | B [@deriving compare]`
and a `equal_a` function is defined as `let equal_a = [%compare.equal : a]`.
In case of comparison with a polymorphic variant `= `Yes`, the equality
defined in `PVariant.(=)` is used. Typically, `open! Pvariant` is added
at the beginning of the file to cover all the uses.
Reviewed By: jberdine
Differential Revision: D4456129
fbshipit-source-id: f31c433
Summary: This will be useful in upcoming changes to the thread-safety analysis as well.
Reviewed By: dkgi
Differential Revision: D4402146
fbshipit-source-id: c750127
Summary:
A domain should not definite its initial state, since distinct users of the domain may want to choose different initial values.
For example, one user might want to bind all of the formals to some special values, and one user might want the initial domain to be an empty map
This diff makes this distinction clear in the types by (a) requiring the initial state to be passed to the abstract interpreter and (b) lifting the requirement that abstract domains define `initial`.
Reviewed By: jberdine
Differential Revision: D4359629
fbshipit-source-id: cbcee28
Summary:
There's a lot of boilerplate work to be done when adding a new kind of source.
This diff tries to reduce the boilerplate by making a functor do all the work.
The functor:
(1) adds a notion of "footprint kind" to the source
(2) packages the source with a call site
Reviewed By: jvillard
Differential Revision: D4349224
fbshipit-source-id: 5e1701a
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:
Use In_channel and Out_channel operations instead of those in Pervasives. Don't
use physical equality on values that aren't heap-allocated since it doesn't help
the compiler generate faster code and the semantics is unspecified. Also use
phys_equal for physical equality.
Reviewed By: sblackshear
Differential Revision: D4232459
fbshipit-source-id: 36fcfa8
Summary:
Utils contains definitions intended to be in the global namespace for
all of the infer code-base, as well as pretty-printing functions, and
assorted utility functions mostly for dealing with files and processes.
This diff changes the module opened into the global namespace to
IStd (Std conflict with extlib), and moves the pretty-printing
definitions from Utils to Pp.
Reviewed By: jvillard
Differential Revision: D4232457
fbshipit-source-id: 1e070e0
Summary:
In Java, we handle unknown code by propagating behavior from the parameters of the unknown function call to the return value (or constructed object, in the case of a constructor). But we do this in a somewhat silly way--generating a new summary with these semantics at each unknown call site. Instead, this diff introduces these two options as predefined behaviors and adds specialized code for them.
As a side effect of this approach, unknown functions are no longer counted as passthroughs. This is ok; the original behavior was less of a reasoned decision and more of an unintended consequence of the way we decided to handle unknown code.
This new approach ought to be more efficient than the old one, and as a virtuous side effect it will be easier to specify how to handle unknown code in other languages like C++.
Reviewed By: jeremydubreil
Differential Revision: D4205624
fbshipit-source-id: bf97445
Summary:
There's not really a concept of callee here, so s/callee/callsite/, and "to"
suggests we get the callee whereas we update it, so s/to/with/.
Feel free to bikeshed further.
Reviewed By: sblackshear
Differential Revision: D4153426
fbshipit-source-id: 6ea762c
Summary: A must-have for reporting taint errors and any other interprocedural error where the trace is sufficiently complex.
Reviewed By: jvillard
Differential Revision: D4124072
fbshipit-source-id: 26b3b2b
Summary: A must-have for reporting taint errors and any other interprocedural error where the trace is sufficiently complex.
Reviewed By: jvillard
Differential Revision: D4106352
fbshipit-source-id: b2677e6
Summary:
Previously, we recorded direct sinks as sinks and transitive sinks as passthroughs. This makes it difficult to create an expanded interprocedural trace when recording an error because we can't distinguish between sinks (which we want to expand) and passthroughs (which we don't). This diff changes recording of sinks so that a sink is now the *last* function in a trace to call a sink. To find out what the original sink was, the summary for the transitive sink in the trace will now need to be (recursively) expanded until we bottom out in the original sink.
Will do the same for sources in a follow-up diff.
Reviewed By: cristianoc
Differential Revision: D4103759
fbshipit-source-id: 6f435f5
Summary:
this makes frontends no longer depend on SymExec.ml. `ModelBuiltins` was split into two modules:
- `BuiltinDecl` with procnames for builtins (used to determine whether some function is a builtin)
- `BuiltinDefn` with implementations used by `SymExec`
- they both have similar type defined in `BUILTINS.S` which makes sure that new builtin gets added into both modules.
During the refactor I ran some scripts:
`BuiltinDecl.ml`:
let X = create_procname "X"
cat BuiltinDecl.ml | grep "create_procname" | tail -70 | awk ' { print $1,$2,$3,$4,"\42"$2"\42"} '
then manually confirm string match. Exceptions:
"__exit" -> "_exit"
"objc_cpp_throw" -> "__infer_objc_cpp_throw"
__objc_dictionary_literal
nsArray_arrayWithObjects
nsArray_arrayWithObjectsCount
`BuiltinDefn.ml`:
let X = Builtin.register BuiltinDecl.X execute_X
cat BuiltinDecl.ml | grep "create_procname" | tail -70 | awk ' { print $1,$2,$3,"Builtin.register BuiltinDecl."$2,"execute_"$2} '
then, fix all compilation problems
Reviewed By: jberdine
Differential Revision: D3951035
fbshipit-source-id: f059602
Summary:
Right now, taint gets lost if it flows into a constructor or procedure whose implementation is missing.
Since the core Java (e.g., String) and Android classes (e.g, Intent) are among these, this is bad.
We could handle this by writing a bunch of models instead, but that would be a lot of work (plus we may still miss cases).
Reviewed By: jvillard
Differential Revision: D4051591
fbshipit-source-id: 65851c8
Summary:
Change Sil.Call instruction to have only a single optional return
identifier, insted of a list. Essentially none of the code handled
multiple return identifiers. Also, add the type of the return
identitifier to Call instructions.
Reviewed By: sblackshear
Differential Revision: D3919358
fbshipit-source-id: d2d4f72
Summary:
Move Sil.call_flags type and operations into separate CallFlags
module.
Reviewed By: dulmarod
Differential Revision: D3548086
fbshipit-source-id: 6d264e9
Summary: Move Sil.unop type and operations into separate Unop module.
Reviewed By: dulmarod
Differential Revision: D3548077
fbshipit-source-id: 49d3d83
Summary: Move Sil.const type and operations into separate Const module.
Reviewed By: dulmarod
Differential Revision: D3548073
fbshipit-source-id: 388d03e
Summary:
Move closure values from const to exp. They are not constants, and
this reduces interdependence between Sil types.
Reviewed By: sblackshear
Differential Revision: D3541364
fbshipit-source-id: 1a2f998
Summary:
Now that array types record only static - and therefore constant -
lengths, Sil typ and exp no longer need to be mutually recursive.
This diff:
- splits the recursion in the type definitions of typ and exp,
- splits the recursion in the comparison and pretty-printing
functions,
- and then refactors typ into a separate module.
Reviewed By: cristianoc
Differential Revision: D3423575
fbshipit-source-id: 6130630
Summary:
This diff refactors Sil.Int, which represents integer literals, into a
separate module IntLit. There are no dependencies forcing Sil.Int to
be a submodule of Sil, and it is also no simpler as a submodule.
Reviewed By: cristianoc
Differential Revision: D3422910
fbshipit-source-id: 63013f2
Summary:
Results of AbsInt checkers are node id -> abstract state maps.
It's hard to compare/combine the results of multiple analyses if the node id types are different.
Needed for the upcoming improvements of the preanalysis.
Reviewed By: jvillard
Differential Revision: D3235669
fb-gh-sync-id: c5251cf
fbshipit-source-id: c5251cf
Summary:public
When a conditional is the last instruction, there will be a join node leading directly to the exit node.
Some instructions, such as nullification of dead variables, and abstraction, are added to the control flow graph automatically. But, join nodes cannot contain instructions. So when a procedure ends with a conditional, there might be no place to store these instructions.
This diff adds one extra node between the join and the exit node in that situation.
Reviewed By: jvillard
Differential Revision: D3179056
fb-gh-sync-id: 2b9cd7e
fbshipit-source-id: 2b9cd7e
Summary:public
Eliminate the use of the -open Utils command line option passed to the compiler in favor of `open! Utils` in each source file. While slightly convenient, this option causes more headaches than it is worth with other tools e.g. merlin.
Reviewed By: jvillard
Differential Revision: D3168193
fb-gh-sync-id: 4285ef6
fbshipit-source-id: 4285ef6
Summary:public It's useful for the transfer functions to understand what the current procedure is. Applications include debugging, implementing context-sensitivity, asking which program variables are parameters, and the list goes on.
In the future, we'll almost certainly want to pass the tenv to the transfer functions as well.
Reviewed By: jberdine
Differential Revision: D3104997
fb-gh-sync-id: 1c0df8f
fbshipit-source-id: 1c0df8f
Sam Blackshear
Cristiano Calcagno
Kihong Heo
Josh Berdine
Jules Villard
Andrzej Kotulski
Lázaro Clapp Jiménez Labora