Summary:
In all other places, we index params from 0, but accidentally recorded
the wrong number in json. It was because of the confusion between index
and user-visible param position that we show for the user message.
This diff fixes it: now we use 0-based indices internally (but of course still
report 1-based ones in the error message).
Reviewed By: artempyanykh
Differential Revision: D24916878
fbshipit-source-id: 45532c5ff
Summary:
Sometimes there are annotations that don't correspond to the user facing
code.
Previously we would fail, now process them gracefully.
Reviewed By: artempyanykh
Differential Revision: D24890895
fbshipit-source-id: e64a866ec
Summary:
Split the translation of return more aggressively between:
1. the instruction that has to happen before the translation of the sub-expr
2. the sub-expr
3. the instruction that has to happen after the sub-expr
This is needed for the next diff which creates potentially large CFGs in
(2).
Reviewed By: da319
Differential Revision: D24954071
fbshipit-source-id: a7e7e2527
Summary: Model `folly::Optional::get_pointer` which returns an address to a value if exists or `nullptr` if empty.
Reviewed By: jvillard
Differential Revision: D24935677
fbshipit-source-id: 9d990fe07
Summary:
We deliberately stopped as soon as an error was detected when applying a
function call. This is not good as other pre/posts of the function may
apply cleanly, which would allow us to cover more behaviours of the
code.
Went on a bit of a refactoring tangeant while fixing this, to clarify
the `Ok None`/`Ok Some _`/`Error _` datatype returned by PulseInterproc.
Now we report errors as soon as we find them during function calls but
continue accumulating specs afterwards.
Reviewed By: da319
Differential Revision: D24888768
fbshipit-source-id: d5f2c29d7
Summary:
- The SDK path is different for Objective C when command line tools for Xcode are installed without Xcode itself.
- Related issue https://github.com/facebook/infer/issues/1002.
I still have a problem with the mentioned issue. I think that it is possible that the SDK paths differs for Objective C on Mac OS when I have installed just command line tools for Xcode without Xcode itself. This change solves the issue for me. Is it a good solution? Could it be possibly added to Infer? Or is there any better way?
Pull Request resolved: https://github.com/facebook/infer/pull/1345
Reviewed By: ngorogiannis
Differential Revision: D24917636
Pulled By: jvillard
fbshipit-source-id: 534ce4b49
Summary: It is too easy to mix up multiple arguments of the same type.
Reviewed By: jvillard
Differential Revision: D24934116
fbshipit-source-id: 6e595b26e
Summary:
Global constants have reliable types, and their sizes can be used
instead of storing the size of the initializer separately.
Reviewed By: jvillard
Differential Revision: D24934114
fbshipit-source-id: 2426ab5be
Summary:
In practice this has not been observed to matter so far, since
treating `Splat N` as interpreted or uninterpreted does not matter
when `N` is a literal constant, and code seen so far only uses `Splat`
for zero initializers or memset with literal constant bytes.
Reviewed By: jvillard
Differential Revision: D24934118
fbshipit-source-id: 213e9724e
Summary:
optimize the git clone using --depth flag in term of size of clone
and also in term's of time taken to fetch the files and commit history
of whole repository .
More detail can be found at blog
https://www.atlassian.com/git/tutorials/big-repositories
Signed-off-by: Pratik Raj <rajpratik71@gmail.com>
Pull Request resolved: https://github.com/facebook/infer/pull/1341
Reviewed By: da319
Differential Revision: D24917645
Pulled By: jvillard
fbshipit-source-id: 6764fc971
Summary:
In the next diff we need to know when a destructor is needed for sure
before calling some of these auxiliary methods.
Reviewed By: da319
Differential Revision: D24832078
fbshipit-source-id: 6d4e17de2
Summary:
Communicate new facts from the arithmetic domain to the memory domain to
detect contradictions between the two.
Reviewed By: jberdine
Differential Revision: D24832079
fbshipit-source-id: 2caf8e9af
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
Summary:
0 and Splat 0 need to be treated the same since code relies on knowing
that 0 consists of all-0 bytes, and extracting a subsequence of a
Splat 0 yields 0. For example, initializing a struct to all-zeros and
then reading a member of pointer type out of it needs to produce the
null pointer. Therefore 0 and Splat 0 are redundant representations,
and all uses of Splat need to be updated to also handle 0.
This unfortunately leads to some near code duplication that seems to
be necessary. The issue is that 0 and Splat 0 are, from the backend's
perspective, constants in two distinct theories. Since 0 is chosen
over Splat 0 as the representation, the sequence theory solver needs
to treat 0 as if it was Splat 0, which duplicates some code handling
the general Splat cases.
Reviewed By: jvillard
Differential Revision: D24920758
fbshipit-source-id: 7c02be62b
Summary:
Global variables and function names in LLAIR are constant and so do
not need to be handled like normal assignable or shadowable
variables. This diff does this by changing the translation from LLAIR
to FOL to map globals and functions to uninterpreted constants instead
of variables.
Reviewed By: jvillard
Differential Revision: D24886571
fbshipit-source-id: efb8c9f49
Summary:
Localizing the entry of a procedure needs the globals (that the
procedure uses), but later creating a summary does not.
Reviewed By: jvillard
Differential Revision: D24886570
fbshipit-source-id: 8a7b18c58
Summary:
The computation of provable reachability through the heap currently
uses a set of variables whose values are either determined by the
desired roots or by the heap constraints. This requires globals to be
treated as variables. In preparation for distinguishing globals from
variables, this diff changes the reachability computation to use a set
of atomic terms instead of variables.
Reviewed By: jvillard
Differential Revision: D24886573
fbshipit-source-id: c0e6763b6
Summary:
No functional change, only simplifiying and making easier to
generalize.
Reviewed By: jvillard
Differential Revision: D24886572
fbshipit-source-id: e487b815d
Summary:
The general xlate_value function can be used in place of
xlate_func_name.
Reviewed By: jvillard
Differential Revision: D24846677
fbshipit-source-id: 603ebd388
Summary:
Calling an inline asm function that might raise is not currently
supported.
Reviewed By: jvillard
Differential Revision: D24846675
fbshipit-source-id: a7cfe6050
Summary:
Mangled names start with _Z, so only demangle those names. This leaves
unmangled C names alone.
Reviewed By: jvillard
Differential Revision: D24846674
fbshipit-source-id: d239a61aa
Summary:
Distinguish expressions that name globals from registers. This leads
to clearer code, and globals are semantically distinct from general
registers. In particular, they are constant, so any machinery for
handling assignment does not need to consider them. This diff only
adds the distinction to LLAIR, it is not pushed through to FOL, which
will come later.
Reviewed By: jvillard
Differential Revision: D24846676
fbshipit-source-id: 3aca025bf
Summary:
This module represents the definition of a global constant, rather
than the global itself.
Reviewed By: jvillard
Differential Revision: D24846673
fbshipit-source-id: d47e67984
Summary:
Distinguish expressions that name functions from registers. This leads
to clearer code, and function names are semantically distinct from
general registers. In particular, they are constant, so any machinery
for handling assignment does not need to consider them. Unlike general
globals, they never have initializer expressions, and in particular
not recursive initializers. This diff only adds the distinction to
LLAIR, it is not pushed through to FOL, which will come later.
Reviewed By: jvillard
Differential Revision: D24846672
fbshipit-source-id: 2101f353f
Summary:
LLVM and Llair use a form of records, in particular for values of
constant structs and arrays. In Llair, these use standard `select` and
`update` operations a la McCarthy's theory of functional arrays, with
a compact `record` operation for constructing complete records. This
is fine and logically well-understood. The issue is that once
constructed, these values are accessed using instructions that (may)
operate over byte-ranges, rather than struct member indices. The
backend uses a theory of sequences to represent such values (the
contents of memory). So some code depends on high fidelity
interoperation between these two views.
This diff resolves this by removing the record theory from the backend
and instead encoding them using the sequence theory. The approach
taken keeps records in Llair and translates them to sequences in
Llair_to_Fol. This choice is made since the encoding into the sequence
theory involves terms that do not have types that are expressible in
terms of the source types. In particular, `(update r i e)`, is encoded
as the concatenation of the prefix of `r` up to the offset of index
`i`, followed by `e` (possibly with padding), and then the suffix of
`r` from index `i+1` on. The prefix and suffix sequences do not
necessarily have source-expressible types.
Reviewed By: jvillard
Differential Revision: D24800866
fbshipit-source-id: e7238c558
Summary:
The support for recursive references to globals from within their
initializers is enough to handle all the cases of recursive structs
that have been encountered so far. Therefore this diff removes the
complication of recursive records entirely.
Reviewed By: jvillard
Differential Revision: D24772955
fbshipit-source-id: f59f06257
Summary:
It happens so seldomly that it is not worth it to optimistically
assume that linking will make opaque types sized. In particular, it is
incongruent for `Typ.is_sized` to hold and then `Typ.size_of` to
raise.
Reviewed By: jvillard
Differential Revision: D24772956
fbshipit-source-id: 96a72a5cf
Summary:
This information is needed to mediate between index-based
operations (such as on records) and offset-based operations (such as
load/store). Since it is fragile to recompute, the approach here is to
query llvm during translation and store the result.
Reviewed By: jvillard
Differential Revision: D24772954
fbshipit-source-id: ad22c3ecf
Summary:
If these failures are observed in real code of interest, they will
need to be resolved, so they are `todo` rather than an internal
`fail`ure.
Reviewed By: jvillard
Differential Revision: D24746222
fbshipit-source-id: 6b924be58
Summary:
While vector types can be translated, vector operations are not
currently handled. Translating type can lead to later failures (such
as bogus Convert expressions) that are not clearly identifiable as
originating from vector types. So fail eagerly when a vector type is
seen. Note that the only vectors that the frontend sees are due to
incompleteness of the LLVM vectorizer pass.
Reviewed By: jvillard
Differential Revision: D24746224
fbshipit-source-id: 30c0715eb
Summary: Do not fail when resolving the realpath of a debug info path.
Reviewed By: jvillard
Differential Revision: D24746237
fbshipit-source-id: b9dc35176
Summary:
Applicative command line argument interpretation fail: using map
instead of compose meant that the type of the ignored arg was the
action function.
Reviewed By: jvillard
Differential Revision: D24746226
fbshipit-source-id: f1dd67067
Summary:
Change Arith.map to not descend through non-interpreted arithmetic
operators. For example, in `2×(x × y) + 3×z + 4`, `map ~f` will apply
`f` to the subterms `x × y` and `z`, but not `x` or `y`.
The logical notion of "subterm" that is needed by the solver does not
coincide with the representation. This is essentially due to not
"flattening" or "purifying" terms. That is, traditionally `x × y`
would not be permitted as an indeterminate of a polynomial. Instead, a
new variable would need to be introduced: `v = x × y` and then the
polynomial would be expressed as `2×v + 3×z + 4`. Taking maximal
non-interpreted subterms as the definition of "subterm" results in
subterms in the non-flattened representation that are equivalent to
those that would result from flattening the representation.
Reviewed By: jvillard
Differential Revision: D24746235
fbshipit-source-id: d8fcf46a1
Summary:
The implementation of Arithmetic relies on the partial projection from
terms to polynomials. This diff enables it to also embed polynomials
back into terms.
Reviewed By: jvillard
Differential Revision: D24746223
fbshipit-source-id: b6010e7b7
Summary:
Add a distinction between interpreted and uninterpreted arithmetic
terms, and use it in Context.classify. This enables correctly
classifying non-linear terms such as `x × y` as uninterpreted.
Reviewed By: ngorogiannis
Differential Revision: D24746228
fbshipit-source-id: 1a4b0e3bd
Mitya Lyubarskiy
Jules Villard
Daiva Naudziuniene
Dominik Harmim
Josh Berdine
Pratik raj
Ezgi Çiçek