Summary: Add some basic command line API to run Infer using Buck genrules. Remains to fix issues with absolute vs relative paths and to see how to create these genrules on the fly for a given java or android library.
Reviewed By: sblackshear
Differential Revision: D4245622
fbshipit-source-id: 1cda4ee
Summary:
Clean up code related to --changed-files-index option:
1. Store DB.SourceFileSet.t in DB.changed_source_files_set
2. Refactor rest of the code to use it
3. Bunch of minor changes to make code more consise
Reviewed By: jberdine
Differential Revision: D4238736
fbshipit-source-id: 51e5684
Summary:
Implement heuristic to capture more of the user code:
In C++ there is a lot of interesting code in header files. On the other hand,
that code gets included in multiple places and we don't want to capture it by default (for performance reasons).
Right now we capture everything from source file + all symbols from headers that source file needs.
New heuristic will extend "capturing everything" to matching header files (ie. capture everything in X.h if source file is X.cpp)
Reviewed By: jberdine
Differential Revision: D4238008
fbshipit-source-id: 0528250
Summary:
Dealing with symbolic links in project root is tricky. To avoid it, always normalize all paths to sources with `realpath`.
Changes to tests are expected - infer started to resolve symbolic links which screws up with our testing mechanism.
Reviewed By: jberdine
Differential Revision: D4237587
fbshipit-source-id: fe1cb01
Summary:
Before, we were using a set domain of strings to model a boolean domain.
An explicit boolean domain makes it a bit clear what's going on.
There are two things to note here:
(1) This actually changed the semantics from the old set domain. The set domain wouldn't warn if the lock is held on only one side of a branch, which isn't what we want.
(2) We can't actually test this because the modeling for `Lock.lock()` etc doesn't work :(.
The reason is that the models (which do things like adding attributes for `Lock.lock`) are analyzed for Infer, but not for the checkers.
We'll have to add separate models for thread safety.
Reviewed By: peterogithub
Differential Revision: D4242487
fbshipit-source-id: 9fc599d
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:
Let's introduce some concepts. A "known unknown" function is one for which no Java code exists (e.g., `native`, `abstract`, and `interface methods`). An "unknown unknown" function is one for which Java code may or may not exist, but we don't have the code or we choose not to analyze it (e.g., non-modeled methods from the core Java or Android libraries).
Previously, Quandary handled both known unknowns and unknown unknowns by propagating taint from the parameters of the unknown function to its return value. It turns out that it is really expensive to do this for known unknown functions. D4142697 was the diff that starting handling known unknown functions in this way, and bisecting shows that it was the start of the recent performance problems for Quandary.
This diff essentially reverts D4142697 by handling known unknowns as skips instead. Pragmatically, doing the propagation trick for Java/Android library functions (e.g., `String` functions!) matters much more, so i'm not too worried about the missed behaviors from this. Ideally, we will go back to the old handling once performance has improved (have lots of ideas there). But I need this to unblock me in the meantime.
Reviewed By: jeremydubreil
Differential Revision: D4205507
fbshipit-source-id: 79cb9c8
Summary:
Useful for refactoring purposes, to provide a list of modules in
dependency order.
Reviewed By: jeremydubreil
Differential Revision: D4232363
fbshipit-source-id: 2adaaf5
Summary:
Implement heuristics to get from corresponding source files for header files.
We already had initial implementation for CaptureCompilationDatabase - moved it and extended it
to handle C/C++/objC/objC++
Reviewed By: jberdine
Differential Revision: D4231864
fbshipit-source-id: 4516287
Summary:
introduce `AttributesTable.load_defined_attributes` which will return proc attributes only if the procedure is defined. In order to not mess up
with existing caching, create another hashmap to store those procdescs.
We need to do that because with reactive capture we no longer can assume that all proc attributes are final before analysis starts
Reviewed By: jberdine
Differential Revision: D4231575
fbshipit-source-id: e795bcb
Summary:
This will help during the creation of new checkers, and will prevent errors like misspelling of AST node names.
It will also make it possible to fail immediately during the parsing of CTL inputs.
Reviewed By: ddino
Differential Revision: D4205434
fbshipit-source-id: ed8631a
Summary: Make backend know filenames of compilation database. It will allow it to compile extra files when needed
Reviewed By: cristianoc
Differential Revision: D4231521
fbshipit-source-id: c462448
Summary: Pure refactoring simplifying the code doing the case analysis for execturing the cast instruction.
Reviewed By: dulmarod
Differential Revision: D4215238
fbshipit-source-id: 9f0f163
Summary: Currently the thread safety checker neglects to analyze and files methods we don't want to report on. Like constructors and private methods, and classes where no superclass is marked ThreadSafe. For interprocedural analysis we want to analyze all these to get summaries, even if we don't report on them.
Reviewed By: jberdine
Differential Revision: D4226515
fbshipit-source-id: 7571573
Summary:
`abs_source_file_from_path` is dangerous because
1. It's not trying to make path relative to `project_root` - you can't compare two files created with two different methods
2. It's making relative paths relative to `getcwd` instead of `project_root` which is different from `from_string` function.
Also remove unused `DB.inode_equal`
Reviewed By: cristianoc
Differential Revision: D4204891
fbshipit-source-id: c4c2f99
Summary:
1. Always store cpp model source_file with relative path. This will make them cache friendly independent of infer location
2. Distinguish between "relative to project root" and "relative to infer models src"
3. Unify `source_file_from_path` used by java and C frontends into one function. There are no improvements to that logic yet
4. Move `is_cpp_model_file` to use `source_file` instead of `filename`
Reviewed By: jberdine
Differential Revision: D4204548
fbshipit-source-id: 6e21771
Summary:
This will help during the creation of new checkers, and will prevent errors like misspelling of operators' kinds.
It will also make it possible to fail immediately during the parsing of CTL inputs.
Reviewed By: ddino
Differential Revision: D4212956
fbshipit-source-id: c3c7fe7
Summary: This option is unused and it's making DB.source_file_* API harder to simplify
Reviewed By: cristianoc
Differential Revision: D4219803
fbshipit-source-id: 23ce697
Summary:
Sometimes, we'll need to have extra information in compilation database.
To keep the code simple just keep information about all files and filter
data when scheduling jobs. Perf shouldn't be much worse since we only start one
process for infer invocation.
Reviewed By: jvillard
Differential Revision: D4130884
fbshipit-source-id: 9920c11
Summary:
Using DB.source_file_to_string may return relative path which may or may not be relative to current location.
I went through all calls to `DB.source_file_to_string` and fixed ones that used that output to open files
Reviewed By: jeremydubreil
Differential Revision: D4205407
fbshipit-source-id: b285b7e
Summary:
Add test for compilation database and --changed-files-index option
Fix one bug that the test uncovered
Reviewed By: dulmarod
Differential Revision: D4198502
fbshipit-source-id: 9039c65
Summary:
This diff adds a dependency on ctypes.foreign and uses the example
binding of fts to implement remove_directory_tree.
Reviewed By: yunxing
Differential Revision: D4115569
fbshipit-source-id: 3509955
Summary: D4189956 killed the phantom space printed after types, but the Context leak message was relying on it :).
Reviewed By: jeremydubreil
Differential Revision: D4208591
fbshipit-source-id: 9f0d709
Summary:
Developers will sometimes write GuardedBy("T.f") with the intended semantics: "guarded by the field f of the object with type T in the current state".
We want to support this to avoid false positives.
Reviewed By: peterogithub
Differential Revision: D4197476
fbshipit-source-id: acd00d9
Summary:
- fix python calling function with wrong number of arguments (sic)
- print legend of analysis output, this was lost in translation ('F', '.', ...)
- add "Capturing in <mode> mode..." message before capture
- remove version from "Analyzing..." message (users don't even paste the full output, so this is not as useful as initially hoped)
Reviewed By: akotulski
Differential Revision: D4205072
fbshipit-source-id: 2b6505c
Summary: These are dangerous if you are trying to compare a type to a string, and they're also unsightly.
Reviewed By: jvillard
Differential Revision: D4189956
fbshipit-source-id: 14ce127
Summary:
This diff implements enough of the functionality in the python code in
the OCaml toplevel driver that executing `infer -- analyze` is done
with direct procedure calls instead of forking the python interpreter.
Except for some reporting code that remains in report.py.
Reviewed By: jvillard
Differential Revision: D4074718
fbshipit-source-id: 56a794d
Summary: Using address equality check to short-circuit comparison of equal lists faster + kill use of `next`.
Reviewed By: jeremydubreil
Differential Revision: D4189581
fbshipit-source-id: bdf5d1e
Summary:
SIOF is only for interactions between objects of non-POD types. Previously the
checker was also reporting for POD types.
Reviewed By: akotulski
Differential Revision: D4197620
fbshipit-source-id: 7c56571
Summary:
Record an abstraction of the bug traces in the tests. The abstraction of a
trace is the sequence of descriptions. In practice, descriptions are either
empty, or of the form "start/end/return from/call to procedure X". They seem
pretty stable.
Motivation: there is nothing testing the traces reported by Infer right now,
even though they are surfaced to developers. For instance, Quandary uses
--issues-txt instead of --issues-tests to make sure the traces do not regress.
This change would make this approach more widespread.
Reviewed By: sblackshear
Differential Revision: D4159597
fbshipit-source-id: 9c83952
Summary:
clang errors would get diverted to log files instead of being printed on the
console. Also log other types of clang errors and warnings more often.
Before:
$ infer -- clang -c nosuchfile.c
$ # nothing gets printed, exit code set to 1
After:
$ infer -- clang -c nosuchfile.c
clang-4.0: error: no such file or directory: 'nosuchfile.c'
Also print more messages in case of compilation errors:
$ echo ')' >> a.c
$ infer -- clang -c a.c
a.c:1:1: error: expected identifier or '('
)
^
1 error generated.
Error: the following clang command did not run successfully:
'/home/jul/infer/facebook-clang-plugins/clang/install/bin/clang-4.0' "@/home/jul/infer/examples/infer-out/clang/clang_command_2a0a84.txt"
(only the last 2 lines are new)
Reviewed By: akotulski
Differential Revision: D4183039
fbshipit-source-id: b9a9065
Summary: clang has very complicated logic what to translate based on `project_root` and filename. Add tests for different situations in regard of symbolic links in path/project_root
Reviewed By: jvillard
Differential Revision: D4168551
fbshipit-source-id: 586b364
Summary: Generalizing jvillard's awesome work to include passthroughs in traces, then calling it from Quandary.
Reviewed By: jvillard
Differential Revision: D4172108
fbshipit-source-id: 0296c59
Summary:
Translate local variable names using the bytecode directly instead of JBir. The bytecode has more precise type information.
We still need to declare the temporary variables intruduced by Sawja until we can base the translation directly on the bytecode.
Reviewed By: sblackshear
Differential Revision: D4185309
fbshipit-source-id: 81904a0
Summary: Refactoring to make thread safety checker interpocedural. This should not change funcitonality, and will only set things up for making the interprocedural part more serious.
Reviewed By: sblackshear
Differential Revision: D4124316
fbshipit-source-id: 6721953
Summary:
When loading results from a json file, sort them. This prints results in some
sane order for both --issues-test and --issues-txt, removing the need for
post-processing of the result.
Reviewed By: cristianoc
Differential Revision: D4167029
fbshipit-source-id: 37e9f1c
Summary: The thread safety checker is run independently of other analyses, using the command "infer -a threadsafety -- <build-command>".
Reviewed By: sblackshear
Differential Revision: D4148553
fbshipit-source-id: bc7b3f9
Summary: Infer starts from bytecode generated by the compilation commands, so there isn't much need to check if the bytecode is compliant or not.
Reviewed By: sblackshear
Differential Revision: D4179218
fbshipit-source-id: 2f27148
Summary: If seems that we were dropping succesful loaded classes for no reason
Reviewed By: sblackshear
Differential Revision: D4178079
fbshipit-source-id: 827c0b9
Summary:
Our patch to Javalib has been accepted, so we can parse programs with invokedynamic!
invokedynamic still crashes Sawja, but I have worked around this by replacing all invokedynamic's with invokestatic's before passing them to Sawja.
This means we can handle everything about invokedynamic except calling the correct function (I call a dummy function with the correct signature for now).
We can try to actually call the right method in the future.
Reviewed By: jvillard
Differential Revision: D4160384
fbshipit-source-id: a8ef4e1
Summary: When searching for cast errors, types that were not Java objects, e.g. arrays of primitive types were not taken into account, leading to incorrect class cast excpetion reports.
Reviewed By: sblackshear
Differential Revision: D4166184
fbshipit-source-id: 7157c95
Summary: Speed up capture on MacOS by caching results of realpath. Improvements to lint speed on MacOS will be sent later
Reviewed By: jvillard
Differential Revision: D4166902
fbshipit-source-id: 4d065bc
Summary:
This adds generic support for reporting error traces as usual infer issues
traces (instead of putting them in the textual description of the error) to
Trace.ml and SinkTrace.ml.
The siof checker is made to use these new traces, and gets an improved error
message mentioning the name of the problematic global as well, which requires a
slight API change in Pvar.re.
The support in Trace.ml is incomplete: passthroughs are ignored. This missing
feature will be needed by Quandary to migrate its error messages.
Reviewed By: sblackshear
Differential Revision: D4159542
fbshipit-source-id: 8c1101d
Summary:
`install` will not do anything if the file didn't change, which should give
`make` more opportunities to not do work.
Reviewed By: jeremydubreil
Differential Revision: D4161918
fbshipit-source-id: 9b9061a
Summary:
- set SHELL to bash explicitly in Makefiles (Debian uses dash)
- avoid using system headers when using our own clang's headers in tests
- do not rely on the name of the object file to write the frontend debugging scripts. It turns out that `-o` is *not* always present in the arguments of `-cc1` functions so the `Option.get` could crash. Since we don't actually need to get the object file name, just a nice enough name, don't try to be smarter at guessing what object will be created and pick a different name built from the source name instead.
Reviewed By: akotulski
Differential Revision: D4159516
fbshipit-source-id: c7bc2b9
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:
It was defined in two places and I'm about to add a third, so let's share
instead.
Reviewed By: sblackshear
Differential Revision: D4153420
fbshipit-source-id: 3d2c519
Summary:
In some error conditions, it could happen that the log file directory
was created early in execution, and then deleted, causing the creation
of log files to then fail.
Reviewed By: sblackshear
Differential Revision: D4157986
fbshipit-source-id: 1548b08
Summary: If a procedure is both a source and a sink for the same value, and it's a sink first, you will get a false positive when applying the summary for the procedure.
Reviewed By: cristianoc
Differential Revision: D4145246
fbshipit-source-id: 97f0022
Summary:
This diff ports checkCopyright to Core, builds it separately from other
executables, and fixes the build command in the linter.
Reviewed By: jvillard
Differential Revision: D4148217
fbshipit-source-id: 8aefc98
Summary: Mark native methods as defined so that the analysis generates a summary for those methods. When analyzing Java projects compiled with Buck, the summaries for the dependencies methods of are retrieved from the classpath. In this case, having access to the summary is useful to access the attributes of a callee when the callee is part of a, previously analyzed, Buck target.
Reviewed By: sblackshear
Differential Revision: D4141362
fbshipit-source-id: 75888c8
Summary:
Only the python code passes --models, always does so, and only ever
passes Config.models_jar.
Reviewed By: jeremydubreil
Differential Revision: D4151094
fbshipit-source-id: 8f21c03
Summary:
Move code that initializes the InferAnalyze executable from
InferAnalyze.main to InferAnalyzeExe. This enables InferAnalyze.main to
be called from other executables without conflicts due to
initialization.
Reviewed By: jvillard
Differential Revision: D4137280
fbshipit-source-id: 3dd76db
Summary: Also use the executable as a default name prefix.
Reviewed By: akotulski, jvillard
Differential Revision: D4135539
fbshipit-source-id: 84ba011
Summary:
Analyses should handle methods whose code is unknown and methods whose summary is a no-op differently.
Previously, this was done correctly for some kinds of methods (e.g., native methods, which were recognized as unknown), but not for others (interface and abstract methods).
This diff makes sure we correctly treat all three kinds as unknown.
Reviewed By: jeremydubreil
Differential Revision: D4142697
fbshipit-source-id: c88cff3
Summary:
Location.nLOC was introducing a lot of complexity for little benefit (and edge cases were wrong anyway).
We can restore it in some simplified way if we find that we need it
Reviewed By: jeremydubreil
Differential Revision: D4139868
fbshipit-source-id: 4f8e033
Summary: This fixes the build when `./configure --disable-c-analyzers` is used.
Reviewed By: jberdine
Differential Revision: D4146818
fbshipit-source-id: bec4b48
Summary:
Fix potential performance problems in `CLocation` module
1. Don't call `Unix.stat` to compare files if it's enough to compare paths
2. Use C implementation of `realpath` and call it only when it's really necessary
This diff breaks `Location.nLOC` information for whole clang frontend, but it's going away soon anyway
Reviewed By: jvillard, jberdine
Differential Revision: D4132526
fbshipit-source-id: f01afe8
Summary: Having only place where the code runs the transformation of the Java bytecode into the JBir reporesentation allows to more easily start manipulating the JBir representation and the bytecode together and progressively move the translation based on bytecode instead of JBir.
Reviewed By: sblackshear
Differential Revision: D4137576
fbshipit-source-id: c483528
Summary:
Summaries are modified before saving from disk, for example the attributes of the postcondition can change.
I have observed flaky reports of the internal error NULL_TEST_AFTER_DEREFERENCE. Some attributes (e.g. assigned) are changed before saving, but the spec table in memory is not changed.
So in case:
1) the procedure is analyzed on-demand, then subsequent uses in the same process use the summary in memory with the unchanged attribute, and the issue is not reported.
2) the procedure is already on disk and loaded, then the loaded summary has the changed attributes, and the issue is reported.
Flakiness happens as because of parallelism, whether a procedure is analyzed already or whether it is analyzed on-demand, can change.
The normalization function can change the instrumentation of a symbolic heap because it uses the existing comparison functions, which ignore instrumentations.
So normalization can replace part of a symbolic heap with an identical one but where the instrumentation is different — this is what I have observed.
The diff uses a different comparison function where instrumentations are taken into account.
Reviewed By: jberdine
Differential Revision: D4140031
fbshipit-source-id: f4f119a
Summary:
Instead of the custom filtering done by `InferPrint --issues-tests`, use the
filtering done by `infer` and run without filtering for our e2e tests. We still
test the filtering for our build systems integration tests, and this diff
restores that behaviour for the ant test (hence the bugs removed from
ant/issues.exp).
Also add internal exceptions to most tests to get more signal out of them (eg,
knowing when we add assertion failures and the like).
Retire the old `--issues-tests` to limit the number of ways we do filtering.
Reviewed By: jeremydubreil
Differential Revision: D4131308
fbshipit-source-id: 35805cc
Summary:
curr_file is remnant of the past when we didn't have good location information
coming from Clang_ast_t location. But it was fixed ~1 year ago.
Reviewed By: jvillard
Differential Revision: D4139750
fbshipit-source-id: 4ce7235
Summary:
This will be useful to migrate the existing tests to using report.json to
output the list of bugs found by Infer. This will make the tests reflect what
happens in prod more faithfully: right now running with --issues-tests does its
own filtering starting from the specs.
Moreover, this will allow --issues-tests to support the Buck integration, where
the specs/ directory is not populated after a run (although I suppose we could
also copy them from buck-out/ for InferPrint's benefit).
Reviewed By: jeremydubreil
Differential Revision: D4130851
fbshipit-source-id: 0457fba
Summary:
This makes our python code work (instead of crashing) when the source file
should be found not from the current directory (or absolute path), eg with
`infer --project-root .. -- clang -c hello.c`.
Reviewed By: jeremydubreil
Differential Revision: D4130802
fbshipit-source-id: 001f72d
Summary:
If the project root contains ".." then it doesn't work as expected, eg
infer --project-root .. -- clang hello.c
doesn't report at all. Now it works.
Reviewed By: jeremydubreil
Differential Revision: D4125489
fbshipit-source-id: 06b10ad
Summary: These functions are also called when the summary is guaranteed to exist. Enforcing this within the API
Reviewed By: cristianoc
Differential Revision: D4126839
fbshipit-source-id: 305b484
Summary: For some reason, `Specs.is_active` was re-loading from the specs table the summary that should already be in scope.
Reviewed By: cristianoc
Differential Revision: D4124693
fbshipit-source-id: c0e9113
Summary:
The Quandary-style traces are too general for checkers like SIOF.
This diff adds a "suffix abstraction" of the trace for analyses that just care about sinks.
To show how to use it, we add it to SIOF.
Note: this diff converts the domain, but isn't actually doing the fancier reporting yet.
That will come in a future diff.
Reviewed By: jvillard
Differential Revision: D4124881
fbshipit-source-id: 5b9fd07
Summary: Right now there is no test for compilation database integration. Add one
Reviewed By: jvillard
Differential Revision: D4118769
fbshipit-source-id: 5591de7
Summary:
Our default strategy for handling unknown code is to propagate taint from the actuals to the return value.
But for commonly-used methods like `StringBuilder.append` (used every time you do `+` with a string in Java), this doesn't work.
The taint should be propagated to both the receiver and the return value in these cases.
I'm considering a solution where we always propagate taint to the receiver of unknown functions in the future, but I am concerned about the performance.
So let's stick with a few special string cases for now.
Reviewed By: cristianoc
Differential Revision: D4124355
fbshipit-source-id: 5b2a232
Summary: Other checkers are going to start using these, so they shouldn't live in the Quandary directory anymore
Reviewed By: jvillard
Differential Revision: D4124654
fbshipit-source-id: b1d5bdd
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:
This diff adds a skeleton implementation of the capture and analysis
driver to infer.ml, and removes some unnecessary code from infer.py.
With this, individual capture and analysis modules can be added, or
moved from python.
Reviewed By: jvillard
Differential Revision: D4109547
fbshipit-source-id: 0dce2bf
Summary: Don't use a hardcoded string, and enable reports in --issues-tests.
Reviewed By: jvillard
Differential Revision: D4110731
fbshipit-source-id: 9922557
Summary:
ClusterMakefile need not depend on Sys.executable_name referring to
InferAnalyze, use Config.bin_dir instead.
Reviewed By: jvillard
Differential Revision: D4110730
fbshipit-source-id: c330bb3
Summary:
Child processes invoked in multicore mode get arguments using the usual
INFER_ARGS mechanism already, no need for a special case.
Reviewed By: jvillard
Differential Revision: D4110728
fbshipit-source-id: 0987216
Summary: Don't ignore errors coming from clang when there are exceptions raised by the frontend
Reviewed By: jvillard
Differential Revision: D4117375
fbshipit-source-id: f31d6cf
Summary:
The Quandary-style traces are too general for checkers like SIOF.
This diff adds a "suffix abstraction" of the trace for analyses that just care about sinks.
To show how to use it, we add it to SIOF.
Note: this diff converts the domain, but isn't actually doing the fancier reporting yet.
That will come in a future diff.
Reviewed By: jvillard
Differential Revision: D4117393
fbshipit-source-id: e473665
Summary: Other checkers are going to start using these, so they shouldn't live in the Quandary directory anymore
Reviewed By: jvillard
Differential Revision: D4117359
fbshipit-source-id: e3f151e
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:
New version of clang plugin exports `-x` arg information as a part of
TranslationUnitDecl. Get it from there instead of reading it from
clang argv
Reviewed By: jvillard
Differential Revision: D4112652
fbshipit-source-id: 5c3af1f
Summary:
The build system may expect the assembly commands to run. The only issue with
assembly commands is that we shouldn't attach the plugin to them.
This diff also moves the logic of what to capture to the `Capture.capture`
function to be able to reuse code from Capture. This makes sense because the
Capture module is the one with the knowledge of what to actually capture or
not.
Reviewed By: akotulski
Differential Revision: D4096019
fbshipit-source-id: 7fc99e1
Summary: We want to skip readwrite locks for now, maybe report on their misuses later.
Reviewed By: sblackshear
Differential Revision: D4110998
fbshipit-source-id: 986f77e
Summary:
This diff moves the implementation that considers the default value of
--models to be Config.models_jar if it exists from analyze.py to ZipLib.
Reviewed By: cristianoc
Differential Revision: D4100421
fbshipit-source-id: 322fbcf
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 diff unifies the --specs-library and --zip-specs-library options,
delaying the point where their relative order gets lost to at least
after option parsing. This also moves the treatment of non-cached jar
libraries from Config to where they are used in ZipLib. The
implementation of expanding the cached jars is slightly simplified and
untangled from option parsing.
Reviewed By: jvillard
Differential Revision: D4100015
fbshipit-source-id: cf840a7
Summary: This is assumed everywhere, and so set it in `ClangWrapper` instead of `InferClang`
Reviewed By: jberdine
Differential Revision: D4095766
fbshipit-source-id: f77625e
Summary:
See code comment about `throw exn` being translated as `return exn`.
This problem was revealed by D4081279, which started grabbing access paths from exceptions.
Reviewed By: jvillard
Differential Revision: D4096391
fbshipit-source-id: 9d91513
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: Doing `sychronized(A.class)` where `A` is an inner class was not previously recognized by the `GuardedBy` checker.
Reviewed By: peterogithub
Differential Revision: D4095094
fbshipit-source-id: c832f9e
Summary:
Now that it's possible to run clang wrapper as a function from another process,
Logging module cannot rely on `Config.current_exe` to determine which directory
it should write to.
Reviewed By: jeremydubreil
Differential Revision: D4095455
fbshipit-source-id: d989b06
Summary:
InferClang knows what to do if its name ends in ++. So it is not
necessary to pass whether or not the original clang executable ended in
++ to InferClang using the the INFER_XX environment variable. Instead,
create an InferClang++ symbolic link and make the clang wrapper call
either InferClang or InferClang++ as needed.
Reviewed By: jvillard
Differential Revision: D4078416
fbshipit-source-id: 3b5d5d0
Summary:
We issue a thread safety warning on a class not
marked ThreadSafe, when it has a super that is. This makes some sense. But,
it will be nice to remind that a super is so maeked, else the mesg could
seem out of context or surprising
Reviewed By: sblackshear
Differential Revision: D4075145
fbshipit-source-id: ebc2b83
Summary:
This diff revises the makefiles for java tests so that they are based on
the files actually produced and depended on, instead of the existing
imperative style. This is, I think, clearer and easier to modify, and
enables a little more parallelism.
Reviewed By: jvillard
Differential Revision: D4072560
fbshipit-source-id: c16d4bd
Summary: Now that InferClang is in ocaml there is nothing stopping us from exposing functionality of `InferClang` as a function in addition to binary
Reviewed By: jvillard
Differential Revision: D4081026
fbshipit-source-id: 86d500b
Summary:
Change command line options for dynamic dispatch to capture that the
alternatives are mutually exclusive.
Reviewed By: jeremydubreil
Differential Revision: D4074540
fbshipit-source-id: c329717
Summary: The lazy dynamic dispatch algorithm works by re-analyzing the generic methods with the more specialized types encountered during the symbolic execution. In order to do that, the analysis must access the procedure description of the method to reanalyze in order to run the analysis of the specialized procedure description on demand. This diff adds the procedure description on the summary as the summary are stored in the Buck cache and can easily be retrieved by procname.
Reviewed By: sblackshear
Differential Revision: D4077415
fbshipit-source-id: c2f1cc8
Summary:
- do a semantic analysis of each variable initializer to figure out if they need initialization
- add a flag to globals that is true when they are `constexpr`. In that case, no analysis is needed as the user + compile guarantee that it is a compile-time constant.
Reviewed By: sblackshear
Differential Revision: D4081273
fbshipit-source-id: 44dbe29
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: For some reason, the frontend was always caching the name of the translated classes even when the `--dependencies` was not passed
Reviewed By: jberdine
Differential Revision: D4074225
fbshipit-source-id: 8aa2c79
Summary:
Merging the results directories of targets on buck projects involved creating symbolic links into buck-out.
The bulk of files are .attr files: one per procedure. Creating these links can be a bottleneck, and the merge phase can be slower than the analysis phases on projects with many procedures.
This diff introduces multilinks to speed up merge.
A multilink is a file `multilink.txt` containing a sequence of paths
```
path/to/file1.ext
path/to/file2.ext
...
```
A multilink file is a compact way to represent a link for each entry.
This diff creates a multilink file for each `attributes/dir` directory, instead of one symbolic link for each file.
Reviewed By: jberdine
Differential Revision: D4067428
fbshipit-source-id: 911f8a9
Summary: The frontend replaces global variables that are constant with their values as a quick hack to improve the precision of the analysis. This should apply to `constexpr` too.
Reviewed By: dulmarod
Differential Revision: D4058097
fbshipit-source-id: be4fea6
Summary: This diff simplifies the workflow of creating the procedure descriptions. Instead of creating the all procedure descriptions in a first step and translating the method bodies afterwards, it is simpler to translate to do the two in one step.
Reviewed By: cristianoc
Differential Revision: D4067674
fbshipit-source-id: be9e853
Summary: Doing like this makes it easier to keep the phony declarations in sync with the target definitions
Reviewed By: jvillard
Differential Revision: D4067679
fbshipit-source-id: 723bc0e
Summary: Fix the resolution of symbolic links. The previous version did not work if the path itself was not a symbolic link, but there was a symbolic link somewhere up in the path tree. For example: `path/to/file` where `file` is not a symbolic link, but `to` is a symbolic link.
Reviewed By: jberdine
Differential Revision: D4062947
fbshipit-source-id: 394221d
Summary: Creating a "fake" procedure description the methods that are called is no longer required by the backend. So this diff cleans up the creation of the procedure descriptions
Reviewed By: jberdine
Differential Revision: D4057185
fbshipit-source-id: b444756
Summary: This code is an old experiement and has never really be used in prod because it was creating false positive. Dealing static final fields should be done in the backend instead so that it can used by the different languages C, Objective C, C++ and Java.
Reviewed By: jberdine
Differential Revision: D4055292
fbshipit-source-id: f1dc715
Summary:
Config.analyze_models, set by the INFER_ANALYZE_MODELS environment
variable, is redundant with Config.models_mode.
Reviewed By: jvillard
Differential Revision: D4047338
fbshipit-source-id: 4522d65
Summary:
Also be more careful when escaping arguments and create a module for shared
functionality between the clang frontend and the buck compilation database.
Reviewed By: jberdine
Differential Revision: D4036627
fbshipit-source-id: c981184
Summary: Some arguments passed from infer.ml to infer.py were only used to pass further to infer.ml invocations. Those args should be passed by env variable anyway (???)
Reviewed By: jberdine
Differential Revision: D4048003
fbshipit-source-id: 6f5fbeb
Summary:
Fix an issue where, when `-reactive` mode is used, files captured in the first second are not considered modified, and are not analyzed. This happens because file timestamps are used, and the resolution is one second.
Change the front-ends to change the timestamp of the directory where artifacts are created, so that the timestamps are 1 second in the future.
Small reactive commands such as the following now analyze correctly:
rm -rf infer-out && infer --reactive -- clang -c test.c
Reviewed By: jberdine
Differential Revision: D4050689
fbshipit-source-id: 6271860
Summary:
Move compilation database into separate module which loads said database from json file.
It will allow to load database from json file without calling buck.
Reviewed By: dulmarod
Differential Revision: D4049255
fbshipit-source-id: b2fa29f
Summary:
The integration would not work if other arguments were passed to Buck via infer
using Xbuck.
Reviewed By: akotulski
Differential Revision: D4044371
fbshipit-source-id: 742b5b3
Summary:
Declared and defined procedure attributes are now saved in different files (hashed_name.decl.attr and hashname.attr).
We always try to load using the filename of defined procedure attributes first,
and fall back to loading the file for declared ones if it does not exist.
The logic for replacing an existing file stays the same, with one extra thing:
when a file for a defined attribute is written, the one for the declared one
is deleted if it exists.
At the end of a capture, either a declared or a defined file exist, but not both.
The reason for this change is that when captures of different subprojects are
merged together, it can happen that a link gets created to a declared attributes
file even though a defined one exists, so the body of the procedure will not be analyzed.
After this diff, both links will be created, and the defined one will be loaded
by the back-end.
Reviewed By: dulmarod
Differential Revision: D4037423
fbshipit-source-id: 74fb7e6
Summary: failing to resolve was making the Java analysis to report errors with absolute paths instead of relative paths.
Reviewed By: sblackshear
Differential Revision: D4032764
fbshipit-source-id: e316193
Summary:
Python isn't needed anymore to pass options between `infer` and `InferClang`.
However, it is still needed to set up `PATH` so that we pick up compilation
commands.
Reviewed By: jberdine
Differential Revision: D4008469
fbshipit-source-id: 05c5716
Summary: This avoids issues where the command-line may get too large.
Reviewed By: jberdine
Differential Revision: D4008328
fbshipit-source-id: c1558b9
Summary:
This also adds `-a compile` support to `InferClang`. This is needed for the
`xcodebuild` integration, which is hard to fold into the same binary as the
rest.
Reviewed By: jberdine
Differential Revision: D4008262
fbshipit-source-id: 0bbd53f
Summary:
Checker for the Static Initialization Order Fiasco pattern:
https://isocpp.org/wiki/faq/ctors#static-init-order
1. Collect all globals (transitively) accessed in any given procedure.
2. Once the interprocedural analysis has finished, look at globals accessed in
initializers that do not belong to the current translation unit.
Reviewed By: sblackshear
Differential Revision: D3780266
fbshipit-source-id: 1d07161
Summary:
Create dummy functions representing the initializers of global variables. This
is so we can implement checks in the backend that can look at the initializer
expressions of global variables. We try not to create these dummy functions
when the initializer is not present, although for some reason we sometimes end
up with empty initializers.
Also add source file info to global variables in the backend (Pvar.re).
Reviewed By: sblackshear
Differential Revision: D3780238
fbshipit-source-id: 2dca87e
Summary:
There's no reason for infer to be in lib/ anymore, move it to the same place as
the other binaries. Thus all binaries are in the same directory and Config.ml
can better know where things are.
Reviewed By: jberdine
Differential Revision: D4015958
fbshipit-source-id: c5e851f
Summary: `tput cols` spams the terminal when it finds `$TERM` confusing. Reimplement what we need, which is very little, in C.
Reviewed By: jberdine
Differential Revision: D3960620
fbshipit-source-id: afe357e
Summary:
Before, if I wrote code like
```
x = src()
sink(x)
sink(x)
```
we would report three times instead of two.
The first flow would be double-reported.
Reviewed By: jeremydubreil
Differential Revision: D4024678
fbshipit-source-id: fcd5b30
Summary: when a method has writes to a field outside of synchrnoization, issue an appropriate error message identifying the fields
Reviewed By: sblackshear
Differential Revision: D4015612
fbshipit-source-id: 4f697fc
Summary:
This diff adds a make target to generate interface files from
implementation files. These generated interface files can then be used
as a starting point for documenting and restricting the exposed module
interface. For example, to generate an interface for JavaTaintAnalysis.ml,
execute:
```
make -C infer/src M=quandary/JavaTaintAnalysis mli
```
Note that this relies on `ocamlc -i`, which for reason currently
produces syntactically ill-formed files.
Reviewed By: sblackshear, jvillard
Differential Revision: D3998175
fbshipit-source-id: f653737
Summary: Also make sure it's not dead code, so we don't break it again by accident.
Reviewed By: jeremydubreil
Differential Revision: D4015793
fbshipit-source-id: 017d862
Summary:
During the development/debugging of AST checks, it will be possible to emit dotty graphs with a representation of the evaluation of formulas.
The formulas, expressed using the notation of CTL (https://en.wikipedia.org/wiki/Computation_tree_logic) are represented in a graph alongside the current ast-node and their final evaluation result (green for true, red for false)
To get the dotty graph, run infer with the `--debug` flag
Reviewed By: ddino
Differential Revision: D3937787
fbshipit-source-id: 163e17d
Summary: Avoid polluting stdout and stderr for executables that are always supposed to log into files.
Reviewed By: dulmarod
Differential Revision: D4008888
fbshipit-source-id: 1366498
Summary: Nothing mutates those fields so there is no need to make them `mutable`
Reviewed By: cristianoc
Differential Revision: D4009166
fbshipit-source-id: b840a4b
Summary:
This fixes a perf issue on large files, where a copy of the type environment and control flow graph were loaded for each procedure analyzed in the file.
If the type environment or the control flow graph are big, and the file contains many procedures, this can cause a big memory overhead.
Reviewed By: jvillard
Differential Revision: D4008655
fbshipit-source-id: 11d07c1
Summary:
This changes executions of the former InferClang into a function call. In
particular, it can be called several times per execution.
The new InferClang must be called as if it was clang, and knows how to run
clang with our plugin to get the AST of the source file.
Reviewed By: akotulski
Differential Revision: D3981017
fbshipit-source-id: 7af6490
Summary:
This diff removes the unused support for reporting props, which enables
refactoring so that the 'base' directory has no dependencies, and the
'IR' directory depends only on 'base'.
Reviewed By: jvillard
Differential Revision: D3981352
fbshipit-source-id: 3700a23
Summary:
Color modules in dependency graph based on directory, and cluster
modules together into a subgraph if their directory is listed in the
`clusters` variable of infer/src/Makefile.
Reviewed By: akotulski
Differential Revision: D3979253
fbshipit-source-id: dffd76b
Summary: Introduce `--enable-ocamlopt-custom-cc` configure flag (disabled by default). Normally it doesn't have to be set. However, when cross-compiling infer itself for another platform it may have to be set.
Reviewed By: jberdine
Differential Revision: D3995032
fbshipit-source-id: ce2fd72
Summary: it seems to have no effect on analysis. As such it should be ok to add cg nodes for builtin model calls
Reviewed By: jberdine
Differential Revision: D3967399
fbshipit-source-id: 06c32e5
Summary:
This changes the algorithm for pure join to keep the constraints that,
after normalization, occur in both arguments. Previously pure join
would normalize, filter, and then union the constraints of the
arguments.
Reviewed By: sblackshear
Differential Revision: D3970394
fbshipit-source-id: 3dc1672
Summary:
This is needed for later: InferClang will no longer be started once for each
source file to be analysed. Instead, it will be called to analyse several files
at once, and will analyse them one by one. Thus, `clang_lang` and `source_file`
are moved to `cFrontend_config` as references.
The biggest change this entailed was the new logging infrastructure, which was
depending on `Config.source_file`. This diff moves the logic entirely to
`Logging`, and changes the API so that executables wishing to log into files
have to set it up using `Logging.set_log_file_identifier`. This can be called
several times during the execution, allowing to dynamically change the log file
(eg, when analysing several source files one by one!).
Reviewed By: jberdine
Differential Revision: D3944148
fbshipit-source-id: 6129090
Summary:
Let's start migrating some of our bash script to OCaml to make them easier to
maintain and extend.
For now replace just one script and put it in lib/clang_wrappers/ at compile
time, where the former script used to be. Further simplifications will come
later.
Reviewed By: jberdine
Differential Revision: D3929988
fbshipit-source-id: b2d8b37
Summary:
We were previously leaking the passthroughs of the callee into the caller.
We definitely don't want to do this since it could make the summaries higher up in the call stack explode.
If we need to know the passthroughs of a callee, we can always read them from the callee's summary.
Reviewed By: jeremydubreil
Differential Revision: D3972679
fbshipit-source-id: 5b5903f
Summary: That data was never used and removing it can simplify frontends quite a bit.
Reviewed By: jberdine
Differential Revision: D3967389
fbshipit-source-id: d65c3da
Summary: The code has not much to do with IR and should be part of backend/ directory.
Reviewed By: sblackshear
Differential Revision: D3950834
fbshipit-source-id: 315ea19
Summary:
Move most of common dependencies out of backend/ into base/
Diff doesn't change any code and hence files in base/ may still depend on
code outside of base/. There will be followup diff cleaning those up.
There are also files that maybe should be in common/ but haven't been moved there yet.
Reviewed By: jberdine
Differential Revision: D3950695
fbshipit-source-id: 00612b1
Summary:
Just adds a language agnostic option to skip the analysis of some files based on the path name. Can be used from the command line with:
infer infer --skip-analysis-in-path "some/path/" ...
or via the `.inferconfig` file:
{
"skip-analysis-in-path": [
"infer/demo/Resource.java"
]
}
Reviewed By: jberdine
Differential Revision: D3954809
fbshipit-source-id: d0d2b9f
Summary:
- Use the module types in cModule_types.ml instead of redefining them.
- A few occurrences of \n in formatted output replaced by @\n to let the
formatter know of line breaks (by no means complete, these were just a few I
came across while doing something else)
Reviewed By: jberdine
Differential Revision: D3944081
fbshipit-source-id: 4460427
Summary:
It's nice to be able to know how well the process we started did, although this
diff ignores it for now.
Reviewed By: dulmarod
Differential Revision: D3937902
fbshipit-source-id: 80bf20f
Summary:
In order to have only InferJava depend on JBasics, do not use
JBasics.java_lang_object in the IR or backend. Note that this implies
that the Java frontend should ideally translate JBasics.java_lang_object
to Typename.Java.java_lang_Object.
Reviewed By: jeremydubreil
Differential Revision: D3956468
fbshipit-source-id: def64dd
Summary:
This diff introduces a first version of a front-end checkers specification
language. The language is based on the CTL temporal logic that is interpreted
on trees. In this case the model for a formula is the AST of the program produced
by clang.
This diff introduce the language and translate most of the existing checks on
this new language. In other diff I will translate all the other checks.
Then I will generalize the framework to allow the developer to specify only
the CTL formula.
Reviewed By: martinoluca
Differential Revision: D3819211
fbshipit-source-id: f8e01eb
Summary:
Change implementation of NSArray and NSDictionary model builtins to use
the method return type instead of magicking up types from strings.
Reviewed By: jvillard
Differential Revision: D3919815
fbshipit-source-id: f07a993
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:
Refactor Sil.struct_typ and associated operations into a separate
StructTyp module. This is possible now that Typ.Tstruct only carries a
type name instead of the definition directly, and is helpful to simplify
module dependencies.
Reviewed By: cristianoc
Differential Revision: D3919357
fbshipit-source-id: a37a656
Summary:
It is no longer necessary to keep the name of a struct within the
struct, as the name will just have been used to look it up.
Reviewed By: cristianoc
Differential Revision: D3919355
fbshipit-source-id: ab65168
Summary:
Pass the exe_env to checker cluster callbacks, and add it to the domain
extras for BoundedCallTree, and use the Exe_env instead of
AttributesTable to obtain the tenv.
Reviewed By: sblackshear
Differential Revision: D3921850
fbshipit-source-id: 9edf324
Summary: The Infer builtins can be used in the e2e tests, but those tests should not depend on the Infer models to avoid cyclic dependencies. This diff separates the models and the Infer builtins in two directories so that the test can depend on the builtins without depending on the models
Reviewed By: sblackshear
Differential Revision: D3929478
fbshipit-source-id: 7d0ab79
Summary:
The global reference `DB.current_source` is used internally in the module DB, by all the front-ends, and directly and indirectly by the back-end, including saving and restoring the state in case of on-demand procedure calls. In particular, it is heavily used in printing functions.
This diff cleans up the flow of information about what the current file is, making it explicit, and removes the reference.
Reviewed By: jberdine
Differential Revision: D3901247
fbshipit-source-id: ef596bd
Summary:
Create the log directory even if the parent results directory does not
exist. In particular, the python buck module will delete the results
directory after it is created the first time, so it needs to be
re-created or else there is nowhere for the log files.
Reviewed By: sblackshear
Differential Revision: D3896546
fbshipit-source-id: 834cf79
Summary:
Rename symbols in test files so they are not duplicated and files can be analyzed together without affecting analysis results.
Fix some compilation errors, where files could be analyzed but would fail direct compilation.
Add Makefile mimicking the same analysis parameters used for the existing tests.
Reviewed By: dulmarod
Differential Revision: D3869993
fbshipit-source-id: 6db1baf
Josh Berdine
Jeremy Dubreil
Andrzej Kotulski
Sam Blackshear
Dulma Churchill
Dino Distefano
Martino Luca
Peter O'Hearn
Ryan Rhee
Cristiano Calcagno
Jules Villard
Yunxing Dai