facebook / infer

A static analyzer for Java, C, C++, and Objective-C
http://fbinfer.com/
MIT License
14.89k stars 2.01k forks source link

[java] add tests documenting the dependencies analysis mode #1679

Closed jeremydubreil closed 1 year ago

jeremydubreil commented 1 year ago

This pull request adds a test to document the --dependencies options triggering the analysis of the classes appearing in the Java classpath. This feature is especially useful when analysing JAR files packaging all the code of a Java application including the dependencies.

The examples added in the test are outlining how running the Pulse taint analysis with the dependencies mode enabled can eliminate cases of false negatives and cases of false positives.

All the tests are passing locally.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

jvillard commented 1 year ago

The macos tests on GitHub are failing: https://github.com/facebook/infer/actions/runs/3039326327/jobs/4894127613

[*ERROR**][82386] ./lib/Framework.java:30: error: cannot find symbol
[*ERROR**][82386]             String content = new String(inputStream.readAllBytes(), StandardCharsets.UTF_8);
[*ERROR**][82386]                                                    ^
[*ERROR**][82386]   symbol:   method readAllBytes()
[*ERROR**][82386]   location: variable inputStream of type FileInputStream

Maybe that method is not available in Java 8, can we just use another?

jeremydubreil commented 1 year ago

Yes, the choice of the method is arbitrary so we can use another one that is available with Java 8. However, I wonder independently if we should make sure that we are using the same version of the Java SDK on Linux and on macOS. We can discuss this on https://github.com/facebook/infer/pull/1680.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

jeremydubreil commented 1 year ago

@jvillard , this last version should be ready. I simplified the test code to avoid pulling in too many dependencies to run the analysis on. I was also trying to understand why the disable-issue-type was not working (it was because of the --debug-exceptions). Should be good now.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

jvillard commented 1 year ago

I get more errors in the test, and they are reported inside infer-out/ for some reason!

diff --git a/mnt/btrfs/trunk-git-infer-38-1665654773/infer/tests/codetoanalyze/java/dependencies/issues.exp b/mnt/btrfs/trunk-git-infer-38-1665654773/infer/tests/codetoanalyze/java/dependencies/issues.exp.test
index 65c0e8821f..485fb40455 100644
--- a/mnt/btrfs/trunk-git-infer-38-1665654773/infer/tests/codetoanalyze/java/dependencies/issues.exp
+++ b/mnt/btrfs/trunk-git-infer-38-1665654773/infer/tests/codetoanalyze/java/dependencies/issues.exp.test
@@ -1 +1,4 @@
{+codetoanalyze/java/dependencies/infer-out/classnames/java/util/stream/StreamOpFlag.class, java.util.stream.StreamOpFlag.isStreamFlag():boolean, 0, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Map.get()` (modelled),is assigned to the null pointer,assigned,in call to `cast` (modelled),invalid access occurs here]+}
{+codetoanalyze/java/dependencies/infer-out/classnames/java/util/stream/StreamOpFlag.class, java.util.stream.StreamOpFlag.canSet(java.util.stream.StreamOpFlag$Type):boolean, 0, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Map.get()` (modelled),is assigned to the null pointer,assigned,in call to `cast` (modelled),invalid access occurs here]+}
{+codetoanalyze/java/dependencies/infer-out/classnames/java/util/stream/StreamOpFlag.class, java.util.stream.StreamOpFlag.createMask(java.util.stream.StreamOpFlag$Type):int, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Map.get()` (modelled),is assigned to the null pointer,assigned,in call to `cast` (modelled),invalid access occurs here]+}
codetoanalyze/java/dependencies/my/Application.java, my.Application.indirectNPE():java.lang.String, 1, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Object Framework.returnNull()`,is assigned to the null pointer,returned,return from call to `Object Framework.returnNull()`,assigned,invalid access occurs here]

Test output (codetoanalyze/java/dependencies/issues.exp.test) differs from expected test output codetoanalyze/java/dependencies/issues.exp
Run the following command to replace the expected test output with the new output:

  make -C infer/tests/codetoanalyze/java/dependencies replace
jeremydubreil commented 1 year ago

@jvillard what version of Java is used to run the tests on the internal CI?

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

jvillard commented 1 year ago

@jeremydubreil Java 8

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

jvillard commented 1 year ago

The new tests are still failing on our end, although they succeeded in the GitHub CI:

[*ERROR**][146229] diff --git a/mnt/btrfs/trunk-git-infer-0-1666026411/infer/tests/codetoanalyze/java/dependencies/issues.exp b/mnt/btrfs/trunk-git-infer-0-1666026411/infer/tests/codetoanalyze/java/dependencies/issues.exp.test
[*ERROR**][146229] index 65c0e8821f..27dabd5833 100644
[*ERROR**][146229] --- a/mnt/btrfs/trunk-git-infer-0-1666026411/infer/tests/codetoanalyze/java/dependencies/issues.exp
[*ERROR**][146229] +++ b/mnt/btrfs/trunk-git-infer-0-1666026411/infer/tests/codetoanalyze/java/dependencies/issues.exp.test
[*ERROR**][146229] @@ -1 +1,4 @@
[*ERROR**][146229] {+/java/util/stream/StreamOpFlag.class, java.util.stream.StreamOpFlag.isStreamFlag():boolean, 0, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Map.get()` (modelled),is assigned to the null pointer,assigned,in call to `cast` (modelled),invalid access occurs here]+}
[*ERROR**][146229] {+/java/util/stream/StreamOpFlag.class, java.util.stream.StreamOpFlag.canSet(java.util.stream.StreamOpFlag$Type):boolean, 0, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Map.get()` (modelled),is assigned to the null pointer,assigned,in call to `cast` (modelled),invalid access occurs here]+}
[*ERROR**][146229] {+/java/util/stream/StreamOpFlag.class, java.util.stream.StreamOpFlag.createMask(java.util.stream.StreamOpFlag$Type):int, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Map.get()` (modelled),is assigned to the null pointer,assigned,in call to `cast` (modelled),invalid access occurs here]+}
[*ERROR**][146229] codetoanalyze/java/dependencies/my/Application.java, my.Application.indirectNPE():java.lang.String, 1, NULLPTR_DEREFERENCE, no_bucket, ERROR, [in call to `Object Framework.returnNull()`,is assigned to the null pointer,returned,return from call to `Object Framework.returnNull()`,assigned,invalid access occurs here]
[*ERROR**][146229] 
[*ERROR**][146229] Test output (codetoanalyze/java/dependencies/issues.exp.test) differs from expected test output codetoanalyze/java/dependencies/issues.exp
[*ERROR**][146229] Run the following command to replace the expected test output with the new output:
[*ERROR**][146229] 
[*ERROR**][146229]   make -C infer/tests/codetoanalyze/java/dependencies replace
facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

jeremydubreil commented 1 year ago

@jvillard what make the internal test failing? Is there something to change here?

jvillard commented 1 year ago

It's failing on Java 11 due to test changes in direct_java_dependencies_test. Maybe we can mark those java8-only?

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jeremydubreil has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 year ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

jvillard commented 1 year ago

Thank you @jeremydubreil!