Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpelCompiler VerifyError - Incompatible argument to function [SPR-15192] #19758

Closed
spring-issuemaster opened this issue Jan 26, 2017 · 7 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Jan 26, 2017

Gary Russell opened SPR-15192 and commented

Method execution with an Elvis operator on a Map in the method argument expression.

java.lang.IllegalStateException: Failed to instantiate CompiledExpression
	at org.springframework.expression.spel.standard.SpelCompiler.compile(SpelCompiler.java:111)
	at org.springframework.expression.spel.standard.SpelExpression.compileExpression(SpelExpression.java:467)
	at org.springframework.expression.spel.standard.SpelExpression.checkCompile(SpelExpression.java:437)
	at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:331)
	at com.example.CompilerTests.test(CompilerTests.java:67)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:678)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: java.lang.VerifyError: (class: spel/Ex5, method: getValue signature: (Ljava/lang/Object;Lorg/springframework/expression/EvaluationContext;)Ljava/lang/Object;) Incompatible argument to function
	at java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
	at java.lang.Class.getConstructor0(Class.java:3075)
	at java.lang.Class.newInstance(Class.java:412)
	at org.springframework.expression.spel.standard.SpelCompiler.compile(SpelCompiler.java:108)
	... 27 more

public class CompilerTests {

	@Test
	public void test() {
		SpelParserConfiguration configuration = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null);

		Expression exp = new SpelExpressionParser(configuration).parseExpression("bar()"); // OK
		assertEquals("BAR", exp.getValue(new Foo(), String.class));
		assertEquals("BAR", exp.getValue(new Foo(), String.class));
		assertEquals("BAR", exp.getValue(new Foo(), String.class));
		assertNotNull(new DirectFieldAccessor(exp).getPropertyValue("compiledAst"));

		exp = new SpelExpressionParser(configuration).parseExpression("bar('baz')"); // OK
		assertEquals("BAZ", exp.getValue(new Foo(), String.class));
		assertEquals("BAZ", exp.getValue(new Foo(), String.class));
		assertEquals("BAZ", exp.getValue(new Foo(), String.class));
		assertNotNull(new DirectFieldAccessor(exp).getPropertyValue("compiledAst"));

		StandardEvaluationContext context = new StandardEvaluationContext();
		context.setVariable("map", Collections.singletonMap("foo", "qux"));

		exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'])"); // OK
		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
		assertNotNull(new DirectFieldAccessor(exp).getPropertyValue("compiledAst"));

		exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'] ?: 'qux')"); // VerifyError
		assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); // OK
		assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); // BOOM
		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
		assertNotNull(new DirectFieldAccessor(exp).getPropertyValue("compiledAst"));
	}

	public static class Foo {

		public String bar() {
			return "BAR";
		}

		public String bar(String arg) {
			return arg.toUpperCase();
		}

	}

}

Also occurs with the Elvis expanded

"bar(#map['foo'] != null ? #map['foo'] : 'qux')"

Affects: 4.3.6, 5.0 M4

Issue Links:

  • INT-4217 Compiled SpEL Improvement for MMIH
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 28, 2017

Gary Russell commented

I took a look at this (mainly for my own education). I noticed that there was no cast check when the object is received from the map.

This is because when the if/else type descriptors don't match, the Elvis prefers to use the nullValue descriptor in computeExitTypeDescriptor().

ALOAD 1
CHECKCAST Foo
ALOAD 2 
LDC "map"
INVOKEINTERFACE ec.lookupVariable
CHECKCAST map
LDC "foo"
INVOKEINTERFACE Map.get
// Elvis
DUP
IFNULL else
GOTO ifEnd
else:
POP
LDC "qux"
ifEnd:
// No CHECKCAST when the object was received from the map <<<<<<<<<<<<<<<<<<<<<<<<<<
INVOKEVIRTUAL Foo.bar() (Ljava/lang/String;)Ljava/lang/String;
ARETURN

When I changed computeExitTypeDescriptor() to always return Object when the descriptors don't match, the SpEL compiles OK.

private void computeExitTypeDescriptor() {
	if (this.exitTypeDescriptor == null && this.children[0].exitTypeDescriptor != null &&
			this.children[1].exitTypeDescriptor != null) {
		String conditionDescriptor = this.children[0].exitTypeDescriptor;
		String ifNullValueDescriptor = this.children[1].exitTypeDescriptor;
		if (conditionDescriptor.equals(ifNullValueDescriptor)) {
			this.exitTypeDescriptor = conditionDescriptor;
		}
		else {
			// Use the easiest to compute common super type
			this.exitTypeDescriptor = "Ljava/lang/Object";
		}
	}
}

Same result with Ternary, as one would expect.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 30, 2017

Andy Clement commented

Thanks for digging Gary - I need to get through something for SCDF but then I was going to sort this out.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 30, 2017

Gary Russell commented

Thanks Andy Clement, no problem - this is not a blocker for us; I found it while trying to figure out why one of our expressions wouldn't compile (and fell back to interpreted only).

In that case the ternary else part's exitDescriptor was null and so not compilable - I re-jigged that expression to use a function instead.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 6, 2017

Andy Clement commented

Whenever a bug comes in like this I spend a bit of time playing around with the construct that is having problems because I'm smarter now than when I first wrote that compiler code. Gary is right that the code for inferring the exit type can be deleted but I also uncovered two more issues that I have fixed in the same commit:

  • there were problems with code gen if the value being queried was an empty string as opposed to null.
  • there were verify errors if working with primitive values due to a missing boxing.

This is all fixed up, in addition to Garys proposed change, and blanketed in testcases. I put it in master - Juergen do you want to copy this to other relevant branches?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 17, 2017

Gary Russell commented

Juergen Hoeller Just a heads-up - I see this is marked for a fix in 4.3.7, and resolved, but I don't see a commit on the 4.3.x branch, just master.

This is not critical for my projects, but we should remove the fixVersion if it's not going to be back ported.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 17, 2017

Juergen Hoeller commented

Thanks for raising this, Gary Russell... This was intended for a backport (and therefore marked here) but somehow remained unnoticed. Backporting it right away now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 18, 2017

Andy Clement commented

I think my fault as I didn't use the right construct to tag Juergen in my comment, I'll do it properly next time. Thanks Juergen Hoeller

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.