This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new ae55de4 GROOVY-10424: SC: prevent infinite recursion for
`isExtended(ClassNode)`
ae55de4 is described below
commit ae55de4f76c48d0189a1c9491b772e3aafaacc2f
Author: Eric Milles <[email protected]>
AuthorDate: Sat Dec 18 10:56:39 2021 -0600
GROOVY-10424: SC: prevent infinite recursion for `isExtended(ClassNode)`
---
.../transformers/BooleanExpressionTransformer.java | 150 ++++++++++-----------
src/test/groovy/transform/stc/BugsSTCTest.groovy | 36 +++++
2 files changed, 106 insertions(+), 80 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
index 82e8261..671a261 100644
---
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
+++
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
@@ -21,7 +21,6 @@ package org.codehaus.groovy.transform.sc.transformers;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.GroovyCodeVisitor;
-import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.BooleanExpression;
@@ -32,7 +31,6 @@ import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.classgen.asm.BytecodeHelper;
import org.codehaus.groovy.classgen.asm.OperandStack;
import org.codehaus.groovy.classgen.asm.WriterController;
-import org.codehaus.groovy.classgen.asm.sc.StaticTypesTypeChooser;
import org.codehaus.groovy.transform.stc.ExtensionMethodNode;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
@@ -41,9 +39,6 @@ import java.lang.reflect.Modifier;
import java.util.Iterator;
import java.util.List;
-import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
-import static org.codehaus.groovy.ast.ClassHelper.isWrapperBoolean;
import static
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDGMMethodsByNameAndArguments;
import static org.objectweb.asm.Opcodes.DUP;
import static org.objectweb.asm.Opcodes.GOTO;
@@ -52,75 +47,61 @@ import static org.objectweb.asm.Opcodes.IFNONNULL;
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
import static org.objectweb.asm.Opcodes.POP;
-public class BooleanExpressionTransformer {
- private final StaticCompilationTransformer transformer;
+class BooleanExpressionTransformer {
- public BooleanExpressionTransformer(StaticCompilationTransformer
staticCompilationTransformer) {
- transformer = staticCompilationTransformer;
+ private final StaticCompilationTransformer scTransformer;
+
+ BooleanExpressionTransformer(final StaticCompilationTransformer
scTransformer) {
+ this.scTransformer = scTransformer;
}
- Expression transformBooleanExpression(final BooleanExpression
booleanExpression) {
- if (booleanExpression instanceof NotExpression) {
- return transformer.superTransform(booleanExpression);
- }
- final Expression expression = booleanExpression.getExpression();
- if (!(expression instanceof BinaryExpression)) {
- StaticTypesTypeChooser typeChooser = transformer.getTypeChooser();
- final ClassNode type = typeChooser.resolveType(expression,
transformer.getClassNode());
- BooleanExpression transformed = new
OptimizingBooleanExpression(transformer.transform(expression),type);
- transformed.setSourcePosition(booleanExpression);
- transformed.copyNodeMetaData(booleanExpression);
- return transformed;
+ Expression transformBooleanExpression(final BooleanExpression be) {
+ if (!(be instanceof NotExpression || be.getExpression() instanceof
BinaryExpression)) {
+ ClassNode type =
scTransformer.getTypeChooser().resolveType(be.getExpression(),
scTransformer.getClassNode());
+ return optimizeBooleanExpression(be, type, scTransformer);
}
- return transformer.superTransform(booleanExpression);
+ return scTransformer.superTransform(be);
}
- private static boolean isExtended(ClassNode owner,
Iterator<InnerClassNode> classes) {
- while (classes.hasNext()) {
- InnerClassNode next = classes.next();
- if (next!=owner && next.isDerivedFrom(owner)) return true;
- }
- if (owner.getInnerClasses().hasNext()) {
- return isExtended(owner, owner.getInnerClasses());
- }
- return false;
+ private static Expression optimizeBooleanExpression(final
BooleanExpression be, final ClassNode targetType, final ExpressionTransformer
transformer) {
+ Expression opt = new
OptimizingBooleanExpression(transformer.transform(be.getExpression()),
targetType);
+ opt.setSourcePosition(be);
+ opt.copyNodeMetaData(be);
+ return opt;
}
+
//--------------------------------------------------------------------------
+
private static class OptimizingBooleanExpression extends BooleanExpression
{
- private final Expression expression;
private final ClassNode type;
- public OptimizingBooleanExpression(final Expression expression, final
ClassNode type) {
+ OptimizingBooleanExpression(final Expression expression, final
ClassNode type) {
super(expression);
- this.expression = expression;
// we must use the redirect node, otherwise InnerClassNode would
not have the "correct" type
this.type = type.redirect();
}
@Override
public Expression transformExpression(final ExpressionTransformer
transformer) {
- Expression ret = new
OptimizingBooleanExpression(transformer.transform(expression), type);
- ret.setSourcePosition(this);
- ret.copyNodeMetaData(this);
- return ret;
+ return optimizeBooleanExpression(this, type, transformer);
}
@Override
public void visit(final GroovyCodeVisitor visitor) {
if (visitor instanceof AsmClassGenerator) {
- AsmClassGenerator acg = (AsmClassGenerator) visitor;
- WriterController controller = acg.getController();
+ WriterController controller = ((AsmClassGenerator)
visitor).getController();
+ MethodVisitor mv = controller.getMethodVisitor();
OperandStack os = controller.getOperandStack();
- if (isPrimitiveBoolean(type)) {
- expression.visit(visitor);
+ if (ClassHelper.isPrimitiveBoolean(type)) {
+ getExpression().visit(visitor);
os.doGroovyCast(ClassHelper.boolean_TYPE);
return;
}
- if (isWrapperBoolean(type)) {
- MethodVisitor mv = controller.getMethodVisitor();
- expression.visit(visitor);
+
+ if (ClassHelper.isWrapperBoolean(type)) {
+ getExpression().visit(visitor);
Label unbox = new Label();
Label exit = new Label();
// check for null
@@ -130,57 +111,66 @@ public class BooleanExpressionTransformer {
mv.visitInsn(ICONST_0);
mv.visitJumpInsn(GOTO, exit);
mv.visitLabel(unbox);
- // unbox
- // GROOVY-6270
- if (!os.getTopOperand().equals(type))
BytecodeHelper.doCast(mv, type);
+ if (!os.getTopOperand().equals(type))
BytecodeHelper.doCast(mv, type); // GROOVY-6270
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Boolean",
"booleanValue", "()Z", false);
mv.visitLabel(exit);
os.replace(ClassHelper.boolean_TYPE);
return;
}
+
ClassNode top = type;
if (ClassHelper.isPrimitiveType(top)) {
- expression.visit(visitor);
- // in case of null safe invocation, it is possible that
what was supposed to be a primitive type
- // becomes the "null" constant, so we need to recheck
- top = controller.getOperandStack().getTopOperand();
+ getExpression().visit(visitor);
+ // in case of null-safe invocation, it is possible that
what
+ // was supposed to be a primitive type becomes "null"
value,
+ // so we need to recheck
+ top = os.getTopOperand();
if (ClassHelper.isPrimitiveType(top)) {
-
BytecodeHelper.convertPrimitiveToBoolean(controller.getMethodVisitor(), top);
-
controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
+ BytecodeHelper.convertPrimitiveToBoolean(mv, top);
+ os.replace(ClassHelper.boolean_TYPE);
return;
}
}
+
List<MethodNode> asBoolean =
findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(),
top, "asBoolean", ClassNode.EMPTY_ARRAY);
if (asBoolean.size() == 1) {
- MethodNode node = asBoolean.get(0);
- if (node instanceof ExtensionMethodNode) {
- MethodNode dgmNode = ((ExtensionMethodNode)
node).getExtensionMethodNode();
- ClassNode owner = dgmNode.getParameters()[0].getType();
- if (isObjectType(owner)) {
- // we may inline a var!=null check instead of
calling a helper method iff
- // (1) the class doesn't define an asBoolean
method (already tested)
- // (2) no subclass defines an asBoolean method
- // For (2), we check that we are in one of those
cases
- // (a) a final class
- // (b) a private inner class without subclass
- if (Modifier.isFinal(top.getModifiers())
- || (top instanceof InnerClassNode
- && Modifier.isPrivate(top.getModifiers())
- && !isExtended(top,
top.getOuterClass().getInnerClasses()))
- ) {
- CompareToNullExpression expr = new
CompareToNullExpression(
- expression, false
- );
- expr.visit(acg);
- return;
- }
+ MethodNode method = asBoolean.get(0);
+ if (method instanceof ExtensionMethodNode) {
+ ClassNode selfType = (((ExtensionMethodNode)
method).getExtensionMethodNode()).getParameters()[0].getType();
+ // we may inline a var!=null check instead of calling
a helper method iff
+ // (1) the class doesn't define an asBoolean method
(already tested)
+ // (2) no subclass defines an asBoolean method
+ // For (2), check that we are in one of these cases:
+ // (a) a final class
+ // (b) an effectively-final inner class
+ if (ClassHelper.isObjectType(selfType) &&
(Modifier.isFinal(top.getModifiers()) || isEffectivelyFinal(top))) {
+ Expression opt = new
CompareToNullExpression(getExpression(), false);
+ opt.visit(visitor);
+ return;
}
}
}
- super.visit(visitor);
- } else {
- super.visit(visitor);
}
+
+ super.visit(visitor);
+ }
+
+ private static boolean isEffectivelyFinal(final ClassNode type) {
+ if (!Modifier.isPrivate(type.getModifiers())) return false;
+
+ List<ClassNode> outers = type.getOuterClasses();
+ ClassNode outer = outers.get(outers.size() - 1);
+ return !isExtended(type, outer.getInnerClasses());
+ }
+
+ private static boolean isExtended(final ClassNode type, final
Iterator<? extends ClassNode> inners) {
+ while (inners.hasNext()) { ClassNode next = inners.next();
+ if (next != type && next.isDerivedFrom(type))
+ return true;
+ if (isExtended(type, next.getInnerClasses()))
+ return true;
+ }
+ return false;
}
}
-}
\ No newline at end of file
+}
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy
b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index c405b65..488c279 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -1101,4 +1101,40 @@ Printer
assert n == 2
'''
}
+
+ // GROOVY-10424
+ void testPrivateInnerClassOptimizedBooleanExpr1() {
+ assertScript '''
+ class Outer {
+ private static class Inner {
+ private Inner() {} // triggers creation of Inner$1 in
StaticCompilationVisitor$addPrivateBridgeMethods
+ }
+ void test() {
+ def inner = new Inner()
+ if (inner) { // optimized boolean expression;
StackOverflowError
+ assert true
+ }
+ }
+ }
+ new Outer().test()
+ '''
+ }
+
+ // GROOVY-10424
+ void testPrivateInnerClassOptimizedBooleanExpr2() {
+ assertScript '''
+ class Outer {
+ private static class Inner {
+ static class Three {}
+ }
+ void test() {
+ def inner = new Inner()
+ if (inner) { // optimized boolean expression;
StackOverflowError
+ assert true
+ }
+ }
+ }
+ new Outer().test()
+ '''
+ }
}
