From a7aedbe0a17700a11a94ce097d59c448f7539344 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Fri, 17 Jun 2016 00:30:09 +0200 Subject: [PATCH] Fix compound assignment with string concats. in Java 9 there is no stringbuilder on stack! This closes #18929 --- .../main/java/org/elasticsearch/painless/MethodWriter.java | 7 ++++++- .../java/org/elasticsearch/painless/WriterConstants.java | 2 +- .../main/java/org/elasticsearch/painless/node/EChain.java | 7 ++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java index 1290633879a..6241b5b4207 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java @@ -209,15 +209,20 @@ public final class MethodWriter extends GeneratorAdapter { } } - public void writeNewStrings() { + /** Starts a new string concat. + * @return the size of arguments pushed to stack (the object that does string concats, e.g. a StringBuilder) + */ + public int writeNewStrings() { if (INDY_STRING_CONCAT_BOOTSTRAP_HANDLE != null) { // Java 9+: we just push our argument collector onto deque stringConcatArgs.push(new ArrayList<>()); + return 0; // nothing added to stack } else { // Java 8: create a StringBuilder in bytecode newInstance(STRINGBUILDER_TYPE); dup(); invokeConstructor(STRINGBUILDER_TYPE, STRINGBUILDER_CONSTRUCTOR); + return 1; // StringBuilder on stack } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java index 7906a125f35..6960e906339 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java @@ -131,7 +131,7 @@ public final class WriterConstants { // not Java 9 - we set it null, so MethodWriter uses StringBuilder: bs = null; } - INDY_STRING_CONCAT_BOOTSTRAP_HANDLE = null; // Disabled until https://github.com/elastic/elasticsearch/issues/18929 + INDY_STRING_CONCAT_BOOTSTRAP_HANDLE = bs; } public final static int MAX_INDY_STRING_CONCAT_ARGS = 200; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java index d0d26542f5c..3be7f068261 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java @@ -294,8 +294,9 @@ public final class EChain extends AExpression { // track types going onto the stack. This must be done before the // links in the chain are read because we need the StringBuilder to // be placed on the stack ahead of any potential concatenation arguments. + int catElementStackSize = 0; if (cat) { - writer.writeNewStrings(); + catElementStackSize = writer.writeNewStrings(); } ALink last = links.get(links.size() - 1); @@ -312,7 +313,7 @@ public final class EChain extends AExpression { // Handle the case where we are doing a compound assignment // representing a String concatenation. - writer.writeDup(link.size, 1); // dup the StringBuilder + writer.writeDup(link.size, catElementStackSize); // dup the top element and insert it before concat helper on stack link.load(writer); // read the current link's value writer.writeAppendStrings(link.after); // append the link's value using the StringBuilder @@ -323,7 +324,7 @@ public final class EChain extends AExpression { writer.writeAppendStrings(expression.actual); // append the expression's value unless it's also a concatenation } - writer.writeToStrings(); // put the value of the StringBuilder on the stack + writer.writeToStrings(); // put the value for string concat onto the stack writer.writeCast(back); // if necessary, cast the String to the lhs actual type if (link.load) {