Summary:
Conversion and reformat of infer source using ocamlformat
auto-formatting tool.
Current status:
- Because Reason does not handle docstrings, the output of the
conversion is not 'Warning 50'-clean, meaning that there are
docstrings with ambiguous placement. I'll need to manually fix
them just before landing.
Reviewed By: jvillard
Differential Revision: D5225546
fbshipit-source-id: 3bd2786
Summary:
Add support for converting without actually converting. To be used as
a landing pad for rebasing over the conversion.
Reviewed By: jvillard
Differential Revision: D5379967
fbshipit-source-id: 1bd932d
Summary: Adding to the Quandary tests, the list of tests that are already working for the bi-abduction based taint analysis.
Reviewed By: sblackshear
Differential Revision: D5395734
fbshipit-source-id: c4f2e79
Summary:
Error'ing when getting several subcommands on the command line only makes sense
when the user typed these options herself. This was causing a bug in the Gradle
integration, and possibly others (see GitHub issue).
Fixes#668
Reviewed By: jeremydubreil
Differential Revision: D5381850
fbshipit-source-id: f332377
Summary:
Fixes#683
Disclaimer: I don't know what XML namespaces/namespace prefixes are, but this seems to do the trick.
Reviewed By: mbouaziz
Differential Revision: D5381480
fbshipit-source-id: 3da16e7
Summary: This will allow to replace type vars into concrete types in expressions.
Reviewed By: jvillard, mbouaziz
Differential Revision: D5209276
fbshipit-source-id: c1650f8
Summary:
The thread safety domain manipulates access paths that are a variable
followed by a sequence of field or index accesses. Some expressions
from C++ code do not fit that form, such as cases where subtraction of
an offset from a pointer is used to obtain another pointer, whose
fields are then accessed. Previously the analyzer would crash on such
expressions. This diff partially treats them by introducing dummy
variables.
Reviewed By: da319
Differential Revision: D5343567
fbshipit-source-id: f73b520
Summary:
:
because otherwise people would believe they can use the internal representation of these std lib but it fails for our models.
Reviewed By: jvillard
Differential Revision: D5368671
fbshipit-source-id: 4e53d5a
Summary: So that it get rebuilt. Test is clang_compilation_db_relpath
Reviewed By: mbouaziz
Differential Revision: D5364561
fbshipit-source-id: 8cae984
Summary:
:
Get rid of model location in reports.
The goal is to avoid changing `issues.exp` whenever a model is updated.
Reviewed By: jvillard
Differential Revision: D5356608
fbshipit-source-id: 88ecaba
Summary: This could generate enormous amounts of logs on medium-sized projects (eg rocksdb) when running with `--stats`. Review other debug levels in inferbo as well.
Reviewed By: mbouaziz
Differential Revision: D5364139
fbshipit-source-id: 5c8d206
Summary:
In C++ it may happen that a procedure return 'variable' is an access
path that cannot be translated to Hil. For now just skip these instead
of crashing.
Reviewed By: mbouaziz
Differential Revision: D5356134
fbshipit-source-id: 977dfba
Summary:
They are expected to occur in C++ code, so don't fail on them. For now
just skip them, although a better treatment of dynamic dispatch may be
needed later.
Reviewed By: da319, mbouaziz
Differential Revision: D5292462
fbshipit-source-id: 4285514
Summary:
Indexing into a string literal expression would generate a fresh
variable on every application of a transformer. This violated
finiteness of the domain, and caused divergence.
Reviewed By: da319
Differential Revision: D5342951
fbshipit-source-id: e95e84e
Summary: It was passing the argument multiple times rather than once with all the accepted values
Reviewed By: jvillard
Differential Revision: D5338509
fbshipit-source-id: 22b86a5
Summary: This is useful when we only want to examine the html for some but not all source files.
Reviewed By: jvillard
Differential Revision: D5334786
fbshipit-source-id: 774c23c
Summary:
It instantiates fields of structures when a pointer to which is given
as a function parameter, e.g., `foo(&s);`.
Reviewed By: mbouaziz, jvillard
Differential Revision: D5337645
fbshipit-source-id: c06da29
Summary: Rename historical option to its new form, since the old form was no longer accepted by infer.
Reviewed By: martinoluca
Differential Revision: D5329033
fbshipit-source-id: 4fa9402
Summary:
We keep track of both `beginPtr` and `endPtr` but the modelling was mostly
about `beginPtr` as some sort of approximation I guess. This shouldn't change
much but will be useful later when doing more iterator stuff.
Reviewed By: mbouaziz
Differential Revision: D5255772
fbshipit-source-id: 0f6e3e8
Summary:
This will be needed to re-use the functions now in Driver.ml in other contexts
without always adding to infer.ml. For instance, this is used in a later diff
to do a diff-analysis orchestrator that needs to run the capture and analysis
several times.
Reviewed By: jberdine
Differential Revision: D5319862
fbshipit-source-id: caf9551
Summary: Needed in a later diff to be able to compute the set of changed files *during* an infer execution.
Reviewed By: jberdine
Differential Revision: D5319667
fbshipit-source-id: 226ec91
Summary: This seems to move in the right direction. Also, `const operator[]` did not do an `access_at`, which I fixed.
Reviewed By: mbouaziz
Differential Revision: D5320427
fbshipit-source-id: c31c5ea
Summary: Unknown library returns the unknown pointer as well as the top interval.
Reviewed By: mbouaziz, jvillard
Differential Revision: D5282669
fbshipit-source-id: 34c7e18
Summary:
Thanks to the logging introduced in D5293334 (or because of, depending on your liking of spam), I noticed that there lots of errors being logged of the form
Couldn't read multilink file '/home/jul/infer/infer/tests/codetoanalyze/cpp/errors/infer-out/attributes/d0/multilink.txt': /home/jul/infer/infer/tests/codetoanalyze/cpp/errors/infer-out/attributes/d0/multilink.txt: No such file or directory
I don't think it makes sense to care about multilink files except when `--merge` is specified.
Also introduced a .mli and self-documented a function.
Reviewed By: akotulski
Differential Revision: D5310129
fbshipit-source-id: c3a6276
Summary:
Once the fixed/preexisting/introduced sets have been computed, they endure
further filtering which may decide that more of them are equal. These bugs just
get dropped on the floor. Put these into preexisting as well instead, at least
in the case of the "skip_duplicated_types_on_filenames" filter.
Reviewed By: martinoluca
Differential Revision: D5274248
fbshipit-source-id: 99b3f3d
Summary:
This diff tries to achieve the followings: if we have the following C++ codes:
```
bool foo(int x, int y) {
return &x == &y;
}
```
We want the C++ frontend to emit Sil as if the input is written as
```
bool foo(int x, int y) {
if (&x == &y) return 1; else return 0;
}
```
This matches the behavior of our Java frontend.
The reason why we prefer an explicit branch is that it will force the backend to eagerly produce two different specs for `foo`. Without the explicit branch, for the above example the backend would produce one spec with `return = (&x == &y)` as the post condition, which is not ideal because (1) we don't want local variables to escape to the function summary, and (2) with the knowledge that no two local variables may alias each other, the backend could actually determines that `&x == &y` is always false, emitting a more precise postcondition `return = 0`. This is not possible if we do not eagerly resolve the comparison expression.
Reviewed By: akotulski
Differential Revision: D5260745
fbshipit-source-id: 6bbbf99
Summary:
:
There are throw wrapper functions like `std::__throw_bad_alloc()` defined in both libstdc++ (https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/functexcept.h) and libc++ (e.g. 907c1196a7/include/new (L145)). Folly actually exports some of them as well (diffusion/FBS/browse/master/fbcode/folly/portability/BitsFunctexcept.h). The function body of those wrappers merely throws the corresponding exception. My understanding is that the primary purpose of the wrappers is to throw the exception if everything goes well and to fall back to something reasonable when exception is disabled (e.g. when `-fno-exceptions` is passed to the compiler).
The problem is that infer doesn't really understand what those functions do, and I've seen some false positives get reported as a result of it. So to remove those FPs we need to either model them or handle them specially. Modeling those wrappers by either whitelisting them or overriding the include files turns out to be difficult, as those wrappers are only declared but not defined in the STL headers. Their implementations are not available to Infer so whitelisting them does nothing, and if I provide custom implementations in the headers then normal compilation process will be disrupted because the linker would complain about duplicated implementation.
What I did here is to replace functions whose name matches one of the throw wrapper's name with a `BuiltinDecls.exit`. I have to admit that this is a bit hacky: initially I was trying to do something more general: replacing functions with `noreturn` attribute with `BuitinDecls.exit`. That did not work because, CMIIW, the current frontend only exports function attributes for functions with actual bodies, not declaration-only functions. I'd love to be informed if there are better ways to handle those wrappers.
Reviewed By: jeremydubreil
Differential Revision: D5266030
fbshipit-source-id: 4580227
Summary:
:
Do not store dummy `_` into the stack.
This makes debugging a lot easier
Reviewed By: skcho
Differential Revision: D5275941
fbshipit-source-id: ce329a5
Summary:
:
What is relevant for the Buck integration is not the list of bugs that we find in a single target, which is essentially identical to testing `infer -- javac ...`, but to make sure that we still find the issues that are involving several Buck targets, and later other things like the caching mechanism.
This should also make the tests faster.
Reviewed By: jberdine, jvillard
Differential Revision: D5250205
fbshipit-source-id: 7f66b68
Summary:
I changed the link, so that now it works.
Closes https://github.com/facebook/infer/pull/666
Differential Revision: D5256662
Pulled By: dulmarod
fbshipit-source-id: 5ae622d
Summary: This change introduces the a new argument that lets you restrict the results of a differential report to only certain files.
Reviewed By: mbouaziz
Differential Revision: D5236626
fbshipit-source-id: 52711e9
Summary: Looks much less confusing when C++ templates with `<stuff>` are involved.
Reviewed By: mbouaziz
Differential Revision: D5255551
fbshipit-source-id: f4a93e6
Summary:
After D5245416 I was taking a closer look and decided it's best to get rid of the `Interprocedural` module altogether.
Since jeremydubreil's refactoring to pass the summaries around everywhere, this module doesn't do much (it used to make sure the summary actually got stored to disk).
Client code is shorter and simpler without this module.
Reviewed By: mbouaziz
Differential Revision: D5255400
fbshipit-source-id: acd1c00
Summary:
Only one instance will win in the end so it's not useful to double register.
Log when that happens. Currently it happens in the Java tests on
`InferBuiltins` but I don't understand why so I left it alone.
Reviewed By: jberdine
Differential Revision: D5217928
fbshipit-source-id: dc7ccca
Summary:
This avoids race conditions when two processes or more try to lock a file for
writing. It could be that the process losing the race writes less than the
winner, then we get rubbish at the end of the file. Calling `ftruncate(2)` inside the critical section makes sure the
contents of the file are erased first. The harmful race was observed in
xcodebuild sometimes, as it can call infer on the same file several times in
parallel (!).
Reviewed By: jberdine
Differential Revision: D5209177
fbshipit-source-id: 744169c
Summary: I tested on Fresco and this reduced the number of calls to Prover.check_disequal by 30%.
Reviewed By: cristianoc
Differential Revision: D5237774
fbshipit-source-id: 377545e
Summary: The docs for this said that it stores the summary to disk, which is no longer true. `compute_summary` is more descriptive of what it actually does now.
Reviewed By: jberdine
Differential Revision: D5245416
fbshipit-source-id: f5138cd
Summary: We had a model for `Pools.SimplePool`, but were missing models for `Pools.Pool`. Since `SimplePool` and `SynchronizedPool` both extend `Pool`, modeling it should cover all of the cases.
Reviewed By: ngorogiannis
Differential Revision: D5236280
fbshipit-source-id: 9bbdb25
Summary:
When dealing with differential's comparisons, SourceFiles in reports may not exist anymore because of changes between revisions.
Disable user visible warnings if desired, through the labeled argument `~warn_on_error`
Reviewed By: jvillard
Differential Revision: D5217097
fbshipit-source-id: 7a1542b
Summary:
The ThreadSafety analysis currently reports on methods only if some
class in the file defining the method is annotated ThreadSafe, or
if it is called by some other such method call. Conflating files and
classes is a bit of a Javaism that seems to be unnecessary.
Reviewed By: sblackshear
Differential Revision: D5182319
fbshipit-source-id: aa77754
Summary: This will make merlin aware of the sources in src/unit/, which are compiled when running tests
Reviewed By: jvillard
Differential Revision: D5217193
fbshipit-source-id: 18bf01a
Summary: This makes it possible to see which tainted parameter can flow to a sink, which is quite useful.
Reviewed By: jeremydubreil
Differential Revision: D5213297
fbshipit-source-id: 1371b5a
Summary:
:
No longer use deprecated reporting function for the suggest nullable checker
Depends on D5205009
Reviewed By: grievejia
Differential Revision: D5205843
fbshipit-source-id: f6dd059
Summary:
Now that we can run several inter-procedural analyses at the same time, we should no longer use the function `Reporting.log_error_deprecated` as it logs the errors in the specs table. This specs table is normally used for caching and will be deprecated in favor of having a cache summaries for the callees in the `Ondemand` module (to avoid deserialising a callee more than once within the same process).
This revision just renames the reporting functions.
Reviewed By: sblackshear
Differential Revision: D5205009
fbshipit-source-id: b066549
Summary:
Read/write race errors should always show one trace for a read and one trace for a write.
We forget to pass the conflicting writes to the reporting function in one case, which prevented us from showing a well-formed trace.
Fixed it by making the `conflicts` parameter non-optional
Reviewed By: jberdine
Differential Revision: D5209332
fbshipit-source-id: 05da01a
Summary: Can be useful in case there is compilation issue with the models
Reviewed By: jvillard
Differential Revision: D5208830
fbshipit-source-id: f31d84f
Summary:
This messes up with the Buck cache: if infer recaptures a file with only trivial changes that shouldn't affect the capture Buck will still believe it has to reanalyze everything that depends on that file if there's non-deterministic data in infer's output.
Re-use the `--buck` flag used by the Java Buck integration for mostly the same purposes. Add a few special cases for the flavours integration (eg: keep capture data).
Change perf stats registration to take `Config.buck_cache_mode` into account instead of relying on each call site to handle that peculiarity correctly. Also, there's no need to create the perf stats directories before calling the registration function since it will do that too.
Reviewed By: jeremydubreil
Differential Revision: D5192311
fbshipit-source-id: 334ea6e
Summary:
It is not used, so one fewer thing to track inside Python.
Note that Python does pass `--buck` to OCaml infer to make it aware that it is
running from within Buck, and the OCaml code uses that flag to trim infer-out
to only what's strictly necessary (and especially to remove non-deterministic
output that would mess with Buck's cache).
Reviewed By: jberdine
Differential Revision: D5200043
fbshipit-source-id: 1c84442
Summary:
Mutating a formatter in `dup_formatter` had unintended consequences of
double-printing newlines. There's actually no need not to make a fresh copy,
which avoids this problem.
Also, don't warn if the log file is still a buffer at the end of execution as
that can happen in some error cases, eg:
```
$ infer -- build
ERROR: (Invalid_argument "Unsupported build command build")
```
Reviewed By: jberdine
Differential Revision: D5199664
fbshipit-source-id: fc2731d
Summary:
We were saving them in infer-out/clang/ but that's bad for Buck's cache as
these files have randomly-generated names.
Reviewed By: dulmarod
Differential Revision: D5191884
fbshipit-source-id: b3a478b
Summary: These can be useful in other checkers that have a notion of footprint.
Reviewed By: jvillard
Differential Revision: D5189193
fbshipit-source-id: c5bd91b
Summary: Also makes the error message for clang commands a bit nicer to the eye.
Reviewed By: mbouaziz
Differential Revision: D5191664
fbshipit-source-id: 801ec72
Summary:
It's easier to distinguish what's checked into infer and what comes from the
plugin this way.
Reviewed By: akotulski
Differential Revision: D5191581
fbshipit-source-id: 579311e
Summary:
This should help prevent new modules being created that do not `open! IStd`.
Documented this in CONTRIBUTING.md.
Reviewed By: jberdine
Differential Revision: D5183552
fbshipit-source-id: 0f0a8ce
Summary:
First step toward addressing bad traces that happen in examples like
```
void sourceMethod() {
Obj source = (Obj) InferTaint.inferSecretSource();
callSameSink(null, source); // index: 1
}
void callSameSink(Obj o1, Obj o2) {
callMySink1(o1); // flows via o1 ~= index 0, don't expand
callMySink2(o2); // flows via o2 ~= index 1, can expand
}
void callMySink1(Obj o) {
... // maybe interesting something happens here that doesn't happen in callMySink2
InferTaint.inferSensitiveSink(o); // flows via o ~= index 0, can expand
}
void callMySink2(Obj o) {
InferTaint.inferSensitiveSink(o); // flows via o ~= index 0, can expand
}
```
The issue is that when we recreate a trace to the sink starting from `sourceMethod`, we don't know which of the calls to `callMySink` to expand/include in the trace.
If we expand the call to `callMySink(o1)`, we'll get a bogus trace.
In this example that's not such a big deal, but imagine the case where the first call to `callMySink` is a different function that transitively calls the sink through some long and confusing path.
Remembering the index at which taint flows into each sink will let us choose which sinks are safe to expand.
This diff just adds indexes to the API; it's not actually propagating the index info or using it during expansion yet.
Reviewed By: jeremydubreil
Differential Revision: D5170563
fbshipit-source-id: ba4b096
Summary:
Change the API of `Logging` wrt to writing to files and to the console (see
changes in logging.mli).
Write only to one log file: infer-out/log. Prefix each line with the kind of
warning and the PID of the process emitting it. Writing with `O_APPEND` is
atomic so the file should not get garbled by concurrent writes. To get the
output of a single process, find out which one interests you by looking at
infer-out/log, then `grep ^[<PID>] infer-out/log`.
Introduce 3 log levels for debug output and command-line options to set them
for various categories individually.
Change tons of `"\n"` to `"@\n"` so the `Format` module is aware of newlines
without us having to look through every character of every logged string for
`\n` characters.
Reviewed By: mbouaziz
Differential Revision: D5165317
fbshipit-source-id: 93c922f
Summary:
This makes it clearer that something went wrong. Most `failwith` did not set
this prefix already, so I opted to append it automatically and remove it from
the few instances that added it manually.
Also add quotes around bad user arguments to lessen possible confusion.
Reviewed By: jberdine
Differential Revision: D5182272
fbshipit-source-id: 20e4769
Summary:
Introduce a new option `--no-report` (conversely `--report`) to stop reporting
after the analysis. This is useful to call sub-`infer-analyze` processes with
as they shouldn't compute "result.json" or report bugs to stderr. This bug
would only manifest itself when per-procedure parallelism disabled, which
explains why it was noticed only on Java.
Reviewed By: jberdine
Differential Revision: D5182110
fbshipit-source-id: a892470
Summary:
This is a refactoring diff to put the info into the abstract domain
to track when we have done steps which would invalidate "I think I have a proof".
Subsequent diffs will start manipulating ThumbsUpDomain
Reviewed By: sblackshear
Differential Revision: D5172181
fbshipit-source-id: 51ceba6
Summary:
The logic is not that simple and this will be needed in a later diff to create
the log file as early as possible.
Reviewed By: jberdine
Differential Revision: D5173128
fbshipit-source-id: 830f105
Summary: This also allows us to better test that the new commands will keep working.
Reviewed By: jeremydubreil
Differential Revision: D5172891
fbshipit-source-id: 169bd6f
Summary:
Spacetime profiling showed that this was allocating a lot more than it needs
to. Switching to a `Hashtbl` makes the memory overhead go away, and halves the
memory consumption of the whole frontend on some pathological files.
This also brings the file "clang_ast_main.ml" into the infer repo, renamed to
"ClangPointers.ml" (and given an mli). It's useful to have something like
"clang_ast_main.ml" checked into the facebook-clang-plugins repo to illustrate
how to use the OCaml AST visitor generated by atdgen, but it can be simplified
to be more pedagogical instead of being the visitor used in infer.
Reviewed By: jberdine
Differential Revision: D4884383
fbshipit-source-id: 88f324a
Summary: We were almost always using `~report_reachable:true`, and in the cases where we weren't it is fine to do so. In general, a sink could read any state from its parameters, so it makes sense to complain if anything reachable from them is tainted.
Reviewed By: mbouaziz
Differential Revision: D5169067
fbshipit-source-id: ea7d659
Summary:
This was a subtle one. The ranking function of `aux` is the cardinality of `m`..
But if `may_alias` is not reflexive, then `k_part` will be empty, `non_k_part` will be the same size, and we'll diverge.
Sneakily, `may_alias` is actually *not* reflexive because `is_subtype t1 t2` doesn't check for the equality of `t1` and `t2`.
That is confusing and should be fixed separately.
For now, just make sure `may_alias` is always reflexive and add an assertion that `k_part` is never empty.
Reviewed By: jeremydubreil
Differential Revision: D5177427
fbshipit-source-id: 0549d6a
Summary: Have found this useful in Quandary for fbcode, where we want to do this for folly due to its use of assembly (details in comments).
Reviewed By: mbouaziz
Differential Revision: D5167564
fbshipit-source-id: bf6d7e0
Summary:
Now `infer analyze` really has the same behaviour as `infer -- analyze`.
Previously it wouldn't create report.json or report.
Reviewed By: jeremydubreil
Differential Revision: D5172875
fbshipit-source-id: 8f9ddd1
Summary: Needed higher-up the stack, useful to have in the API in general.
Reviewed By: jberdine
Differential Revision: D5165153
fbshipit-source-id: 714aeea
Summary:
This will be needed higher up in the stack because the new `ProcessPool` module
will need to call into `Logging` to refresh the logging formatters to get the
right PID when writing to the log file.
+remove dead code `iter_parallel`
Reviewed By: jberdine
Differential Revision: D5165130
fbshipit-source-id: 95c949b
Summary:
I should not push debugging code to the repo
I should not push debugging code to the repo
I should not push debugging code to the repo
I should not push debugging code to the repo
I should not push debugging code to the repo
I should not push debugging code to the repo
...
Reviewed By: martinoluca
Differential Revision: D5164723
fbshipit-source-id: d944916
Summary:
For now, we just support clearing the taint on a return value.
Ideally, we would associate a kind with the sanitizer and only clear taint that matches that kind.
However, it's fairly complicated to make that work properly with footprint sources.
I have some ideas about how to do it with passthroughs instead, but let's just do the simple thing for now.
Reviewed By: jeremydubreil
Differential Revision: D5141906
fbshipit-source-id: a5b8b5e
Summary: Allow type variables in `Typ.desc`. It will be used to store template type arguments.
Reviewed By: jberdine
Differential Revision: D5154757
fbshipit-source-id: 55b8e81
Summary:
- model `exit` as `Bottom`
- model `fgetc` as returning `[-1; 255]` rather than `[-1; +oo]`
- reduced the number of model functions for simple models
Reviewed By: KihongHeo
Differential Revision: D5137485
fbshipit-source-id: 943eeeb
Summary:
1. `noexcept` was missing from `unique_ptr` constructors leading to compilation errors in some edge cases
2. In case `unique_ptr` specified custom deleter with custom `deleter::pointer`, there could be compilation errors due to invalid cast from `deleter::pointer` to `void*`. Add extra overload of `model_set` to prevent this issue
Reviewed By: jberdine
Differential Revision: D5147071
fbshipit-source-id: 2586701
Summary:
This is a minimal change to (poorly) recognize and model std::mutex
lock and unlock methods, and to surface all thread safety issues for
C++ based on the computed summaries with no filtering.
This ignores much of the Java analysis, including everything about the
Threads domain. The S/N is comically low at this point.
Reviewed By: sblackshear
Differential Revision: D5120485
fbshipit-source-id: 0f08caa
Summary:
ThreadSafety.may_alias crashed on C++ code because it assumed Java
field names.
Reviewed By: sblackshear
Differential Revision: D5147284
fbshipit-source-id: d10841f
Summary: The previous error message recommended annotating the method in question with `GuardedBy`, which doesn't actually work.
Reviewed By: jeremydubreil
Differential Revision: D5149661
fbshipit-source-id: d935aec
Dulma Churchill
azhdar
Josh Berdine
Jeremy Dubreil
Mehdi Bouaziz
Jules Villard
Jia Chen
Andrzej Kotulski
Kihong Heo
Martino Luca
Daiva Naudziuniene
Alexander Slesarev
Radu Grigore
Sungkeun Cho
Dino Distefano
Sam Blackshear
Kyriakos Nikolaos Gkorogiannis
Marco Concetto Rudilosso
Don Stewart
Peter O'Hearn