Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

junit: Compute a default for jazzer.instrument based on the class path #732

Merged
merged 3 commits into from
May 31, 2023
Merged

junit: Compute a default for jazzer.instrument based on the class path #732

merged 3 commits into from
May 31, 2023

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Apr 26, 2023

The new heuristic walks the classpath directory and considers all directories (but not jar files) to constitute the classes of the current project. All subdirectories that include .class files are translated into packages and all their subpackages are subject to instrumentation.

This is expected to give much better results than the previous heuristic, which only considered the first two or three segments of the test class package.

@fmeum
Copy link
Contributor Author

fmeum commented Apr 26, 2023

@florianGla Could you test whether this works for cifuzz? You can build your own version of jazzer-junit via bazel build //deploy:jazzer-junit.

@florianGla
Copy link
Contributor

I replaced jazzer-junit in my local maven repository, but I does not make any difference. I probably did something wrong though.

@fmeum
Copy link
Contributor Author

fmeum commented Apr 27, 2023

@florianGla Could you provide the verbose output of cifuzz? It would be interesting to see how exactly it executed Jazzer.

@florianGla
Copy link
Contributor

florianGla commented Apr 27, 2023

🔍 Command: NO_CIFUZZ='1' JAVA_HOME='/usr/lib/jvm/java-17-openjdk-amd64' RULES_JNI_TRACE='1' "/usr/lib/jvm/java-17-openjdk-amd64/bin/java" "-cp" "/home/flogla/.m2/repository/org/junit/jupiter/junit-jupiter-engine/5.9.2/junit-jupiter-engine-5.9.2.jar:/home/flogla/.m2/repository/org/junit/platform/junit-platform-engine/1.9.2/junit-platform-engine-1.9.2.jar:/home/flogla/.m2/repository/org/opentest4j/opentest4j/1.2.0/opentest4j-.2.0.jar:/home/flogla/.m2/repository/org/junit/platform/junit-platform-commons/1.9.2/junit-platform-commons-1.9.2.jar:/home/flogla/.m2/repository/org/junit/jupiter/junit-jupiter-api/5.9.2/junit-jupiter-api-5.9.2.jar:/home/flogla/.m2/repository/org/apiguardian/apiguardian-api/1.1.2/apiguardian-api-1.1.2.jar:/home/flogla/.m2/repository/com/code-intelligence/jazzer-junit/0.16.0/jazzer-ju
nit-0.16.0.jar:/home/flogla/.m2/repository/com/code-intelligence/jazzer-api/0.16.0/jazzer-api-0.16.0.jar:/home/flogla/.m2/repository/com/code-intelligence/jazzer/0.16.0/jazzer-0.16.0.jar:/home/flogla/.m2/repository/org/junit/jupiter/junit-jupiter-params/5.9.0/junit-jupite
r-params-5.9.0.jar:/home/flogla/.m2/repository/org/junit/platform/junit-platform-launcher/1.9.0/junit-platform-launcher-1.9.0.jar:/home/flogla/Code/cifuzz/examples/maven/target/classes:/home/flogla/Code/cifuzz/examples/maven/target/test-classes:/home/flogla/Code/cifuzz/examples/maven/src/main/resources:/home/flogla/Code/cifuzz/examples/maven/src/test/resources" "com.code_intelligence.jazzer.Jazzer" "--target_class=com.example.FuzzTestCase" "--target_method=" "-max_total_time=0" "-rss_limit_mb=3000"

I have replaced this jar with your version: /home/flogla/.m2/repository/com/code-intelligence/jazzer-junit/0.16.0/jazzer-ju
nit-0.16.0.jar

@fmeum
Copy link
Contributor Author

fmeum commented Apr 27, 2023

@florianGla Could you print the content of the jazzer.instrumentation_includes system property in your fuzz test? If you can share reproduction steps, I can also take a look.

@florianGla
Copy link
Contributor

2023-04-27_15-03
I just replaced the jazzer-junit.jar file in my ~/.m2/repository folder and ran the example maven project. The instrumentation includes look fine but external classes are still instrumented.

Copy link
Contributor

@bertschneider bertschneider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@bertschneider
Copy link
Contributor

@florianGla based on your logs you're using version 0.16.0. I'm not entirely sure if something important changed in the interaction between Jazzer and its JUnit integration, but could you try to use 0.16.1 and replace jazzer-junit in that version?

@florianGla
Copy link
Contributor

Unfortunately I get the same outputs. It is strange that the instrumentation includes are set, but seem to have no effect.

@fmeum
Copy link
Contributor Author

fmeum commented May 24, 2023

@florianGla I found a bug that caused both the old and the new heuristic to not affect the scope of instrumentation when running within cifuzz - I will submit a fix.

@fmeum fmeum requested a review from bertschneider May 25, 2023 14:04
@fmeum
Copy link
Contributor Author

fmeum commented May 25, 2023

@florianGla I pushed a fix, could you test this again?

@florianGla
Copy link
Contributor

@florianGla I pushed a fix, could you test this again?

Unfortunately it does not seem to be fixed.

@fmeum
Copy link
Contributor Author

fmeum commented May 25, 2023

@florianGla This was broken in yet another way. I added a full end-to-end test, so this should be the last time I ask you to try it out. Fingers crossed ;⁠-⁠)

@florianGla
Copy link
Contributor

@florianGla This was broken in yet another way. I added a full end-to-end test, so this should be the last time I ask you to try it out. Fingers crossed ;⁠-⁠)

It still does not seem to work. Maybe we can have a look together.

Copy link
Contributor

@bertschneider bertschneider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me. The failing test also works locally and don't seem to be flaky. Restarted it.

@fmeum
Copy link
Contributor Author

fmeum commented May 28, 2023

The test is flaky, but with an interesting failure:

java.lang.NullPointerException
	at java.lang.String.getBytes(String.java:940)
	at com.code_intelligence.jazzer.runtime.TraceDataFlowNativeCallbacks.encodeForLibFuzzer(TraceDataFlowNativeCallbacks.java:106)
	at com.code_intelligence.jazzer.runtime.TraceDataFlowNativeCallbacks.traceStrcmp(TraceDataFlowNativeCallbacks.java:41)
	at com.code_intelligence.jazzer.api.Jazzer.guideTowardsEquality(Jazzer.java:117)
	at com.code_intelligence.jazzer.sanitizers.RegexRoadblocks.nodeMatchHook(RegexRoadblocks.java:182)
	at java.util.regex.Pattern$Curly.match0(Pattern.java:4264)
        ...

I will submit a separate fix.

@florianGla Yes, let's look into it together next week.

@fmeum
Copy link
Contributor Author

fmeum commented May 30, 2023

Rebased onto #748

fmeum added 3 commits May 31, 2023 11:25
This reverts commit 4dfee6d and adds a
comment explaining why it wasn't a good idea.
The `Driver` accesses `Opt` settings and thus locked in their values
before the JUnit integration, specifically `AgentConfigurator`, had a
chance to update the instrumentation filter.

This is worked around by evaluating the instrumentation settings lazily,
with a more conceptual fix coming with the ongoing config overhaul.
The new heuristic walks the classpath directory and considers all
directories (but not jar files) to constitute the classes of the current
project. All subdirectories that include `.class` files are translated
into packages and all their subpackages are subject to instrumentation.

This is expected to give much better results than the previous
heuristic, which only considered the first two or three segments of the
test class package.
@fmeum
Copy link
Contributor Author

fmeum commented May 31, 2023

@florianGla I tested this on the cifuzz Maven example and the instrumentation filter is effective now.

@fmeum fmeum enabled auto-merge (rebase) May 31, 2023 09:37
@fmeum fmeum merged commit 6f5b3c5 into CodeIntelligenceTesting:main May 31, 2023
@fmeum fmeum deleted the CLI-941-instrumentation-default branch May 31, 2023 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants