Summary:
Currently cfg nodes are written into dot files in whatever order they
appear in a hash table. This seems unnecessarily sensitive, so this
diff sorts the nodes.
Reviewed By: dulmarod
Differential Revision: D4232377
fbshipit-source-id: a907cc6
Summary:
introduce `AttributesTable.load_defined_attributes` which will return proc attributes only if the procedure is defined. In order to not mess up
with existing caching, create another hashmap to store those procdescs.
We need to do that because with reactive capture we no longer can assume that all proc attributes are final before analysis starts
Reviewed By: jberdine
Differential Revision: D4231575
fbshipit-source-id: e795bcb
Summary: Pure refactoring simplifying the code doing the case analysis for execturing the cast instruction.
Reviewed By: dulmarod
Differential Revision: D4215238
fbshipit-source-id: 9f0f163
Summary:
1. Always store cpp model source_file with relative path. This will make them cache friendly independent of infer location
2. Distinguish between "relative to project root" and "relative to infer models src"
3. Unify `source_file_from_path` used by java and C frontends into one function. There are no improvements to that logic yet
4. Move `is_cpp_model_file` to use `source_file` instead of `filename`
Reviewed By: jberdine
Differential Revision: D4204548
fbshipit-source-id: 6e21771
Summary: D4189956 killed the phantom space printed after types, but the Context leak message was relying on it :).
Reviewed By: jeremydubreil
Differential Revision: D4208591
fbshipit-source-id: 9f0d709
Summary: These are dangerous if you are trying to compare a type to a string, and they're also unsightly.
Reviewed By: jvillard
Differential Revision: D4189956
fbshipit-source-id: 14ce127
Summary:
SIOF is only for interactions between objects of non-POD types. Previously the
checker was also reporting for POD types.
Reviewed By: akotulski
Differential Revision: D4197620
fbshipit-source-id: 7c56571
Summary:
Record an abstraction of the bug traces in the tests. The abstraction of a
trace is the sequence of descriptions. In practice, descriptions are either
empty, or of the form "start/end/return from/call to procedure X". They seem
pretty stable.
Motivation: there is nothing testing the traces reported by Infer right now,
even though they are surfaced to developers. For instance, Quandary uses
--issues-txt instead of --issues-tests to make sure the traces do not regress.
This change would make this approach more widespread.
Reviewed By: sblackshear
Differential Revision: D4159597
fbshipit-source-id: 9c83952
Summary: The thread safety checker is run independently of other analyses, using the command "infer -a threadsafety -- <build-command>".
Reviewed By: sblackshear
Differential Revision: D4148553
fbshipit-source-id: bc7b3f9
Summary: When searching for cast errors, types that were not Java objects, e.g. arrays of primitive types were not taken into account, leading to incorrect class cast excpetion reports.
Reviewed By: sblackshear
Differential Revision: D4166184
fbshipit-source-id: 7157c95
Summary:
This adds generic support for reporting error traces as usual infer issues
traces (instead of putting them in the textual description of the error) to
Trace.ml and SinkTrace.ml.
The siof checker is made to use these new traces, and gets an improved error
message mentioning the name of the problematic global as well, which requires a
slight API change in Pvar.re.
The support in Trace.ml is incomplete: passthroughs are ignored. This missing
feature will be needed by Quandary to migrate its error messages.
Reviewed By: sblackshear
Differential Revision: D4159542
fbshipit-source-id: 8c1101d
Summary:
It was defined in two places and I'm about to add a third, so let's share
instead.
Reviewed By: sblackshear
Differential Revision: D4153420
fbshipit-source-id: 3d2c519
Summary:
Location.nLOC was introducing a lot of complexity for little benefit (and edge cases were wrong anyway).
We can restore it in some simplified way if we find that we need it
Reviewed By: jeremydubreil
Differential Revision: D4139868
fbshipit-source-id: 4f8e033
Summary:
Summaries are modified before saving from disk, for example the attributes of the postcondition can change.
I have observed flaky reports of the internal error NULL_TEST_AFTER_DEREFERENCE. Some attributes (e.g. assigned) are changed before saving, but the spec table in memory is not changed.
So in case:
1) the procedure is analyzed on-demand, then subsequent uses in the same process use the summary in memory with the unchanged attribute, and the issue is not reported.
2) the procedure is already on disk and loaded, then the loaded summary has the changed attributes, and the issue is reported.
Flakiness happens as because of parallelism, whether a procedure is analyzed already or whether it is analyzed on-demand, can change.
The normalization function can change the instrumentation of a symbolic heap because it uses the existing comparison functions, which ignore instrumentations.
So normalization can replace part of a symbolic heap with an identical one but where the instrumentation is different — this is what I have observed.
The diff uses a different comparison function where instrumentations are taken into account.
Reviewed By: jberdine
Differential Revision: D4140031
fbshipit-source-id: f4f119a
Summary: Don't use a hardcoded string, and enable reports in --issues-tests.
Reviewed By: jvillard
Differential Revision: D4110731
fbshipit-source-id: 9922557
Summary:
this makes frontends no longer depend on SymExec.ml. `ModelBuiltins` was split into two modules:
- `BuiltinDecl` with procnames for builtins (used to determine whether some function is a builtin)
- `BuiltinDefn` with implementations used by `SymExec`
- they both have similar type defined in `BUILTINS.S` which makes sure that new builtin gets added into both modules.
During the refactor I ran some scripts:
`BuiltinDecl.ml`:
let X = create_procname "X"
cat BuiltinDecl.ml | grep "create_procname" | tail -70 | awk ' { print $1,$2,$3,$4,"\42"$2"\42"} '
then manually confirm string match. Exceptions:
"__exit" -> "_exit"
"objc_cpp_throw" -> "__infer_objc_cpp_throw"
__objc_dictionary_literal
nsArray_arrayWithObjects
nsArray_arrayWithObjectsCount
`BuiltinDefn.ml`:
let X = Builtin.register BuiltinDecl.X execute_X
cat BuiltinDecl.ml | grep "create_procname" | tail -70 | awk ' { print $1,$2,$3,"Builtin.register BuiltinDecl."$2,"execute_"$2} '
then, fix all compilation problems
Reviewed By: jberdine
Differential Revision: D3951035
fbshipit-source-id: f059602
Summary:
- do a semantic analysis of each variable initializer to figure out if they need initialization
- add a flag to globals that is true when they are `constexpr`. In that case, no analysis is needed as the user + compile guarantee that it is a compile-time constant.
Reviewed By: sblackshear
Differential Revision: D4081273
fbshipit-source-id: 44dbe29
Summary:
Merging the results directories of targets on buck projects involved creating symbolic links into buck-out.
The bulk of files are .attr files: one per procedure. Creating these links can be a bottleneck, and the merge phase can be slower than the analysis phases on projects with many procedures.
This diff introduces multilinks to speed up merge.
A multilink is a file `multilink.txt` containing a sequence of paths
```
path/to/file1.ext
path/to/file2.ext
...
```
A multilink file is a compact way to represent a link for each entry.
This diff creates a multilink file for each `attributes/dir` directory, instead of one symbolic link for each file.
Reviewed By: jberdine
Differential Revision: D4067428
fbshipit-source-id: 911f8a9
Summary:
Declared and defined procedure attributes are now saved in different files (hashed_name.decl.attr and hashname.attr).
We always try to load using the filename of defined procedure attributes first,
and fall back to loading the file for declared ones if it does not exist.
The logic for replacing an existing file stays the same, with one extra thing:
when a file for a defined attribute is written, the one for the declared one
is deleted if it exists.
At the end of a capture, either a declared or a defined file exist, but not both.
The reason for this change is that when captures of different subprojects are
merged together, it can happen that a link gets created to a declared attributes
file even though a defined one exists, so the body of the procedure will not be analyzed.
After this diff, both links will be created, and the defined one will be loaded
by the back-end.
Reviewed By: dulmarod
Differential Revision: D4037423
fbshipit-source-id: 74fb7e6
Summary:
Checker for the Static Initialization Order Fiasco pattern:
https://isocpp.org/wiki/faq/ctors#static-init-order
1. Collect all globals (transitively) accessed in any given procedure.
2. Once the interprocedural analysis has finished, look at globals accessed in
initializers that do not belong to the current translation unit.
Reviewed By: sblackshear
Differential Revision: D3780266
fbshipit-source-id: 1d07161
Summary:
Create dummy functions representing the initializers of global variables. This
is so we can implement checks in the backend that can look at the initializer
expressions of global variables. We try not to create these dummy functions
when the initializer is not present, although for some reason we sometimes end
up with empty initializers.
Also add source file info to global variables in the backend (Pvar.re).
Reviewed By: sblackshear
Differential Revision: D3780238
fbshipit-source-id: 2dca87e
Summary: Nothing mutates those fields so there is no need to make them `mutable`
Reviewed By: cristianoc
Differential Revision: D4009166
fbshipit-source-id: b840a4b
Summary:
This diff removes the unused support for reporting props, which enables
refactoring so that the 'base' directory has no dependencies, and the
'IR' directory depends only on 'base'.
Reviewed By: jvillard
Differential Revision: D3981352
fbshipit-source-id: 3700a23
Summary: That data was never used and removing it can simplify frontends quite a bit.
Reviewed By: jberdine
Differential Revision: D3967389
fbshipit-source-id: d65c3da
Summary: The code has not much to do with IR and should be part of backend/ directory.
Reviewed By: sblackshear
Differential Revision: D3950834
fbshipit-source-id: 315ea19
Summary:
In order to have only InferJava depend on JBasics, do not use
JBasics.java_lang_object in the IR or backend. Note that this implies
that the Java frontend should ideally translate JBasics.java_lang_object
to Typename.Java.java_lang_Object.
Reviewed By: jeremydubreil
Differential Revision: D3956468
fbshipit-source-id: def64dd
Summary:
Change Sil.Call instruction to have only a single optional return
identifier, insted of a list. Essentially none of the code handled
multiple return identifiers. Also, add the type of the return
identitifier to Call instructions.
Reviewed By: sblackshear
Differential Revision: D3919358
fbshipit-source-id: d2d4f72
Summary:
Refactor Sil.struct_typ and associated operations into a separate
StructTyp module. This is possible now that Typ.Tstruct only carries a
type name instead of the definition directly, and is helpful to simplify
module dependencies.
Reviewed By: cristianoc
Differential Revision: D3919357
fbshipit-source-id: a37a656
Summary:
It is no longer necessary to keep the name of a struct within the
struct, as the name will just have been used to look it up.
Reviewed By: cristianoc
Differential Revision: D3919355
fbshipit-source-id: ab65168
Summary:
Pass the exe_env to checker cluster callbacks, and add it to the domain
extras for BoundedCallTree, and use the Exe_env instead of
AttributesTable to obtain the tenv.
Reviewed By: sblackshear
Differential Revision: D3921850
fbshipit-source-id: 9edf324
Summary: The Infer builtins can be used in the e2e tests, but those tests should not depend on the Infer models to avoid cyclic dependencies. This diff separates the models and the Infer builtins in two directories so that the test can depend on the builtins without depending on the models
Reviewed By: sblackshear
Differential Revision: D3929478
fbshipit-source-id: 7d0ab79
Summary:
The global reference `DB.current_source` is used internally in the module DB, by all the front-ends, and directly and indirectly by the back-end, including saving and restoring the state in case of on-demand procedure calls. In particular, it is heavily used in printing functions.
This diff cleans up the flow of information about what the current file is, making it explicit, and removes the reference.
Reviewed By: jberdine
Differential Revision: D3901247
fbshipit-source-id: ef596bd
Summary:
This diff removes the redundancy in the representation of types where
struct types could be represented either directly using Tstruct or
indirectly using Tvar to refer to the type environment. A consequence
is that it is much harder to construct large type values.
Reviewed By: sblackshear, cristianoc
Differential Revision: D3839753
fbshipit-source-id: cf04ea5
Summary:
Rename Typ.mk_struct to internal_mk_struct, and add Tenv.mk_struct that
ensures types are added to the environment under the right name.
Reviewed By: cristianoc
Differential Revision: D3791865
fbshipit-source-id: fd4b667
Summary:
The Typ.struct_typ.csu field is now redundant with the Csu.t in the
name: Typename.t field.
Reviewed By: cristianoc
Differential Revision: D3791861
fbshipit-source-id: 5370885
Summary: Replace the struct_name: Mangled.t option field of Typ.struct_typ with name: Typename.t
Reviewed By: sblackshear
Differential Revision: D3791860
fbshipit-source-id: 3ee1d00
Summary:
When a project defines the same symbol in different files,
the results of the capture will be different depending on
the order in which files are captured.
In particular, if a procedure with the same name is defined in different files,
it will be associated with the last file being captured.
So the results can be different if the files are processed in different order.
In case of parallel capture, this has in effect a non-deterministic behaviour.
This diff defines a canonical file (alphabetically) when a procedure is multiply
defined, so that the procedure is associated to the same file whatever the order.
Reviewed By: jeremydubreil
Differential Revision: D3805002
fbshipit-source-id: 2c561f4
Summary:
Infer doesn't go looking into field values when looking for unsigned
expressions, which could cause some unintended reports.
Reviewed By: sblackshear
Differential Revision: D3724232
fbshipit-source-id: 9c4cd97
Josh Berdine
Sam Blackshear
Andrzej Kotulski
Jeremy Dubreil
Ryan Rhee
Cristiano Calcagno
Jules Villard
Peter O'Hearn
Yunxing Dai
Dulma Churchill