RFR: 8337221: CompileFramework: test library to conveniently compile java and jasm sources for fuzzing
Emanuel Peter
epeter at openjdk.org
Thu Sep 12 15:57:28 UTC 2024
On Sun, 28 Jul 2024 16:25:43 GMT, Evgeny Nikitin <enikitin at openjdk.org> wrote:
>> **Motivation**
>>
>> I want to write small dedicated fuzzers:
>> - Generate `java` and `jasm` source code: just some `String`.
>> - Quickly compile it (with this framework).
>> - Execute the compiled code.
>>
>> The primary users of the CompileFramework are Compiler-Engineers. Imagine you are working on some optimization. You already have a list of **hand-written tests**, but you are worried that this does not give you good coverage. You also do not trust that an existing Fuzzer will catch your bugs (at least not fast enough). Hence, you want to **script-generate** a large list of tests. But where do you put this script? It would be nice if it was also checked in on git, so that others can modify and maintain the test easily. But with such a script, you can only generate a **static test**. In some cases that is good enough, but sometimes the list of all possible tests your script would generate is very very large. Too large. So you need to randomly sample some of the tests. At this point, it would be nice to generate different tests with every run: a "mini-fuzzer" or a **fuzzer dedicated to a compiler feature**.
>>
>> **The CompileFramework**
>>
>> Java sources are compiled with `javac`, jasm sources with `asmtools` that are delivered with `jtreg`.
>> An important factor: Integration with the IR-Framwrork (`TestFramework`): we want to be able to generate IR-rules for our tests.
>>
>> I implemented a first, simple version of the framework. I added some tests and examples.
>>
>> **Example**
>>
>>
>> CompileFramework comp = new CompileFramework();
>> comp.add(SourceCode.newJavaSourceCode("XYZ", "<source-code-here>"));
>> comp.compile();
>> comp.invoke("XYZ", "test", new Object[] {5}); // XYZ.test(5);
>>
>>
>> https://github.com/openjdk/jdk/blob/e869cce8092ee995cf2f3ad1ab2bca69c5e256ab/test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/SimpleJavaExample.java#L42-L74
>>
>> **Below some use cases: tests that would have been better with the CompileFramework**
>>
>> **Use case: test/hotspot/jtreg/compiler/loopopts/superword/TestAlignVectorFuzzer.java**
>>
>> I needed to test loops with various `init / stride / limit / scale / unrolling-factor / ...`.
>>
>> For this I used `MethodHandle constant = MethodHandles.constant(int.class, value);`,
>>
>> to be able to chose different values before the C2 compilation, and then the C2 compilation would see them as constants and optimize assuming those constants. This works, but is difficult to extract reproducers once something i...
>
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 28:
>
>> 26: import compiler.lib.compile_framework.SourceCode;
>> 27: import compiler.lib.compile_framework.CompileFrameworkException;
>> 28: import compiler.lib.compile_framework.InternalCompileFrameworkException;
>
> Not needed?
What do you mean? I have multiple uses in the file.
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 42:
>
>> 40: import java.util.List;
>> 41: import java.util.concurrent.TimeUnit;
>> 42: import jdk.test.lib.process.ProcessTools;
>
> 1. URI and Arrays are not used
> 2. imports list is unsorted.
thanks!
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 54:
>
>> 52: }
>> 53:
>> 54: public void printSourceCodes() {
>
> I'd decouple debug dumping from the stream it prints to. Not always we agree with filling the `stdout` with garbage.
> Something like this:
>
> public String sourceCodesAsString() {...}
> public Set<SourceCode> getSourceCodes() {...}
Ok, I'll make it dependent on `-DCompileFrameworkVerbose`.
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 66:
>
>> 64: }
>> 65:
>> 66: printSourceCodes();
>
> Make debug printouts controllable? Fuzzing generators compiling thousands files, would generate a whole big data of those printouts.
Good point! I'll make it dependent on -DCompileFrameworkVerbose.
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 73:
>
>> 71: switch (sourceCode.kind) {
>> 72: case SourceCode.Kind.JASM -> { jasmSources.add(sourceCode); }
>> 73: case SourceCode.Kind.JAVA -> { javaSources.add(sourceCode); }
>
> Simplify (and shorten imports also)?
> Suggestion:
>
> case JASM -> { jasmSources.add(sourceCode); }
> case JAVA -> { javaSources.add(sourceCode); }
Hmm. I think I like it the way it is, a bit more explicit.
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 87:
>
>> 85: final String sourceDir;
>> 86: try {
>> 87: sourceDir = "compile-framework-sources-" + ProcessTools.getProcessId();
>
> Make it MT-safe, say by utilising the `File.createTempFile(prefix, suffix, directory)`?
I would like the generated and saved files to be available int the `JTWork/scratch` directory. This allows us to download the source-code of a reproducer from our CI pipeline when there is a failure. Is that possible with `File.createTempFile` as well?
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 131:
>
>> 129: private static String getAsmToolsPath() {
>> 130: for (String path : getClassPaths()) {
>> 131: System.out.println("jtreg.jar?: " + path);
>
> A weird debug printout here
Ah, good point. Needed that for debugging on windows
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 150:
>
>> 148: return;
>> 149: }
>> 150: System.out.println("Compiling Java sources: " + javaSources.size());
>
> Same wish to make debug printouts controllable.
> A dedicated (mini-?)logger, a static on/off variable, a dedicated PrintStream the user could set up themselves, etc.
I think the flag will be off by default, so no printing. But if there ever arises a need for a logger like that, we can make this happen in a follow-up RFE.
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 174:
>
>> 172: }
>> 173:
>> 174: private static List<String> writeSourcesToFile(String sourceDir, List<SourceCode> sources) {
>
> `String sourceDir` -> `Path sourceDir` ?
Good idea!
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 185:
>
>> 183: }
>> 184:
>> 185: private static void writeCodeToFile(String code, String fileName) {
>
> `String fileName` -> `Path fullPath` ?
Yes, will do it systematically.
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 189:
>
>> 187: File file = new File(fileName);
>> 188: File dir = file.getAbsoluteFile().getParentFile();
>> 189: if (!dir.exists()){
>
> A space is missing
> Suggestion:
>
> if (!dir.exists()) {
thanks!
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 203:
>
>> 201:
>> 202: ProcessBuilder builder = new ProcessBuilder(command);
>> 203: builder.redirectErrorStream(true);
>
> Same streams question. Not always we'd like to have errors in the `stderr`
But here I think I want everything to go to the `stdout`. That way, I can check below that the compilation had neither any errors nor warnings. See `output.equals("")`.
> test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 222:
>
>> 220: }
>> 221:
>> 222: if (exitCode != 0 || !output.equals("")) {
>
>> Note: FuzzerUtils.java uses or overrides a deprecated API.
>> Note: Recompile with -Xlint:deprecation for details.
>
> Warnings could corrupt the output. And we probably need to at least make it controllable.
I see. I have so far not encountered any issues. I would like to keep it simple for now. We can invest in a more complicated solution in a follow up RFE. For now I suppose `FuzzerUtils` would just not be allowed.
> test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/CombinedJavaJasmExample.java line 72:
>
>> 70: out.println("}");
>> 71: out.close();
>> 72: return writer.toString();
>
> Use a multi-line string here (and in other code samples)?
>
> Suggestion:
>
> return """
> package p/xyz;
>
> super public class XYZJasm {
> public static Method test:"(I)I"
> stack 20 locals 20
> {
> iload_0;
> iconst_2;
> imul;
> invokestatic Method p/xyz/XYZJava."mul3":"(I)I"; // reference java class
> ireturn;
> }
>
> public static Method mul5:"(I)I"
> stack 20 locals 20
> {
> iload_0;
> ldc 5;
> imul;
> ireturn;
> }
> }""";
Yes, I'll use a multi-line string, good idea!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694644541
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694854892
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694671396
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694698616
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694701904
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694704950
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694753498
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694775690
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694812582
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694812823
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694855298
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694816041
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1694819394
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1693257830
More information about the hotspot-compiler-dev
mailing list