From 155ecd0a76b3bd32c2896eda5f327b14a3c52ec1 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 15 Oct 2019 07:46:29 -0700 Subject: [PATCH] Change Painless regex node to use SField instead of Globals (#47944) * Change Painless regex node to use SField instead of Globals * Use reflection instead of ASM to specify modifiers * Remove synthetic from SField --- .../elasticsearch/painless/ClassWriter.java | 25 +++++++++++++++++++ .../painless/node/ECallLocal.java | 10 +++----- .../elasticsearch/painless/node/ERegex.java | 7 ++++-- .../elasticsearch/painless/node/SClass.java | 10 -------- .../elasticsearch/painless/node/SField.java | 11 ++++---- 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ClassWriter.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ClassWriter.java index 57eb6afbc4e..873af9f73bf 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ClassWriter.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ClassWriter.java @@ -27,6 +27,7 @@ import org.objectweb.asm.util.Printer; import org.objectweb.asm.util.TraceClassVisitor; import java.io.Closeable; +import java.lang.reflect.Modifier; import java.util.BitSet; /** @@ -35,6 +36,30 @@ import java.util.BitSet; */ public class ClassWriter implements Closeable { + /** + * Converts Java reflection modifiers to ASM access constants. + * @param modifiers Java reflection {@code Modifiers} + * @param synthetic {@code true} if the item is synthetically generated + * @return ASM access constants + */ + public static int buildAccess(int modifiers, boolean synthetic) { + int access = synthetic ? Opcodes.ACC_SYNTHETIC : 0; + + if (Modifier.isFinal(modifiers)) access |= Opcodes.ACC_FINAL; + if (Modifier.isInterface(modifiers)) access |= Opcodes.ACC_INTERFACE; + if (Modifier.isNative(modifiers)) access |= Opcodes.ACC_NATIVE; + if (Modifier.isPrivate(modifiers)) access |= Opcodes.ACC_PRIVATE; + if (Modifier.isProtected(modifiers)) access |= Opcodes.ACC_PROTECTED; + if (Modifier.isPublic(modifiers)) access |= Opcodes.ACC_PUBLIC; + if (Modifier.isStatic(modifiers)) access |= Opcodes.ACC_STATIC; + if (Modifier.isStrict(modifiers)) access |= Opcodes.ACC_STRICT; + if (Modifier.isSynchronized(modifiers)) access |= Opcodes.ACC_SYNCHRONIZED; + if (Modifier.isTransient(modifiers)) access |= Opcodes.ACC_TRANSIENT; + if (Modifier.isVolatile(modifiers)) access |= Opcodes.ACC_VOLATILE; + + return access; + } + protected final CompilerSettings compilerSettings; protected final BitSet statements; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java index 40d3a37df6a..b791589d1f8 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java @@ -34,15 +34,13 @@ import org.objectweb.asm.Label; import org.objectweb.asm.Type; import org.objectweb.asm.commons.Method; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Set; import static org.elasticsearch.painless.WriterConstants.CLASS_TYPE; -import static org.objectweb.asm.Opcodes.ACC_PRIVATE; -import static org.objectweb.asm.Opcodes.ACC_PUBLIC; -import static org.objectweb.asm.Opcodes.ACC_STATIC; /** * Represents a user-defined call. @@ -144,13 +142,13 @@ public final class ECallLocal extends AExpression { actual = classBinding.returnType; bindingName = scriptRoot.getNextSyntheticName("class_binding"); scriptRoot.getClassNode().addField(new SField(location, - ACC_PRIVATE, bindingName, classBinding.javaConstructor.getDeclaringClass(), null)); + Modifier.PRIVATE, bindingName, classBinding.javaConstructor.getDeclaringClass(), null)); } else if (instanceBinding != null) { typeParameters = new ArrayList<>(instanceBinding.typeParameters); actual = instanceBinding.returnType; bindingName = scriptRoot.getNextSyntheticName("instance_binding"); - scriptRoot.getClassNode().addField(new SField(location, - ACC_STATIC | ACC_PUBLIC, bindingName, instanceBinding.targetInstance.getClass(), instanceBinding.targetInstance)); + scriptRoot.getClassNode().addField(new SField(location, Modifier.STATIC | Modifier.PUBLIC, + bindingName, instanceBinding.targetInstance.getClass(), instanceBinding.targetInstance)); } else { throw new IllegalStateException("Illegal tree structure."); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ERegex.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ERegex.java index 0c8b35db3ac..7ac10be445f 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ERegex.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ERegex.java @@ -29,6 +29,7 @@ import org.elasticsearch.painless.MethodWriter; import org.elasticsearch.painless.ScriptRoot; import org.elasticsearch.painless.WriterConstants; +import java.lang.reflect.Modifier; import java.util.Set; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; @@ -87,8 +88,10 @@ public final class ERegex extends AExpression { new IllegalArgumentException("Error compiling regex: " + e.getDescription())); } - constant = new Constant( - location, MethodWriter.getType(Pattern.class), "regexAt$" + location.getOffset(), this::initializeConstant); + String name = scriptRoot.getNextSyntheticName("regex"); + scriptRoot.getClassNode().addField( + new SField(location, Modifier.FINAL | Modifier.STATIC | Modifier.PRIVATE, name, Pattern.class, null)); + constant = new Constant(location, MethodWriter.getType(Pattern.class), name, this::initializeConstant); actual = Pattern.class; } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java index e9776d0abba..9099dc175a5 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java @@ -303,16 +303,6 @@ public final class SClass extends AStatement { if (false == globals.getConstantInitializers().isEmpty()) { Collection inits = globals.getConstantInitializers().values(); - // Fields - for (Constant constant : inits) { - classVisitor.visitField( - Opcodes.ACC_FINAL | Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC, - constant.name, - constant.type.getDescriptor(), - null, - null).visitEnd(); - } - // Initialize the constants in a static initializer final MethodWriter clinit = new MethodWriter(Opcodes.ACC_STATIC, WriterConstants.CLINIT, classVisitor, globals.getStatements(), settings); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SField.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SField.java index fbacb6867fd..f12e5797cb2 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SField.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SField.java @@ -35,7 +35,7 @@ import java.util.Set; */ public class SField extends ANode { - private final int access; + private final int modifiers; private final String name; private final Class type; private final Object instance; @@ -43,15 +43,15 @@ public class SField extends ANode { /** * Standard constructor. * @param location original location in the source - * @param access asm constants for field modifiers + * @param modifiers java modifiers for the field * @param name name of the field * @param type type of the field * @param instance initial value for the field */ - public SField(Location location, int access, String name, Class type, Object instance) { + public SField(Location location, int modifiers, String name, Class type, Object instance) { super(location); - this.access = access; + this.modifiers = modifiers; this.name = name; this.type = type; this.instance = instance; @@ -86,7 +86,8 @@ public class SField extends ANode { } void write(ClassWriter classWriter) { - classWriter.getClassVisitor().visitField(access, name, Type.getType(type).getDescriptor(), null, null).visitEnd(); + classWriter.getClassVisitor().visitField( + ClassWriter.buildAccess(modifiers, true), name, Type.getType(type).getDescriptor(), null, null).visitEnd(); } @Override