RFR: 7903808: IOOBE in compile action when processing class files only [v2]

[Christian Stein]

On Mon, 9 Sep 2024 08:39:51 GMT, Christian Stein <cstein at openjdk.org> wrote:

>> Please review this fix preventing an `IndexOutOfBoundsException` in the compile action when processing class files only.
>> 
>> As an library used by a test might need `--enable-preview` it is possible that the `insertPos` variable is still assigned with the value `-1`: the `for (String currArg : args) { ... }` loop may exit normally without setting a valid insertion position.
>> 
>> This change checks for `-1` values and use `0` instead in order to prepend `--enable-preview` and `--source N` before any variadic arguments.
>
> Christian Stein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add test for 7903808

Another way to fix the issue and also simplify the implementation is to always prepend additional arguments instead of finding a "good enough" insertion index.


Subject: [PATCH] Always prepend additional arguments
---
Index: src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java b/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java
--- a/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java	(revision 3e8216d86aea380964aa6e09064d3d45f25c9f2c)
+++ b/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java	(revision c40335c32ea9e4349f5eb8a7dfc5c5c8ff028bf8)
@@ -327,7 +327,6 @@
         List<String> jcodArgs = new ArrayList<>();
         boolean runJavac = process;
 
-        int insertPos = -1;
         boolean seenSourceOrRelease = false;
         boolean seenEnablePreview = false;
 
@@ -335,9 +334,6 @@
             if (currArg.endsWith(".java")) {
                 if (!(new File(currArg)).exists())
                     throw new TestRunException(CANT_FIND_SRC + currArg);
-                if (insertPos == -1) {
-                    insertPos = javacArgs.size();
-                }
                 javacArgs.add(currArg);
                 runJavac = true;
             } else if (currArg.endsWith(".jasm")) {
@@ -353,9 +349,6 @@
                     case "-source":
                     case "--source":
                     case "--release":
-                        if (insertPos == -1) {
-                            insertPos = javacArgs.size();
-                        }
                         seenSourceOrRelease= true;
                         break;
                 }
@@ -368,14 +361,12 @@
                 && !seenEnablePreview
                 && (script.enablePreview() || usesLibraryCompiledWithPreviewEnabled())
                 && (libLocn == null || libLocn.isTest())) {
-            if (insertPos == -1) { // happens for example in "-proc:only CLASS" cases
-                insertPos = 0;     // prepend in order to not mess with variadic arguments
-            }
-            javacArgs.add(insertPos, "--enable-preview");
+            // prepend additional arguments in order to not mess with variadic arguments
+            javacArgs.add(0, "--enable-preview");
             if (!seenSourceOrRelease) {
                 int v = script.getTestJDKVersion().major;
-                javacArgs.add(insertPos + 1, "-source");
-                javacArgs.add(insertPos + 2, String.valueOf(v));
+                javacArgs.add(1, "-source");
+                javacArgs.add(2, String.valueOf(v));
             }
         }

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

PR Comment: https://git.openjdk.org/jtreg/pull/224#issuecomment-2338534454