RFR: 8337221: CompileFramework: test library to conveniently compile java and jasm sources for fuzzing [v17]

Christian Hagedorn chagedorn at openjdk.org
Mon Sep 23 08:54:47 UTC 2024 

On Wed, 18 Sep 2024 06:53:44 GMT, Emanuel Peter <epeter 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...
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   another small suggestion from Christian

Almost there, some minor nits left, mostly code style. Otherwise, looks good to me now!

test/hotspot/jtreg/compiler/lib/compile_framework/Compile.java line 31:

> 29: 
> 30: /**
> 31:  * Helper class for compilation of Java and Jasm {@code SourceCode}.

Suggestion:

 * Helper class for compilation of Java and Jasm {@link SourceCode}.

test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 48:

> 46:      * Set up a new Compile Framework instance, for a new compilation unit.
> 47:      */
> 48:     public CompileFramework() {}

You can probably omit that since it's empty.

test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 106:

> 104:      *
> 105:      * @param name Name of the class to be retrieved.
> 106:      * @return A class corresponding to the {@code name}.

The?
Suggestion:

     * @return The class corresponding to the {@code name}.

test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 117:

> 115: 
> 116:     /**
> 117:      * Invoke a static method from the compiled code.

Maybe add a new line for separation:
Suggestion:

     * Invoke a static method from the compiled code.

test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 157:

> 155: 
> 156:     /**
> 157:      * Returns the classpath appended with the {@code classesDir}, where

Suggestion:

     * Returns the classpath appended with the {@link classesDir}, where

test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 160:

> 158:      * the compiled classes are stored. This enables another VM to load
> 159:      * the compiled classes. Note, the string is already backslash escaped,
> 160:      * so that the windows paths which use backslashes can be used directly

Suggestion:

     * so that Windows paths which use backslashes can be used directly

test/hotspot/jtreg/compiler/lib/compile_framework/CompileFramework.java line 169:

> 167:     }
> 168: }
> 169: 

Suggestion:

test/hotspot/jtreg/compiler/lib/compile_framework/Utils.java line 127:

> 125:      * Write sources to file.
> 126:      */
> 127:     public static List<Path> writeSourcesToFile(List<SourceCode> sources, Path sourceDir) {

Since you write multiple files, should we name this "writeSourcesToFile**s**"?
Suggestion:

    /**
     * Write each source in {@code sources} to a file inside {@code sourceDir}.
     */
    public static List<Path> writeSourcesToFiles(List<SourceCode> sources, Path sourceDir) {

test/hotspot/jtreg/compiler/lib/compile_framework/Utils.java line 128:

> 126:      */
> 127:     public static List<Path> writeSourcesToFile(List<SourceCode> sources, Path sourceDir) {
> 128:         List<Path> storedFiles = new ArrayList<Path>();

Suggestion:

        List<Path> storedFiles = new ArrayList<>();

test/hotspot/jtreg/compiler/lib/compile_framework/Utils.java line 170:

> 168:             throw new CompileFrameworkException("Compilation failed.");
> 169:         }
> 170:     }

These methods only seem to be used by the class `Compile` and look specific to the needs of that class. Maybe they better live there (or are put into separate classes). What do you think?

test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/CombinedJavaJasmExample.java line 87:

> 85:     }
> 86: 
> 87:     public static void main(String args[]) {

You should use the Java style instead of C-style and put the `[]` at the type:
Suggestion:

    public static void main(String[] args) {


Same in other examples.

test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/CombinedJavaJasmExample.java line 113:

> 111:             throw new RuntimeException("wrong value: " + i);
> 112:         }
> 113: 

Suggestion:

test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/IRFrameworkJavaExample.java line 45:

> 43:  * might not compile it because it is not present in the class, only in the dynamically compiled
> 44:  * code.
> 45:  *

Suggestion:

 * <p>

test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/IRFrameworkJavaExample.java line 47:

> 45:  *
> 46:  * Additionally, we must set the classpath for the Test-VM, so that it has access to all compiled
> 47:  * classes (see {@code getEscapedClassPathOfCompiledClasses}).

Suggestion:

 * classes (see {@link CompileFramework#getEscapedClassPathOfCompiledClasses}).

test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/IRFrameworkJavaExample.java line 57:

> 55: 
> 56:     // Generate a source java file as String
> 57:     public static String generate_X1(CompileFramework comp) {

Generally, since these are motivating example, we should probably not use underlines in the method names since Java advocates camelCase.

test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/IRFrameworkJavaExample.java line 129:

> 127: 
> 128:         // Load the compiled class.
> 129:         Class c = comp.getClass("X2");

Suggestion:

        Class<?> c = comp.getClass("X2");

test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/MultiFileJavaExample.java line 73:

> 71:         comp.compile();
> 72: 
> 73: 

Suggestion:

-------------

PR Review: https://git.openjdk.org/jdk/pull/20184#pullrequestreview-2321341939
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770900687
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770923762
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770969825
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770970223
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770971689
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770973056
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770973518
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770987188
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770982248
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770913796
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770989589
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770989881
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770992003
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770991812
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770995166
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770992852
PR Review Comment: https://git.openjdk.org/jdk/pull/20184#discussion_r1770996920

More information about the hotspot-compiler-dev mailing list