Summary:
This simplifies a bit the code to run the analysis on all the prcedures in the cluster. Before, the functions procedure_should_be_analyzed, which loads the attributes, and get_proc_desc were called twice for the analysis of every procedure.
The objective is to remove the calls to procedure_should_be_analyzed and hide it from the ondemand API since it is already called before the analysis of every procedure.
Reviewed By: sblackshear
Differential Revision: D4553397
fbshipit-source-id: 02cffaf
Summary:
One gets very obscure errors when trying to run infer for clang when it was
compiled for Java, or vice-versa. This diff makes sure we crash early with the
appropriate error message. For instance:
```
$ ./build-infer java
$ infer -- clang -c hello.c
Uncaught exception:
(Failure
"Unsupported build mode: make/cc\
\nInfer was built with clang analyzers disabled.\
\nPlease rebuild infer with clang enabled.\
\n")
Raised at file "pervasives.ml", line 30, characters 22-33
Called from file "backend/infer.ml", line 398, characters 6-48
Called from file "backend/infer.ml", line 449, characters 20-38
$ infer --clang-compilation-db-files foo.json
Uncaught exception:
(Failure
"Unsupported build mode: clang compilation database\
\nInfer was built with clang analyzers disabled.\
\nPlease rebuild infer with clang enabled.\
\n")
Raised at file "pervasives.ml", line 30, characters 22-33
Called from file "backend/infer.ml", line 392, characters 8-65
Called from file "backend/infer.ml", line 449, characters 20-38
```
Reviewed By: sblackshear
Differential Revision: D4566641
fbshipit-source-id: d9a118f
Summary:
- inferbo introduced a dependency to extlib. When building Java analyzers, this
is implicitly pulled in by javalib, but it's missing when building only the
clang analyzers. Add `extlib` to the packages we build against.
- infer.ml and Javac.ml depend on Javalib, but it's easy to push down the code
that needs it to `jMain.ml` so that we can build without javalib for the
clang-only case.
- jMain.mli had 2 copies: one in java/ and one in java_stubs/. Make one a symlink to the other.
Reviewed By: jeremydubreil
Differential Revision: D4566581
fbshipit-source-id: 214a4eb
Summary:
Sevel auxiliary files made it to the output directory of the analysis of individual targets when analyzing Java projects build with Buck. However, these files are then taken into account= to compute the target rule key and then to decide whether to analyze the dependent targets. Since these auxiliary files were containing time sentive information, every cach miss on a given target would then invalitate the cache entries for all the dependent targets.
This diff cleans up the output directory to only keep the specs files, the `global.tenv` and the `report.json` files which are the only artifacts needed to analyze the dependent targets
This diff makes a minimal number of changes to see how it behaves in prod, but I intend to refoctor this more when continuing to add support for running Infer with genrules
Reviewed By: sblackshear
Differential Revision: D4562615
fbshipit-source-id: 4628420
Summary:
Xcode's compilation databases follows a different convention than cmake's and
escape the `"file"` and `"dir"` fields of each unit to make them shell-ready.
We need to treat them differently when reading them.
This adds a new `--clang-compilation-db-files-escaped` option and makes the
code related to reading compilation databases deal correctly with both
conventions.
Reviewed By: akotulski
Differential Revision: D4559239
fbshipit-source-id: 51120ae
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:
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:
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:
One of the things that confuses me about the current annotations API is that there's a lot of ways to do the same thing.
Some of the concepts like `annotated_signature` are only really needed by Eradicate.
This diff removes usages of `annotated_signature` outside of Eradicate (everyone else was just using `get_annotated_signature` as a roundabout way to get the return annotation of a procedure).
In the future, I'll move `get_annotated_signature` and other Eradicate-specific functionality into its own module inside the Eradicate directory.
Reviewed By: jberdine
Differential Revision: D4472058
fbshipit-source-id: 5bb0846
Summary:
`pdesc_has_annot` checks the annotations of both the return values and the parameters, which seems like a bad idea in general.
The client should have to specify which annotations they actually care about.
Converting existing uses of `pdesc_has_annot` to what I read as the intended behavior (checking the return annotation).
Will make better use of the other new functions in a follow-up.
Reviewed By: jeremydubreil
Differential Revision: D4469885
fbshipit-source-id: de5531e
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:
Epilogue tasks such as closing logs or putting files back were we found them
run automatically at the end of our executables by registering them with
`at_exit`. They do not run if the program is interrupted by a signal. This diff
makes sure they are run when the user stops infer with Ctrl-C (SIGINT).
Reviewed By: cristianoc
Differential Revision: D4435575
fbshipit-source-id: c3ab702
Summary:
This replaces the previous integration written in Python, which consisted in 1)
run the mvn command and parse its output to locate "directories containing
source files", 2) run on files named "*.java" in these directories. This meant
we had to run javac twice on each source file, and more importantly this
mechanism of finding source files was very fragile. In fact, I could not make
it work on several mvn projects I tried.
The new integration is based on parsing "pom.xml" to add an "infer-capture"
profile which instructs mvn to run `/path/to/infer` instead of `javac`. We also
add this profile to each maven submodule.
Users can specify an "infer-capture" profile themselves if the default one
doesn't work; in that case we don't inject our own "infer-capture" profile.
Reviewed By: jeremydubreil
Differential Revision: D4409613
fbshipit-source-id: d664274
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:
Make the html output available to checkers when -g is used on the command-line.
A checker needs to call a function to start and finish the processing of each node,
and add prints during the processing.
This diff illustrates the case for Eradicate, by adding printing of the pre-state
and post-states.
Reviewed By: sblackshear
Differential Revision: D4421379
fbshipit-source-id: 67501ba
Summary:
Previously, we would first compute which build command is at hand, based on the
first argument after "infer --", then do everything depending on that piece of
information. However, the build command alone is not enough to know in which
"build mode" we are operating. For instance, there are several build modes
corresponding to "buck" build commands.
This led to duplication of the logic (to retrieve which build mode we are in in
the various phases of an infer run), and some invariants that had to be
re-asserted at various points in the code, eg that the arguments are not empty.
This diff adds a `build_mode` type (renaming the previous `build_mode` to
`build_system`) that identifies the various integrations we support. We compute
the build mode at the start of infer, then pass the build mode around.
Also, move `run_javac` to a new `integration/Javac.ml` file given that it's a
bit large.
Reviewed By: jberdine
Differential Revision: D4415074
fbshipit-source-id: db854a0
Summary:
Currently, if we don't find `-d` or `-classes_out` on the command line then we
tell javac to redirect the compiled classes in some other directory, by default
the initial working directory. But we don't detect when these arguments are
hidden inside files (`foo` arguments on the javac command line) so the
heuristic was incomplete. Look inside these files to better tell whether we need
to make up an output directory or not.
Reviewed By: jeremydubreil
Differential Revision: D4397716
fbshipit-source-id: 30c5e4f
Summary:
Sometimes we don't want to analyze but a message gets printed that there was
nothing to analyze and we exit with error, which is confusing.
Reviewed By: jberdine
Differential Revision: D4398120
fbshipit-source-id: 43ce3ab
Summary:
Add more debug output to be able to trace the calls to javac more easily
when --stats or --debug is passed to infer.
Reviewed By: sblackshear
Differential Revision: D4398100
fbshipit-source-id: 3012900
Summary:
This would fail before and works as expected now:
```
$ infer -- clang -c hello.c
$ cd infer-out/ && ln -s ../foo && cd ..
$ infer -- clang -c hello.c # crashes because it fails to delete infer-out/foo
```
Reviewed By: jberdine
Differential Revision: D4398763
fbshipit-source-id: 38465f8
Summary:
One of the tests was failing without `make clean` because infer-out didn't get
deleted when rerunning the clang db test. This was because infer thinks it's in
`Analyze` mode when capturing clang db files.
Reviewed By: akotulski
Differential Revision: D4397731
fbshipit-source-id: 26f423a
Summary:
This error message is confusing when the user is not actually running
InferPrint, eg `infer foo`: `Load Error: file foo: arguments must be .specs
files`.
With this diff, we don't get any error for `infer foo`, which is not great
either and will need to be addressed (do we support all the python arguments in
OCaml now too and are able to turn on argument parsing errors in OCaml land?).
Reviewed By: jberdine
Differential Revision: D4397765
fbshipit-source-id: e7ca48f
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:
Force clients to specify the path relative to which relative paths
should be made absolute.
Reviewed By: akotulski
Differential Revision: D4370262
fbshipit-source-id: 36a2807
Summary:
There is not much to redirect except for an uninformative line before proper
logging files are set up. This is from before the current logging system, which
has builtin support for logging into custom files.
Reviewed By: jberdine
Differential Revision: D4365988
fbshipit-source-id: 044290a
Summary:
Instead of opening new log files each time with non-deterministic names, keep
appending to the same log files. This only removes the randomized part of the
names in the files. In particular, it keeps the name prefixes for, eg, clang
source files.
Also changed most "<executable>/<executable>-out.log" to simply "<executable>/out.log".
Reviewed By: jberdine
Differential Revision: D4365983
fbshipit-source-id: 46792dc
Summary: This more easily allow to switch between the different modes for handeling dynamic dispatch
Reviewed By: sblackshear
Differential Revision: D4367556
fbshipit-source-id: 795d2c4
Summary: 957b243 removed the last use of `Exe_env.get_tenv ~create:true`
Reviewed By: jeremydubreil
Differential Revision: D4364521
fbshipit-source-id: 819efee
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: Adding the information that a procedure has been modelled as part of the attributes, during the translation, instead of getting this information from where is the summary loaded from. This is more consistent with the use of the attributes in other parts of the analysis, but is also useful in the context of the lazy dynamic dispatch algorithm where the procedures, including the models, are cloned and reanalyzed with more specialized parameters. The information about whether a procedure is a model must persist when cloning the procedures.
Reviewed By: sblackshear
Differential Revision: D4356892
fbshipit-source-id: 40ff5ca
Summary:
When you try to log an error on a procedure P and a summary for P doesn't exist, the error gets quietly dropped on the floor.
But we should fail loudly instead, because this should only happen in the case of a user error.
Got burned by this today; I was trying to log an error on the *caller* of `Integer.parseInt`, but was accidentally logging it to `Integer.parseInt` itself instead.
Since no summary for that method exists, my error wasn't appearing.
Reviewed By: jvillard, jeremydubreil
Differential Revision: D4355546
fbshipit-source-id: db2a0e6
Summary:
The two concepts are not negation of each other. The type environment created by the different frontends is not guaranteed to contain a full view of the type hierarchy. In this case, there can be holes preventing Infer to prove that `t <: t'` if the type definition between `t` and `t'` is missing. There are now two functions:
# `is_known_subtype` when the subtyping relation can be proven
# `is_known_not_subtype` when it can be proven that there is no subtyping relation between two types
This diff is intended to make no functional changes but to add functionality to detect cast error angelically, i.e. assuming that the program is probably fine where there is not enough information to prove the cast error.
Reviewed By: jberdine
Differential Revision: D4345803
fbshipit-source-id: 39b79bc
Summary: Different analyses need different preanalyses to run. It doesn't make sense for all of the pre-analyses to be bundled together into one package.
Reviewed By: jvillard
Differential Revision: D4348243
fbshipit-source-id: 46a8ebd
Dulma Churchill
Jeremy Dubreil
Jules Villard
Cristiano Calcagno
Sam Blackshear
Andrzej Kotulski
Kihong Heo
Josh Berdine