Summary:
We use `procedure_name` which is coming from `Procname.get_method` in explanation of cost issues. For blocks, procedure name includes a prefix `objc_block` and a suffix with `_x` where x is the block counter. However, displaying this name to the user is not pretty. Especially when we have nested blocks, procedure name looks like `objc_blockobjc_blockdirectUIMessageFromContentAndMetadata_10_23`.
This diff drops the block index suffix and replaces `objc_block` with a prettier version `^`(signifying block).
so instead, in the cost report, we will have `^^blockdirectUIMessageFromContentAndMetadata`.
Reviewed By: skcho
Differential Revision: D26945333
fbshipit-source-id: 9d135423c
Summary: We shouldn't report a complexity increase here because `existing_block_here` is a removed function (that doesn't exist in current version)!
Reviewed By: skcho
Differential Revision: D26947439
fbshipit-source-id: 6620804be
Summary:
This diff runs `infer reportdiff` on config impact results, ie previous and current
`config-impact-report.json`s. Ungated and added/removed callees are reported at `introduced.json`.
Reviewed By: ezgicicek
Differential Revision: D26723054
fbshipit-source-id: efabd0d5f
Summary:
Now that the buck java flavour is fully deployed, the genrule-based integrations for java can be removed. We also remove the combined (clang+java) integration as this will be reimplemented using flavours in the future.
Also, remove a bunch of deprecated arguments linked to these integrations.
Reviewed By: jvillard
Differential Revision: D26104384
fbshipit-source-id: 6b0059407
Summary: using 'buck clean' rather than 'rm -rf buck-out' makes buck happier, apply to all buck integration tests
Reviewed By: ngorogiannis
Differential Revision: D25558469
fbshipit-source-id: 6c07341d6
Summary:
ndkbuild builds for all supported targets by default, giving errors
for clangs that doesn't support MIPS arch (which isn't relevant for this test).
Reviewed By: da319
Differential Revision: D25533986
fbshipit-source-id: 25c6001ce
Summary:
On centos8 devservers, this test failed on bizarre buck-out/tmp java.nio.file.NoSuch
FileException. I can't tell exactly what going on with rm -rf buck-out, but my guess would be that it puts the running buckd in a bad state.
using 'buck clean' rather than 'rm -rf buck-out' makes buck happier
Reviewed By: jvillard
Differential Revision: D25534471
fbshipit-source-id: 215f993e3
Summary:
Developers complain when a function that used to only throw an exception has complexity increase in the updated revision. Let's suppress such issues by giving those functions 0 cost which is already suppressed by differential reporting.
One common case to the above throw pattern is Java methods that throw an unsupported implementation exception for a functionality that has not been implemented yet. When the developer adds the supported implementation, we don't want to warn them with complexity increase since they are adding new functionality.
This is a design choice/heuristic to prevent noisy results for now.
Reviewed By: skcho
Differential Revision: D25495151
fbshipit-source-id: 94a82b062
Summary: In Java, public class name should be the same to file name.
Reviewed By: ezgicicek
Differential Revision: D25245194
fbshipit-source-id: 49fd16748
Summary:
The current source parser is based on ocamllex only.
In order to track field declaration locations, we propose a
new parser using ocamllex/menhir. This is a more ambitious
project that closely follows the official Java syntax.
Reviewed By: jvillard
Differential Revision: D24858280
fbshipit-source-id: 22d6766e5
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:
This diff adds an option hiding function pointers in costs to users: `cost-suppress-func-ptr` is
true by default.
Reviewed By: ezgicicek
Differential Revision: D24448212
fbshipit-source-id: 88f6b5ea1
Summary:
Testing procedure for java source parser
- we can run directly the parser without compiling and analysing the source file
- we add a test file
Reviewed By: ngorogiannis
Differential Revision: D23705199
fbshipit-source-id: 2103c1681
Summary:
Upgrade to latest clang release, needed for xcode12.
clang-8/9 won't be able to read the Xcode 12 SDK since there's annotations that will fail compilation.
Also removing unused (and hard to compile) binary `ast_exporter_bin` from facebook-clang-plugins/libtooling.
Reviewed By: ngorogiannis
Differential Revision: D23780089
fbshipit-source-id: 2314125a9
Summary: Also, fix some leftover meaningless logging, enable all cost issue types on diff testing so that we can see the unreachable cost issues
Reviewed By: skcho
Differential Revision: D23784563
fbshipit-source-id: 1b4a2b582
Summary:
Following the previous diff, the reason we mistakingly introduced new block as complexity increases is because when we are computing the hash of block, we return the string `block` to compute the hash for all block.
This diff fixes this case by returning the non-verbose name of the block when returning hashable name.
Reviewed By: ezgicicek
Differential Revision: D23729710
fbshipit-source-id: a345d3045
Summary:
When inspecting silent introduced issues, we found that an introduced issue is about a complexity increase of a block that is only created in the current diff. Based on the trace view, we find out that this is caused by infer mistakingly consider another block that exists in the previous diff as the same block that is newly created in the current diff.
This diff adds a test case that reproduces this case, and this will be fixed in the next diff.
facebook
Trace view: https://www.internalfb.com/intern/traceview/?id=109896337
Reviewed By: ezgicicek
Differential Revision: D23681550
fbshipit-source-id: 78341268b
Summary:
For complexity issues from O(m) to O(n), we only include the trace of the current complexity O(n). However, this makes it difficult to understand what the original complexity O(m) was. Especially in fixed issues where n=1, we only get a constant cost with no trace attached, so it is difficult to see how the symbol m disappeared.
This diff includes the traces for the previous cost in the cost issues.
Reviewed By: skcho
Differential Revision: D23680360
fbshipit-source-id: 3f2b21b20
Summary: Polynomial category zero corresponds to unreachable but "zero" is a misnomer and rather confusing. Let's fix it.
Reviewed By: skcho
Differential Revision: D23597735
fbshipit-source-id: f0c96ed26
Summary:
This diff extends the cost_item json format to print the autoreleasepool_size field. Not yet, there
is no semantics for that code kind, so the results will always be zero with no traces.
Reviewed By: ezgicicek
Differential Revision: D23540665
fbshipit-source-id: 94442e376
Summary: There is still a bug in using marshalled values as keys (exposed up the diff stack) where structurally equal procnames get serialised to distinct keys. This diff addresses that bug by using the existing functionality for string unique IDs.
Reviewed By: ezgicicek
Differential Revision: D23133761
fbshipit-source-id: 3cbafb51b
Summary:
This diff finishes the migration from the specialization of methods that take blocks as arguments. Here we delete all the old code and change the way we model dispatch functions so that the tests pass.
- Remove the code for specializing the methods in biabduction.
- Remove the call flags `cf_with_block_parameters` that was only used in this algorithm.
- Removes models for dispatch functions.
- Adds models for dispatch functions as program transformation only in biabduction. To be added in other checkers in the future.
Reviewed By: ngorogiannis
Differential Revision: D23345342
fbshipit-source-id: b5e8542ce
Summary:
As preparation for splitting summaries into some of their components, and then iterating over only those when reporting (thus gaining performance) we need procnames in the table.
Also, this fixes the now-broken use case of `infer report` with spec files, by using `--procedures-filter` to restrict printing of summaries accordingly.
Reviewed By: skcho
Differential Revision: D23101853
fbshipit-source-id: 1ae878d8e
Summary:
`java_type` aka `JavaSplitName.t` is a pair of strings. It's used in a procname to store the return type. Replace that with `Typ.t`.
This is step one of deleting `JavaSplitName`. Step two will be to replace parameters with `Typ.t`.
Reviewed By: skcho
Differential Revision: D20323748
fbshipit-source-id: f0029c3ca
Summary: New, experimental for now, integration with buck on Java using the `#infer-java-capture` flavor.
Reviewed By: artempyanykh
Differential Revision: D22187748
fbshipit-source-id: 62cdafe6b
Summary: This diff prevents printing line numbers of loop in the trace description, which helps to keep the same descriptions even when the line number of a function is changed in tests.
Reviewed By: ezgicicek
Differential Revision: D22375584
fbshipit-source-id: 676d1a7cc
Summary: This linters were not used much anymore, so we can delete them.
Reviewed By: ngorogiannis
Differential Revision: D22233895
fbshipit-source-id: f31180a05
Summary:
Move the implementation of implicit getters and setters from the biabduction to the clang frontend so these methods are accessible to all the checkers.
*Background*: In Objective-C when properties are created in the interface of a class, the compiler creates automatically the instance variable for it and also the getter and setter in the implementation of the class. In the frontend we collect the information about which method is the implicit getter and setter of which instance variable (we get the method declaration but not the implementation), and here we add the implicit implementation.
Reviewed By: skcho
Differential Revision: D22187238
fbshipit-source-id: 76e0508ed
Summary:
A bug in docusaurus makes relative URLs fail depending on how the page
was accessed, because the URL of a page in docs/ will end in / if
accessed directly or via hyperlink, but that / will be omitted when
clicking on the sidebar. The final / makes all the difference when
interpreting relative URLs so relative URLs are essentially broken.
See https://github.com/facebook/docusaurus/issues/2832 for more details.
This changes URL generation to generate URLs /docs/next/..., and
manually substitute relative URLs that had been written by hand.
Also fix a few other things about outdated links/comments.
Finally, `make doc-publish`.
Reviewed By: dulmarod
Differential Revision: D22117187
fbshipit-source-id: 32e2ba7e1
Summary: The new memory leaks analysis is now ready to be enabled by default and turned on in production. This also replaces the biabduction one which is now disabled.
Reviewed By: jvillard
Differential Revision: D21998666
fbshipit-source-id: 9cd95e894
Summary:
Turns out it was useful, so it is now reborn in OCaml.
Fixes https://github.com/facebook/infer/issues/1262.
Reviewed By: skcho
Differential Revision: D22016185
fbshipit-source-id: 31ccb7540
Summary:
This diff gives an order on running `test1`, `test2`, and `test3`. If they run parallel, they may
have a data race on writing `infer-out/.infer_runstate.json`.
Another minor fix is the object file path to remove.
Reviewed By: jvillard
Differential Revision: D21995671
fbshipit-source-id: eb9950cae
Summary:
There was an issue on `make build_ant_test` that newly generated `issues.exp.test` is empty. When
infer runs with `-- ant`, if there is already built objects in `ant_out`, infer analyzes nothing,
which result in the empty `issues.exp.test`. For example,
```
~/infer/infer/tests/build_systems/ant$ make test
[11:26:49][36924] Testing ant integration...
[ 2s][36924] SUCCESS Testing ant integration
~/infer/infer/tests/build_systems/ant$ touch `which infer`
```
The makefile tries to reanalyze ant targets because `infer` binary has been changed. However, there
is nothing to build/analyze, since `ant_out` still exists as the same.
```
~/infer/infer/tests/build_systems/ant$ make test
[11:26:57][37078] Testing ant integration...
[ 1s][37078] SUCCESS Testing ant integration
diff --git a/Users/scho/infer/infer/tests/build_systems/ant/issues.exp b/Users/scho/infer/infer/tests/build_systems/ant/issues.exp.test
index 376146b5f..e69de29bb 100644
--- a/Users/scho/infer/infer/tests/build_systems/ant/issues.exp
+++ b/Users/scho/infer/infer/tests/build_systems/ant/issues.exp.test
@@ -1 +0,0 @@
[-build_systems/ant/src/Hello.java, Hello.test():int, 2, NULL_DEREFERENCE, no_bucket, ERROR, [start of procedure test()]-]
Test output (build_systems/ant/issues.exp.test) differs from expected test output build_systems/ant/issues.exp
Run the following command to replace the expected test output with the new output:
make -C build_systems/ant replace
make: *** [test] Error 1
```
To handle this issue, this cleans `ant_out` before running infer.
Reviewed By: jvillard
Differential Revision: D21950916
fbshipit-source-id: f57056f6e
Summary:
A buck integration for capturing simultaneously clang and java targets.
Just like the java-specific `JavaGenruleCapture` integration, it relies on
dummy targets that depend on the flavoured clang versions.
For example, a `cxx_library` target named `//clang:hello` will have an associated target
called `//clang:hello_infer` that depends on `//clang:hello#infer-capture-all`,
and whose output is a text file containing the output path of the dependency.
Reviewed By: jvillard
Differential Revision: D21620458
fbshipit-source-id: 23919387b
Summary:
We stopped relying on an external perf data file to determine which functions are on the cold start. Let's remove this issue now.
NB: Keeping the `--perf-profiler-data-file` as deprecated to prevent issues on the CI and prod.
Reviewed By: skcho
Differential Revision: D21594150
fbshipit-source-id: faa58782d
Summary:
This function had been computing the name for ObjC methods wrong, with only the class name. This was causing wrong error messages in Pulse.
The main issue was that `Procname.to_simplified_string` was writing `Classname::methodname` for ObjC methods, which is not the convention. This confused the `hashable_name` funtion. So changing the method name to `Classname.methodname` which is more standard, and this also fixes `hashable_name`.
Reviewed By: ngorogiannis, jvillard
Differential Revision: D21570880
fbshipit-source-id: 13ed62cf8
Summary:
It suppresses cost reports of access methods, anonymous class methods, and auto-generated methods.
For those methods, this diff blocks reporting both expensive
issue (`EXPENSIVE_COLD_START/_UI_THREAD`) and complexity increase issue (`COMPLEXITY_INCREASE`).
Reviewed By: jvillard
Differential Revision: D21303068
fbshipit-source-id: 1c9533956
Summary:
The directory names had some interesting variety due to historical
reasons.
- {c,cpp,objc,objcpp}/errors/ date from the time when infer was only
biabduction
- java/infer/ dates from the time when we had an "--analyzer" option and
"infer" was one of them (sic), and eg another was "eradicate".
- c/biabduction/ dates from the time when the biabduction analysis was
being migrated to the "checkers" (AI) framework. For some reasons the
tests there are not a subset of c/infer/ but seem to be entirely new
tests.
The convention now dictates that we should name all of these
*/biabduction/. This diff moves the existing tests from c/biabduction/
into c/biabduction/misc/.
Reviewed By: mityal
Differential Revision: D21300147
fbshipit-source-id: 516d1cb15