From f8c55a5e7bc1e12f78a4a270f92a74ff60393885 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Wed, 22 Jun 2016 08:40:20 -0400 Subject: [PATCH] painless: fix disabled loop counter --- .../elasticsearch/painless/MethodWriter.java | 19 ++++++++------- .../org/elasticsearch/painless/node/SDo.java | 4 +++- .../org/elasticsearch/painless/node/SFor.java | 8 +++++-- .../elasticsearch/painless/node/SWhile.java | 8 +++++-- .../painless/BasicStatementTests.java | 23 +++++++++++++++++++ 5 files changed, 47 insertions(+), 15 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 97686655a97..43fd54c51a4 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 @@ -118,17 +118,16 @@ public final class MethodWriter extends GeneratorAdapter { } public void writeLoopCounter(int slot, int count, Location location) { - if (slot > -1) { - writeDebugInfo(location); - final Label end = new Label(); + assert slot != -1; + writeDebugInfo(location); + final Label end = new Label(); - iinc(slot, -count); - visitVarInsn(Opcodes.ILOAD, slot); - push(0); - ifICmp(GeneratorAdapter.GT, end); - throwException(PAINLESS_ERROR_TYPE, "The maximum number of statements that can be executed in a loop has been reached."); - mark(end); - } + iinc(slot, -count); + visitVarInsn(Opcodes.ILOAD, slot); + push(0); + ifICmp(GeneratorAdapter.GT, end); + throwException(PAINLESS_ERROR_TYPE, "The maximum number of statements that can be executed in a loop has been reached."); + mark(end); } public void writeCast(final Cast cast) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java index e214703daf4..62572e54610 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java @@ -113,7 +113,9 @@ public final class SDo extends AStatement { condition.fals = end; condition.write(writer, globals); - writer.writeLoopCounter(loopCounter.getSlot(), Math.max(1, block.statementCount), location); + if (loopCounter != null) { + writer.writeLoopCounter(loopCounter.getSlot(), Math.max(1, block.statementCount), location); + } writer.goTo(start); writer.mark(end); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java index 06ad19d204d..c324682040b 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java @@ -176,10 +176,14 @@ public final class SFor extends AStatement { ++statementCount; } - writer.writeLoopCounter(loopCounter.getSlot(), statementCount, location); + if (loopCounter != null) { + writer.writeLoopCounter(loopCounter.getSlot(), statementCount, location); + } block.write(writer, globals); } else { - writer.writeLoopCounter(loopCounter.getSlot(), 1, location); + if (loopCounter != null) { + writer.writeLoopCounter(loopCounter.getSlot(), 1, location); + } } if (afterthought != null) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java index dff8ae2592f..bd6740dd9b8 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java @@ -113,13 +113,17 @@ public final class SWhile extends AStatement { condition.write(writer, globals); if (block != null) { - writer.writeLoopCounter(loopCounter.getSlot(), Math.max(1, block.statementCount), location); + if (loopCounter != null) { + writer.writeLoopCounter(loopCounter.getSlot(), Math.max(1, block.statementCount), location); + } block.continu = begin; block.brake = end; block.write(writer, globals); } else { - writer.writeLoopCounter(loopCounter.getSlot(), 1, location); + if (loopCounter != null) { + writer.writeLoopCounter(loopCounter.getSlot(), 1, location); + } } if (block == null || !block.allEscape) { diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java index 98921f88c38..01f3ee42ae6 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java @@ -1,5 +1,7 @@ package org.elasticsearch.painless; +import java.util.Collections; + /* * Licensed to Elasticsearch under one or more contributor * license agreements. See the NOTICE file distributed with @@ -223,4 +225,25 @@ public class BasicStatementTests extends ScriptTestCase { assertEquals(10, exec("def i = 1; if (i == 1) {i = 2; return 10}")); assertEquals(10, exec("def i = 1; if (i == 1) {i = 2; return 10} else {return 12}")); } + + public void testArrayLoopWithoutCounter() { + assertEquals(6L, exec("long sum = 0; long[] array = new long[] { 1, 2, 3 };" + + "for (int i = 0; i < array.length; i++) { sum += array[i] } return sum", + Collections.emptyMap(), + Collections.singletonMap(CompilerSettings.MAX_LOOP_COUNTER, "0"), + null + )); + assertEquals(6L, exec("long sum = 0; long[] array = new long[] { 1, 2, 3 };" + + "int i = 0; while (i < array.length) { sum += array[i++] } return sum", + Collections.emptyMap(), + Collections.singletonMap(CompilerSettings.MAX_LOOP_COUNTER, "0"), + null + )); + assertEquals(6L, exec("long sum = 0; long[] array = new long[] { 1, 2, 3 };" + + "int i = 0; do { sum += array[i++] } while (i < array.length); return sum", + Collections.emptyMap(), + Collections.singletonMap(CompilerSettings.MAX_LOOP_COUNTER, "0"), + null + )); + } }