diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/BytecodeMethod.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/BytecodeMethod.java index d475d443c7c..69be2e92342 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/BytecodeMethod.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/BytecodeMethod.java @@ -2851,10 +2851,30 @@ private int nextExecutable(int from) { return -1; } + /// The instruction that produced the value on top of the operand stack just + /// before `from + 1`, or -1 when no single instruction did. + /// + /// Stops at a jump target, like srNextRealNoJoin: past a join the value can + /// arrive from a predecessor that never executed the instruction before the + /// label. javac compiles `for (x : c ? a : b)` to + /// + /// IFEQ L2 ; ; GOTO L3 ; L2: ALOAD b ; L3: INVOKE iterator + /// + /// and skipping L3 made `ALOAD b` the loop's receiver: intrinsifyForEach + /// folded it into the setup and deleted it from the else branch, so the + /// loop always walked `b` -- a NullPointerException when `b` was null and + /// `c` true -- while the value `a` pushed was never popped. Every caller + /// pairs a consumer with its producer, so none may look through a join. private int prevExecutable(int from) { for (int i = from; i >= 0; i--) { Instruction ins = instructions.get(i); - if (ins instanceof LabelInstruction || ins instanceof LineNumber || ins instanceof TryCatch) { + if (ins instanceof LabelInstruction) { + if (LabelInstruction.isJumpTarget(((LabelInstruction) ins).getLabel())) { + return -1; + } + continue; + } + if (ins instanceof LineNumber || ins instanceof TryCatch) { continue; } return i; @@ -2961,15 +2981,14 @@ public void intrinsifyForEach() { || !"()Ljava/util/Iterator;".equals(inv.getDesc())) { continue; } + // -1 when the receiver arrives through a join (a `?:`): there is no + // one producer to fold, and the loop reads it off the stack below. int recvIdx = prevExecutable(i - 1); - if (recvIdx < 0) { - continue; - } // Fold an expression when it is already available. Raw stack // producers remain in the IR and use a non-assignable consumer, // so later expression folding cannot discard their evaluation. StringBuilder recvCode = new StringBuilder(); - Instruction recv = instructions.get(recvIdx); + Instruction recv = recvIdx < 0 ? null : instructions.get(recvIdx); Invoke foldedMapView = null; int viewIndex = -1; if (recv instanceof Invoke) { diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/BulkCompareTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/BulkCompareTest.java new file mode 100644 index 00000000000..6cbdba76545 --- /dev/null +++ b/vm/tests/src/test/java/com/codename1/tools/translator/BulkCompareTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.tools.translator; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/// The bulk comparison must fail a check that proves nothing, not just one +/// whose output differs: equal output from a check that printed nothing, or +/// that threw out of its body on HotSpot, passed silently. +class BulkCompareTest { + private static List compare(String hotspot, String parparvm) { + Map> expected = BulkConformanceIntegrationTest.parse(hotspot); + Map> actual = BulkConformanceIntegrationTest.parse(parparvm); + List failures = new ArrayList<>(); + BulkConformanceIntegrationTest.compare(expected, actual, failures); + return failures; + } + + @Test + void equalOutputPasses() { + assertEquals(0, compare("### a\nx=1\n", "### a\nx=1\n").size()); + } + + @Test + void differentOutputFails() { + assertEquals(1, compare("### a\nx=1\n", "### a\nx=2\n").size()); + } + + @Test + void aCheckThatPrintsNothingFails() { + List failures = compare("### a\n### b\nx=1\n", "### a\n### b\nx=1\n"); + assertEquals(1, failures.size(), failures.toString()); + assertTrue(failures.get(0).startsWith("a: printed nothing"), failures.toString()); + } + + @Test + void aDiagnosticLineIsNotEvidence() { + assertEquals(1, compare("### a\n[GC-INSTR] x\n", "### a\n").size()); + } + + @Test + void aCheckThatThrowsOnHotSpotFailsEvenWhenParparVmMatches() { + String run = "### a\nx=1\nTHREW java.lang.NullPointerException: null\n"; + List failures = compare(run, run); + assertEquals(1, failures.size(), failures.toString()); + assertTrue(failures.get(0).contains("threw out of its body on HotSpot"), failures.toString()); + } + + @Test + void aSkippedCheckStillFailsWhenParparVmThrows() { + List failures = compare("### a\nSKIPPED jdk differs\n", + "### a\nTHREW java.lang.NullPointerException: null\n"); + assertEquals(1, failures.size(), failures.toString()); + assertTrue(failures.get(0).contains("skipped on HotSpot"), failures.toString()); + } + + @Test + void aSkippedCheckIgnoresOrdinaryDifferences() { + assertEquals(0, compare("### a\nSKIPPED jdk differs\n", "### a\nx=2\n").size()); + } +} diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/BulkConformanceIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/BulkConformanceIntegrationTest.java index 53ad819dfc3..f0c39b22594 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/BulkConformanceIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/BulkConformanceIntegrationTest.java @@ -186,6 +186,11 @@ void everyCheckAnswersTheSameOnHotSpotAndParparVM() throws Exception { Run parpar = run(Collections.singletonList(executable.toString()), parparDir); Map> expected = parse(hotspot.output); + // Bulk.end() prints "(end)". Without it the HotSpot run stopped early + // -- a check that exits the JVM -- and every later check would be + // missing from both sides and compare as equal. + assertTrue(expected.containsKey("(end)"), "the HotSpot run never reached Bulk.end():\n" + + tail(hotspot.output)); Map> actual = parse(parpar.output); compare(expected, actual, failures); BulkGeneratedCodeChecks.checkOutput(actual, failures); @@ -237,7 +242,32 @@ static void compare(Map> expected, Map failures.add(name + ": never ran on ParparVM"); continue; } - if (!e.isEmpty() && e.get(0).startsWith("SKIPPED ")) { + // "Both runtimes printed the same" is only evidence when something + // was printed and every line of the check ran. A check that prints + // nothing compares nothing, and one that throws out of its body on + // HotSpot stops at the throw on both sides -- its remaining + // assertions then pass on ParparVM without running there. Both + // passed silently; neither is ever intended. + if (e.isEmpty()) { + failures.add(name + ": printed nothing on HotSpot, so it compares nothing"); + continue; + } + for (String line : e) { + if (line.startsWith("THREW ")) { + failures.add(name + ": threw out of its body on HotSpot (" + clip(line) + + "); the rest of the check never ran. Catch the expected exception" + + " with Bulk.pThrown"); + } + } + if (e.get(0).startsWith("SKIPPED ")) { + // The JDK's answer differs from Codename One's here, so there is + // nothing to compare -- but ParparVM still runs the body, and an + // uncaught exception there is a failure, not a difference. + for (String line : a) { + if (line.startsWith("THREW ")) { + failures.add(name + " (skipped on HotSpot): ParparVM " + clip(line)); + } + } continue; } if (e.size() == 1 && "CN1-ONLY".equals(e.get(0))) { diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/BulkGeneratedCodeChecks.java b/vm/tests/src/test/java/com/codename1/tools/translator/BulkGeneratedCodeChecks.java index 554ab51d3e1..98fada90c68 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/BulkGeneratedCodeChecks.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/BulkGeneratedCodeChecks.java @@ -219,6 +219,29 @@ static void checkOutput(Map> parparvm, List failure caseValue(collections, "ihm.valNulls", "1", "the value iterator must hand back a null value as null", failures); caseValue(collections, "ihm.bigSeen", "500", "key iteration must survive a rehash", failures); + // A `?:` receiver joins two branches right before the loop. Each + // must walk the branch that ran -- the else branch's load used to + // be folded in as the receiver of both, so a true condition with a + // null else threw. Without the fix trueNullElse, trueOther, + // localsFirst, localsFirstNullElse, staticsNullElse, keySetFirst, + // keySetFirstNullElse and valuesFirst fail; the other cases pin the + // branches that were right by construction. Equal output catches + // them too; named so the failure says what broke. + String[][] ternary = { + {"ternary.trueNullElse", "o:1"}, {"ternary.trueOther", "o:2"}, + {"ternary.falseOther", "uv:2"}, {"ternary.falseNull", "THREW java.lang.NullPointerException"}, + {"ternary.localsFirst", "a1a2"}, {"ternary.localsSecond", "uv"}, + {"ternary.localsFirstNullElse", "a1a2"}, + {"ternary.fieldsNullElse", "c"}, {"ternary.fieldsElse", "b"}, + {"ternary.staticsNullElse", "sc"}, {"ternary.staticsElse", "sb"}, + {"ternary.keySetFirst", "1"}, {"ternary.keySetSecond", "6"}, + {"ternary.keySetFirstNullElse", "1"}, + {"ternary.valuesFirst", "3"}, {"ternary.valuesSecond", "6"}, + {"ternary.iteratorFirst", "a1"}, {"ternary.iteratorSecond", "u"}, + }; + for (String[] c : ternary) { + caseValue(collections, c[0], c[1], "a ?: receiver must use the branch that ran", failures); + } } List rethrow = parparvm.get("CatchRethrowApp"); if (rethrow != null && !rethrow.contains("RESULT=127")) { diff --git a/vm/tests/src/test/resources/com/codename1/tools/translator/bulk/CollectionSemanticsApp.java b/vm/tests/src/test/resources/com/codename1/tools/translator/bulk/CollectionSemanticsApp.java index 204aacd44f8..3297012874b 100644 --- a/vm/tests/src/test/resources/com/codename1/tools/translator/bulk/CollectionSemanticsApp.java +++ b/vm/tests/src/test/resources/com/codename1/tools/translator/bulk/CollectionSemanticsApp.java @@ -244,6 +244,140 @@ private void fieldTraversalCases() { } catch (NullPointerException expected) { emit("fieldTraversal.null", traversalCalls); } } + /// A for-each and a map view whose receiver is a `?:`, plus a + /// behaviour-only iterator call. javac joins the two branches at a label + /// right before the consumer, and the translator used to take the else + /// branch's load for the receiver on whichever path ran (and NPE when that + /// value was null). + private Iterable ownTraversal() { traversalCalls++; return traversalField; } + + private String ternaryLoop(boolean all, List updated) { + String value = ""; + for (String item : (all ? ownTraversal() : updated)) value += item; + return value; + } + + private static String ternaryLocals(boolean first, List a, List b) { + String value = ""; + for (String item : first ? a : b) value += item; + return value; + } + + // The keySet/values fold fires only on a receiver whose exact class the + // translator has proved, so both maps are built right here; with the + // parameters typed HashMap these cases passed even before the fix. + // ternaryIteratorCall is behaviour only: lowerIteratorCalls does not retype + // this shape (the iterator stores are folded first), so no translator + // revision has been seen to fail it. + // The shape JpqlQueryImpl walks a CASE expression with: both branches read + // a field, and the else branch's is null exactly when the then branch runs. + // A guard, not a reproduction: an instance field read is not folded into + // the loop, so master already took this receiver off the stack. + private List ternaryChildren; + private List ternaryBranches; + + private String ternaryFields() { + String value = ""; + for (String item : ternaryBranches == null ? ternaryChildren : ternaryBranches) value += item; + return value; + } + + // A static field read IS folded, so this shape failed like a local did. + private static List staticChildren; + private static List staticBranches; + + private static String ternaryStatics() { + String value = ""; + for (String item : staticBranches == null ? staticChildren : staticBranches) value += item; + return value; + } + + private static int ternaryKeySet(boolean first, boolean secondNull) { + java.util.HashMap a = new java.util.HashMap(); + a.put("k", "vvv"); + java.util.HashMap b = secondNull ? null : new java.util.HashMap(); + if (b != null) { b.put("kk", "v"); b.put("kkkk", "vvvvv"); } + int seen = 0; + for (String key : (first ? a : b).keySet()) seen += key.length(); + return seen; + } + + private static int ternaryValues(boolean first) { + java.util.HashMap a = new java.util.HashMap(); + a.put("k", "vvv"); + java.util.HashMap b = new java.util.HashMap(); + b.put("kk", "v"); b.put("kkkk", "vvvvv"); + int seen = 0; + for (String v : (first ? a : b).values()) seen += v.length(); + return seen; + } + + private static String ternaryIteratorCall(boolean first) { + ArrayList a = new ArrayList(); + a.add("a1"); + // Two iterator classes the translator can name: if a future lowering + // retypes this shape, a retype onto the wrong branch would read the + // wrong layout. + java.util.ArrayDeque b = new java.util.ArrayDeque(); + b.add("u"); + Iterator ia = a.iterator(); + Iterator ib = b.iterator(); + return (first ? ia : ib).next(); + } + + private interface TernaryCase { + Object run(); + } + + /// One case per call, so a case that throws cannot hide the ones after it. + private static void ternaryCase(String key, TernaryCase body) { + Object value; + try { + value = body.run(); + } catch (RuntimeException e) { + value = "THREW " + e.getClass().getName(); + } + emit(key, value); + } + + private void ternaryTraversalCases() { + traversalField = new ArrayList(); + traversalField.add("o"); + traversalCalls = 0; + final List other = new java.util.LinkedList(); + other.add("u"); other.add("v"); + ternaryCase("ternary.trueNullElse", () -> ternaryLoop(true, null) + ":" + traversalCalls); + ternaryCase("ternary.trueOther", () -> ternaryLoop(true, other) + ":" + traversalCalls); + ternaryCase("ternary.falseOther", () -> ternaryLoop(false, other) + ":" + traversalCalls); + ternaryCase("ternary.falseNull", () -> ternaryLoop(false, null)); + final List a = new ArrayList(); + a.add("a1"); a.add("a2"); + ternaryCase("ternary.localsFirst", () -> ternaryLocals(true, a, other)); + ternaryCase("ternary.localsSecond", () -> ternaryLocals(false, a, other)); + ternaryCase("ternary.localsFirstNullElse", () -> ternaryLocals(true, a, null)); + ternaryChildren = new ArrayList(); + ternaryChildren.add("c"); + ternaryBranches = null; + ternaryCase("ternary.fieldsNullElse", () -> ternaryFields()); + ternaryBranches = new ArrayList(); + ternaryBranches.add("b"); + ternaryCase("ternary.fieldsElse", () -> ternaryFields()); + staticChildren = new ArrayList(); + staticChildren.add("sc"); + staticBranches = null; + ternaryCase("ternary.staticsNullElse", () -> ternaryStatics()); + staticBranches = new ArrayList(); + staticBranches.add("sb"); + ternaryCase("ternary.staticsElse", () -> ternaryStatics()); + ternaryCase("ternary.keySetFirst", () -> ternaryKeySet(true, false)); + ternaryCase("ternary.keySetSecond", () -> ternaryKeySet(false, false)); + ternaryCase("ternary.keySetFirstNullElse", () -> ternaryKeySet(true, true)); + ternaryCase("ternary.valuesFirst", () -> ternaryValues(true)); + ternaryCase("ternary.valuesSecond", () -> ternaryValues(false)); + ternaryCase("ternary.iteratorFirst", () -> ternaryIteratorCall(true)); + ternaryCase("ternary.iteratorSecond", () -> ternaryIteratorCall(false)); + } + public static void fixtureMain(String[] args) throws InterruptedException { // The migration result exposes this exact wrapper. Its toString calls // Object.toString through a Collection receiver, whose interface also @@ -265,6 +399,7 @@ public static void fixtureMain(String[] args) throws InterruptedException { emit("unmodifiable.collection", java.util.Collections.unmodifiableCollection(migrations).toString()); emit("interface.string", stringFromCollection(applied)); new CollectionSemanticsApp().fieldTraversalCases(); + new CollectionSemanticsApp().ternaryTraversalCases(); new CollectionSemanticsApp().directCallCases(); nativeStorageCases(); bulkCopyCases();