Commit a43122d6 authored by Lubomir Bulej's avatar Lubomir Bulej

Simplify changed method tracking and avoid a corner case

If a method was not instrumented but still changed by a class
transformer, we would call the CodeMerger with a null instance
of InstrumentedClass. Given that transformers are not used too
often, this is not much of a problem.

Also log information when duplicating class code to implement
dynamic bypass, and log errors when asked to split method code
(the splitting is not yet implemented).
parent 22d58967
package ch.usi.dag.disl;
public class DiSLInMethodException extends DiSLException {
@SuppressWarnings ("serial")
final class DiSLInMethodException extends DiSLException {
private static final long serialVersionUID = 4819683200958042417L;
public DiSLInMethodException() {
super();
}
public DiSLInMethodException(String message, Throwable cause) {
super(message, cause);
}
public DiSLInMethodException(String message) {
super(message);
}
public DiSLInMethodException(Throwable cause) {
super(cause);
public DiSLInMethodException (
final Throwable cause, final String className, final String methodName
) {
super (cause, "%s.%s", className, methodName);
}
}
package ch.usi.dag.disl;
import ch.usi.dag.disl.dynamicbypass.BypassCheck;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
import org.objectweb.asm.commons.CodeSizeEvaluator;
import org.objectweb.asm.tree.*;
import java.lang.reflect.Method;
import java.util.List;
import java.util.Set;
import java.util.function.IntConsumer;
import java.util.stream.IntStream;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.commons.CodeSizeEvaluator;
import org.objectweb.asm.tree.ClassNode;
import org.objectweb.asm.tree.InsnList;
import org.objectweb.asm.tree.JumpInsnNode;
import org.objectweb.asm.tree.LabelNode;
import org.objectweb.asm.tree.MethodInsnNode;
import org.objectweb.asm.tree.MethodNode;
import org.objectweb.asm.tree.TryCatchBlockNode;
abstract class CodeMerger {
import ch.usi.dag.disl.dynamicbypass.BypassCheck;
private static final Type BPC_CLASS = Type.getType (BypassCheck.class);
import ch.usi.dag.util.logging.Logger;
private static final String BPC_METHOD = "executeUninstrumented";
private static final Type BPC_DESC = Type.getMethodType ("()Z");
abstract class CodeMerger {
private static final Logger __log = Logging.getPackageInstance ();
private static final int ALLOWED_SIZE = 64 * 1024; // 64KB limit
private static final Method executeUninstrumented = ReflectionHelper.getMethod (
BypassCheck.class, "executeUninstrumented"
);
/**
* Method size limit.
*
* @see https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.3
*/
private static final int METHOD_SIZE_LIMIT = 65534;
// NOTE: the instCN ClassNode will be modified in the process
......@@ -29,10 +43,6 @@ abstract class CodeMerger {
final Set <String> changedMethods, final ClassNode origCN,
final ClassNode instCN
) {
if (changedMethods == null) {
throw new DiSLFatalException ("Set of changed methods cannot be null");
}
//
// Select instrumented methods and merge into their code the original
// (un-instrumented) method to be executed when the bypass is active.
......@@ -66,9 +76,7 @@ abstract class CodeMerger {
// <original code>
// }
//
final MethodInsnNode checkNode = AsmHelper.invokeStatic (
BPC_CLASS, BPC_METHOD, BPC_DESC
);
final MethodInsnNode checkNode = AsmHelper.invokeStatic (executeUninstrumented);
instCode.insert (checkNode);
final LabelNode origLabel = new LabelNode ();
......@@ -92,11 +100,11 @@ abstract class CodeMerger {
// instrumented class.
//
final IntConsumer fixupStrategy = splitLongMethods ?
i -> __splitLongMethod (i, instCN, origCN) :
i -> __revertToOriginal (i, instCN, origCN);
mi -> __splitLongMethod (mi, instCN, origCN) :
mi -> __revertToOriginal (mi, instCN, origCN);
IntStream.range (0, instCN.methods.size ()).parallel ().unordered ()
.filter (i -> __methodSize (instCN.methods.get (i)) > ALLOWED_SIZE)
.filter (i -> __methodSize (instCN.methods.get (i)) > METHOD_SIZE_LIMIT)
.forEach (i -> fixupStrategy.accept (i));
return instCN;
......@@ -115,6 +123,13 @@ abstract class CodeMerger {
// method
// add original method to the instrumented code
// rename instrumented method
final MethodNode instMN = instCN.methods.get (methodIndex);
__log.error (
"instrumented method %s.%s%s is too long (%d), but splitting is NOT implemented",
AsmHelper.typeName (instCN), instMN.name, instMN.desc,
__methodSize (instMN)
);
}
......@@ -129,11 +144,10 @@ abstract class CodeMerger {
final MethodNode origMN = __findMethodNode (origCN, instMN.name, instMN.desc);
instCN.methods.set (instIndex, origMN);
System.err.printf (
"warning: method %s.%s%s not instrumented, because its size "+
"(%d) exceeds the maximal allowed method size (%d)\n",
__log.warn (
"instrumented method %s.%s%s is too long (%d), reverted to original",
AsmHelper.typeName (instCN), instMN.name, instMN.desc,
__methodSize (instMN), ALLOWED_SIZE
__methodSize (instMN)
);
}
......
......@@ -373,21 +373,13 @@ public final class DiSL {
ClassNode classNode
) throws DiSLException {
//
// Track changed classes methods. A class can be actually changed
// without changing any methods, i.e., adding thread-local fields
// to the Thread class.
// Instrument each method of the given class and collect names of
// methods that have been changed. Intercept any exceptions and
// propagate them upwards with the name of the method in which
// instrumentation failed.
//
boolean classChanged = false;
final Set <String> changedMethods = new HashSet <> ();
//
// Instrument each method of the given class. Intercept any
// exceptions and propagate them upwards with the name of the
// method in which instrumentation failed.
//
for (final MethodNode methodNode : classNode.methods) {
boolean methodChanged = false;
try {
__log.trace (
"processing [%s] method: %s.%s%s",
......@@ -395,44 +387,48 @@ public final class DiSL {
classNode.name, methodNode.name, methodNode.desc
);
methodChanged = instrumentMethod (classNode, methodNode);
if (instrumentMethod (classNode, methodNode)) {
changedMethods.add (methodNode.name + methodNode.desc);
}
} catch (final DiSLException e) {
throw new DiSLInMethodException (
classNode.name + "." + methodNode.name, e);
}
if (methodChanged) {
changedMethods.add (methodNode.name + methodNode.desc);
classChanged = true;
e, classNode.name, methodNode.name
);
}
}
//
// If the instrumented class is the Thread class, add fields that
// will provide thread-local variables to the code in the snippets.
// Track if a class has been changed. A class can be actually changed
// without changing any methods, i.e., by adding thread-local fields
// to the Thread class.
//
boolean classChanged = !changedMethods.isEmpty ();
//
// If the class being instrumented is java.lang.Thread, add fields to it
// which correspond to @ThreadLocal variables in snippets. If generation
// of dynamic bypass is enabled, add a field containing bypass state.
//
if (Type.getInternalName (Thread.class).equals (classNode.name)) {
// get all thread locals in snippets
final Set <ThreadLocalVar> tlvs = __collectReferencedTLVs (__dislClasses.getSnippets ());
final Set <ThreadLocalVar> tlvs = __collectReferencedTLVs (
__dislClasses.getSnippets ()
);
// dynamic bypass
if (__codeOptions.contains (CodeOption.DYNAMIC_BYPASS)) {
tlvs.add (__createBypassTlv ());
}
if (!tlvs.isEmpty ()) {
// instrument fields
final ClassNode cnWithFields = new ClassNode (Opcodes.ASM5);
classNode.accept (new TLVInserter (cnWithFields, tlvs));
// Add fields to the class and replace the original.
final ClassNode threadWithFields = new ClassNode (Opcodes.ASM6);
classNode.accept (new TLVInserter (threadWithFields, tlvs));
// replace original code with instrumented one
classNode = cnWithFields;
classNode = threadWithFields;
classChanged = true;
}
}
// we have changed some methods
return classChanged ?
new InstrumentedClass (classNode, changedMethods) :
null;
......@@ -482,39 +478,45 @@ public final class DiSL {
// nor by any of the transformers, bail out early and return NULL
// to indicate that the class has not been modified in any way.
//
final InstrumentedClass instResult = instrumentClass (inputCN);
if (instResult == null && transformedBytes == originalBytes) {
return null;
}
final InstrumentedClass instrumentedClass = instrumentClass (inputCN);
if (instrumentedClass != null) {
//
// If creation of bypass code is requested, merge the original method
// code with the instrumented method code and create code to switch
// between the two versions based on the result of a bypass check.
//
// XXX TODO LB: Try to avoid unmarshaling the class again (duplicate it).
//
final ClassNode origCN = ClassNodeHelper.FULL.unmarshal (transformedBytes);
final ClassNode instCN = instrumentedClass.classNode;
if (__codeOptions.contains (CodeOption.CREATE_BYPASS)) {
__log.trace (
"merging original and instrumented code for class: %s", origCN.name
);
// TODO LB: Try to avoid unmarshaling the class again (duplicate it).
final ClassNode origCN = ClassNodeHelper.FULL.unmarshal (transformedBytes);
CodeMerger.mergeOriginalCode (
instrumentedClass.changedMethods, origCN, instCN
);
}
//
// If creating bypass code is requested, merge the original method code
// with the instrumented method code and create code to switch between
// the two versions based on the result of a bypass check.
//
final ClassNode instCN = instResult.classNode;
if (__codeOptions.contains (CodeOption.CREATE_BYPASS)) {
CodeMerger.mergeOriginalCode (
instResult.changedMethods, origCN, instCN
);
}
//
// Fix-up methods that have become too long due to instrumentation.
// To split out the instrumented version of the method, we will need
// to preserve the instrumented version in the previous step.
//
// XXX LB: This will not help long methods produced by transformers.
//
CodeMerger.fixupLongMethods (
__codeOptions.contains (CodeOption.SPLIT_METHODS), origCN, instCN
);
//
// Fix-up methods that have become too long due to instrumentation.
// To split out the instrumented version of the method, we will need
// to preserve the instrumented version in the previous step.
//
// XXX LB: This will not help long methods produced by the transformers.
//
CodeMerger.fixupLongMethods (
__codeOptions.contains (CodeOption.SPLIT_METHODS), origCN, instCN
);
return ClassNodeHelper.marshal (instCN);
return ClassNodeHelper.marshal (instCN);
} else {
// Class was not instrumented, but maybe it was transformed.
return transformedBytes != originalBytes ? transformedBytes : null;
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment