Summary:
Now,
infer -a infer -- ...
and
infer -a checkers --biabduction -- ...
will return the same list of errors
Reviewed By: sblackshear
Differential Revision: D5023223
fbshipit-source-id: f52ce5d
Summary:
Needed because this is how the Clang frontend translates returns of non-POD, non pointer values (I think)?
Will handle the more general case of pass by reference soon.
Reviewed By: jvillard
Differential Revision: D5017653
fbshipit-source-id: 1fbcea5
Summary:
Ran the build with -w,-32 , delete code, repeat, until a fixpoint of no more warnings is reach.
Unfortunately we cannot fatal on w32 because ppx_compare can generate dead code (eg `compare_t` and only `compare` is used).
Reviewed By: mbouaziz
Differential Revision: D4945800
fbshipit-source-id: c95afb6
Summary:
`[a, b] < [a, _]` and `[_, a] < [b, a]` are most probably false (it comes from size < size)
Mark definitely unsatisfied conditions as B1, others as B2+
Reviewed By: KihongHeo, jvillard
Differential Revision: D4962107
fbshipit-source-id: ba8f469
Summary:
The bufferoverrun checkers can now be run with:
infer -a checkers --bufferoverrun -- ...
Reviewed By: jeremydubreil
Differential Revision: D5010689
fbshipit-source-id: 2eaa396
Summary:
The Siof checkers can now be run with:
infer -a checkers --siof -- ...
and also runs by default using:
infer -a checkers -- ...
Reviewed By: jberdine
Differential Revision: D5009731
fbshipit-source-id: e0e2168
Summary: Pass the classpath infromation from the javac command line option file to allow the dection of errors accros Buck targets
Reviewed By: sblackshear
Differential Revision: D5018329
fbshipit-source-id: cf13198
Summary:
First step to be able to enable and disable the checkers to run in the following form:
> infer -a checkers --checker1 --checker2 --checker3 -- ...
and have a predefined list of checkers that are run by default with:
> infer -a checkers -- ...
Reviewed By: sblackshear
Differential Revision: D5007377
fbshipit-source-id: d7339ef
Summary:
While working on making the AI framework simpler to use, it become hard to change the shared API while keeping these unused checkers compiling, and even harder to keep their functionality since there is no tests for them.
Also, these checkers are not useful as proof of concept since using the AI framework is the preferred way to write new kinds of analysis now.
Reviewed By: sblackshear
Differential Revision: D4999387
fbshipit-source-id: 497284b
Summary:
Bufferoverrun-specific model for std::vector
Requires `--bufferoverrun` command line flag
Reviewed By: akotulski
Differential Revision: D4962136
fbshipit-source-id: f6b5f15
Summary:
This gives the option to run the biabduction analysis together with the other Clang-based checkers with the command:
infer -a checkers --biabduction -- ...
The filtering does not work yet because the filtering for the biabduction analysis matches the analyzer `Infer`, and does not filter much when the analyzer is `Checkers`, which is the case here.
Reviewed By: sblackshear
Differential Revision: D4773834
fbshipit-source-id: 16300cc
Summary:
`Location.dummy` is often used in a situation where we know the source file, but not the line/column.
Use `Location.none` for this instead.
Reviewed By: jeremydubreil
Differential Revision: D4991232
fbshipit-source-id: fc361a4
Summary:
Last step for converting thread-safety and quandary to HIL.
Push the logic for managing the id map and converting the instructions into a functor.
This way, client analyses can simply write HIL transfer functions and call the functor.
Reviewed By: jberdine
Differential Revision: D4989987
fbshipit-source-id: 485169e
Summary:
For now we just want to find bugs, let's do something smarter later (smash heap variables only when needed).
Depends on D4962107
Reviewed By: jvillard
Differential Revision: D4962121
fbshipit-source-id: 1b777a6
Summary:
Don't store redundant information in C++ template Type.Name.t.
New signature:
```
| CppClass (qual_name, template_args)
```
For example, for `std::shared_ptr<int>`, will look like this:
```
| CppClass (["std", "shared_ptr"], Template [int])
```
While it used to be:
```
| CppClass (["std", "shared_ptr<int>"], Template (["std", "shared_ptr"], [int]))
```
Reviewed By: jberdine, mbouaziz
Differential Revision: D4834512
fbshipit-source-id: cb1c570
Summary: The name of the source file was passed around everywhere but can also be accessed from the location associated to every node.
Reviewed By: sblackshear
Differential Revision: D4981848
fbshipit-source-id: 2ee592e
Summary: Now, all the summary access functions in the module `Specs` are of the form: `Specs.summary -> 'a`. This is a step toward making the analysis flow stateless.
Reviewed By: sblackshear
Differential Revision: D4976126
fbshipit-source-id: 28b6da1
Summary: This function is always used in the frontend where summaries don't exist yet
Reviewed By: akotulski
Differential Revision: D4979132
fbshipit-source-id: 8d49c52
Summary:
Before we understood ownership, we needed this to avoid a mountain of Builder-related FP's.
Now that we have fairly sophisticated understanding of ownership, we can kill this hack.
Reviewed By: jaegs
Differential Revision: D4940238
fbshipit-source-id: 8d86e57
Summary: Before, running any of these would crash with `Unsupported infer analyzer with Buck flavors:`
Reviewed By: jberdine
Differential Revision: D4970769
fbshipit-source-id: 76be6d5
Summary:
Add `volatile` and `restrict` type qualifiers. Change `Ast_expressions.create_*_type` functions
to always get optional type quals argument.
update-submodule: facebook-clang-plugins
Reviewed By: jberdine
Differential Revision: D4969634
fbshipit-source-id: 9a63bf7
Summary: This code only runs when Infer is running and is not reached when any other analyzer is used
Reviewed By: sblackshear
Differential Revision: D4973824
fbshipit-source-id: 700e24b
Summary: The Java frontend creates a single `tenv` file per `javac` invocation, but the code loading the `tenv` for a given Java procedure in the backend was not taking advantage of it. Also, with the lazy dynamic dispatch algorithm, the procedure name can be created on-demand and therefore defeat the approach to load the tenv by looking at the call graph to associate existing procedure names to the corresponding serialized tenv file. This diff should also fix this last point.
Reviewed By: sblackshear
Differential Revision: D4969254
fbshipit-source-id: 66ed318
Summary:
Title.
The way types are printed is completely valid, but little weird for some C++ programmers:
`int const` - same as `const int`
`int * const` - pointer is `const`, value under it is not
`int const *` - pointer is not `const`, but the value is
`int const * const` - both pointer and value are const
Reviewed By: jberdine
Differential Revision: D4962180
fbshipit-source-id: dcb02e3
Summary:
Modify the type of `Exp.Sizeof ...` to include the value that the expression
evaluates to according to the compiler, or None if it cannot be known
statically.
Use this information in inferbo.
Mostly unused in the BiAbduction checker for now, although it could be useful
there too.
update-submodule: facebook-clang-plugins
Reviewed By: mbouaziz
Differential Revision: D4953634
fbshipit-source-id: be0999d
Summary:
`libstdc++` includes `bits/unique_ptr.h` via `include "unique_ptr.h"` in `bits/locale_conv.h` which infer can't redirect to model headers. This leads to compilation issues.
To work around that, redirect include of `locale_conv.h` to include `unique_ptr` via `include <...>` before normal `locale_conv.h`.
Reviewed By: mbouaziz
Differential Revision: D4962314
fbshipit-source-id: d4b9830
Summary:
Bottom means unreachable, not unknown.
Replace Bottom by Top in:
- unknown procedure return value
- unknown constant
- closure, exceptions
- when there are more formals than actuals
Depends on D4961989
Reviewed By: KihongHeo
Differential Revision: D4962006
fbshipit-source-id: e2a153d
Summary:
- Use Logging for logging
- Add a few extra debug (not ON by default)
- No space before colon
Depends on D4961957
Reviewed By: KihongHeo
Differential Revision: D4961989
fbshipit-source-id: 7c8697d
Summary:
Backend needs to know whether type is const or not. In order to achieve it, frontend needs to know it first.
This diff changes bunch of things:
- update clang plugin to have AST exporter actually export that info most of the time
- change types of functions in clang frontend until it compiles
- replace `type_ptr` with `qual_type` and `tp` with `qt` in names where applicable
- cleanup some things in the process
update-submodule: facebook-clang-plugins
Reviewed By: jvillard
Differential Revision: D4938567
fbshipit-source-id: 716b3ef
Summary: Sometimes reports need traces to be fully understood, but sometimes reporting where the exception takes place can save time to developers.
Reviewed By: jvillard
Differential Revision: D4914037
fbshipit-source-id: 039ab63
Summary: The purpose of the annotation reachability analysis is to report when a method annotated with `X` never calls, directly or indirectly, another method annotated with `Y`. However, there can be different call stacks following different execution paths from `X` to `Y`. Reporting more than one call stack ending with the same annotated procedure does not bring more signal to the end user. So the purpose of this diff is to avoid those duplicated reports and report at most one annotation reachability issue per end of call stack.
Reviewed By: sblackshear
Differential Revision: D4942765
fbshipit-source-id: 46325a7
Summary: The analysis logic was split between the treatment of the instructions and the definition of the domain, making the code more complicated that it should. This diff moves more of the logic into the domain definition and change to variable names to more descriptive ones
Reviewed By: sblackshear
Differential Revision: D4936414
fbshipit-source-id: ff59de7
Summary:
- Bottom-lift abstract memory domain to express unreachable node
- Two cases to make a node unreachable
+ constant: when an evaluation result of condition expression is
bottom or false, e.g., "prune(0)".
+ alias: when the same structure e is compared to itself with "<",
">", and "!=", e.g., "prune(e < e)".
- Add test for the new prune (prune_constant.c, prune_alias.c)
- Debug the semantics of comparison
Reviewed By: mbouaziz
Differential Revision: D4938055
fbshipit-source-id: d0fadf0
Summary: This will be needed to generate man pages with an accurate date.
Reviewed By: jberdine, mbouaziz
Differential Revision: D4937498
fbshipit-source-id: b5ebd31
Summary:
Went through the trouble of killing these warnings, make sure they do not creep
up again in the future.
Reviewed By: mbouaziz
Differential Revision: D4937493
fbshipit-source-id: 0e2eda4
Summary:
We won't be needing to know the terminal size when we move the help to
manpages. Hopefully if we need C stubs again in the future we can refer back to
this diff to revive them.
Reviewed By: jberdine
Differential Revision: D4937491
fbshipit-source-id: 99139fa
Summary: This is necessary to create a `report` subcommand (see later diffs).
Reviewed By: jberdine, dulmarod
Differential Revision: D4937488
fbshipit-source-id: 3fec0b5
Summary:
As an interprocedural checker, SIOF should not run unless explicitly required.
Make it a new type of analyzer like other similar checkers.
Reviewed By: mbouaziz
Differential Revision: D4937820
fbshipit-source-id: a9e2d38
Summary: There was unncecessary abstraction here - instead of looking up sil type in the map, simply store it as part of variant payload
Reviewed By: jberdine
Differential Revision: D4938114
fbshipit-source-id: 9fb8dd4
Summary: `bufferoverrun` is not part of `checkers` yet so bufferoverrun tests require their own Makefile
Reviewed By: jvillard
Differential Revision: D4937558
fbshipit-source-id: 20380f1
Summary:
Whenever running CTL in debug mode, you can now stop the execution and attach a step-by-step debugger by placing a breakpoint in the source code.
A breakpoint is the comment containing `INFER_BREAKPOINT`, placed on the line of interest.
You can attach the debugger to your breakpoint by running Infer with the following command
```
infer --linters-def-file my_linter.al --linters-developer-mode -- clang -c file.m
```
Reviewed By: dulmarod
Differential Revision: D4843358
fbshipit-source-id: acebde4
Summary: Handling special case of DeclStmt with VarDecl: emit the VarDecl node then emit the statements in DeclStmt as children of VarDecl. This is because despite being equal, the statements inside VarDecl and those inside DeclStmt belong to different instances, hence they fail the phys_equal check that should colour them
Reviewed By: dulmarod
Differential Revision: D4850681
fbshipit-source-id: b9683dc
Summary:
We lost InferClang++ a long time ago... It was installed properly on `make
install` but not during normal compilation!
Reviewed By: akotulski
Differential Revision: D4905947
fbshipit-source-id: 56375c2
Summary:
The error was complaining about a missing `etree` package but that's actually
part of the `lxml` package and that wasn't clear from the output.
See for instance https://github.com/facebook/infer/issues/635
Reviewed By: martinoluca
Differential Revision: D4905953
fbshipit-source-id: 28b26af
Summary: Sawja assigns them on multiple control-flow paths, so they're not SSA.
Reviewed By: peterogithub
Differential Revision: D4896745
fbshipit-source-id: c805216
Summary:
There are false positives in the current analysis due to the
use of conjunction in the treatment of threaded. Changing conjunction to disjunction
removes these false positives. Some new false negatives arise, but all the old tests pass.
This is a stopgap towards a better solution being planned.
Reviewed By: sblackshear
Differential Revision: D4883280
fbshipit-source-id: c2a7e6e
Summary:
These were showing up as allocation huge amounts of memory on some analysis
profiles from OpenSSL using Spacetime. Doesn't hurt to make them allocate less.
Reviewed By: jeremydubreil
Differential Revision: D4884520
fbshipit-source-id: e79b815
Summary:
Running `infer --help` with spacetime showed that `Str` was a hog. And indeed perf is 74% better now:
```
before:
bash -c 'for i in $(seq 100); do infer --help > /dev/null 2>/dev/null; done' 28.02s user 0.62s system 98% cpu 29.205 total
after:
bash -c 'for i in $(seq 100); do infer --help > /dev/null 2>/dev/null; done' 6.34s user 0.64s system 92% cpu 7.557 total
```
Running on many files one after the other is now 64% faster too:
```
$ cd infer/tests/codetoanalyze/c/errors
$ # before
$ rm -fr infer-out && time bash -c 'for src in */*.c; do infer -a capture --continue -- clang -c $src; done'
bash -c 7.77s user 0.66s system 97% cpu 8.647 total
$ # after
$ rm -fr infer-out && time bash -c 'for src in */*.c; do infer -a capture --continue -- clang -c $src; done'
bash -c 2.35s user 0.56s system 93% cpu 3.119 total
$ time infer -a capture -- clang -c */*.c
infer -a capture -- clang -c */*.c 0.54s user 0.20s system 99% cpu 0.737 total
```
Reviewed By: mbouaziz
Differential Revision: D4875803
fbshipit-source-id: cfcfa69
Summary:
OCaml 4.04.0 new warnings raised a few valid points!
Fixing warning 57 in two ways:
- best way: introduce an auxiliary function to avoid code duplication
- not-so-best way: introduce code duplication. I did that when the branches body are small. Typically the number of bound variables in the pattern is high, so an auxiliary function would need to take many arguments and the whole thing will not be readable (we'd still duplicate the arguments we pass to the function for instance).
Reviewed By: jberdine
Differential Revision: D4851006
fbshipit-source-id: fbf1867
Summary:
Limit the use of `SourceFile.invalid` (renamed from `SourceFile.empty`) as much
as possible. In particular, do not generate bogus procnames for external global
variables: their translation unit was set to the invalid source file, now we
distinguish between extern/non-extern global variables more explicitly.
`SourceFile.invalid` is still used in too many places to actually remove it, often as a dummy initial value that never gets used, but sometimes as an actual value... Worse, we cannot fail on all operations on `SourceFile.Invalid` yet: the `SourceFile.to_string` method is used in too many places where it could get `SourceFile.Invalid` as argument. It's easy to see where it's used by making it raise in the code, then running the tests. This results in spaghetti backtraces that are hard to trace back to a root cause.
Reviewed By: akotulski, jeremydubreil
Differential Revision: D4860019
fbshipit-source-id: 45be040
Summary:
Try to read .inferconfig in the current directory, then in .., then in ../..,
etc. This can be overriden with the `INFERCONFIG` environment variable.
This removes the need for two-phase parsing, so clean up that code too.
Paths in .inferconfig are interpreted relative to where .inferconfig is located.
This does not apply to other path-sensitive things like regexpes... this is not
a show stopper because regexpes can account for the fact that infer may be
called from different project roots.
Make sure we fail when .inferconfig exists but cannot be read.
Reviewed By: jberdine
Differential Revision: D4843142
fbshipit-source-id: 340a63f
Summary: The flakiness is exercised by upgrading the OCaml compiler to 4.04.0.
Reviewed By: jberdine
Differential Revision: D4859904
fbshipit-source-id: 0dd7ff6
Summary:
If we couldn't project the callee access path into the caller during summary application, we still added the corresponding trace to the caller state.
This was wasteful; it just bloats the caller with state it will never look at.
Fixed it by making `get_caller_ap_node` return `None` when the state won't be visible in the caller.
Reviewed By: jeremydubreil
Differential Revision: D4727937
fbshipit-source-id: 87665e9
Summary: This is required to upgrade OCaml as our ancient Reason is not available on 4.04.0.
Reviewed By: yunxing
Differential Revision: D4851582
fbshipit-source-id: 994a9a8
Summary:
It's distracting to see the debug HTML for the preanalysis when you're trying to debug something else.
Also, it breaks the nice bi-abduction debug feature of marking the visited nodes as green.
Reviewed By: akotulski
Differential Revision: D4858578
fbshipit-source-id: 8e77976
Summary: This should make the reports much easier to understand. We can generalize to reporting a stack trace for all of the writes in the future if we wish.
Reviewed By: peterogithub
Differential Revision: D4845641
fbshipit-source-id: 589fdbc
Summary: Prereq for reporting a call stack for both the read and write in a read/write race.
Reviewed By: peterogithub
Differential Revision: D4845603
fbshipit-source-id: ebfeb9b
Summary:
We shouldn't encourage contributors to run only a subset of the tests with
`make test`, but it's more helpful to complain clearly should they try to do
so.
Now `make test` will ensure that both the clang and the Java analyzers are
enabled, otherwise it will refuse to run and suggest to run `make config_tests`
instead.
It can still be useful to be able to run only the Java or Clang tests. The diff
also fixes the cases where that previously failed.
Fixes#634
Reviewed By: jberdine
Differential Revision: D4826515
fbshipit-source-id: 4b76029
Summary:
This debugger stops the execution of CTL during the evaluation of formulas, then shows useful info about the current state:
- Which formula is being evaluated
- In which part of the AST is such formula being evaluated
- Line number of the source file
- Whether a CTL-Transition will take place or not
- Node-ID that can be related to the dotty file containing the evaluation graph (the one in infer-out/lint_dotty/)
The AST is shown with coloured nodes, where each colour has the following meaning:
- [Default terminal colour], Bold: The node is being evaluated
- Green, Bold: The formula returned true on the current node
- Red, Bold: The formula returned false on the current node
Reviewed By: ddino
Differential Revision: D4834451
fbshipit-source-id: c9aa970
Summary:
Make it possible to write one model which will be used by all template instantiations.
There is one big missing piece: infer never tries to do template instantiation by itself. With current code, it's possible to use generic models
as long as header contains `__infer_generic_model` annotation (see the test as an example).
This is not viable to modify all headers with this annotation hence infer will try to do template instantiation for generic models in later diffs.
Reviewed By: jberdine
Differential Revision: D4826365
fbshipit-source-id: 2233e42
Summary:
Initial version of naming, required for generic models. It's simply non mangled name stripped from any template arguments.
This makes it impossible to have two generic models with
1. different template arguments
2. different overloads (function with same name, but different types of arguments)
Reviewed By: jberdine
Differential Revision: D4826358
fbshipit-source-id: 42ac763
Summary: If two public methods touch the same state and only one is marked `ThreadSafe`, it's reasonable to report unsafe accesses on both of them.
Reviewed By: peterogithub
Differential Revision: D4785038
fbshipit-source-id: 5a80da4
Summary: `__attribute__((annotate("")))` is better way of passing information to the frontend
Reviewed By: jberdine
Differential Revision: D4818805
fbshipit-source-id: 6e8add2
Summary:
- Do not print OCaml backtrace unless in developer mode.
- Print javac output when it fails.
- Refactor Javac.ml so that rerunning the command doesn't duplicate code.
The output of the clang and Javac integration is good, the output for mvn is
ok, the output for Buck is still fairly "meh".
Reviewed By: jberdine
Differential Revision: D4818722
fbshipit-source-id: 1973b93
Summary: This will avoid the redefine this Map and Set module as pretty printable when used to create abstract domains.
Reviewed By: sblackshear
Differential Revision: D4811849
fbshipit-source-id: e2f6763
Summary:
*Unless* the unprotected write runs on the main thread and the read doesn't.
Otherwise, we'll already report on the unprotected write, and we don't want to duplicate.
Reviewed By: peterogithub
Differential Revision: D4798357
fbshipit-source-id: 5de06a0
Summary:
Currently --per-procedure-parallelism defaults to a chunk size of 1
procedure, which has a high overhead. Add a command line option to
control it, and raise the default value.
Reviewed By: jvillard
Differential Revision: D4794692
fbshipit-source-id: 7715a40
Jeremy Dubreil
Sam Blackshear
Jules Villard
Mehdi Bouaziz
Ryan Rhee
Dulma Churchill
Dino Distefano
Andrzej Kotulski
Peter O'Hearn
Martino Luca
Sungkeun Cho
Kihong Heo
Josh Berdine
htxs