Summary:
This way we don't rely on existing summaries already having a
MustBeValid for self and on them having possibly other conditions that
might prevent a report.
Reviewed By: skcho
Differential Revision: D29550321
fbshipit-source-id: b9822fcd4
Summary: Typ.pp does not print types by default, use Typ.pp_full instead.
Reviewed By: ngorogiannis
Differential Revision: D29557158
fbshipit-source-id: 62ae20bd5
Summary:
Before, atoms we didn't understand but could be a contradiction became
"true" if they were only about dead variables. This is unsound for
incorrectness logic, and introduces false positives.
Be more conservative and try to identify when that could happen and
prune these paths instead, thus regaining under-approximation.
This is a bit ad-hoc: we expect reasoning on equalities and
disequalities to be precise enough, but we know that inequalities have
very little solving power at the moment (eg we don't detect `x < 0, x >
0 => false`). In addition, we remark that having *one* `<` atom can
never be a contradiction. So, we claim a possible contradiction whenever
we find *two* (>1) atoms containing inequalities and involving the same
variable.
Reviewed By: skcho
Differential Revision: D29232544
fbshipit-source-id: ff91eb1e4
Summary: Add some test cases to potentially non-exhaustive case clauses.
Reviewed By: rgrig
Differential Revision: D29555525
fbshipit-source-id: f710e93e6
Summary: New inferconfig flags which will pass through to buck config flags. Purpose is to allow using a buck target to distribute and use infer binaries as a buck toolchain.
Reviewed By: martintrojer
Differential Revision: D29485801
fbshipit-source-id: 86169cdae
Summary: If a test function `F` has multiple calls to a possibly problematic function `G`, our current expected output does not tell exactly which call to `G` causes the problem. This diff splits up such functions `F` into smaller parts `F1`, `F2`, ..., so that each part only calls `G` once. This way our expected output can match more precisely.
Reviewed By: rgrig
Differential Revision: D29554848
fbshipit-source-id: bdc62731c
Summary:
"glue" together the trace from the caller with the trace from the callee
to form the access trace when applying LatentInvalidAccess summaries.
This is consistent with what happens in function calls and allows Pulse
to better keep track of the history of the value. The goal is to prevent
bugs from getting filtered out because the access trace doesn't contain
the invalidation event for the value.
Reviewed By: da319
Differential Revision: D29550320
fbshipit-source-id: b600c188d
Summary:
There should be no change in semantics:
- before: the "crash node" modelling throwing an exception was connected directly to the exit of the procedure
- now: the "crash node" goes to exit_failure, which should end up being connected to the exit of the procedure by the `translate_one_function`
The latter is closer to what we'd need to do when we eventually model
throwing and catching exceptions (rather than just reporting an error
the moment we see them thrown).
Differential Revision: D29540834
fbshipit-source-id: c4de4c391
Summary:
Topl matches procedure names against whatever Procname.hashable_name
returns. For Java, it returns something like "java.util.ArrayList.add(...)",
but for Erlang it used to return something like "add/1": no module and
no parenthesis after. Now, Erlang returns "lists:add/1"; that is, it
includes module name. Also, Topl doesn't insist anymore on having a
paranthesis after the name.
Differential Revision: D29520365
fbshipit-source-id: d23be1cc8
Summary: `call-graph-schedule` was a boolean, now we have a variant type for scheduler.
Reviewed By: jvillard
Differential Revision: D29547773
fbshipit-source-id: ec787065c
Summary:
The gist of this diff is a set of Pulse models for the built-ins
that have been put in Sil by the Erlang frontend:
- __erlang_make_nil, for creating an empty list like []
- __erlang_make_cons, for creating a nonempty list like [1|[]]
- __erlang_pattern_fail, which marks that no pattern matched
The models for the first two update the abstract state. The model for
the third generates a reportable error.
The diff also includes a test.
And also a few bugfixes:
- a type ErlangAny was used sometimes instead of ErlangAny*
- a load that was meant to be n=13 was instead n=*13;
changed to (*tmp=13; n=*tmp)
- the Makefile's guard for the rebar build-system test was inside the large
guard for java analyzers; of course, we don't need java to run rebar
Reviewed By: jvillard
Differential Revision: D29230161
fbshipit-source-id: c8fd6d88a
Summary: Adding more logging for cases where we see a lock/unlock but we see no arguments.
Reviewed By: skcho
Differential Revision: D29427413
fbshipit-source-id: 58b76b865
Summary: Add support for translating binary expressions, and implement a few operators.
Reviewed By: rgrig
Differential Revision: D29452471
fbshipit-source-id: d7c25ae91
Summary: Extract helper methods related to block creation into a module.
Reviewed By: rgrig
Differential Revision: D29436139
fbshipit-source-id: 92f9466d3
Summary: Add support for translating case expressions (without guards yet) and integer literals (including in patterns). Note that we use the same infrastructure for case expressions and function clauses, so adding support for guards will only have to be implemented once.
Reviewed By: rgrig
Differential Revision: D29424141
fbshipit-source-id: 0d6f1e661
Summary: Flag defaults to false. (For now, only the buck integration supports capturing any Kotlin.)
Reviewed By: jvillard
Differential Revision: D29388274
fbshipit-source-id: 8dbec9555
Summary: Adding constraint `self > 0` after computing specs for Objective-C instance methods were causing false positives of the heuristics not to discard `self=0 /\ self|->-` (added new test testAnotherObjectUseSelfOk was FP). This diff adds constrain `self > 0` before computing Objective-C specs, discarding unsatisfiable `self > 0 /\ self =0`.
Reviewed By: skcho
Differential Revision: D29462462
fbshipit-source-id: 088ee447e
Summary: Useful to write other tests, and also probably worth modelling.
Reviewed By: ezgicicek
Differential Revision: D29232545
fbshipit-source-id: ecb24f6f7
Summary:
Investigations into the number of disjuncts we get in summaries
revealed that we sometimes blow past the limit. Rework the code to make
sure that does not happen.
We can probably raise the disjunct limit as a result, more experiments
needed.
Reviewed By: ezgicicek
Differential Revision: D29229249
fbshipit-source-id: 4a06594fa
Summary: Refactor Erlang expression translation. Previously expressions were translated to a list of instructions to be put in the single node. This will not work for expressions that require control flow (e.g., case, if). With this change, expressions are translated to blocks.
Reviewed By: skcho
Differential Revision: D29391068
fbshipit-source-id: 5f353e490
Summary:
There are two changes:
1/ Fix incorrect merge of two cases (empty and not-empty)
```
let<*> astate_equal_zero = ... in
let<*> astate_not_zero = ... in
[Ok (ContinueProgram astate_equal_zero); Ok (ContinueProgram astate_not_zero)]
```
This was an incorrect merge, because if there is an error in the former
case `astate_equal_zero`, it doesn't even evaluate the latter case
`astate_not_zero`.
2/ Cover all cases precisely. There are actually three cases to cover:
* `TextUtils.is_empty(null) = true`
* `TextUtils.is_empty("") = true`
* `TextUtils.is_empty("abc") = false`
However, in the previous model, it missed the second case.
Reviewed By: jvillard
Differential Revision: D29393579
fbshipit-source-id: e59d9a60d
Summary:
This diff adds some semantic models for C++ string length. It introduces
an virtual field for string length and use the value in the models.
* basic_string constructor given a constant string
* string.empty
* string.length
Reviewed By: jvillard
Differential Revision: D29390815
fbshipit-source-id: 99d67e48e
Summary: Option is not used anywhere. More importantly the list of source file extensions in `Config` might give someone the impression the rest of Infer cares about it somehow.
Reviewed By: skcho
Differential Revision: D29361222
fbshipit-source-id: 22f7f66b2
Summary: Narrow container read/write modelled methods by requiring they are all non-static.
Reviewed By: ezgicicek
Differential Revision: D29331533
fbshipit-source-id: e0e445573
Summary:
After activation, a few bugs manifested and are fixed in this commit:
1. Create an (empty) Tenv per file. Previously, each file said there
will be a global Tenv, but no such global Tenv was ever saved. (There is
no static typing at all at the moment in the Erlang frontend.)
2. Procedures are now marked as defined. If the is_defined attribute is
false, then the analysis will not run even if the flowgraph in Procdesc
is nontrivial.
3. Non-dummy Start/Exit nodes for Procdesc. When a Procdesc is created,
it gets dummy Start/Exit nodes. These dummy nodes have a dummy location,
which causes debug output to have wrong directories/links.
4. Filenames from Erlang function names. By default, the verbose
procedure name was used to make file names for the debug output. But,
Erlang function names contain '/', which doesn't play well with
filenames. That's now replaced by '#' for the purposes of constructing
filenames.
Reviewed By: jvillard
Differential Revision: D29166049
fbshipit-source-id: 48346a05b
Summary:
Generates Sil instructions for Erlang some expressions:
- empty list: e.g. []
- cons: e.g. [1,2]
- atom literals: e.g. ok
- string literals: e.g. "foo"
- calls to particular (no higher-order), unqualified functions
Differential Revision: D28996467
fbshipit-source-id: fc7f7e3e9
Summary: Associations in map patterns should only be exact (`:=`). We did not validate this previously (D28832961 (65884018dc)), because we didn't have certain AST transformation passes that would eliminate some special functions that allowed invalid association kinds (later removed by transformation). However, with D29103633 (25711189a8), we get the AST from a different source and these invalid cases are removed, so the check is added.
Reviewed By: mmarescotti
Differential Revision: D29162325
fbshipit-source-id: a47d9f05d
Summary: This workflow still uses the parse transform hook to detect which files are compiled. The parse transform now adds an entry to a list of compiled BEAMs that will be traversed after compilation by the new `extract.escript` to produce the JSON ASTs in the provided outdir.
Differential Revision: D29103633
fbshipit-source-id: 12b8720f4
Summary: There was a typo (probably copy paste) in an error message for unknown unary operators in the Erlang frontend.
Differential Revision: D29062028
fbshipit-source-id: b79e783ba
Summary:
Implement validator for Erlang AST to check various invariants and well-formedness constraints (based on https://erlang.org/doc/apps/erts/absform.html), including:
- Restrictions on expressions used as patterns
- Restrictions on expressions used as guards
- Case and catch clauses have 1 pattern
- If clause has no patterns
See comments for some limitations:
- Some constraints would require type information (e.g., unary/binary operators in patterns/guards), we are not enforcing these yet.
- Map patterns can only have `:=` association, except in some special functions that get translated during compilation. However, we get the AST before these transformation so we are not enforcing this rule yet.
Reviewed By: rgrig
Differential Revision: D28832961
fbshipit-source-id: 7df4425e4
Summary:
{F623534971}
instead, we want the procedure name/type to be stylized as code
Reviewed By: skcho
Differential Revision: D29027480
fbshipit-source-id: 35f8e0c4a
Summary:
Patterns like X, [], [H | T], [X, Y | T] should now be translated.
Also, environment now stores Location.t rather than SourceFile.t: this
simplifies tracking locations during translation.
Reviewed By: skcho
Differential Revision: D28873581
fbshipit-source-id: 09171f9bf
Summary: The `Utils` module has another version of try-finally than can swallow exceptions from the restart scheduler. This diff fixes that, but has to also fix the dependency cycle introduced between `SymOp` and `Utils`. This is done by extracting the exception handling from `SymOp` into a new base module `Exception`.
Reviewed By: jvillard
Differential Revision: D28930082
fbshipit-source-id: 7894d8c89
Summary:
We were defaulting to treating some values as strings regardless of
their intended types. In particular, for options that take no arguments
this was making it impossible to specify them in .inferconfig. Now they
take "null" as argument.
Reviewed By: ezgicicek
Differential Revision: D28959484
fbshipit-source-id: 46327f3d3
Summary: Adapted the AST to reflect the official [documentation](https://erlang.org/doc/apps/erts/absform.html): a clause can have a *guard sequence*, which is a list of *guards*. Each *guard* is a list of *guard tests*. A *guard test* is a (restricted) expression. Therefore, in the AST we now have `guard_test = expression`, and a clause has `guard_test list list`. Furthermore, the JSON translator simply applies `to_expression` to the 2D list, instead of flattening one level with `andalso`. Using `andalso` to flatten was wrong because it handles exceptions differently, see [note](https://learnyousomeerlang.com/syntax-in-functions#guards-guards).
Reviewed By: rgrig
Differential Revision: D28936558
fbshipit-source-id: 510930998
Summary:
- Break down large and deeply nested report function for readability
- Move callees closer to callers
Reviewed By: skcho
Differential Revision: D28873391
fbshipit-source-id: fc3f22708
Summary:
Make the biabduction machinery for detecting (biabduction) exceptions that can be swallowed recognise the one thrown by the restart scheduler.
The dependency hierarchy requires declaring that exception in `base`.
Reviewed By: jvillard
Differential Revision: D28773898
fbshipit-source-id: 2136346da
Summary:
Translates case_clause, ignoring guards for now.
Also, refactored a bit `translate_one_function`, because trying cases
one by one in sequence is similar to trying argument patterns one by
one in sequence.
Also, changed the strategy for storing the value of an (Erlang)
expression: instead of letting the translation function decide where to
put the result and return their choice (in a `block`), the choice is
done at a higher level and passed down in the environment. (This means,
for example, theres no need to copy the result of each case_clause to
the special `return` variable of the function.)
Reviewed By: skcho
Differential Revision: D28834672
fbshipit-source-id: d5d33be5f
Summary: Die loudly if DB to merge is not accessible, before even getting to the Sqlite statement.
Reviewed By: skcho
Differential Revision: D28872539
fbshipit-source-id: af38edd9a
Summary:
`Initializer` is used (mostly by Nullsafe) to signal that a method will only be run from/as a constructor, even if public.
RacerD should recognise this annotation; this diff makes RacerD treat methods annotated as `Initializer` like constructors with regards to the ownership of the receiver object.
Reviewed By: skcho
Differential Revision: D28748068
fbshipit-source-id: 5dd060865
Summary: The number of times summaries get overwritten is useful as a measure of wasted work. Use a hashtable to remember how many times each procedure is overwritten. Use integers as the keys to avoid wasting memory, with the keys set to the string hashes of the procedure UID.
Reviewed By: ezgicicek
Differential Revision: D28794862
fbshipit-source-id: b2737ab23
Summary: Sometimes the type definition is missing, especially in not-well supported build systems. When missing, default to true if on Java since you'd have to explicitly select a non-recursive one on purpose, and vice versa for clang.
Reviewed By: jvillard
Differential Revision: D28745172
fbshipit-source-id: 8b0e26e1a
Summary:
This handles function definitions. (Expressions, aka function bodies,
are not translated.)
Reviewed By: skcho
Differential Revision: D28606721
fbshipit-source-id: 0fd5dc57e
Summary:
Some funky C++ way of calling the parent's constructor triggers a
function in the frontend that used to reverse the order of parameters.
Reviewed By: skcho
Differential Revision: D28832500
fbshipit-source-id: 1032de2ca
Summary:
Unknown functions may create false positives as well as false negatives
for Pulse. Let's consider that unknown functions behave "functionally",
or at least that a functional behaviour is a possible behaviour for
them: when called with the same parameter values, they should return the
same value.
This is implemented purely in the arithmetic domain by recording
`v_return = f_unknown(v1, v2, ..., vN)` for each call to unknown
functions `f_unknown` with values `v1`, `v2`, ..., `vN` (and return
`v_return`). The hope is that this will create more false negatives than
false positives, as several FPs have been observed on real code that
would be suppressed with this heuristic.
The other effect this has on reports is to record hypotheses made on the
return values of unknown functions into the "pruned" part of formulas,
which inhibits reporting on paths whose feasibility depends on the
return value of unknown functions (by making these issues latent
instead). This should allow us to control the amount of FPs until we
model more functions.
Reviewed By: skcho
Differential Revision: D27798275
fbshipit-source-id: d31cfb8b6
Summary: Auditing exception handling so as to remove catch-all clauses that could potentially swallow exceptions from the restart scheduler.
Reviewed By: jvillard
Differential Revision: D28772739
fbshipit-source-id: 99a8d516d
Summary:
This complements a bug fix for InferSharp in which we weren't handling type-checking correctly.
Pull Request resolved: https://github.com/facebook/infer/pull/1447
Reviewed By: ngorogiannis
Differential Revision: D28772069
Pulled By: jvillard
fbshipit-source-id: be0210836
Summary:
Hi.
Thanks for the great tool!
This is just a simple correction for string literals in the `Util` module.
Pull Request resolved: https://github.com/facebook/infer/pull/1448
Reviewed By: ngorogiannis
Differential Revision: D28772080
Pulled By: jvillard
fbshipit-source-id: 36e2145c9
Summary:
This seems needed in some cases, might as well provide the option since
Infer is supposed to work with or without it.
Reviewed By: ngorogiannis
Differential Revision: D28712272
fbshipit-source-id: 35c0708f2
Summary: This diff prints cost and config impact checkers' json reports only for the changed files.
Reviewed By: ezgicicek
Differential Revision: D28707192
fbshipit-source-id: d949771f2
Summary: Litho (https://fblitho.com/) does some operations in the background. Add RacerD messaging specific to Litho.
Reviewed By: ezgicicek
Differential Revision: D28675504
fbshipit-source-id: e76f9f538
Summary: Same as MustBeValid: we want to report the first error on the path.
Reviewed By: skcho
Differential Revision: D28674724
fbshipit-source-id: a2ac04b5b
Summary:
Add a new `PathContext.t` component to the abstract state. For now it
tracks only the current "timestamp" of symbolic execution inside the
procedure, i.e. which step of symbolic execution we are in (bumped by 1
each time we've executed one instruction). In the future this will also
hold, eg, which conditionals we've been through on the path (for
reporting traces with that information).
Most of the diff is about propagating the path context through many of
the APIs.
We use timestamps only in `MustBeValid` attributes to report the first
incorrect access in a function call for now.
Reviewed By: skcho
Differential Revision: D28674726
fbshipit-source-id: 2cd825e73
Summary:
It's better to remember the first reason why an address must be valid,
etc.
Reviewed By: skcho
Differential Revision: D28674729
fbshipit-source-id: 3b69de7ef
Summary:
Spoiler alert: we don't. The next diffs fix that.
When there are several invalid accesses to report at a function call
instruction, we want to report the first one to occur within the
function. This is to avoid confusing reports where pulse reports, eg, a
null dereference for a pointer at a point where it's already been
dereferenced before in the same function.
Reviewed By: skcho
Differential Revision: D28674730
fbshipit-source-id: acb029e4b
Summary:
That was just broken before, but apparently nothing cared. It's needed
for the next diffs.
Reviewed By: skcho
Differential Revision: D28674731
fbshipit-source-id: 2f080238b
Summary:
Each Erlang function now has a Procdesc in `results.db`. The
ProcAttributes record if a function is exported or not by using the
access Public or Private, respectively.
This adds also `ErlangTypeName`. We use a fixed set of "type names" for
the different types of values in Erlang (i.e., for Erlang's "dynamic types").
Reviewed By: jvillard
Differential Revision: D28385954
fbshipit-source-id: f8278505a
Summary:
Update the website to include nil related issues for objective-c
Also fixed a typo + syntax highlighting
Reviewed By: jvillard
Differential Revision: D28638621
fbshipit-source-id: 148f2dd3f
Summary:
This is needed for the next diff. It was a bit annoying to report leaks
in two different places, now it's just in one.
Reviewed By: skcho
Differential Revision: D28576768
fbshipit-source-id: 4f23b43cb
Summary:
Add an option for realloc and fiddle with the other options' help for
consistency.
Moved the memory leak test to memory_leak.c and added more.
Moved the place where we take the options into account closer to their
corresponding models to defend a bit against modifying one without
modifying the other.
Reviewed By: da319
Differential Revision: D28543340
fbshipit-source-id: 75894d06d
Summary:
Let's model all the dynamic memory management functions as they all work
together and are important for a lot of C projects.
Reviewed By: ezgicicek
Differential Revision: D28543008
fbshipit-source-id: f130e1ab6
Summary: We even have matchers in PulseModels that can do the same thing.
Reviewed By: skcho
Differential Revision: D28540278
fbshipit-source-id: 4bfd8a13e
Summary:
It's unclear whether this can happen but it doesn't cost much to do a
last check before reporting an error to the user.
Reviewed By: skcho
Differential Revision: D28382670
fbshipit-source-id: e23f07ebd
Summary:
This fixes a memory leak false positive. When collecting unreachable
values we should be careful to take the equality relation into account.
Equal values are normally canonicalised but only with respect to "known"
equalities. This makes sure variables that are live thanks to the
"pruned" equalities are not discarded from the state.
Reviewed By: skcho
Differential Revision: D28382642
fbshipit-source-id: 2b898d754
Summary:
This makes reports more readable: they were all at the end of functions,
currently.
This is actually quite tricky to do as it involves detecting which
locations are unreachable.
Some of this logic can/should probably be shared with
`AbductiveDomain.discard_unreachable` but at the moment that's not the
case.
Reviewed By: skcho
Differential Revision: D28382590
fbshipit-source-id: bd4239a0c
Summary:
Hi all,
This is just a small fix tries to resolve the leaked type lost issue. It was excluded from previous CIL race condition PR due to irrelevance.
Thanks!
Pull Request resolved: https://github.com/facebook/infer/pull/1446
Reviewed By: skcho
Differential Revision: D28566755
Pulled By: ngorogiannis
fbshipit-source-id: 1c9938d9c
Summary: For Remodel-generated class (https://github.com/facebook/remodel), their properties are stored/loaded at internal fields named "_<property name>". This diff prepends "_" to property names when writing field info to the type environment when the field is of Remodel-generated class.
Reviewed By: ezgicicek
Differential Revision: D28541495
fbshipit-source-id: d0a1e5a4f
Summary: An anonymous class name includes an index number, for example `AnonymousClass$2` represents that it is the 2nd anonymous class implemented in the `AnonymousClass` class. Problem is that when we insert a new anonymous class, all index of anonymous class names below increase by one, which introduces incorrect comparison on reportdiff.
Reviewed By: ezgicicek
Differential Revision: D28568753
fbshipit-source-id: 2a6c576eb
Summary: Objective-C dispatch methods are not specialized, but have a special case during symbolic execution in biabduction. Reuse the same approach for Pulse: retrieve the given block name and its arguments and call it.
Reviewed By: skcho
Differential Revision: D28550468
fbshipit-source-id: 5017bb71e
Jules Villard
Daiva Naudziuniene
Akos Hajdu
Nikos Gorogiannis
Radu Grigore
Sungkeun Cho
Matteo Marescotti
Loc Le
Ezgi Çiçek
Xiaoyu Liu
Matthew Jin
Sangwoo Joh