# Fix #629: Incorrect method reference results when return value requires boxing

bug_investigation_report.md

@@ -0,0 +1,68 @@
+# Issue #629 Investigation and PR #630 Report
+
+## Root Cause
+When a method reference targets a method returning a primitive type, but the functional interface expects a reference type (e.g., `Function<String, Integer> f = String::length`), Java's `LambdaMetafactory` generates an adapter class that automatically boxes the primitive return value into a wrapper object before returning it. 
+Prior to PR #630, JPF short-circuited `LambdaMetafactory` by generating a synthetic `DirectCall` method directly from the `implMethod` (e.g. `String.length()I`). It incorrectly inspected only the functional interface's expected return type, ignoring the actual primitive return type of the underlying method. It generated an `areturn` instruction, assuming an object reference was on the operand stack. However, since the underlying method returned a primitive, a raw primitive value was on the stack. JPF treated this primitive value (e.g., `5`) as an object reference ID, leading to garbage object references or JVM crashes.
+
+## Implementation
+JPF processes method references by intercepting the `LambdaMetafactory` bootstrap method in `JVMClassInfo.java` (specifically inside `setLambdaDirectCallCode`). It synthesizes a new class and method that directly invoke the target method (`implMethod`) and returns the result. 
+To fix the boxing issue, the implementation now correctly compares the primitive return type of the callee method (`calleeReturnType`) with the reference return type of the expected SAM (`samReturnType`). If the callee returns a primitive (length 1, not 'V') and the SAM expects a reference (length > 1), JPF inserts an `invokestatic` call to the corresponding wrapper class's `valueOf` method (e.g., `java.lang.Integer.valueOf`) before executing the `areturn` instruction.
+
+## Files Changed
+File: src/main/gov/nasa/jpf/jvm/JVMClassInfo.java
+Method: setLambdaDirectCallCode(MethodInfo miDirectCall, BootstrapMethodInfo bootstrapMethod)
+Old behavior: Only checked `samReturnType.length()` to determine which return instruction to emit, blindly emitting `areturn` if length > 1, without checking if the callee actually returned an object.
+New behavior: Added a condition to check if `samRetLen > 1 && calleeReturnType.length() == 1 && calleeReturnType.charAt(0) != 'V'`. If true, it determines the correct wrapper type and emits `invokestatic` to its `valueOf` method, followed by `areturn`.
+
+File: src/tests/java8/LambdaTest.java
+Method: N/A
+Old behavior: Did not cover primitive-to-reference boxing in method references.
+New behavior: Added 63 lines of regression tests to cover this exact scenario.
+
+## Exact Lines Changed
+JVMClassInfo.java
+Lines 1056-1065
+
+## Tests Added
+The PR added regression tests in `LambdaTest.java` that cover returning primitives when objects are expected (e.g., `String::length`, `Integer::parseInt`, `String::isEmpty`).
+
+## Test Results
+Clean upstream/master:
+1034 passed
+2 failed (URLClassLoaderTest > testFindResources, testFindResource)
+
+PR #630:
+1040 passed
+2 failed (URLClassLoaderTest > testFindResources, testFindResource)
+
+The PR introduced 0 regressions. The 2 failures on Windows (and the 20 failures reported by the maintainer on macOS) are pre-existing, environment-specific failures present on the `master` branch.
+
+## Environment
+OS: Microsoft Windows 11 Home Single Language, Version 10.0.26200
+Architecture: Intel64 Family 6 Model 140 Stepping 2 (amd64)
+JDK: 11.0.24 (Microsoft)
+JDK vendor: Microsoft
+Gradle: 8.4
+
+## Before/After
+Testing with the custom reproduction:
+
+**Before (upstream/master):**
+`MethodRefBoxingRepro` crashed with missing classes or returned garbage IDs because `areturn` misinterpreted the primitive value as an object reference.
+
+**After (PR #630):**
+```
+ToIntFunction String::length -> 5
+Function String::trim -> [hi]
+Function String::length -> 5
+```
+
+## Why the fix is safe
+- `ToIntFunction<String> f = String::length;` continues to work because both `samRetLen == 1` and `calleeReturnType.length() == 1`. The boxing condition is bypassed, and it emits `ireturn`.
+- `Function<String, String> f = String::trim;` continues to work because both are reference types. The boxing condition is bypassed, and it emits `areturn`.
+- `Function<String, Integer> f = String::length;` now works because `samRetLen > 1` (reference) and `calleeReturnType.length() == 1` (primitive). JPF emits the `valueOf` boxing instruction before returning, fixing the operand stack mismatch.
+
+## Final PR Recommendation
+The PR should **be kept as-is**. It correctly implements the necessary boxing logic for method reference primitive return values without introducing regressions. The test failures reported by the maintainer on macOS are completely unrelated to this PR and exist on upstream master. 
+
+Note: While investigating, I found that *unboxing* (e.g., `IntSupplier f = () -> new Integer(42);`) is also broken in JPF for the exact same reason (missing unboxing logic before `ireturn`). However, as requested, I kept this investigation focused entirely on the reported issue (#629) and did not implement additional adaptations. You may want to document the unrelated test failures separately and potentially open a new issue for method reference unboxing.

src/main/gov/nasa/jpf/jvm/JVMClassInfo.java

@@ -1046,13 +1046,26 @@ protected void setLambdaDirectCallCode (MethodInfo miDirectCall, BootstrapMethod
       break;
     }
 
-    String returnType = Types.getReturnTypeSignature(samSignature);
-    int  len = returnType.length();
-    char c = returnType.charAt(0);
-
+    String samReturnType = Types.getReturnTypeSignature(samSignature);
+    String calleeReturnType = Types.getReturnTypeSignature(calleeSig);
+    int  samRetLen = samReturnType.length();
+    char samRetChar = samReturnType.charAt(0);
+
+    // If the referenced method returns a primitive but the SAM expects a
+    // reference (e.g. String::length returns int, Function.apply returns Object),
+    // we need to box the primitive via Wrapper.valueOf() before areturn.
+    if (samRetLen > 1 && calleeReturnType.length() == 1 && calleeReturnType.charAt(0) != 'V') {
+      // callee returns a primitive, SAM expects a reference — insert boxing
+      byte calleeType = Types.getBuiltinTypeFromSignature(calleeReturnType);
+      String wrapperSimpleName = Types.getBoxedType(calleeType);
+      String wrapperClassName = "java.lang." + wrapperSimpleName;
+      String valueOfDescriptor = "(" + calleeReturnType + ")L" + wrapperClassName.replace('.', '/') + ";";
+      cb.invokestatic(wrapperClassName, "valueOf", valueOfDescriptor);
+      cb.areturn();
+    }
     // adding a return statement for function object method
-    if (len == 1) {
-      switch (c) {
+    else if (samRetLen == 1) {
+      switch (samRetChar) {
       case 'B':
       case 'I':
       case 'C':

src/tests/java8/LambdaTest.java

@@ -26,7 +26,9 @@
 import java.util.HashSet;
 import java.util.Set;
 import java.util.function.BiFunction;
+import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.function.ToIntFunction;
 import java.util.stream.Collectors;
 
 import org.junit.Test;
@@ -433,4 +435,65 @@ public void testSerializableLambdaExpression() {
       assertEquals(arr[2][1], 3);
     }
   }
+
+  // --- Regression tests for primitive method reference return value boxing ---
+
+  @Test
+  public void testMethodRefPrimitiveBoxingIntViaFunction() {
+    if (verifyNoPropertyViolation()) {
+      Function<String, Integer> f = String::length;
+      int result = f.apply("hello");
+      assertEquals(result, 5);
+    }
+  }
+
+  @Test
+  public void testMethodRefPrimitiveBoxingToIntFunction() {
+    if (verifyNoPropertyViolation()) {
+      ToIntFunction<String> f = String::length;
+      assertEquals(f.applyAsInt("hello"), 5);
+    }
+  }
+
+  @Test
+  public void testMethodRefReturningReference() {
+    if (verifyNoPropertyViolation()) {
+      Function<String, String> f = String::trim;
+      assertEquals(f.apply(" hi "), "hi");
+    }
+  }
+
+  @Test
+  public void testStaticMethodRefPrimitiveBoxing() {
+    if (verifyNoPropertyViolation()) {
+      Function<String, Integer> f = Integer::parseInt;
+      int result = f.apply("123");
+      assertEquals(result, 123);
+    }
+  }
+
+  public static class BoxHolder {
+    private final int value;
+    public BoxHolder(int value) { this.value = value; }
+    public int getValue() { return value; }
+  }
+
+  @Test
+  public void testBoundMethodRefPrimitiveBoxing() {
+    if (verifyNoPropertyViolation()) {
+      BoxHolder box = new BoxHolder(42);
+      Supplier<Integer> f = box::getValue;
+      int result = f.get();
+      assertEquals(result, 42);
+    }
+  }
+
+  @Test
+  public void testMethodRefBooleanBoxing() {
+    if (verifyNoPropertyViolation()) {
+      Function<String, Boolean> f = String::isEmpty;
+      assertTrue(f.apply(""));
+      assertFalse(f.apply("x"));
+    }
+  }
 }