Summary:
It works now. Jumping to definition or calling merlin-document on files from
opam libraries doesn't seem to work when several libraries used by infer define
the same module (e.g. string.ml exists in ocaml lib, core_kernel, and base).
Reviewed By: let-def
Differential Revision: D7708218
fbshipit-source-id: 8e74b9b
Summary: Random edits made while reading ZipLib.ml. They're really not interesting, sorry...
Reviewed By: sblackshear
Differential Revision: D7686159
fbshipit-source-id: 19ef1ce
Summary:
Given a difference between two files, return the relevant lines in the new file; a line is relevant when a change took place in it, or nearby. To generate a valid input for this parser, use unix-diff command with the following formatter arguments:
```
diff --unchanged-line-format="U" --old-line-format="O" --new-line-format="N" File1 File2
```
Reviewed By: ezgicicek
Differential Revision: D7385267
fbshipit-source-id: 0cc3143
Summary:
This is to make sure we do not influence the analysis of the models by accident
when a .inferconfig exists in some directory higher up (eg, `$HOME`, infer/,
...).
Reviewed By: sblackshear
Differential Revision: D7686136
fbshipit-source-id: 9a250ff
Summary: It makes sense to run the same prelude and epilogue for `infer analyze` as for `infer run`.
Reviewed By: da319
Differential Revision: D7639363
fbshipit-source-id: 26170c1
Summary:
Upgrade ocamlformat, and base which needs to be done in sync in order to build
ocamlformat, and the other deps can come for the ride.
Reviewed By: jvillard
Differential Revision: D7663537
fbshipit-source-id: 3e90970
Summary: We already suppress race reports if the field is marked in this way; makes sense to do the same thing for these reports.
Reviewed By: ngorogiannis
Differential Revision: D7589275
fbshipit-source-id: 8f0aeab
Summary:
We were using the "filename" as the key because it's (kinda) unique *and* human
readable, but with the `infer explore --procedures` interface we don't really
need the human readable part anymore, so we can just use the OCaml marshalling
of the pname as the key. The human-readable version (sans unique-fying hash) is
now another column in the table, used to match procedure names in
`--procedures-filter`.
Reviewed By: sblackshear
Differential Revision: D7639158
fbshipit-source-id: e714605
Summary:
Add a `--procedures` option to `infer explore` to print information about the
procedures captured by infer. More precisely, `infer explore --procedures` will
print each row of the "procedures" table in the results database. A new
`--procedures-filter` controls which procedures to print information about, and
there is one flag per column in the db too to print more or less options about
each procedure (in particular, we can now print attributes), with some defaults.
Reviewed By: sblackshear
Differential Revision: D7639062
fbshipit-source-id: 034a2b8
Summary: Currently when we look for already abduced expression and find an assertion [exp|->strexp:typexp], we use typexp rather than strexp.
Reviewed By: sblackshear
Differential Revision: D7617193
fbshipit-source-id: c089720
Summary:
Now that everything can run at the same time and we have preanalyses, it can be quite hard to read debug sessions.
Here come session names!
Depends on D7607336
Reviewed By: sblackshear
Differential Revision: D7607481
fbshipit-source-id: 676af86
Summary:
- Less `^`
- `pp_print_string` instead of `F.fprintf fmt "%s"`
- and stuff like that
Reviewed By: jvillard
Differential Revision: D7607336
fbshipit-source-id: 5d985ef
Summary:
Use an adhoc type for `StructuralConstraints` instead of hacky `Exp`s.
Also use a cleaner `Node.IdMap` instead of `Int.Map`.
Depends on D7586645
Reviewed By: ddino
Differential Revision: D7587241
fbshipit-source-id: f9d65bb
Summary:
So we can share stuff between analyses using the same CFG and node representation.
Depends on D7586302
Depends on D7586348
Depends on D7568701
Reviewed By: sblackshear
Differential Revision: D7586645
fbshipit-source-id: ed64b2c
Summary:
Run with `SHELL = bash -e -u -o pipefail` to catch many kinds of failures. We
were silently failing during `make install` because of some missing escaping,
and the failure was hidden because it was happening inside a bash `for` loop.
This fixes the escaping issue and makes sure such issues will result in an
error as of now.
Also removes dangerous `find -exec` instances: `find` will `exit 0` event if
some commands failed.
Fixes#887
Reviewed By: mbouaziz
Differential Revision: D7569054
fbshipit-source-id: 542fe50
Summary:
Let's see in later diffs if that's that useful
Instead of rebuilding these modules, let's share them
Reviewed By: sblackshear
Differential Revision: D7586302
fbshipit-source-id: de69b39
Summary:
Try to skip running even `clang -###` when we know there's nothing to capture
in the clang command. Do that by recording in the `ClangCommand.t` whether the
command is pre- or post-running `clang -###`, and apply slightly different
filters in each case. In particular, the filter has to let in more commands
than necessary pre-`clang -###` because the flags are not normalised at that
point.
Also regard `-x cuda` commands as unsupported.
Reviewed By: mbouaziz
Differential Revision: D7584934
fbshipit-source-id: c27bb63
Summary: This will facilitate accessing Inferbo abstract state at the instruction level from the Cost analysis
Reviewed By: ddino
Differential Revision: D7568701
fbshipit-source-id: 84ac648
Summary:
Got rid of `Itv.equal` which was ambiguous and use an abstract boolean type for abstract comparison results
Depends on D7568573
Reviewed By: jvillard
Differential Revision: D7568583
fbshipit-source-id: 0e897e9
Summary:
Convenience function for `prune_eq` with zero (needed for stacked diffs).
Renamed `prune_zero` to `prune_ne_zero` to avoid ambiguity.
Reviewed By: jvillard
Differential Revision: D7568556
fbshipit-source-id: b95ab6d
Summary: Only RacerD uses the Cluster callbacks but we used to compute a lot of things anyway.
Reviewed By: sblackshear
Differential Revision: D7516279
fbshipit-source-id: 22f2b86
Summary:
This information is already available in the trace, and can contain absolute
paths to system includes (or infer's own clang runtime), which confuses the
diff analysis.
Reviewed By: mbouaziz
Differential Revision: D7534609
fbshipit-source-id: 5bd8f8b
Summary:
This can be noticed when the format of the DB changes, and other fun things
like that. No longer require to `make clean` to be able to pass these tests.
Reviewed By: mbouaziz
Differential Revision: D7533559
fbshipit-source-id: 670cb60
Summary:
It renames `eval_locs` to `eval_arr` and we use it for getting array block values the given input expressions are pointing to. For example, when given a program variable `x` as an input, `eval_arr` returns array blocks that `x` is pointing to, on the other hand, `eval` returns an abstract location of `x`.
Depends on D7471891
Reviewed By: mbouaziz
Differential Revision: D7471915
fbshipit-source-id: b994944
Summary: In the pointer arithmetics, it returns top, if we cannot precisely follow the physical memory model, e.g., (&x + 1).
Reviewed By: mbouaziz
Differential Revision: D7453510
fbshipit-source-id: db8738e
Summary:
It feels risky to me to swallow the `Sys_error` here since this could hide deeper issues with the deserialization of summaries.
This is only used with the Buck Integration for Java when extracting the summaries from the jar files.
Reviewed By: sblackshear
Differential Revision: D7490123
fbshipit-source-id: 68cd556
Summary:
This is unused, but the logic for keeping its value up to date was still alive
and kicking.
I don't know that making use of this flag would be easy: we could just use it
in `Ondemand.should_analyze`, but it would be unsound because a procedure might
need to be re-analysed becauese its dependencies have changed. Since there's
not code to deal with that currently I think it's best to remove it and
re-introduce it when we have some idea how to use it.
Reviewed By: sblackshear, jeremydubreil
Differential Revision: D7444179
fbshipit-source-id: 99a1ec5
Summary:
Report nullable inconsistencies by relying on the bytecode, and not on the presence of analysis summary on disk.
This use the `--external-java-packages` to avoid reporting inconsistencies outside of the codebase.
Reviewed By: sblackshear
Differential Revision: D7481101
fbshipit-source-id: 281135d
Summary:
Utility function that's basically `Logging.progress` but with a more obvious
name for debugging.
The `[@deprecated ...]` ensures no uses gets landed, and the `[@warning "-32"]` ensures that it's not a compile error that there are no uses of this function.
Inspired by #716.
Reviewed By: mbouaziz
Differential Revision: D7443224
fbshipit-source-id: e94798f
Summary:
Without it `make` will not rebuild infer when "infer/src/infer.ml" changes
(since D7381842 moved it out of backend/).
Reviewed By: mbouaziz
Differential Revision: D7443275
fbshipit-source-id: 33ea10b
Summary:
Partial revert of a21644685f / D7381857 due to an
issue introduced in 1.0+beta19 that prevents jbuilder from generating the
correct .merlin (https://github.com/ocaml/dune/issues/657).
Reviewed By: mbouaziz
Differential Revision: D7414970
fbshipit-source-id: 10561e9
Summary:
It's already turned of systematically for the Java integration, this just
generalises it. The Buck daemon seems to cause issues with infer from time to
time that are hard to debug.
Reviewed By: jeremydubreil
Differential Revision: D7400068
fbshipit-source-id: f05ee07
Summary:
Looks like the remaining issues were solved. In particular, build the toplevel
with the same ppx as infer so that the ppx information makes it to .merlin.
Reviewed By: mbouaziz
Differential Revision: D7381857
fbshipit-source-id: 5847d56
Summary:
There's actually a nice separation between IR/, base/, istd/, and the rest of
infer, so they can be made into separate jbuilder libraries so that the
separation remains. This helps make sense of the infer codebase.
Also:
- move everything biabduction-related out of backend/ and into a new
biabduction/ directory. This clarifies the current situation where backend/
contains a mix of analysis-independent code (still there now), and
biabduction-specific code (moved to biabduction/).
- move everything from base/ that is not infer-specific into istd/, e.g. IList.ml
- kill unused `FbTraceCalls`
- A couple of files needed to move around to complete the separation of base/ and IR/
Reviewed By: mbouaziz
Differential Revision: D7381842
fbshipit-source-id: cd86dea
Summary:
If an aggregate `a` has a field `f` whose type has a constructor (e.g., `std::string`), we translate creating a local aggregate `A { "hi" }` as `string(&(a.f), "hi")`.
This diff makes sure that we recognize this as initializing `a`.
Reviewed By: jeremydubreil
Differential Revision: D7404624
fbshipit-source-id: 0ba90a7
Summary:
New command line option job-id meant to contain the Sandcastle job id
itslog
- New command line option job-id meant to contain the ID of the job that Infer is running in
- If the id is passed to infer, then log it with every row in EventLogger
Reviewed By: dulmarod
Differential Revision: D7398537
fbshipit-source-id: c5186e9
Summary: - No need to log the string of stats type when registering a perf Epilogue
Reviewed By: dulmarod
Differential Revision: D7398544
fbshipit-source-id: 681a956
Summary:
Limit the scope of what gets included into IStd.ml to only values that we want
to shadow. New values go into other files.
Also, build istd/ with `Core` open.
Reviewed By: mbouaziz
Differential Revision: D7382111
fbshipit-source-id: 969f0e8
Summary:
- Time only stats are now reported from the driver for
- TotalFrontend
- TotalBackend
Reviewed By: dulmarod
Differential Revision: D7382161
fbshipit-source-id: 6e34ca2
Summary:
- PerfStats can now directly compute the filename / relative path of the stats to be logged
- There is no longer a need to pass in the filename as a parameter, simplifying the API to just be a one-liner
Reviewed By: dulmarod
Differential Revision: D7381886
fbshipit-source-id: e6623c3
Summary: `stats_type` can just include a `SourceFile.t`, as the existence of a `source_file` parameter previously depended on the value of `stats_type` anyway
Reviewed By: dulmarod
Differential Revision: D7381605
fbshipit-source-id: 953ee27
Summary:
`Bottom` is only useful for unreachable code detection, which is already handled.
Let's not check `Bottom` states.
Depends on D7289084
Reviewed By: skcho
Differential Revision: D7289153
fbshipit-source-id: 8333ce7
Summary:
Infer reads the arguments passed to clang, in particular to filter out some
incompatible clang command-line options. But, infer only understands arguments
in arg files if they are presented one per line. That's usually the case, but
infer itself stores clang arguments from compilation databases all on one line.
This breaks filtering.
This changes the type of compilation database items to a list of arguments, so
that they can be stored one per line when possible.
Also does some cleanup/renamings, and remove trailing `_` from temp file names, eg:
clang_command_.tmp.2b2602.txt -> clang_command.tmp.2b2602.txt
Also, do not escape arguments from arg files when printing them in logs (that
was useless and ugly).
Reviewed By: mbouaziz
Differential Revision: D7365907
fbshipit-source-id: 5a3fe70
Summary:
The goal is not to end up with a deep copy when nothing has changed, as this
puts lots of pressure on the memory.
Reviewed By: mbouaziz
Differential Revision: D7113976
fbshipit-source-id: 1b85ecd
Summary:
There's no real reason not to use `Core` lists in this module. Changed the
interface to be more `Core`-like. Changed the `*_changed` functions to use a
ref to track changes instead of passing the changed state around.
Reviewed By: mbouaziz
Differential Revision: D7123211
fbshipit-source-id: b27791a
Summary: This makes the code cleaner, and also makes it easier to look at the type of a var (needed in a successor).
Reviewed By: jeremydubreil
Differential Revision: D7359466
fbshipit-source-id: 5acdb7a
Summary: The "advice" section of the error description does not seem to be used in practice.
Reviewed By: mbouaziz
Differential Revision: D7348815
fbshipit-source-id: 1d7c8c7
Summary:
- InferPrint reports time stats only, logged in `reporting_stats/` as well as a `PerformanceStats` event with `stats_type: reporting`
- Perf reporting can now be registered at one point (with desired start real and cpu times) and triggered later
Reviewed By: dulmarod
Differential Revision: D7332435
fbshipit-source-id: 2e17c7f
Summary:
Perf reporting can now be registered at one point (with desired start real and cpu times) and triggered later
- This is accomplished by storing perf file names in a hashtable mapping to the reporting function
- New `stats_kind` parameter that is either Time, Memory, or TimeAndMemory, where Time requires a Unix.process_times and an Mtime_clock.counter to be passed in, but TimeAndMemory just uses the time from the start of the process.
Reviewed By: dulmarod
Differential Revision: D7337158
fbshipit-source-id: 94699cd
Summary:
Perf reporting can now be registered at one point (with desired start real and cpu times) and triggered later
- This is accomplished by storing perf file names in a hashtable mapping to the reporting function
- New `stats_kind` parameter that is either Time, Memory, or TimeAndMemory, where Time requires a Unix.process_times and an Mtime_clock.counter to be passed in, but TimeAndMemory just uses the time from the start of the process.
Reviewed By: dulmarod
Differential Revision: D7337158
fbshipit-source-id: 3890cc7
Summary:
- PerfStats can now report either time and/or memory stats, by default reporting both
- This is in preparation for adding just time or memory related logging in the future
Reviewed By: mbouaziz, dulmarod
Differential Revision: D7324556
fbshipit-source-id: e265b12
Summary:
Show where the invalidation occurred in the trace.
Should make things easier to understand.
Reviewed By: jeremydubreil
Differential Revision: D7312182
fbshipit-source-id: 44ba9cc
Summary: This was causing false positives when returning the constant integer 0.
Reviewed By: sblackshear
Differential Revision: D7330143
fbshipit-source-id: 45d19dd
Summary: Should be no semantic change, just trying to avoid code duplication.
Reviewed By: jeremydubreil
Differential Revision: D7268588
fbshipit-source-id: 8b00125
Summary: This will make it easier to detect regressions, until we can completely solve the issue with the classpath and change the `No_class_found` exception into a hard failure.
Reviewed By: sblackshear
Differential Revision: D7307464
fbshipit-source-id: eab67fb
Summary: It adds an issue type, `BUFFER_OVERRUN_U5`, for alarms involving unknown values, i.e., when the trace set includes an unknown function call.
Reviewed By: mbouaziz
Differential Revision: D7178841
fbshipit-source-id: bfe857b
Summary:
At the moment, Java and Clang sources/sinks live in the same inferconfig entry.
If we try to parse a Java procedure that happens to be an invalid Clang qualified name (e.g., `MyClass.<init>`),
parsing will crash.
As a temporary fix, throw an exception and catch it instead.
In the future, we can avoid this by requiring that JSON source/sink specifications to indicate the language.
Reviewed By: mbouaziz
Differential Revision: D7291880
fbshipit-source-id: f8f4502
Summary: Add new clang_method_kind field to AnalysisIssue, logged similarly to the existing one in AnalysisStats
Reviewed By: dulmarod
Differential Revision: D7273660
fbshipit-source-id: d1ca79b
Summary:
- Rather than passing a directory name to PerfStats reporting api to write the stats to, and then determining the EventLogger `stats_type` from that dirname, there is now a `stats_type` type in PerfStats, and the directory name and EventLogger tag is automatically determined from that type after it is passed to the API
- This allows us to determine whether we're in capture mode or linters mode dynamically from PerfStats itself, and use the appropriate tag when creating an event
Reviewed By: dulmarod
Differential Revision: D7251580
fbshipit-source-id: 0dec071
Summary:
Aggregate initialization (e.g., `S s{1, 2}`) doesn't invoke a contructor.
Our frontend translates aggregation initialization as assigning to each field in the struct.
To avoid the appearance of the struct being uninitialized, count any assignment to a field of an aggregate struct as initializing the struct.
Reviewed By: jeremydubreil
Differential Revision: D7189671
fbshipit-source-id: ace02fc
Summary: Backend perf stats were previously being logged as both `driver_stats` and `backend_stats`. Now they are correctly logged only as `backend_stats`, and a single `driver_stats` exists for the top-level driver process.
Reviewed By: dulmarod
Differential Revision: D7195879
fbshipit-source-id: 0f2ddc0
Summary:
Previously, `backend_stats` were getting logged correctly only when `infer analyze` was directly called, not `infer run`. Now, we report `backend_stats` directly, as part of the `iterate_callbacks` function in the task passed to the `ProcessPool`.
As a side benefit, `aggregated_stats` are also logged correctly now.
Reviewed By: dulmarod
Differential Revision: D7195525
fbshipit-source-id: fb2a400
Summary: It corrects a precision bug in the interval domain, with adding some test cases.
Reviewed By: mbouaziz
Differential Revision: D7230918
fbshipit-source-id: 3ec641a
Summary:
:
Previously, we did not have information about type of `exp` in `sizeof exp` from clang plugin which led to `Bad_footprint` errors. Infer did not understand `sizeof *p` in `struct Person* p = malloc(sizeof *p);` and used some default type.
This resulted in `Bad_footprint` error when trying to assign to a field `age` in `p->age=42;`.
This diff uses the version of clang plugin which exports the appropriate type information.
update-submodule: facebook-clang-plugins
Reviewed By: dulmarod
Differential Revision: D7179870
fbshipit-source-id: 4104f10
Summary:
Show some `SymAssign`s (corresponding to parameters) in the trace.
Depends on D7194448
Reviewed By: skcho
Differential Revision: D7194479
fbshipit-source-id: 0deff6c
Summary:
- Noticed that there were two different type aliases for the same type, representing the return value of `__POS__`
- Combined them under `ocaml_pos` name which more closely matches the pervasive
- Moved to Logging module
Reviewed By: dulmarod
Differential Revision: D7194034
fbshipit-source-id: 22cb949
Summary:
- PerformanceStats rows will be logged to EventLogger regardless of whether Infer is in developer mode
- PerformanceStats files will still not be created unless Infer is in developer mode
Reviewed By: dulmarod
Differential Revision: D7169403
fbshipit-source-id: 85bd7de
Summary:
- New Event type in EventLogger: PerformanceStats
- Contains some of the information logged to disk by PerfStats, as well as a `stats_type` tag to indicate which type of process the stats are from
- PerfStats.register_report_at_exit now takes 3 string arguments and constructs the file path and determines stats_type based off of them
- Changed all clients of PerfStats to use new api
Reviewed By: dulmarod
Differential Revision: D7131904
fbshipit-source-id: 9226b5d
Summary: It corrects a bug that `&(x.f[n])` was evaluated to `&(x.f[0])`.
Reviewed By: mbouaziz
Differential Revision: D7179620
fbshipit-source-id: 04cbaa7
Summary: It simply resizes the target structure instead of allocating new heap memories and copying values.
Reviewed By: mbouaziz
Differential Revision: D7179353
fbshipit-source-id: 9c20f64
Summary: If a `Closure` expression `e` captures variable `x`, consider `e` as borrowing from `x`. When the closure is invoked via `operator()`, check that the borrow is still valid.
Reviewed By: jeremydubreil
Differential Revision: D7071839
fbshipit-source-id: d923a6a
Summary: It avoids that locations of array fields are evaluated to the `unknown` location incorrectly by addressing the case in the `eval_lindex` function.
Reviewed By: mbouaziz
Differential Revision: D7152736
fbshipit-source-id: 2dc825e
Summary: It collects array accesses from all sub expressions in commands.
Reviewed By: mbouaziz
Differential Revision: D7165098
fbshipit-source-id: 584dc80
Summary: It does not only malloc a new heap memory, but also copy its contents.
Reviewed By: mbouaziz
Differential Revision: D7152194
fbshipit-source-id: 58cba5e
Summary:
- Add support for floating point values to JsonBuilder
- This allows EventLogger to log fields of type float
Reviewed By: dulmarod
Differential Revision: D7138542
fbshipit-source-id: 8a4e353
Summary: This is to make sure than the analysis produces the same results independently from the order in which the members of a call cycle are analyzed.
Reviewed By: sblackshear, mbouaziz
Differential Revision: D6881971
fbshipit-source-id: 23872e1
Summary: Add a new command-line option `--external-java-packages` which allows the user to specify a list of Java package prefixes for external packages. Then the analysis will not report non-actionable warnings on those packages (e.g., inconsistent `Nullable` annotations in external packages).
Reviewed By: jeremydubreil
Differential Revision: D7126960
fbshipit-source-id: c4f3c7c
Summary: When a property was defined in a protocol, we were not translating its attributes which leads to retain cycles false positives. This diff fixes it. It also refactors the code for translating fields a bit.
Reviewed By: mbouaziz
Differential Revision: D7136355
fbshipit-source-id: b5e7445
Summary:
A matcher that matches procnames only.
To be used in bi-abduction.
Depends on D7124877
Reviewed By: skcho
Differential Revision: D7125113
fbshipit-source-id: e44f3a8
Summary:
Because it matches calls (procnames and argument list) and because I will create a `Procname`-only one.
Depends on D7124847
Reviewed By: skcho
Differential Revision: D7124877
fbshipit-source-id: eca7c21
Summary:
Fairly simple approach here:
- If the RHS of an assignment is a frontend-generated temporary variable, assume it transfers ownership to the LHS variable
- If the RHS of an assignment is a program variable, assume that the LHS variable is borrowing from it.
- If we try to access a variable that has borrowed from a variable that is now invalid, complain.
Reviewed By: jeremydubreil
Differential Revision: D7069947
fbshipit-source-id: 99b8ee2
Summary:
Just a bunch of minor changes.
- The mutable field doesn't need to be mutable: it's only mutated once in the code somewhere that doesn't even need to mutate
- Hence the type can be public
- All the `ms_get_*` functions are replaced by field accesses
- `CMethod_signature` -> `CMethodSignature` while I'm at it, although maybe that's bad since other files in clang/ follow the former convention
- `type method_signature` -> `type t`
- `pp` function instead of `to_string`, since it's used with `fprintf`. This gets rid of the only caller of `IList.to_string`.
Reviewed By: dulmarod
Differential Revision: D7123795
fbshipit-source-id: fdfae42
Summary:
`Sequence` API to walk over free variables in expressions, instead of computing lists with uniqueness constraints that make them have linear complexity for insertion.
Switch to a Set representation when we don't care about the order of elements,
otherwise to a `Hash_queue`:
https://ocaml.janestreet.com/ocaml-core/113.33/doc/core/Std/Hash_queue.mod/S.modt/
Often, we don't even need to compute the sequence of free variables, as we are
just testing membership/emptiness/...
Reviewed By: mbouaziz
Differential Revision: D7099294
fbshipit-source-id: e96f84b
Summary:
At function calls, it copies a subset of heap memory that is newly
allocated by callees and is reachable from the return value.
Reviewed By: mbouaziz
Differential Revision: D7081425
fbshipit-source-id: 1ce777a
Summary:
This is to fix the conflicts between Eradicate and the Biabduction when reporting the same kind of errors: when Eradicate is on, the Eradicate warnings will have priority over the null deference reported by the biabduction.
If this approach proved to be successful in prod, I will refactor the reporting mechanism in the analysis itself to simply not report the null dereference in this case at all. For the codebases that aren't yet fully consistently using `Nullable`, this combined approach looks like a good way to deploy Infer toward full null safety.
Reviewed By: mbouaziz
Differential Revision: D7102119
fbshipit-source-id: 35d3add
Summary:
Enrich capability domain with borrowing info. Inline comments explain what the domain is doing.
The analyzer isn't actually using the borrowing functionality yet--that comes in the successor.
Reviewed By: jeremydubreil
Differential Revision: D7067942
fbshipit-source-id: 4c03c69
Summary:
Before D7100561, the frontend translated capture-by-ref and capture-by-value in the same way.
Now we can tell the difference and report bugs in the capture-by-value case.
Reviewed By: jeremydubreil
Differential Revision: D7102214
fbshipit-source-id: e9d3ac7
Summary:
The goal is to allocate less and generally be more efficient than handling
lists with uniqueness constraints.
Reviewed By: mbouaziz, jberdine
Differential Revision: D7098904
fbshipit-source-id: 7111f07
Summary:
The `may_last_field` boolean value in the `decl_sym_val` function presents that the location *may* (not *must*) be a flexible array member.
By the modular analysis nature, it is impossible to determine whether a given argument is a flexible array member or not---because of lack of calling context. For example, there are two function calls of `foo` below: (2) passes a flexible array member as an argument and (1) passes a non-flexible array, however it is hard to notice when analyzing the `foo` function.
```
struct T {
int c[1];
};
struct S {
struct T a;
struct T b;
};
void foo(struct T x) { ... }
void goo () {
struct S* x = (struct S*)malloc(sizeof(struct S) + 10 * sizeof(int));
foo(&(x->a)); // (1)
foo(&(x->b)); // (2)
}
```
We assume that any given arguments may stem from the last field of struct, i.e., flexible array member. (This is why `decl_sym_val` is called with `may_last_field:true` at the first time.) With some tests, we noticed that the assumption does not harm the analysis precision, because whether regarding a parameter as a flexible array member or not is about using a symbolic array size instead of a constant array size written in the type during the analysis of callee. Therefore still it can raise correct alarms if the actual parameter is given in its caller.
Reviewed By: mbouaziz
Differential Revision: D7081295
fbshipit-source-id: a4d57a0
Summary:
Instead of storing the type environment in infer-out/captured/foo.c/foo.c.tenv,
store it in the `source_files` table of the SQLite db. This limits the number
of files we create on disk.
The "file local" type environemnts are specific to the clang integration. For
Java, there is a "global tenv" file. Instead of matching on string names, this
diff also makes the API of `Tenv` reflect this situation.
The global tenv is serialized as a separate file in "infer-out/.global.tenv"
instead of "infer-out/captured/global.tenv", because "infer-out/captured/" will
soon be removed as it now only contains the global tenv (except in debug mode,
where it will still be created).
In the DB, we either store the local tenv for the file, or "global" to indicate
that the global tenv should be consulted.
This diff also moves `Cfg.store` to `SourceFiles.add` because that function
deals with more than just `Cfg.t`.
Reviewed By: jeremydubreil
Differential Revision: D6937945
fbshipit-source-id: 001c10a
Summary:
A list of free variables was computed and passed around, but never used. The
reason OCaml wasn't complaining is that the list is eventually passed to a
recursive function, which passes it to its recursive call, so it looks like
it's used there.
Reviewed By: mbouaziz
Differential Revision: D7098556
fbshipit-source-id: b22a591
Summary:
Switch to the current stable branch for clang.
update-submodule: facebook-clang-plugins
Reviewed By: mbouaziz
Differential Revision: D7067890
fbshipit-source-id: aedff90
Summary:
You can capture a variable by reference in a lambda, assign to it, and then invoke the lambda.
This looks like a dead store from the perspective of the current analysis.
This diff mitigates the problem by computing an additional analysis that tracks variables captured by ref at each program point.
It refuses to report a dead store on a variable that has already been captured by reference.
Later, we might want to incorporate the results of this analysis directly into the liveness analysis instead of just using it to gate reporting.
Reviewed By: jeremydubreil
Differential Revision: D7090291
fbshipit-source-id: 25eeffa
Summary:
We frequently want to treat such vars differently than true program variables.
Used in only one place now, but will be used in a successor.
Reviewed By: mbouaziz, jvillard
Differential Revision: D7067882
fbshipit-source-id: 90e0348
Summary:
As far as I can tell nothing uses it and it is ignored by the matching engine:
the "parameters" arguments are collected from config options but never actually
used for matching.
Reviewed By: sblackshear
Differential Revision: D6976273
fbshipit-source-id: 6fed5ff
Summary:
Ran into this issue on Debian Testing, in which assert.h is probably different
due to a more recent toolchain. Without this change I get the following CFG
for `assert(e)`:
```
start
|-> prune (e) -> join
|-> prune (!e) -> __infer_fail("ASSERTION_FAILURE") -> exit
```
Notice that the first branch does not get to the exit, so infer must think that
the assertion is *always* violated, and reports `error: ASSERTION_FAILURE`.
This is broken.
Reviewed By: dulmarod
Differential Revision: D7067822
fbshipit-source-id: a2bf5ac
Summary:
- EventLogger no longer depends on ProcAttributes
- This allows for other backend classes which are used by ProcAttributes to log in the future without creating a cyclic dependency structure
Reviewed By: dulmarod
Differential Revision: D7082444
fbshipit-source-id: f32a6e2
Summary: - When a CallTrace result is CR_skip, then a reason for skipping the call will be included in the record
Reviewed By: dulmarod
Differential Revision: D7082572
fbshipit-source-id: 20ed191
Summary:
It supports flexible array member using the following heuristic:
- a memory for a class is allocated by `malloc(sizeof(C) + n * sizeof(T))` format
- the last field of the class is an array
- the static size of the last field is one, i.e., `T field_name[1]`
When allocating and initializing members of classes, it sets the size of flexible array to `n+1` if the above conditions are met.
Reviewed By: mbouaziz
Differential Revision: D7056291
fbshipit-source-id: 31c5868
Summary:
The semantics of "placement new" is defined simply as an assignment.
For example, `C* x = new (y) C();` is analyzed as if `C* x = y;`.
Reviewed By: mbouaziz
Differential Revision: D7054007
fbshipit-source-id: 1c6754f
Summary:
It abstracts the environments required in modeling functions and type
declarations as a record type.
Reviewed By: mbouaziz
Differential Revision: D7065996
fbshipit-source-id: b60cd3c
Summary:
The struct fields in Cil have been sorted for long time, however the
checkers do not seem to depend on the sortedness.
Reviewed By: sblackshear
Differential Revision: D7027858
fbshipit-source-id: 9e7ab96
Summary:
- During the symbolic execution stage of the backend, Infer will log detailed stats about procedure calls
- Logging is accomplished directly within symExec/Tabulation
- call_result type is moved to tabulation.ml
CallStats was a broken module that allocated a lot of useless memory. Now, Specs.CallStats and InferPrint.CallsCsv as well as the Calls report kind in InferPrint no longer exist.
Reviewed By: dulmarod
Differential Revision: D7016439
fbshipit-source-id: 40911ee
Summary:
This commit improves precision of symbol instantiations.
When a return value of a callee is `[s1 + s2, _]` and if we want to
instantiate `s1` to `c3 + max(c4, s5)`, the lower bound was
substituted to `-oo` because our domain cannot express `c3 + max(c4,
s5) + s2`.
However, we can have instantiations that are preciser than `-oo`:
(1) `c3 + c4 + s2`
(2) or `c3 + s5 + s2`
because they are smaller than the ideal instantiation, `c3 + max(c4,
s5) + s2` and it is on the lower bound position.
For now, the implementation instantiates to (1) between the two ones,
because constant values introduced by `assert` or `assume`(`if`)
command are often used as safety conditions, e.g., `assert(index >=
0);` can place before array accesses. (We can change the stratege
later if we find that it doesn't work on some other cases.)
Reviewed By: mbouaziz
Differential Revision: D7020063
fbshipit-source-id: 62fb390
Summary:
- StatsLogs is now its own functional module in InferPrint
- No longer stores events to be logged in a list to be later logged, just immediately logs them
Reviewed By: dulmarod
Differential Revision: D7010559
fbshipit-source-id: 1837ca9
Summary: Just some minor renaming to be more consistent with other modules. I was about to use these modules and was too lazy to type `Ident.IdentSet`.
Reviewed By: da319, avarun42
Differential Revision: D6999808
fbshipit-source-id: c24edef
Summary:
- change a use of clang_method_kind to match directly on the type rather than a brittle check for equality
- make the clang_method_kind field in AnalysisStats an option
- if procedure lang is not clang, then clang_method_kind field gets skipped while logging
Reviewed By: dulmarod
Differential Revision: D7010060
fbshipit-source-id: 077094d
Summary:
- JsonBuilder now has an add_string_opt function that takes a string option instead of a string
- Further alphabetized EventLogger
Reviewed By: dulmarod
Differential Revision: D7011763
fbshipit-source-id: 73f1a4b
Summary:
A simple intraprocedural analysis that tracks when a storage location is read or deleted.
For now, this works only with local variable storage locations; field and array accesses are ignored.
In order to test this, I added a new "use-after-lifetime" warning. It complains when a variable is read or deleted after it has already been deleted.
Reviewed By: jeremydubreil
Differential Revision: D6961314
fbshipit-source-id: 75e95a2
Summary: We do not inject a destructor call if the destructor declaration does not contain a body in AST. We miss all the cases where the destructor is declared in `.h` file and defined in `.cpp` file as other files include `.h` file and do not contain the body of the destructor when destructor calls are being injected based on AST information. After this diff we inject destructor calls even if we do not have body for the destructor in AST.
Reviewed By: sblackshear
Differential Revision: D6796567
fbshipit-source-id: 1c187ec
Summary: More preparation for extending HIL with dereference and address of. We need left hand side of the assignment to also include dereference and address of.
Reviewed By: sblackshear
Differential Revision: D6976150
fbshipit-source-id: 47d1d76
Summary:
It prunes abstract memories on `assert` commands.
Problem: Since the assert command is sometimes translated to two
sequential `if` statments, it was not able to prune the memory
precisely at `assert` commands in Inferbo---the pruned memory at the
first branch was joined before the second branch.
Solution: To avoid losing the pruning information at the first branch,
now, it records which locations are pruned at the first branch and
applies the same pruning at the next branch if they have
semantically the same condition.
Reviewed By: mbouaziz
Differential Revision: D6895919
fbshipit-source-id: 15ac1cb
Summary:
See comment--`Prop(resType = blah) myProp` will generate `.myProp`, `.myPropRes`, and `.myPropAttr`, and any of them can be used to set the prop.
Because our annotation parameter parsing is a bit primitive, handle this by simply checking the `Res` and `Attr` suffixes for every `Prop`.
This shouldn't lead to false negatives because these methods will only exist if the `resType` annotation is specified anyway.
Reviewed By: jeremydubreil
Differential Revision: D6955362
fbshipit-source-id: ec59b21
Summary: InferPrint should not generate StatsLogs format at all unless Config.log_events is set to true
Reviewed By: mbouaziz, dulmarod
Differential Revision: D6976352
fbshipit-source-id: d12005a
Summary: In Obj-C blocks, we explicitly insert reads of the captured vars. This does the same thing for C++. For example, `foo() { int x = 1; [x]() { return x; } }` would previously not contain a read of `x` in `foo`. Now, we'll create a temporary that reads from `x` and pass it to the closure value.
Reviewed By: dulmarod
Differential Revision: D6939997
fbshipit-source-id: f218afc
Summary:
1) Fixes some false negatives when a method annotated with `nullable` in the header is not annotated in the implementation and the attribute lookup returns the implementation. In that case, we should follow the information given in the header.
2) Fixes some false positives when annotations are in the other way around, i.e. annotated in the implementation but not in the headers. For now, there should be no report in this case, but the analysis should be extended to report the inconsistency between the header and the implementation
3) Fixes some cases of weird reports caused by name conflicts where the method in the include has the same name has another method annotated differently.
Reviewed By: jvillard
Differential Revision: D6935379
fbshipit-source-id: 3577eb0
Summary: Preparing to extend HIL with Dereference and AddressOf expressions. Next steps: (1) change SIL -> HIL translation to preserve address of and dereference; (2) adapt analyses based on HIL to make use access expressions.
Reviewed By: jeremydubreil
Differential Revision: D6961928
fbshipit-source-id: 51da919
Summary:
Added a check for recursive calls not to add abduced reference parameters constraints. Abduced reference parameters constraints were causing assertion failure when renaming variables in specs, in particular, when transforming variables into callee variables.
A similar check is already in place for abduced retvals constraints.
Reviewed By: jeremydubreil
Differential Revision: D6856919
fbshipit-source-id: acfe840
Summary:
This allows Eradicate to lookup the annotations from the classpath and without requiring the code in the classpath to have been previously analyzed. The benefit is that source files can be analyzed independently of each other as long as the classpath is known.
The main goal is to run be able to run Eradicate as a linter without losing warnings.
We may have to add some more models of the standard libraries as no `Nullable` on a parameter does not necessarily mean that the method does not accept `null`.
Reviewed By: sblackshear
Differential Revision: D6921720
fbshipit-source-id: f525269
Summary: Some tags like `Bucket` are used, but a lot are just added to the list of tags and never read.
Reviewed By: mbouaziz
Differential Revision: D6886980
fbshipit-source-id: 4474d7f
Summary:
There's a lot of code for building up and moving around `Tags`.
When working on cleaning up some of the `Errlog` code, I noticed that `Tags` were included in the JSON and wondered why.
The answer is suprisingly just one thing: only the line tags get used, and even then they are only used to decide what frame to select as the start frame for the trace (i.e., the one that is highlighted first).
That seems like overkill; starting on trace on the actual line where the error occurs, starting at the beginning of the procedure where the error occurs, or starting at the first line of the trace all seem equally reasonable.
If we are happy with any of these alternatives, we can kill `Tags` altogether and potentially save a decent amount space in our JSON artifacts.
Reviewed By: mbouaziz
Differential Revision: D6876752
fbshipit-source-id: 1580127
Summary:
Useful to debug without having to rerun commands.
Looks like this:
```
Usage Error: Failed to execute compilation command:
'/home/jul/infer.fb/infer/bin/../../facebook-clang-plugins/clang/install/bin/clang'
'args.txt'
++Contents of 'args.txt':
'arg1.txt' 'arghorashy_hello_c.txt' 'args.txt'
++Contents of 'arg1.txt':
'arghorashy_c.txt'
++Contents of 'arg_c.txt':
'-c'
++Contents of 'arg_hello_c.txt':
'examples/hello.c'
Error message:
clang-5.0: error: no such file or directory: 'examples/hello.c'
*** Infer needs a working compilation command to run.
```
Reviewed By: mbouaziz
Differential Revision: D6872550
fbshipit-source-id: 65cd026
Summary:
- Combine two fields from ProcAttributes.t into a single field `method_kind` with more information
- New field details whether the procedure is an `OBJC_INSTANCE`, `CPP_INSTANCE`, `OBJ_CLASS`, `CPP_CLASS`, `BLOCK`, or `C_FUNCTION`
- `is_objc_instance_method` and `is_cpp_instance_method` fields no longer necessary
- Changed `is_instance` field in CMethod_signature to `method_kind` field of type ProcAttributes.method_kind
Reviewed By: dulmarod
Differential Revision: D6884402
fbshipit-source-id: 4b916c3
Summary: If the procedure is defined, then the attributes should be the same on the specs files or on the attributes table.
Reviewed By: sblackshear
Differential Revision: D6910086
fbshipit-source-id: 709b290
Summary:
- small optimization by starting deconstructing procnames/types in the dispatcher rather than the matchers
- as a consequence, returns fast for unhandled constructs like Java procnames or types
- Java is still not handled but at least does not crash
- re-enable Inferbo for Java
Reviewed By: jberdine
Differential Revision: D6912304
fbshipit-source-id: 76e95a8
Summary:
In some cases infer runs clang commands where we do not attach the plugin,
notably for pre-processor-only commands like `clang -E foo.c`. Usually these
commands are used by build systems to test their output, so infer should not
swallow it.
This makes infer echo the stdout of clang whenever we do not attach the plugin
to clang. When the plugin is attached, stdout is captured because that's where
the plugin outputs its results, so we cannot do the same in this case (not that
we would want to yet).
Fixes#696.
Reviewed By: mbouaziz
Differential Revision: D6912101
fbshipit-source-id: c4ad2e4
Summary:
Record "capture phases" in the runstate and in the source files table of the
database. Use this instead of filesystem timestamps to decide which files need
re-analyzing in the reactive analysis.
Reviewed By: jeremydubreil
Differential Revision: D6760833
fbshipit-source-id: 7955621
Summary:
The boolean lock domain is simple and surprisingly effective.
But it's starting to cause false positives in the case where locks are nested.
Releasing the inner lock also releases the outer lock.
This diff introduces a new locks domain: a map of locks (access paths) to a bounded count representing an underapproximation of the number of times the lock has been acquired.
For now, we just use a single dummy access path to represent all locks (and thus a count actually would have been sufficiently expressive; we don't need the map yet).
But I'm planning to remove this limitation in a follow-up by refactoring the lock models to give us an access path.
Knowing the names of locks could be useful for error messages and suggesting fixes.
Reviewed By: jberdine
Differential Revision: D6182006
fbshipit-source-id: 6624971
Summary: There's a new `ocaml_pos` type that the other clang frontend exceptions use, but Self.SelfClassException still used the raw tuple. Now, SelfClassException also uses this type.
Reviewed By: dulmarod
Differential Revision: D6900258
fbshipit-source-id: 94c7042
Summary:
- During backend execution, infer will log detailed stats about procedure analysis
- Logging is integrated with EventLogger
- `events_to_log` field added to Stats.t record in InferPrint
- New format in InferPrint - Logs
- `format_list` type changed to have a Utils.Outfile option to support Logs format
Reviewed By: dulmarod
Differential Revision: D6834538
fbshipit-source-id: 8c847f5
Summary:
Previously, we could understand than an access was safe either because it was possibly owned or protected by a thread/lock, but not both. If an access was both protected by a lock and rooted in a paramer (i.e., possibly owned), we would forget the ownership part of the precondition and remember only the lock bit. This leads to false positives in cases where an access protected by a lock is owned, but another unowned access to the same memory is not protected by a lock (see the new `unownedLockedWriteOk` E2E test for an example).
This diff makes access safety conditions disjunctive so we can simultaneously track whether an access is owned and whether an access is protected by a thread/lock. This will fix false positives like the one explained in T24015160.
Reviewed By: jberdine
Differential Revision: D6671489
fbshipit-source-id: d17715f
Summary:
- ast_node argument is now optional in functions `unimplemented` and `incorrect_assumption` in cFrontend_config
- The argument type was already an option, and the majority of the calls were with 'None'. This makes the function more intuitive to use
Reviewed By: sblackshear
Differential Revision: D6846141
fbshipit-source-id: 13deb8f
Summary:
We already knew not to warn when non-resource `Closeable`'s like `ByteArrayOutputStream` weren't closed, but we still warned on their subtypes.
This diff fixes that problem by checking subclasses in the frontend.
This also removes the need for Java source code models of non-resource types, so I removed them.
Reviewed By: jeremydubreil
Differential Revision: D6843413
fbshipit-source-id: 60fe7fb
Summary: The heuristics to determine the end of a block/procedure was too brittle, the new one ignores non significant instructions.
Reviewed By: jvillard
Differential Revision: D6845380
fbshipit-source-id: feab557
Summary:
The infer results directories in buck-out/ are "cleaned up" to avoid polluting
the Buck cache with too much data or non-deterministic data. In particular, the
runstate is deleted, which confused subsequent infer processes trying to read
the pre-existing results directory.
Add a special case in infer to delete pre-existing results directories in
buck-out instead of trying to load their state.
Reviewed By: mbouaziz
Differential Revision: D6845128
fbshipit-source-id: 5c716aa
Summary:
- `NonZeroInt` for added guarantees on the invariants of `SymLinear` coefficients
- some simplifications
- some optimizations
Reviewed By: jvillard
Differential Revision: D6833968
fbshipit-source-id: 39e28a0
Jules Villard
Nikos Gorogiannis
Sam Blackshear
Martino Luca
Josh Berdine
Ezgi Çiçek
Jeremy Dubreil
Daiva Naudziuniene
Mehdi Bouaziz
Tony Hu
Dulma Churchill
Dino Distefano
Sungkeun Cho
Varun Arora
Jérémie Marguerie
Ilya Sergey