From 42e8196782999f19409b7bce8c7d91822dc144b6 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sat, 23 May 2026 07:08:25 +0300 Subject: [PATCH] Fix #3108: conversion opcodes must update runtime type tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #4985. The PUTFIELD fix there was correct but only addressed half of the chained-assignment NPE. Repro (ddyer's original program): private double boardScaleIndex, newBoardScaleIndex; boardScaleIndex = newBoardScaleIndex = Math.min(n, (int) expr); javac emits PUSH_INT (Math.min result), I2D, DUP2_X1, two PUTFIELDs. The I2D / I2L / F2D / F2L / L2D macros only wrote SP[-1].data.* and left the runtime type tag untouched. BC_DUP2_X1 / BC_DUP2_X2 / BC_DUP_X2 dispatch via IS_DOUBLE_WORD(...) on that tag, so an I2D-produced double whose tag still reads CN1_TYPE_INT sent the dup down the cat-1 branch: - SP bumped by +2 instead of +1 - three slots reshuffled as if all single-word - first putfield happened to read the right operands (lucky) - second putfield read the double's IEEE-754 bit pattern as the object reference and crashed with NPE on iOS Reproduced end-to-end with a standalone C test that mirrors the BC_* macros: post-fix the second putfield's target resolves to the expected this pointer; pre-fix it resolved to 0x4028000000000000 (= bits of 12.0). Each cat-changing conversion now rewrites SP[-1].type to the new CN1_TYPE_*. The narrowing inverses (L2I/L2F, D2I/D2F) and the pure cat-1 conversion I2F do the same for symmetry — DUP2/DUP_X2 path can hit them in less obvious bytecode sequences. I2B/I2C/I2S already preserve CN1_TYPE_INT, so they're left alone. Regression test conversionOpcodesUpdateRuntimeTypeTag asserts each of the 12 conversion opcodes emits the corresponding SP[-1].type = CN1_TYPE_* write. Full BytecodeInstructionIntegrationTest (72 tests) passes; verified the fail-on-pre-fix-code shape too. Co-Authored-By: Claude Opus 4.7 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 27 +++++----- .../bytecodes/BasicInstruction.java | 51 ++++++++++--------- .../BytecodeInstructionIntegrationTest.java | 46 +++++++++++++++++ 3 files changed, 89 insertions(+), 35 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 8f92825bb8..dbf6b18716 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -350,29 +350,32 @@ typedef struct clazz* JAVA_CLASS; SP[-1].data.l = SP[-1].data.l ^ (*SP).data.l; \ } -#define BC_I2L() SP[-1].data.l = SP[-1].data.i +// Conversion macros must rewrite the runtime type tag too. BC_DUP2_X1 / +// BC_DUP2_X2 / BC_DUP_X2 dispatch via IS_DOUBLE_WORD on the tag, so a stale +// tag corrupts the stack on chained assignments (issue #3108). +#define BC_I2L() do { SP[-1].data.l = SP[-1].data.i; SP[-1].type = CN1_TYPE_LONG; } while(0) -#define BC_L2I() SP[-1].data.i = (JAVA_INT)SP[-1].data.l +#define BC_L2I() do { SP[-1].data.i = (JAVA_INT)SP[-1].data.l; SP[-1].type = CN1_TYPE_INT; } while(0) -#define BC_L2F() SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.l +#define BC_L2F() do { SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.l; SP[-1].type = CN1_TYPE_FLOAT; } while(0) -#define BC_L2D() SP[-1].data.d = (JAVA_DOUBLE)SP[-1].data.l +#define BC_L2D() do { SP[-1].data.d = (JAVA_DOUBLE)SP[-1].data.l; SP[-1].type = CN1_TYPE_DOUBLE; } while(0) -#define BC_I2F() SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.i +#define BC_I2F() do { SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.i; SP[-1].type = CN1_TYPE_FLOAT; } while(0) -#define BC_F2I() SP[-1].data.i = (JAVA_INT)SP[-1].data.f +#define BC_F2I() do { SP[-1].data.i = (JAVA_INT)SP[-1].data.f; SP[-1].type = CN1_TYPE_INT; } while(0) -#define BC_F2L() SP[-1].data.l = (JAVA_LONG)SP[-1].data.f +#define BC_F2L() do { SP[-1].data.l = (JAVA_LONG)SP[-1].data.f; SP[-1].type = CN1_TYPE_LONG; } while(0) -#define BC_F2D() SP[-1].data.d = SP[-1].data.f +#define BC_F2D() do { SP[-1].data.d = SP[-1].data.f; SP[-1].type = CN1_TYPE_DOUBLE; } while(0) -#define BC_D2I() SP[-1].data.i = (JAVA_INT)SP[-1].data.d +#define BC_D2I() do { SP[-1].data.i = (JAVA_INT)SP[-1].data.d; SP[-1].type = CN1_TYPE_INT; } while(0) -#define BC_D2L() SP[-1].data.l = (JAVA_LONG)SP[-1].data.d +#define BC_D2L() do { SP[-1].data.l = (JAVA_LONG)SP[-1].data.d; SP[-1].type = CN1_TYPE_LONG; } while(0) -#define BC_I2D() SP[-1].data.d = SP[-1].data.i +#define BC_I2D() do { SP[-1].data.d = SP[-1].data.i; SP[-1].type = CN1_TYPE_DOUBLE; } while(0) -#define BC_D2F() SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.d +#define BC_D2F() do { SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.d; SP[-1].type = CN1_TYPE_FLOAT; } while(0) #ifdef CN1_INCLUDE_NPE_CHECKS #define BC_ARRAYLENGTH() { \ diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/bytecodes/BasicInstruction.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/bytecodes/BasicInstruction.java index 06e60b08d7..fce84a34c0 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/bytecodes/BasicInstruction.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/bytecodes/BasicInstruction.java @@ -530,52 +530,57 @@ public void appendInstruction(StringBuilder b, List instructions) { b.append(" SP--; SP[-1].data.l = SP[-1].data.l ^ (*SP).data.l; /* LXOR */\n") ; break; + // The conversion opcodes below must also rewrite SP[-1].type to + // match the new category. BC_DUP2_X1 / BC_DUP2_X2 / BC_DUP_X2 + // dispatch via the runtime tag (IS_DOUBLE_WORD), so a stale tag + // (e.g. INT left over after I2D) sends the dup macro down the + // cat-1 branch and corrupts the stack. See issue #3108. case Opcodes.I2L: - b.append(" SP[-1].data.l = SP[-1].data.i; /* I2L */\n"); + b.append(" SP[-1].data.l = SP[-1].data.i; SP[-1].type = CN1_TYPE_LONG; /* I2L */\n"); break; - + case Opcodes.I2F: - b.append(" SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.i; /* I2F */\n"); + b.append(" SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.i; SP[-1].type = CN1_TYPE_FLOAT; /* I2F */\n"); break; - + case Opcodes.I2D: - b.append(" SP[-1].data.d = SP[-1].data.i; /* I2D */;\n"); + b.append(" SP[-1].data.d = SP[-1].data.i; SP[-1].type = CN1_TYPE_DOUBLE; /* I2D */;\n"); break; - + case Opcodes.L2I: - b.append(" SP[-1].data.i = (JAVA_INT)SP[-1].data.l; /* L2I */\n"); + b.append(" SP[-1].data.i = (JAVA_INT)SP[-1].data.l; SP[-1].type = CN1_TYPE_INT; /* L2I */\n"); break; - + case Opcodes.L2F: - b.append(" SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.l; /* L2F */\n"); + b.append(" SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.l; SP[-1].type = CN1_TYPE_FLOAT; /* L2F */\n"); break; - + case Opcodes.L2D: - b.append(" SP[-1].data.d = (JAVA_DOUBLE)SP[-1].data.l; /* L2D */\n"); + b.append(" SP[-1].data.d = (JAVA_DOUBLE)SP[-1].data.l; SP[-1].type = CN1_TYPE_DOUBLE; /* L2D */\n"); break; - + case Opcodes.F2I: - b.append(" SP[-1].data.i = (JAVA_INT)SP[-1].data.f; /* F2I */\n"); + b.append(" SP[-1].data.i = (JAVA_INT)SP[-1].data.f; SP[-1].type = CN1_TYPE_INT; /* F2I */\n"); break; - + case Opcodes.F2L: - b.append(" SP[-1].data.l = (JAVA_LONG)SP[-1].data.f; /* F2L */\n"); + b.append(" SP[-1].data.l = (JAVA_LONG)SP[-1].data.f; SP[-1].type = CN1_TYPE_LONG; /* F2L */\n"); break; - + case Opcodes.F2D: - b.append(" SP[-1].data.d = SP[-1].data.f; /* F2D */\n"); + b.append(" SP[-1].data.d = SP[-1].data.f; SP[-1].type = CN1_TYPE_DOUBLE; /* F2D */\n"); break; - + case Opcodes.D2I: - b.append(" SP[-1].data.i = (JAVA_INT)SP[-1].data.d; /* D2I */\n"); + b.append(" SP[-1].data.i = (JAVA_INT)SP[-1].data.d; SP[-1].type = CN1_TYPE_INT; /* D2I */\n"); break; - + case Opcodes.D2L: - b.append(" SP[-1].data.l = (JAVA_LONG)SP[-1].data.d; /* D2L */\n"); + b.append(" SP[-1].data.l = (JAVA_LONG)SP[-1].data.d; SP[-1].type = CN1_TYPE_LONG; /* D2L */\n"); break; - + case Opcodes.D2F: - b.append(" SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.d; /* D2F */\n"); + b.append(" SP[-1].data.f = (JAVA_FLOAT)SP[-1].data.d; SP[-1].type = CN1_TYPE_FLOAT; /* D2F */\n"); break; case Opcodes.I2B: diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/BytecodeInstructionIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/BytecodeInstructionIntegrationTest.java index a5d468651a..f46318d1c0 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/BytecodeInstructionIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/BytecodeInstructionIntegrationTest.java @@ -1216,6 +1216,52 @@ void putfieldUnfoldedPathUsesPeekNotPop() { "Object PUTFIELD must not rely on C argument evaluation order:\n" + objC); } + /** + * Regression test for issue #3108 (second cause). + * + * The widening / narrowing conversion opcodes (I2D, I2L, F2D, F2L, L2D and + * their inverses) used to write the new value into SP[-1].data but leave + * the runtime type tag untouched. BC_DUP2_X1 / BC_DUP2_X2 / BC_DUP_X2 + * dispatch via IS_DOUBLE_WORD(...) on that tag, so e.g. PUSH_INT (tag=INT) + * followed by I2D (data updated, tag still INT) followed by DUP2_X1 sent + * the dup through the cat-1 branch and shifted SP by +2 instead of +1. + * The chained assignment "a.x = b.x = (double) someInt" then read garbage + * for the second putfield's operands and crashed with NPE on iOS. + * + * Each cat-changing conversion must rewrite SP[-1].type to the new + * CN1_TYPE_*. Pure-arithmetic conversions (I2B/I2C/I2S, I2F/F2I) are not + * involved in dup-dispatch but are checked for symmetry. + */ + @Test + void conversionOpcodesUpdateRuntimeTypeTag() { + Object[][] cases = { + {Opcodes.I2L, "I2L", "CN1_TYPE_LONG"}, + {Opcodes.I2D, "I2D", "CN1_TYPE_DOUBLE"}, + {Opcodes.I2F, "I2F", "CN1_TYPE_FLOAT"}, + {Opcodes.L2I, "L2I", "CN1_TYPE_INT"}, + {Opcodes.L2F, "L2F", "CN1_TYPE_FLOAT"}, + {Opcodes.L2D, "L2D", "CN1_TYPE_DOUBLE"}, + {Opcodes.F2I, "F2I", "CN1_TYPE_INT"}, + {Opcodes.F2L, "F2L", "CN1_TYPE_LONG"}, + {Opcodes.F2D, "F2D", "CN1_TYPE_DOUBLE"}, + {Opcodes.D2I, "D2I", "CN1_TYPE_INT"}, + {Opcodes.D2L, "D2L", "CN1_TYPE_LONG"}, + {Opcodes.D2F, "D2F", "CN1_TYPE_FLOAT"}, + }; + for (Object[] c : cases) { + int opcode = (Integer) c[0]; + String name = (String) c[1]; + String expectedTypeTag = (String) c[2]; + BasicInstruction instr = new BasicInstruction(opcode, 0); + StringBuilder out = new StringBuilder(); + instr.appendInstruction(out, new ArrayList()); + String emitted = out.toString(); + assertTrue(emitted.contains("SP[-1].type = " + expectedTypeTag), + name + " must rewrite SP[-1].type to " + expectedTypeTag + + " so BC_DUP2_X1 / BC_DUP2_X2 / BC_DUP_X2 dispatch correctly. Emitted:\n" + emitted); + } + } + @Test void testArithmeticExpressionCoverage() { // Use tryReduce to construct an ArithmeticExpression since constructor is private