diff --git a/Makefile b/Makefile index 6a80b5343..3b2ba7c24 100644 --- a/Makefile +++ b/Makefile @@ -10,9 +10,13 @@ CHECKLINK ?= ${DAIKONDIR}/.utils/checklink PLUME_SCRIPTS ?= ${DAIKONDIR}/.utils/plume-scripts ifeq (,$(wildcard ${PLUME_SCRIPTS})) - dummy := $(shell mkdir ${DAIKONDIR}/.utils && git clone --depth=1 -q https://github.com/plume-lib/plume-scripts.git ${PLUME_SCRIPTS}) + dummy := $(shell mkdir -p "$(dir ${PLUME_SCRIPTS})" && git clone --depth=1 -q https://github.com/plume-lib/plume-scripts.git "${PLUME_SCRIPTS}") endif SORT_DIRECTORY_ORDER := ${PLUME_SCRIPTS}/sort-directory-order +ifneq "$(wildcard ${SORT_DIRECTORY_ORDER})" "${SORT_DIRECTORY_ORDER}" + # The clone above did not happen or did not succeed, so sort-directory-order is not available. + SORT_DIRECTORY_ORDER := sort +endif JAVA_RELEASE_NUMBER := $(shell java -version 2>&1 | head -1 | cut -d'"' -f2 | sed '/^1\./s///' | cut -d'.' -f1 | sed 's/-ea//') diff --git a/java/daikon/chicory/Instrument24.java b/java/daikon/chicory/Instrument24.java index 93753eef5..17aebebb5 100644 --- a/java/daikon/chicory/Instrument24.java +++ b/java/daikon/chicory/Instrument24.java @@ -762,6 +762,9 @@ private void instrumentCode( MethodInfo curMethodInfo, int method_info_index) { + // This handler modifies mgen, and may be run more than once. + mgen.resetForCodeBuilder(); + MethodGen24.MInfo24 minfo = new MethodGen24.MInfo24(method_info_index, mgen.getMaxLocals(), codeBuilder); diff --git a/java/daikon/chicory/MethodGen24.java b/java/daikon/chicory/MethodGen24.java index 5fdcdc7d7..c426ab7cd 100644 --- a/java/daikon/chicory/MethodGen24.java +++ b/java/daikon/chicory/MethodGen24.java @@ -155,6 +155,12 @@ public class MethodGen24 { // TODO: Should uses of this be synchronized? private ConstantPoolBuilder poolBuilder; + /** + * The mutable state of this method, as recorded by the first call to {@link + * #resetForCodeBuilder}. Null until then. + */ + private @Nullable State savedState; + /** Information about the current method. */ public static class MInfo24 { @@ -766,6 +772,71 @@ public void setInstructionList(List il) { codeList = il; } + /** + * A copy of the mutable state of a MethodGen24; see {@link MethodGen24#resetForCodeBuilder}. + * These are the fields that instrumentation modifies. The arrays and lists are copies, so that a + * State is unaffected by later modifications to the MethodGen24 it was made from. + * + * @param codeList a copy of {@link MethodGen24#codeList} + * @param localsTable a copy of {@link MethodGen24#localsTable} + * @param maxLocals the value of {@link MethodGen24#maxLocals} + * @param paramTypes a copy of {@link MethodGen24#paramTypes} + * @param paramNames a copy of {@link MethodGen24#paramNames} + * @param origLocalVariables a copy of {@link MethodGen24#origLocalVariables} + */ + @SuppressWarnings("ArrayRecordComponent") // defensive copies previent mutation of array fields + private record State( + List codeList, + List localsTable, + int maxLocals, + ClassDesc[] paramTypes, + @Identifier String[] paramNames, + LocalVariable[] origLocalVariables) {} + + /** + * Undoes every modification made to this MethodGen24 since the first call to this method, and + * returns true if this is that first call. + * + *

Call this at the top of every {@code CodeBuilder} handler that modifies this MethodGen24. + * The java.lang.classfile implementation may run such a handler more than once: if the code the + * handler built contains a branch whose target does not fit in the branch instruction's 2-byte + * operand, the implementation discards what was built and runs the handler again, this time + * widening those branches. The second run starts from a fresh CodeBuilder, but not from a fresh + * MethodGen24, so without this call the handler's modifications -- adding the DCompMarker + * parameter and renumbering the locals that follow it, for instance -- would be applied a second + * time to a MethodGen24 that already has them. + * + *

A handler that has other side effects must use the return value to perform them only once. + * + * @return true if this is the first call to this method on this MethodGen24 + */ + public boolean resetForCodeBuilder() { + State state = savedState; + if (state == null) { + savedState = + new State( + new ArrayList<>(codeList), + new ArrayList<>(localsTable), + maxLocals, + paramTypes.clone(), + paramNames.clone(), + origLocalVariables.clone()); + return true; + } + // As in the constructor, a LinkedList is the right choice for codeList. + @SuppressWarnings("JdkObsolete") + List cl = new LinkedList(state.codeList()); + codeList = cl; + // Modify localsTable in place, because clients hold references to it. + localsTable.clear(); + localsTable.addAll(state.localsTable()); + maxLocals = state.maxLocals(); + paramTypes = state.paramTypes().clone(); + paramNames = state.paramNames().clone(); + origLocalVariables = state.origLocalVariables().clone(); + return false; + } + /** * Returns string representation close to declaration format, 'public static void main(String[])', * e.g. diff --git a/java/daikon/chicory/Runtime.java b/java/daikon/chicory/Runtime.java index 56fd23db4..ac1947dfb 100644 --- a/java/daikon/chicory/Runtime.java +++ b/java/daikon/chicory/Runtime.java @@ -931,9 +931,11 @@ public Class primitiveClass() { } } + /** The major version of the running JVM: 8 for Java 8, 24 for Java 24, and so on. */ + private static final int javaMajorVersion = javaMajorVersion(System.getProperty("java.version")); + /** True if the running JVM is for Java 9 or later. */ - private static final boolean isJava9orLater = - !System.getProperty("java.version").startsWith("1."); + private static final boolean isJava9orLater = javaMajorVersion >= 9; /** * Returns true if the running JVM is for Java 9 or later. @@ -945,10 +947,7 @@ public static boolean isJava9orLater() { } /** True if the running JVM is for Java 24 or later. */ - private static final boolean isJava24orLater = - !System.getProperty("java.version").startsWith("1.") - && !System.getProperty("java.version").startsWith("9.") - && Integer.parseInt(System.getProperty("java.version").substring(0, 2)) >= 24; + private static final boolean isJava24orLater = javaMajorVersion >= 24; /** * Returns true if the running JVM is for Java 24 or later. @@ -958,4 +957,41 @@ public static boolean isJava9orLater() { public static boolean isJava24orLater() { return isJava24orLater; } + + /** + * Returns the major version encoded in a {@code java.version} property value: 8 for Java 8, 24 + * for Java 24, and so on. + * + *

Both version schemes are accepted: the pre-Java-9 {@code "1.8.0_432"} form, whose major + * version is its second component, and the Java 9 and later {@code "24"}, {@code "24.0.1"}, and + * {@code "24-ea"} forms, whose major version is the first. In the latter scheme the major version + * may stand alone with no separator at all, as it does for a GA release such as {@code "9"}. + * + *

Only the leading digits are examined; anything after them is ignored rather than rejected, + * so {@code "9foo"} yields 9. That is deliberate. This runs from a static initializer, so + * throwing on an unrecognized suffix would turn an unanticipated vendor version string into an + * ExceptionInInitializerError inside an instrumented program, which is the failure this method + * exists to prevent. Ignoring the suffix instead yields the right major version for any string + * that begins with one. A value with no leading digits at all is still rejected. + * + * @param version the value of the {@code java.version} system property + * @return the major version it encodes + */ + // Package-private rather than private so that RuntimeTest can exercise it directly; the value + // derived from the running JVM is fixed at class-initialization time and cannot be varied. + static int javaMajorVersion(String version) { + // Java 8 and earlier report "1.N..."; the major version is the second component. + String rest = version.startsWith("1.") ? version.substring(2) : version; + // The major version is the leading run of digits. What follows it is "." for a release with + // minor components, "-" or "+" for a pre-release or build identifier, and nothing at all for a + // bare GA release such as "9". + int end = 0; + while (end < rest.length() && Character.isDigit(rest.charAt(end))) { + end++; + } + if (end == 0) { + throw new IllegalArgumentException("Cannot parse java.version: " + version); + } + return Integer.parseInt(rest.substring(0, end)); + } } diff --git a/java/daikon/chicory/RuntimeTest.java b/java/daikon/chicory/RuntimeTest.java new file mode 100644 index 000000000..1d46873f6 --- /dev/null +++ b/java/daikon/chicory/RuntimeTest.java @@ -0,0 +1,50 @@ +package daikon.chicory; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link daikon.chicory.Runtime}. */ +@RunWith(JUnit4.class) +public class RuntimeTest { + + /** + * Tests {@link Runtime#javaMajorVersion}. The values are real {@code java.version} strings; the + * bare "9" is the one that matters most, because Java 9 GA reported its version with no separator + * after the major number. + */ + @Test + public void testJavaMajorVersion() { + // Java 8 and earlier: the major version is the second component. + assertEquals(8, Runtime.javaMajorVersion("1.8.0_432")); + assertEquals(7, Runtime.javaMajorVersion("1.7.0_80")); + // Java 9 and later, with minor components. + assertEquals(9, Runtime.javaMajorVersion("9.0.4")); + assertEquals(11, Runtime.javaMajorVersion("11.0.28")); + assertEquals(24, Runtime.javaMajorVersion("24.0.1")); + assertEquals(25, Runtime.javaMajorVersion("25.0.2")); + // Java 9 and later, major version alone. + assertEquals(9, Runtime.javaMajorVersion("9")); + assertEquals(24, Runtime.javaMajorVersion("24")); + // Pre-release and build identifiers. + assertEquals(9, Runtime.javaMajorVersion("9-ea")); + assertEquals(26, Runtime.javaMajorVersion("26-ea")); + assertEquals(26, Runtime.javaMajorVersion("26+11")); + + // Trailing junk after the major version is ignored rather than rejected; this is deliberate, + // and javaMajorVersion's javadoc says why. A value with no leading digits still throws. + assertEquals(9, Runtime.javaMajorVersion("9foo")); + assertThrows(IllegalArgumentException.class, () -> Runtime.javaMajorVersion("bogus")); + } + + /** Tests that {@link Runtime#isJava24orLater} agrees with the JVM running the test. */ + @Test + public void testIsJava24orLater() { + int major = Runtime.javaMajorVersion(System.getProperty("java.version")); + assertEquals(major >= 24, Runtime.isJava24orLater()); + assertEquals(major >= 9, Runtime.isJava9orLater()); + } +} diff --git a/java/daikon/dcomp-dummy/DCRuntime.class.dummy b/java/daikon/dcomp-dummy/DCRuntime.class.dummy index 4f1be70e7..2f76b80f1 100644 Binary files a/java/daikon/dcomp-dummy/DCRuntime.class.dummy and b/java/daikon/dcomp-dummy/DCRuntime.class.dummy differ diff --git a/java/daikon/dcomp-dummy/DCRuntime.java.dummy b/java/daikon/dcomp-dummy/DCRuntime.java.dummy index 3d3acfabe..f4d4db275 100644 --- a/java/daikon/dcomp-dummy/DCRuntime.java.dummy +++ b/java/daikon/dcomp-dummy/DCRuntime.java.dummy @@ -54,6 +54,15 @@ public final class DCRuntime { public static void normal_exit_primitive(Object[] tag_frame) { } + public static void uninstrumented_enter(int tagCount) { + } + + public static void uninstrumented_exit() { + } + + public static void uninstrumented_exit_primitive() { + } + public static void exception_exit(Object throwable) { } diff --git a/java/daikon/dcomp-transfer/DCRuntime.java b/java/daikon/dcomp-transfer/DCRuntime.java index 7d393162c..563f049dc 100644 --- a/java/daikon/dcomp-transfer/DCRuntime.java +++ b/java/daikon/dcomp-transfer/DCRuntime.java @@ -56,6 +56,18 @@ public static void normal_exit_primitive(Object[] tag_frame) { daikon.dcomp.DCRuntime.normal_exit_primitive(tag_frame); } + public static void uninstrumented_enter(int tagCount) { + daikon.dcomp.DCRuntime.uninstrumented_enter(tagCount); + } + + public static void uninstrumented_exit() { + daikon.dcomp.DCRuntime.uninstrumented_exit(); + } + + public static void uninstrumented_exit_primitive() { + daikon.dcomp.DCRuntime.uninstrumented_exit_primitive(); + } + public static void exception_exit(Object throwable) { daikon.dcomp.DCRuntime.exception_exit(throwable); } diff --git a/java/daikon/dcomp/ClassGen24.java b/java/daikon/dcomp/ClassGen24.java index c7d7df08a..6c6e334dc 100644 --- a/java/daikon/dcomp/ClassGen24.java +++ b/java/daikon/dcomp/ClassGen24.java @@ -63,9 +63,6 @@ public class ClassGen24 { /** True if this class is an interface. */ private final boolean isInterface; - /** True if this class is static. */ - private final boolean isStatic; - /** * Creates a ClassGen24 object. * @@ -84,7 +81,6 @@ public ClassGen24( accessFlags = classModel.flags(); isInterface = accessFlags.has(AccessFlag.INTERFACE); - isStatic = accessFlags.has(AccessFlag.STATIC); superclassName = getSuperclassName(classModel); @@ -146,15 +142,6 @@ public final boolean isInterface() { return isInterface; } - /** - * Returns true if this class is static. - * - * @return true if this class is static - */ - public final boolean isStatic() { - return isStatic; - } - /** * Returns this class's name, in binary format. * diff --git a/java/daikon/dcomp/DCInstrument.java b/java/daikon/dcomp/DCInstrument.java index 91dd052c6..244a65b2b 100644 --- a/java/daikon/dcomp/DCInstrument.java +++ b/java/daikon/dcomp/DCInstrument.java @@ -729,6 +729,11 @@ public JavaClass instrument() { boolean junit_test_class = false; + // Skipped for JDK classes. A JDK class is never a JUnit test class: the check below confirms + // one only by a junit.framework.TestCase superclass or an org/junit/Test annotation. Skipping + // them loses no state transition -- STARTING and TEST_DISCOVERY are re-evaluated on the next + // class load, and the test classes themselves are never in the JDK -- and it keeps a + // getStackTrace, plus TEST_DISCOVERY's superclass walk, off the JDK class-loading path. if (!in_jdk) { // A very tricky special case: If JUnit is running and the current // class has been passed to JUnit on the command line, then this @@ -996,29 +1001,13 @@ public JavaClass instrument() { remove_local_variable_type_table(mgen); - // Do not copy any problematic annotations from the original - // method to our instrumented method. - AnnotationEntryGen[] aes = mgen.getAnnotationEntries(); - for (AnnotationEntryGen item : aes) { - String type = item.getTypeName(); - if (BLACKLISTED_ANNOTATIONS.contains(type)) { - mgen.removeAnnotationEntry(item); - } - } + remove_blacklisted_annotations(mgen); // Can't duplicate "main" or "clinit" or a JUnit test. boolean replacingMethod = BcelUtil.isMain(mgen) || BcelUtil.isClinit(mgen) || junit_test_class; try { - if (has_code) { - il = mgen.getInstructionList(); - InstructionHandle end = il.getEnd(); - int length = end.getPosition() + end.getInstruction().getLength(); - if (length >= MAX_CODE_SIZE) { - throw new ClassGenException( - "Code array too big: must be smaller than " + MAX_CODE_SIZE + " bytes."); - } - } + check_code_size(mgen); if (replacingMethod) { classGen.replaceMethod(m, mgen.getMethod()); if (BcelUtil.isMain(mgen)) { @@ -1028,36 +1017,51 @@ public JavaClass instrument() { classGen.addMethod(mgen.getMethod()); } } catch (Exception e) { - String s = e.getMessage(); - if (s == null) { + if (!is_code_size_error(e)) { throw e; } - if (s.startsWith("Branch target offset too large") - || s.startsWith("Code array too big")) { - System.err.printf( - "DynComp warning: ClassFile: %s - method %s has too many bytecodes to instrument" - + " and is being skipped.%n", - classname, mgen.getName()); - // Build a dummy instrumented method that has DCompMarker - // parameter and no instrumentation. - // first, restore unmodified method - mgen = new MethodGen(m, classname, pool); - // restore StackMapTable - setCurrentStackMapTable(mgen, classGen.getMajor()); - // Add the DCompMarker parameter - add_dcomp_param(mgen); - remove_local_variable_type_table(mgen); - // try again - if (replacingMethod) { - classGen.replaceMethod(m, mgen.getMethod()); - if (BcelUtil.isMain(mgen)) { - classGen.addMethod(create_dcomp_stub(mgen).getMethod()); - } + System.err.printf( + "DynComp warning: ClassFile: %s - method %s has too many bytecodes to instrument and" + + " is being skipped.%n", + classname, m.getName()); + // Restore the unmodified method, to recover its original signature. + MethodGen original = new MethodGen(m, classname, pool); + if (replacingMethod) { + // A JUnit method keeps its original descriptor, but its instrumented callers leave + // primitive argument tags for it to consume. Main and use the ordinary + // uninstrumented calling convention, so they are emitted unchanged. + debugInstrument.log( + "Copying oversized method without instrumentation: %s%n", original.getName()); + debugInstrument.indent(); + // No add_dcomp_param call here: it returns early for main and . For the + // remaining case -- a JUnit test class -- it would append the marker and alter the + // descriptor, so omit the call to preserve JUnit discovery. That matches the normal + // path above, which adds the marker only if !junit_test_class. + // + // junit_test_class is a property of the class, not of the method, so main and + // are excluded explicitly: they use the uninstrumented calling convention even in a + // JUnit test class, and create_oversized_method cannot give them the DCompMarker + // overload that its fallback stub forwards to. + if (junit_test_class && !BcelUtil.isMain(original) && !BcelUtil.isClinit(original)) { + classGen.replaceMethod(m, create_oversized_method(m, false)); } else { - classGen.addMethod(mgen.getMethod()); + remove_local_variable_type_table(original); + classGen.replaceMethod(m, original.getMethod()); + } + if (BcelUtil.isMain(original)) { + classGen.addMethod(create_dcomp_stub(original).getMethod()); } + debugInstrument.exdent(); + debugInstrument.log("End of copy%n"); } else { - throw e; + // Emit a minimally instrumented copy with the DCompMarker parameter that maintains + // the tag stack; see create_oversized_method. + debugInstrument.log( + "Oversized method, creating minimally instrumented copy: %s%n", original.getName()); + debugInstrument.indent(); + classGen.addMethod(create_oversized_method(m, true)); + debugInstrument.exdent(); + debugInstrument.log("End of copy%n"); } } debug_transform.exdent(); @@ -1280,52 +1284,27 @@ public JavaClass instrument_jdk_class() { remove_local_variable_type_table(mgen); - // Do not copy any problematic annotations from the original - // method to our instrumented method. - AnnotationEntryGen[] aes = mgen.getAnnotationEntries(); - for (AnnotationEntryGen item : aes) { - String type = item.getTypeName(); - if (BLACKLISTED_ANNOTATIONS.contains(type)) { - mgen.removeAnnotationEntry(item); - } - } + remove_blacklisted_annotations(mgen); try { - if (has_code) { - il = mgen.getInstructionList(); - InstructionHandle end = il.getEnd(); - int length = end.getPosition() + end.getInstruction().getLength(); - if (length >= MAX_CODE_SIZE) { - throw new ClassGenException( - "Code array too big: must be smaller than " + MAX_CODE_SIZE + " bytes."); - } - } + check_code_size(mgen); classGen.addMethod(mgen.getMethod()); } catch (Exception e) { - String s = e.getMessage(); - if (s == null) { - throw e; - } - if (s.startsWith("Branch target offset too large") - || s.startsWith("Code array too big")) { - System.err.printf( - "DynComp warning: ClassFile: %s - method %s has too many bytecodes to instrument" - + " and is being skipped.%n", - classname, mgen.getName()); - // Build a dummy instrumented method that has DCompMarker - // parameter and no instrumentation. - // first, restore unmodified method - mgen = new MethodGen(m, classname, pool); - // restore StackMapTable - setCurrentStackMapTable(mgen, classGen.getMajor()); - // Add the DCompMarker parameter - add_dcomp_param(mgen); - remove_local_variable_type_table(mgen); - // try again - classGen.addMethod(mgen.getMethod()); - } else { + if (!is_code_size_error(e)) { throw e; } + System.err.printf( + "DynComp warning: ClassFile: %s - method %s has too many bytecodes to instrument and" + + " is being skipped.%n", + classname, m.getName()); + // Emit a minimally instrumented copy with the DCompMarker parameter that maintains the + // tag stack; see create_oversized_method. + debugInstrument.log( + "Oversized method, creating minimally instrumented copy: %s%n", m.getName()); + debugInstrument.indent(); + classGen.addMethod(create_oversized_method(m, true)); + debugInstrument.exdent(); + debugInstrument.log("End of copy%n"); } debug_transform.exdent(); @@ -1333,10 +1312,7 @@ public JavaClass instrument_jdk_class() { if (debugInstrument.enabled) { t.printStackTrace(); } - // TODO: Is it guaranteed that mgen is non-null by the time control reaches here? - if (mgen != null) { - skip_method(mgen); - } + skip_method(classname, m.getName()); if (quit_if_error) { throw new Error("Error processing " + classname + "." + m.getName(), t); } else { @@ -1467,7 +1443,19 @@ public void instrumentMethod(MethodGen mgen) { * @param m method to add to skipped_methods list */ void skip_method(MethodGen m) { - skipped_methods.add(m.getClassName() + "." + m.getName()); + skip_method(m.getClassName(), m.getName()); + } + + /** + * Adds the method name and containing class name to {@code skip_methods}, the list of + * uninstrumented methods. Use this overload where instrumentation may have failed before the + * method's {@link MethodGen} was built. + * + * @param classname the class that contains the method + * @param methodName the name of the method + */ + void skip_method(String classname, String methodName) { + skipped_methods.add(classname + "." + methodName); } /** @@ -1502,6 +1490,22 @@ public void build_exception_handler(MethodGen mgen) { /** Adds a try/catch block around the entire method. */ public void add_exception_handler(MethodGen mgen, InstructionList catch_il) { + InstructionList cur_il = mgen.getInstructionList(); + add_exception_handler(mgen, catch_il, cur_il.getStart(), cur_il.getEnd()); + } + + /** + * Adds a try/catch block around the given range of the method. Use this overload where the + * handler must not cover the whole method, such as when the method's first instructions establish + * the state that the handler undoes. + * + * @param mgen the method to add the handler to + * @param catch_il the code of the handler, which is entered with the throwable on the stack + * @param start the first instruction the handler covers + * @param end the last instruction the handler covers + */ + public void add_exception_handler( + MethodGen mgen, InstructionList catch_il, InstructionHandle start, InstructionHandle end) { // methods (constructors) are problematic // for adding a whole-method exception handler. The start of @@ -1516,10 +1520,6 @@ public void add_exception_handler(MethodGen mgen, InstructionList catch_il) { } } - InstructionList cur_il = mgen.getInstructionList(); - InstructionHandle start = cur_il.getStart(); - InstructionHandle end = cur_il.getEnd(); - // This is just a temporary handler to get the start and end // address tracked as we make code modifications. global_catch_il = catch_il; @@ -4418,6 +4418,473 @@ boolean is_class_initialized_by_jvm(String classname) { return false; } + /** + * Returns a minimally instrumented copy of a method whose fully instrumented form would exceed + * the JVM's 64K code-size limit. The copy retains the original body rather than forwarding to + * another method, which preserves caller-sensitive and exception-stack semantics. + * + *

The caller leaves a tag on the tag stack for each primitive argument, so this method + * discards those tags on entry. If {@code addDcompMarker} is true, the caller also expects the + * method to produce a tag for a primitive result, so this method pushes one immediately before + * each primitive return. + * + *

A JUnit method has no marker, so its original descriptor is the one its callers use and this + * copy replaces the instrumented version altogether; see {@link #create_oversized_method}. Such a + * method enters with a caller-produced result tag above its argument tags, and the body it + * retains pushes no argument tags for the calls it makes, even though a method of a JUnit test + * class does consume them. So the body is bracketed by {@code DCRuntime.uninstrumented_enter} and + * {@code DCRuntime.uninstrumented_exit}, which discard the caller's tags, keep the body's calls + * from consuming tags that belong to an outer frame, and push the replacement result tag on the + * way out. A catch-all handler performs the same cleanup when the body throws; see {@link + * #uninstrumented_catch_il}. + * + * @param mgen the unmodified method, with its original signature + * @param addDcompMarker whether to append the DCompMarker parameter + * @return a minimally instrumented copy of {@code mgen} + * @throws IOException if the method cannot be built + */ + MethodGen create_oversized_method_copy(MethodGen mgen, boolean addDcompMarker) + throws IOException { + + InstructionList il = mgen.getInstructionList(); + if (il == null) { + // Only a method with code can be oversized. Returning mgen unchanged would ignore + // addDcompMarker, and the caller would add a method whose descriptor is already in use. + throw new ClassGenException("No instruction list for oversized method " + mgen.getName()); + } + + setCurrentStackMapTable(mgen, classGen.getMajor()); + buildUninitializedNewMap(il); + + Type[] paramTypes = mgen.getArgumentTypes(); + if (addDcompMarker) { + fixLocalVariableTable(mgen); + add_dcomp_param(mgen); + } + + int primitiveCount = 0; + for (Type paramType : paramTypes) { + if (is_primitive(paramType)) { + primitiveCount++; + } + } + boolean primitiveResult = is_primitive(mgen.getReturnType()); + if (addDcompMarker) { + // The uninstrumented body's calls use the original descriptors, which name the + // uninstrumented methods, so nothing it calls touches the tag stack. + if (primitiveCount > 0) { + InstructionList entryCode = new InstructionList(); + entryCode.append(ifact.createConstant(primitiveCount)); + entryCode.append(dcr_call("discard_tag", CD_void, intSig)); + insertAtMethodStart(mgen, entryCode); + } + if (primitiveResult) { + for (InstructionHandle ih = il.getStart(); ih != null; ) { + InstructionHandle next = ih.getNext(); + Instruction instruction = ih.getInstruction(); + if (instruction instanceof ReturnInstruction) { + InstructionList returnCode = new InstructionList(); + returnCode.append(dcr_call("push_const", CD_void, noArgsSig)); + returnCode.append(instruction); + replaceInstructions(mgen, il, ih, returnCode); + } + ih = next; + } + } + } else { + // A JUnit method replaces the instrumented version, so the calls its uninstrumented body + // makes reach instrumented methods that expect argument tags. Bracket the body; see the + // method comment. + InstructionList entryCode = new InstructionList(); + entryCode.append(ifact.createConstant(primitiveCount + (primitiveResult ? 1 : 0))); + entryCode.append(dcr_call("uninstrumented_enter", CD_void, intSig)); + // The body must be bracketed on an exceptional exit as well as on a return. Record the + // handler's range before the entry code is inserted, so that the range starts after + // uninstrumented_enter rather than covering it. + add_exception_handler(mgen, uninstrumented_catch_il(), il.getStart(), il.getEnd()); + insertAtMethodStart(mgen, entryCode); + + String exitMethod = primitiveResult ? "uninstrumented_exit_primitive" : "uninstrumented_exit"; + for (InstructionHandle ih = il.getStart(); ih != null; ) { + InstructionHandle next = ih.getNext(); + Instruction instruction = ih.getInstruction(); + if (instruction instanceof ReturnInstruction) { + InstructionList returnCode = new InstructionList(); + returnCode.append(dcr_call(exitMethod, CD_void, noArgsSig)); + returnCode.append(instruction); + replaceInstructions(mgen, il, ih, returnCode); + } + ih = next; + } + assert stackMapTable != null + : "@AssumeAssertion(nullness): set by setCurrentStackMapTable above"; + install_exception_handler(mgen); + } + + updateUninitializedNewOffsets(il); + createNewStackMapAttribute(mgen); + remove_blacklisted_annotations(mgen); + remove_local_variable_type_table(mgen); + mgen.setMaxLocals(); + mgen.setMaxStack(); + return mgen; + } + + /** + * Returns a minimally instrumented copy of a method whose fully instrumented form exceeds the + * JVM's 64K code-size limit; see {@link #create_oversized_method_copy}. The tag-stack bookkeeping + * that copy adds is only a few bytes long, but the method is already near the limit, so the copy + * can exceed the limit too. If it does, this method emits a small forwarding stub that performs + * the bookkeeping and calls the unchanged original method. + * + * @param m the unmodified method, with its original signature + * @param addDcompMarker whether to append the DCompMarker parameter + * @return a minimally instrumented copy of {@code m} + * @throws IOException if the method cannot be built + */ + Method create_oversized_method(Method m, boolean addDcompMarker) throws IOException { + + String classname = classGen.getClassName(); + try { + MethodGen copy = + create_oversized_method_copy(new MethodGen(m, classname, pool), addDcompMarker); + check_code_size(copy); + return copy.getMethod(); + } catch (Exception e) { + if (!is_code_size_error(e)) { + throw e; + } + System.err.printf( + "DynComp warning: ClassFile: %s - method %s is too large even for the minimal" + + " instrumentation; a forwarding stub is being used.%n", + classname, m.getName()); + } + + MethodGen mgen = new MethodGen(m, classname, pool); + if (addDcompMarker) { + return create_oversized_method_stub(mgen).getMethod(); + } + + // A JUnit method must retain its original descriptor, so use that descriptor for the small + // bookkeeping stub and put the unchanged body in a private DCompMarker overload. The caller + // and stub then agree about every primitive argument and result tag even when the original body + // has no room for a single additional instruction. + MethodGen body = new MethodGen(m, classname, pool); + boolean bodyHasMarker = true; + try { + InstructionList bodyIl = body.getInstructionList(); + assert bodyIl != null + : "@AssumeAssertion(nullness): create_oversized_method_copy rejects a method with no" + + " code, and that rejection is not a code-size error, so it was rethrown above"; + // add_dcomp_param renumbers the locals that follow the new parameter, and may widen the + // instructions that reference them, so the stack map has to be rebuilt from the original. + // This also discards the stale stackMapTable left behind by the abandoned attempt above. + setCurrentStackMapTable(body, classGen.getMajor()); + buildUninitializedNewMap(bodyIl); + fixLocalVariableTable(body); + add_dcomp_param(body); + updateUninitializedNewOffsets(bodyIl); + createNewStackMapAttribute(body); + // Widening those instructions can push a body that fit over the limit. BCEL reports that + // only if some other u2 field overflows with it; it does not reject an oversized code array + // itself, and would emit a class file with a code_length that the JVM refuses to load. + check_code_size(body); + } catch (Exception e) { + if (!is_code_size_error(e)) { + throw e; + } + if (BcelUtil.isConstructor(m)) { + // A constructor cannot be distinguished from the stub by name, and its original descriptor + // is the one its callers use, so there is nothing left to try. Emit the original + // constructor and leave its argument tags for its caller's normal_exit to discard. + System.err.printf( + "DynComp warning: ClassFile: %s - constructor %s cannot be given a forwarding stub, so" + + " it is emitted unchanged; the comparability of its arguments is not tracked.%n", + classname, m.getName()); + MethodGen original = new MethodGen(m, classname, pool); + remove_local_variable_type_table(original); + return original.getMethod(); + } + // Distinguish the body by name rather than by descriptor. That leaves the code array + // byte-for-byte unchanged, so unlike the DCompMarker parameter it cannot overflow. + body = new MethodGen(m, classname, pool); + body.setName(unused_oversized_body_name(m.getName(), m.getSignature())); + bodyHasMarker = false; + } + body.isPublic(false); + body.isProtected(false); + body.isPrivate(true); + body.isSynchronized(false); + body.isSynthetic(true); + body.removeAnnotationEntries(); + remove_local_variable_type_table(body); + if (bodyHasMarker) { + // The added parameter occupies a local that the original method did not have. + body.setMaxLocals(); + } + classGen.addMethod(body.getMethod()); + return create_oversized_junit_method_stub(mgen, body.getName(), bodyHasMarker).getMethod(); + } + + /** + * Returns the name of the private method that holds the unchanged body of an oversized JUnit + * method; see {@link #create_oversized_method}. It is used only when the body cannot be + * distinguished from its forwarding stub by adding the DCompMarker parameter. + * + *

The name may already be in use; use {@link #unused_oversized_body_name} to obtain a name + * that can actually be added to the class being generated. + * + * @param methodName the name of the original method + * @return the name to give the method that holds the original body + */ + static @Identifier String oversized_body_name(@Identifier String methodName) { + return methodName + "__$dcomp_body"; + } + + /** + * Returns {@link #oversized_body_name}, made unique by appending a decimal suffix if some method + * of the class being generated already has that name and the given descriptor. + * + *

The body of an oversized JUnit method keeps the original method's descriptor, so its name + * must not be the name of any other method that has that descriptor: {@code classGen.addMethod} + * does not check, and a class with two methods of the same name and descriptor does not load. A + * collision is unlikely but possible, because the class may declare a method with the derived + * name itself, and in a JUnit test class that method keeps its original descriptor. + * + * @param methodName the name of the original method + * @param signature the descriptor of the original method, which the body retains + * @return a name for the method that holds the original body, unused in the generated class + */ + @Identifier String unused_oversized_body_name(@Identifier String methodName, String signature) { + @Identifier String base = oversized_body_name(methodName); + @Identifier String candidate = base; + for (int suffix = 2; classGen.containsMethod(candidate, signature) != null; suffix++) { + candidate = base + suffix; + } + return candidate; + } + + /** + * Returns the code for a catch-all handler that undoes the tag-stack bookkeeping of {@code + * DCRuntime.uninstrumented_enter} and rethrows the original throwable; see {@link + * #create_oversized_method_copy}. Without it, an exception out of an uninstrumented body would + * leave that body's marker, and the tags its calls pushed above the marker, on the tag stack: the + * body belongs to a JUnit test method, whose caller is JUnit's reflective invocation, so no + * enclosing instrumented frame would clean up after it. + * + *

The handler calls {@code uninstrumented_exit} even for a primitive result, because a + * throwing method produces no result tag for its caller to consume. + * + * @return the code of a catch-all handler that cleans up the tag stack and rethrows + */ + InstructionList uninstrumented_catch_il() { + InstructionList il = new InstructionList(); + // The throwable that the handler was entered with is left on the stack for the athrow. + il.append(dcr_call("uninstrumented_exit", CD_void, noArgsSig)); + il.append(new ATHROW()); + return il; + } + + /** + * Returns a JUnit-visible wrapper that maintains the tag-stack calling convention and invokes the + * private method that holds the unchanged original body. This is the final fallback when the + * bookkeeping does not fit in the original method body. A catch-all handler performs the exit + * bookkeeping when the body throws; see {@link #uninstrumented_catch_il}. + * + * @param mgen the unmodified method, with its original signature + * @param bodyName the name of the private method that holds the original body + * @param bodyHasMarker true if that method has an added DCompMarker parameter, false if it is + * distinguished by its name alone + * @return a forwarding stub with the original signature + * @throws IOException if the stub's stack map cannot be built + */ + MethodGen create_oversized_junit_method_stub( + MethodGen mgen, String bodyName, boolean bodyHasMarker) throws IOException { + Type[] paramTypes = mgen.getArgumentTypes(); + Type returnType = mgen.getReturnType(); + + int primitiveCount = 0; + for (Type paramType : paramTypes) { + if (is_primitive(paramType)) { + primitiveCount++; + } + } + + boolean primitiveResult = is_primitive(returnType); + InstructionList il = new InstructionList(); + // The body this forwards to is the unchanged original, which pushes no argument tags for the + // calls it makes even though a method of a JUnit test class consumes them; see + // create_oversized_method_copy. + il.append(ifact.createConstant(primitiveCount + (primitiveResult ? 1 : 0))); + InstructionHandle enterHandle = il.append(dcr_call("uninstrumented_enter", CD_void, intSig)); + + int offset = 0; + if (!mgen.isStatic()) { + il.append(InstructionFactory.createThis()); + offset = 1; + } + for (Type paramType : paramTypes) { + il.append(InstructionFactory.createLoad(paramType, offset)); + offset += paramType.getSize(); + } + Type[] bodyParamTypes = paramTypes; + if (bodyHasMarker) { + il.append(new ACONST_NULL()); + bodyParamTypes = ArraysPlume.append(paramTypes, dcomp_marker); + } + il.append( + ifact.createInvoke( + mgen.getClassName(), + bodyName, + returnType, + bodyParamTypes, + mgen.isStatic() ? INVOKESTATIC : INVOKESPECIAL, + classGen.isInterface())); + il.append( + dcr_call( + primitiveResult ? "uninstrumented_exit_primitive" : "uninstrumented_exit", + CD_void, + noArgsSig)); + InstructionHandle returnHandle = il.append(InstructionFactory.createReturn(returnType)); + + mgen.setInstructionList(il); + mgen.removeExceptionHandlers(); + mgen.removeLineNumbers(); + mgen.removeLocalVariables(); + mgen.removeCodeAttributes(); + remove_blacklisted_annotations(mgen); + // The body this forwards to can throw, and then the uninstrumented_exit* call above does not + // run. Clean up on that path too; see uninstrumented_catch_il. The handler's range starts + // after uninstrumented_enter, which establishes the state that the handler undoes. + // + // removeCodeAttributes above discarded the original method's stack map, so this reads back an + // empty one; the handler is a branch target, so it needs a stack map frame of its own. + setCurrentStackMapTable(mgen, classGen.getMajor()); + InstructionHandle tryStart = enterHandle.getNext(); + assert tryStart != null : "@AssumeAssertion(nullness): the invocation of the body follows"; + add_exception_handler(mgen, uninstrumented_catch_il(), tryStart, returnHandle); + assert stackMapTable != null + : "@AssumeAssertion(nullness): set by setCurrentStackMapTable above"; + install_exception_handler(mgen); + createNewStackMapAttribute(mgen); + mgen.setMaxLocals(); + mgen.setMaxStack(); + return mgen; + } + + /** + * Returns a DCompMarker overload that maintains the tag-stack calling convention and forwards to + * the unchanged original method. This is the final fallback when adding bookkeeping directly to + * an oversized method would itself exceed the JVM's code-size limit. + * + * @param mgen the unmodified method, with its original signature + * @return a forwarding stub with a DCompMarker parameter + */ + MethodGen create_oversized_method_stub(MethodGen mgen) { + Type[] paramTypes = mgen.getArgumentTypes(); + Type returnType = mgen.getReturnType(); + InstructionList il = discard_primitive_tags(paramTypes); + + int offset = 0; + if (!mgen.isStatic()) { + il.append(InstructionFactory.createThis()); + offset = 1; + } + for (Type paramType : paramTypes) { + il.append(InstructionFactory.createLoad(paramType, offset)); + offset += paramType.getSize(); + } + + short kind; + if (mgen.isStatic()) { + kind = INVOKESTATIC; + } else if (mgen.isPrivate() || mgen.getName().equals("")) { + kind = INVOKESPECIAL; + } else if (classGen.isInterface()) { + kind = INVOKEINTERFACE; + } else { + kind = INVOKEVIRTUAL; + } + il.append( + ifact.createInvoke( + mgen.getClassName(), + mgen.getName(), + returnType, + paramTypes, + kind, + classGen.isInterface())); + + if (is_primitive(returnType)) { + il.append(dcr_call("push_const", CD_void, noArgsSig)); + } + il.append(InstructionFactory.createReturn(returnType)); + + MethodGen stub = + new MethodGen( + mgen.getAccessFlags(), + returnType, + ArraysPlume.append(paramTypes, dcomp_marker), + ArraysPlume.append(mgen.getArgumentNames(), "marker"), + mgen.getName(), + mgen.getClassName(), + il, + pool); + stub.setMaxLocals(); + stub.setMaxStack(); + return stub; + } + + /** + * Throws an exception if the method's code array exceeds the JVM's 64K code-size limit. + * + * @param mgen the method to check + */ + void check_code_size(MethodGen mgen) { + InstructionList il = mgen.getInstructionList(); + if (il == null) { + return; + } + InstructionHandle end = il.getEnd(); + int length = end.getPosition() + end.getInstruction().getLength(); + if (length >= MAX_CODE_SIZE) { + throw new ClassGenException( + "Code array too big: must be smaller than " + MAX_CODE_SIZE + " bytes."); + } + } + + /** + * Returns true if the exception reports that a method's code array, one of its branch offsets, or + * some other field that the code array's size bounds is too large for the class file format. + * + * @param e an exception thrown while building an instrumented method + * @return true if {@code e} reports that a method is too large + */ + static boolean is_code_size_error(Exception e) { + String message = e.getMessage(); + return message != null + && (message.startsWith("Branch target offset too large") + || message.startsWith("Code array too big") + // BCEL reports an oversized method indirectly, when some u2 field of the code + // attribute overflows along with the code array: a bytecode offset, or the length of a + // local's live range. The name of the field is at the front of the message and the + // limit is formatted for the default locale, so match only the fixed text between. + || (message.contains("[Value out of range") && message.contains("for type u2:"))); + } + + /** + * Removes from the given method any annotation that must not appear on an instrumented method; + * see {@link #BLACKLISTED_ANNOTATIONS}. + * + * @param mgen the method to remove annotations from + */ + void remove_blacklisted_annotations(MethodGen mgen) { + for (AnnotationEntryGen item : mgen.getAnnotationEntries()) { + if (BLACKLISTED_ANNOTATIONS.contains(item.getTypeName())) { + mgen.removeAnnotationEntry(item); + } + } + } + /** * Creates a method with a DcompMarker parameter that does nothing but call the corresponding * method without the DCompMarker argument. (Currently, only used for ? va main.) diff --git a/java/daikon/dcomp/DCInstrument24.java b/java/daikon/dcomp/DCInstrument24.java index 07b2e5cf0..aeb13d60c 100644 --- a/java/daikon/dcomp/DCInstrument24.java +++ b/java/daikon/dcomp/DCInstrument24.java @@ -465,6 +465,16 @@ public int compare(WorkItem w1, WorkItem w2) { /** Has an {@code } method completed initialization? */ protected boolean constructor_is_initialized; + /** + * The MethodInfo that {@link #instrumentCode} registered for the method it is emitting, or null + * if it has not registered one. Read only by a rerun of {@code instrumentCode} for that same + * method; see {@link MethodGen24#resetForCodeBuilder}. + */ + private @Nullable MethodInfo currentMethodInfo; + + /** The index of {@link #currentMethodInfo} in {@code DCRuntime.methods}. */ + private int currentMethodInfoIndex; + /** * Record used to describe a new LocalVariable. When instrumentation wants to create a new method, * it creates a list containing one of these records for each local variable of the method. This @@ -562,6 +572,14 @@ public record myLocalVariable(int slot, String name, ClassDesc descriptor) {} */ private Set oversizedMethods = new HashSet<>(); + /** + * The subset of {@link #oversizedMethods} that exceeds the JVM's 64K code-size limit even with + * the minimal tag-stack bookkeeping that {@link #copyOversizedMethod} adds. Such a method is + * emitted as a small forwarding stub that performs the bookkeeping and calls the unchanged + * original method. Uses the same keys as {@link #oversizedMethods}. + */ + private Set oversizedMethodsRequiringStub = new HashSet<>(); + /** If we're using an instrumented JDK, then "java.lang"; otherwise, "daikon.dcomp". */ protected @DotSeparatedIdentifiers String dcompMarkerPrefix; @@ -847,6 +865,11 @@ private void instrumentClass( boolean junit_test_class = false; + // Skipped for JDK classes. A JDK class is never a JUnit test class: the check below confirms + // one only by a junit.framework.TestCase superclass or an org/junit/Test annotation. Skipping + // them loses no state transition -- STARTING and TEST_DISCOVERY are re-evaluated on the next + // class load, and the test classes themselves are never in the JDK -- and it keeps a + // getStackTrace, plus TEST_DISCOVERY's superclass walk, off the JDK class-loading path. if (!in_jdk) { // A very tricky special case: If JUnit is running and the current // class has been passed to JUnit on the command line, then this @@ -1328,19 +1351,49 @@ private void processMethod( } // If an earlier build attempt found that this method's instrumented form exceeds the JVM's - // 64K code-size limit, emit it with the original (uninstrumented) body. The method must - // still be emitted with the DCompMarker parameter, because callers of the instrumented - // version look it up by that signature; it simply does no comparability tracking. - // See instrument_jdk_class for how this set is populated. - if (oversizedMethods.contains( - oversizedMethodKey(methodModel.methodName().stringValue(), mtd))) { - debugInstrument.log("Copying oversized method: %s%n", mgen.getName()); + // 64K code-size limit, emit its original body with only the tag-stack bookkeeping required + // by its callers. The method must still have the DCompMarker parameter when addingDcompArg is + // true, because callers of the instrumented version look it up by that signature. See + // instrument_jdk_class for how this set is populated. + String oversizedKey = oversizedMethodKey(methodModel.methodName().stringValue(), mtd); + if (oversizedMethods.contains(oversizedKey)) { + // The bookkeeping is a few bytes long, but the method is already at the limit, so it can + // overflow too; a method that did is emitted as a small forwarding stub. + // + // The stub forwards to the unchanged original, which is emitted under its own descriptor + // only when a DCompMarker parameter is added. Without the marker there is nothing to + // forward to: the stub's call would resolve to the stub itself. Only instrument_jdk_class + // populates oversizedMethods, so the only method that gets here without the marker is + // main, whose copy adds no bookkeeping at all and therefore cannot overflow. + boolean copyOriginalBody = + !addingDcompArg || !oversizedMethodsRequiringStub.contains(oversizedKey); + debugInstrument.log( + "Oversized method, creating %s: %s%n", + copyOriginalBody ? "minimally instrumented copy" : "forwarding stub", mgen.getName()); debugInstrument.indent(); + final boolean addMarker = addingDcompArg; + // A JUnit test class would also leave argument tags for the callee to discard, but as + // noted above only instrument_jdk_class populates oversizedMethods, and a JDK class is + // never a JUnit test class. So the marker is the only thing to test here. + final boolean discardArgumentTags = copyOriginalBody && addingDcompArg; + final boolean pushResultTag = copyOriginalBody && addingDcompArg; classBuilder.withMethod( methodModel.methodName().stringValue(), mtd, methodModel.flags().flagsMask(), - methodBuilder -> copyMethod(methodBuilder, methodModel, mgen)); + methodBuilder -> { + if (copyOriginalBody) { + copyOversizedMethod( + methodBuilder, + methodModel, + mgen, + addMarker, + discardArgumentTags, + pushResultTag); + } else { + createOversizedMethodStub(methodBuilder, methodModel, mgen); + } + }); debugInstrument.exdent(); debugInstrument.log("End of copy%n"); debug_transform.exdent(); @@ -1389,6 +1442,179 @@ private void outputMethodUnchanged(ClassBuilder classBuilder, MethodModel mm, Me methodBuilder -> copyMethod(methodBuilder, mm, mgen)); } + /** + * Copies a method whose fully instrumented form would exceed the JVM's 64K code-size limit, + * adding only the bookkeeping required by its callers. Retaining the original body rather than + * forwarding to another method preserves caller-sensitive and exception-stack semantics. + * + * @param methodBuilder for the output method + * @param methodModel describes the input method + * @param mgen describes the output method + * @param addDcompMarker whether to append the DCompMarker parameter and shift existing locals + * @param discardArgumentTags whether to discard primitive argument tags on entry + * @param pushResultTag whether to push a tag before each primitive return + */ + private void copyOversizedMethod( + MethodBuilder methodBuilder, + MethodModel methodModel, + MethodGen24 mgen, + boolean addDcompMarker, + boolean discardArgumentTags, + boolean pushResultTag) { + + for (MethodElement me : methodModel) { + debugInstrument.log("MethodElement: %s%n", me); + switch (me) { + case CodeModel codeModel -> + methodBuilder.withCode( + codeBuilder -> { + // This handler modifies mgen, and may be run more than once. + mgen.resetForCodeBuilder(); + MethodGen24.MInfo24 minfo = + new MethodGen24.MInfo24(0, mgen.getMaxLocals(), codeBuilder); + mgen.fixLocals(minfo); + if (addDcompMarker) { + add_dcomp_param(mgen, minfo); + } + + for (LocalVariable lv : mgen.localsTable) { + codeBuilder.localVariable( + lv.slot(), + lv.name().stringValue(), + lv.typeSymbol(), + lv.startScope(), + lv.endScope()); + } + + if (discardArgumentTags) { + int primitiveCount = 0; + for (ClassDesc paramType : mgen.getParameterTypes()) { + if (is_primitive(paramType)) { + primitiveCount++; + } + } + if (primitiveCount > 0) { + boolean replaceCallerResultTag = + !pushResultTag && is_primitive(mgen.getReturnType()); + int discardCount = primitiveCount + (replaceCallerResultTag ? 1 : 0); + for (CodeElement ce : discard_tag_code(null, discardCount)) { + codeBuilder.with(ce); + } + if (replaceCallerResultTag) { + codeBuilder.with(dcr_call("push_const", CD_void, noArgsSig)); + } + } + } + + boolean primitiveResult = pushResultTag && is_primitive(mgen.getReturnType()); + for (CodeElement ce : mgen.getInstructionList()) { + if (ce instanceof LocalVariable || ce instanceof LocalVariableType) { + continue; + } + if (primitiveResult && ce instanceof ReturnInstruction) { + codeBuilder.with(dcr_call("push_const", CD_void, noArgsSig)); + } + codeBuilder.with(ce); + } + }); + + case RuntimeVisibleAnnotationsAttribute rvaa -> copyAnnotations(methodBuilder, rvaa); + + default -> methodBuilder.with(me); + } + } + } + + /** + * Builds a DCompMarker overload that maintains the tag-stack calling convention and forwards to + * the unchanged original method. This is the final fallback when adding bookkeeping directly to + * an oversized method would itself exceed the JVM's code-size limit. + * + *

The caller must be emitting the method with an added DCompMarker parameter; the stub calls + * the original descriptor, which is the unchanged original method only in that case. See {@code + * processMethod}, which uses {@link #copyOversizedMethod} instead when no marker is added. + * + * @param methodBuilder for the output method + * @param methodModel describes the input method + * @param mgen describes the output method + */ + private void createOversizedMethodStub( + MethodBuilder methodBuilder, MethodModel methodModel, MethodGen24 mgen) { + for (MethodElement me : methodModel) { + switch (me) { + case CodeModel codeModel -> + methodBuilder.withCode( + codeBuilder -> { + for (CodeElement ce : discard_primitive_tags(mgen.getParameterTypes())) { + codeBuilder.with(ce); + } + + int localIndex = 0; + if (!mgen.isStatic()) { + codeBuilder.with(LoadInstruction.of(TypeKind.REFERENCE, localIndex++)); + } + for (ClassDesc paramType : mgen.getParameterTypes()) { + TypeKind typeKind = TypeKind.from(paramType); + codeBuilder.with(LoadInstruction.of(typeKind, localIndex)); + localIndex += typeKind.slotSize(); + } + + Opcode opcode; + if (mgen.isStatic()) { + opcode = INVOKESTATIC; + } else if (mgen.isConstructor() + || (mgen.getAccessFlagsMask() & ACC_PRIVATE) != 0) { + opcode = INVOKESPECIAL; + } else if (classGen.isInterface()) { + opcode = INVOKEINTERFACE; + } else { + opcode = INVOKEVIRTUAL; + } + ClassEntry owner = poolBuilder.classEntry(ClassDesc.of(mgen.getClassName())); + NameAndTypeEntry nameAndType = + poolBuilder.nameAndTypeEntry( + mgen.getName(), + MethodTypeDesc.of(mgen.getReturnType(), mgen.getParameterTypes())); + codeBuilder.with( + InvokeInstruction.of(opcode, owner, nameAndType, classGen.isInterface())); + + if (is_primitive(mgen.getReturnType())) { + codeBuilder.with(dcr_call("push_const", CD_void, noArgsSig)); + } + codeBuilder.with(ReturnInstruction.of(TypeKind.from(mgen.getReturnType()))); + }); + + case RuntimeVisibleAnnotationsAttribute rvaa -> copyAnnotations(methodBuilder, rvaa); + + default -> methodBuilder.with(me); + } + } + } + + /** + * Copies the given annotations from the original method to our instrumented method, unless any of + * them is one that must not appear on an instrumented method; see {@link + * #BLACKLISTED_ANNOTATIONS}. + * + * @param methodBuilder for the output method + * @param rvaa the annotations of the input method + */ + private void copyAnnotations( + MethodBuilder methodBuilder, RuntimeVisibleAnnotationsAttribute rvaa) { + List filteredAnnotations = new ArrayList<>(); + for (final Annotation item : rvaa.annotations()) { + String description = item.className().stringValue(); + if (BLACKLISTED_ANNOTATIONS.contains(description)) { + debugInstrument.log("Annotation not copied: %s%n", description); + } else { + filteredAnnotations.add(item); + } + } + if (!filteredAnnotations.isEmpty()) { + methodBuilder.with(RuntimeVisibleAnnotationsAttribute.of(filteredAnnotations)); + } + } + /** * Copy the given method from the input class file to the output class with no changes. * @@ -1457,14 +1683,6 @@ private void instrumentMethod( ClassInfo classInfo, boolean trackMethod) { - // Per-method state: constructor_is_initialized records whether the super constructor call - // has been seen in the method now being instrumented, and must start false for every method. - // Without this reset it stays set once any constructor in the class reaches its super() call, - // so a later constructor would be treated as initialized from its first instruction; and - // because instrument_jdk_class may rebuild the class with this same instance, a value left - // over from an abandoned attempt would make the retry differ from the first attempt. - constructor_is_initialized = false; - try { boolean codeModelSeen = false; for (MethodElement me : methodModel) { @@ -1476,21 +1694,7 @@ private void instrumentMethod( codeBuilder -> instrumentCode(codeBuilder, codeModel, null, mgen, classInfo, trackMethod)); } - case RuntimeVisibleAnnotationsAttribute rvaa -> { - // Do not copy any problematic annotations from the original - // method to our instrumented method. - boolean output = true; - for (final Annotation item : rvaa.annotations()) { - String description = item.className().stringValue(); - if (BLACKLISTED_ANNOTATIONS.contains(description)) { - output = false; - debugInstrument.log("Annotation not copied: %s%n", description); - } - } - if (output) { - methodBuilder.with(me); - } - } + case RuntimeVisibleAnnotationsAttribute rvaa -> copyAnnotations(methodBuilder, rvaa); // copy all other MethodElements to output class (unchanged) default -> methodBuilder.with(me); } @@ -1540,6 +1744,19 @@ private void instrumentCode( ClassInfo classInfo, boolean trackMethod) { + // This handler modifies mgen, and may be run more than once. + boolean firstRun = mgen.resetForCodeBuilder(); + + // Per-method state: constructor_is_initialized records whether the super constructor call has + // been seen in the method now being instrumented, and must start false for every run of this + // handler. Without this reset it stays set once any constructor in the class reaches its + // super() call, so a later constructor would be treated as initialized from its first + // instruction. It must be reset here rather than in instrumentMethod because the code builder + // may run this handler more than once (see resetForCodeBuilder): a value left over from an + // earlier run would make a later run emit different code, and would make a field access that + // precedes the super() call use the field's tag accessor on an uninitialized `this`. + constructor_is_initialized = false; + // method_info_index is not used at this point in DCInstrument MethodGen24.MInfo24 minfo = new MethodGen24.MInfo24(0, mgen.getMaxLocals(), codeBuilder); debugInstrument.log("nextLocalIndex: %d%n", minfo.nextLocalIndex); @@ -1630,11 +1847,21 @@ private void instrumentCode( // and add it to the list for this class. MethodInfo mi = null; if (trackMethod && !in_jdk) { - @SuppressWarnings("nullness:assignment") // the method exists - @NonNull MethodInfo miTmp = create_method_info_if_instrumented(classInfo, mgen); - mi = miTmp; - classInfo.method_infos.add(mi); - DCRuntime.methods.add(mi); + if (firstRun) { + @SuppressWarnings("nullness:assignment") // the method exists + @NonNull MethodInfo miTmp = create_method_info_if_instrumented(classInfo, mgen); + mi = miTmp; + classInfo.method_infos.add(mi); + DCRuntime.methods.add(mi); + currentMethodInfo = mi; + currentMethodInfoIndex = DCRuntime.methods.size() - 1; + } else { + // This is a rerun of the handler for the method that the previous run registered; reuse + // that MethodInfo rather than registering the method a second time. + assert currentMethodInfo != null + : "@AssumeAssertion(nullness): set by the first run, under the same conditions"; + mi = currentMethodInfo; + } } @SuppressWarnings("JdkObsolete") @@ -1687,8 +1914,8 @@ private void instrumentCode( if (trackMethod && !in_jdk) { assert mi != null : "@AssumeAssertion(nullness): mi was assigned under same conditions"; - add_enter(mgen, minfo, codeList, DCRuntime.methods.size() - 1); - add_exit(mgen, mi, minfo, codeList, DCRuntime.methods.size() - 1); + add_enter(mgen, minfo, codeList, currentMethodInfoIndex); + add_exit(mgen, mi, minfo, codeList, currentMethodInfoIndex); } // Copy the modified local variable table to the output class. @@ -1838,9 +2065,12 @@ public byte[] instrument_jdk_class(ClassInfo classInfo) { // jdk.internal.classfile.impl.DirectCodeBuilder.build): throw the whole builder away and start // over, this time emitting the offending method unchanged. Each attempt identifies at most one // oversized method, so bound the number of attempts by the number of methods in the class. + // A method that is still oversized with that bookkeeping is emitted as a forwarding stub, so + // a single method can cost two attempts. oversizedMethods.clear(); + oversizedMethodsRequiringStub.clear(); int skippedMethodsMark = skipped_methods.size(); - int maxAttempts = classModel.methods().size() + 1; + int maxAttempts = 2 * classModel.methods().size() + 1; for (int attempt = 0; attempt < maxAttempts; attempt++) { try { @@ -1860,15 +2090,31 @@ public byte[] instrument_jdk_class(ClassInfo classInfo) { // to identify this one case by matching the text of the exception's message. It returns // null for any message it does not recognize, which is treated as a real error below. String method = oversizedMethodName(e); - if (method == null || !oversizedMethods.add(method)) { - // Either this is not the oversized-method error, or we already emitted this method - // uninstrumented and it still does not fit. Both indicate a bug in the instrumentor. + if (method == null) { + // This is not the oversized-method error; it indicates a bug in the instrumentor. throw e; } - System.err.printf( - "DynComp warning: ClassFile: %s - method %s has too many bytecodes to instrument and is" - + " being skipped.%n", - classname, method); + if (oversizedMethods.add(method)) { + System.err.printf( + "DynComp warning: ClassFile: %s - method %s has too many bytecodes to instrument and" + + " is being skipped.%n", + classname, method); + } else if (oversizedMethodsRequiringStub.add(method)) { + System.err.printf( + "DynComp warning: ClassFile: %s - method %s is too large even for the minimal" + + " instrumentation; a forwarding stub is being used.%n", + classname, method); + } else { + // Even the small forwarding stub failed the code-size check. This should be impossible + // unless its construction is broken. + throw new DynCompError( + "Method " + + classname + + "." + + method + + " exceeds the JVM's 64K code-size limit even without instrumentation", + e); + } // Discard anything the abandoned attempt recorded; the retry starts from scratch. skipped_methods.subList(skippedMethodsMark, skipped_methods.size()).clear(); } @@ -2908,12 +3154,13 @@ private void add_exit( } ClassModel cm; try { - @SuppressWarnings("nullness:assignment") - @NonNull ClassModel cmTmp = getClassModel(interfaceName); - cm = cmTmp; + cm = getClassModel(interfaceName); } catch (Throwable t) { throw new DynCompError(String.format("Unable to load class: %s", interfaceName), t); } + if (cm == null) { + throw new DynCompError(String.format("Unable to find class: %s", interfaceName)); + } for (MethodModel jm : cm.methods()) { String jmName = jm.methodName().stringValue(); MethodTypeDesc mtd = jm.methodTypeSymbol(); diff --git a/java/daikon/dcomp/DCInstrumentTest24.java b/java/daikon/dcomp/DCInstrumentTest24.java index 91ec8b3bf..03bb94b94 100644 --- a/java/daikon/dcomp/DCInstrumentTest24.java +++ b/java/daikon/dcomp/DCInstrumentTest24.java @@ -1,8 +1,10 @@ package daikon.dcomp; +import static java.lang.classfile.Opcode.INVOKEVIRTUAL; import static java.lang.constant.ConstantDescs.CD_int; import static java.lang.constant.ConstantDescs.CD_void; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -10,10 +12,14 @@ import daikon.chicory.ClassInfo; import daikon.chicory.Runtime; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; +import java.io.UncheckedIOException; +import java.lang.classfile.Annotation; +import java.lang.classfile.Attribute; import java.lang.classfile.ClassFile; import java.lang.classfile.ClassHierarchyResolver; import java.lang.classfile.ClassModel; @@ -22,20 +28,30 @@ import java.lang.classfile.CodeModel; import java.lang.classfile.Label; import java.lang.classfile.MethodModel; +import java.lang.classfile.MethodTransform; +import java.lang.classfile.attribute.CodeAttribute; +import java.lang.classfile.attribute.RuntimeVisibleAnnotationsAttribute; import java.lang.classfile.attribute.StackMapFrameInfo; import java.lang.classfile.attribute.StackMapTableAttribute; import java.lang.classfile.instruction.InvokeInstruction; +import java.lang.classfile.instruction.LocalVariable; import java.lang.constant.ClassDesc; import java.lang.constant.MethodTypeDesc; import java.lang.reflect.AccessFlag; +import java.lang.reflect.InvocationTargetException; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Supplier; import java.util.regex.Pattern; +import org.apache.bcel.classfile.ClassParser; +import org.apache.bcel.classfile.JavaClass; +import org.apache.bcel.classfile.Method; import org.checkerframework.checker.interning.qual.Interned; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.checker.signature.qual.BinaryName; +import org.checkerframework.checker.signature.qual.Identifier; import org.junit.Test; /** @@ -314,208 +330,1787 @@ public int add(int x) { value += x; return value; } + + /** + * Adds to {@link #value}. Takes two primitive parameters, so an instrumented caller must leave + * two tags on the tag stack for it. + * + * @param x one amount to add + * @param y another amount to add + * @return the new value + */ + public int combine(int x, int y) { + value += x + y; + return value; + } + } + + /** + * A superclass with a parameterized constructor, so that {@link TwoConstructors} can read a field + * in the argument expression of its {@code super()} call -- that is, before the superclass + * constructor has run. + */ + public static class Base { + + /** An arbitrary value. */ + int base; + + /** + * Creates a new Base. + * + * @param base the value to store + */ + public Base(int base) { + this.base = base; + } + } + + /** + * A class with two constructors, the second of which reads a field before calling its superclass + * constructor. Used by {@link #constructorInitializedStateDoesNotLeakBetweenMethods}. + */ + public static class TwoConstructors extends Base { + + /** Creates a new TwoConstructors. This constructor reads no fields before {@code super()}. */ + public TwoConstructors() { + super(0); + } + + /** + * Creates a new TwoConstructors, reading {@code s.value} before {@code super()} runs. + * + * @param s supplies the value to store + */ + public TwoConstructors(Sample s) { + super(s.value); + } + } + + /** + * Tests that a class instrumented by {@link DCInstrument24#instrument_jdk_class} calls the shadow + * runtime class {@code java.lang.DCRuntime} rather than {@code daikon.dcomp.DCRuntime}. A class + * in a pre-instrumented {@code java.base} module may not refer to anything outside {@code + * java.base}. This must hold even when the DCInstrument24 constructor chose {@code + * daikon.dcomp.DCRuntime}, which it does whenever {@code Premain.jdk_instrumented} is false. + * + * @throws IOException if the class file for {@link Sample} cannot be read + */ + @Test + public void testJdkClassCallsShadowRuntime() throws IOException { + boolean savedJdkInstrumented = Premain.jdk_instrumented; + @BinaryName String savedInstrumentationInterface = DCRuntime.instrumentation_interface; + Premain.jdk_instrumented = false; + // BuildJDK24 sets this static field before each class it instruments. + DCRuntime.instrumentation_interface = "daikon.dcomp.DCompInstrumented"; + byte[] instrumented; + try { + instrumented = instrumentAsJdkClass(); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + DCRuntime.instrumentation_interface = savedInstrumentationInterface; + } + Set invoked = invokedClasses(instrumented); + assertTrue( + "instrumented class does not call java/lang/DCRuntime: " + invoked, + invoked.contains("java/lang/DCRuntime")); + assertFalse( + "instrumented class calls daikon/dcomp/DCRuntime: " + invoked, + invoked.contains("daikon/dcomp/DCRuntime")); + } + + /** + * Instruments {@link Sample} as if it were a JDK class. + * + * @return the instrumented bytes of {@link Sample} + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private byte[] instrumentAsJdkClass() throws IOException { + @SuppressWarnings("signature:assignment") // the name of a nested class + @BinaryName String classname = Sample.class.getName(); + InputStream sampleStream = + DCInstrumentTest24.class.getResourceAsStream("DCInstrumentTest24$Sample.class"); + if (sampleStream == null) { + throw new Error("cannot find the class file for " + classname); + } + byte[] original; + try (InputStream is = sampleStream) { + original = is.readAllBytes(); + } + return instrumentAsJdkClass(original); + } + + /** + * Instruments the given definition of {@link Sample} as if it were a JDK class. + * + * @param original the class-file bytes to instrument + * @return the instrumented bytes + */ + private byte[] instrumentAsJdkClass(byte[] original) { + ClassFile classFile = ClassFile.of(); + ClassModel classModel = classFile.parse(original); + ClassInfo classInfo = new ClassInfo(sampleClassName(), classLoader()); + DCInstrument24 dci = new DCInstrument24(classFile, classModel, true); + // instrument_jdk_class throws rather than returning null if it cannot instrument the class. + return dci.instrument_jdk_class(classInfo); + } + + /** Tests that removing a blacklisted annotation does not remove its permitted siblings. */ + @Test + public void preservesAnnotationsThatAreNotBlacklisted() throws IOException { + @BinaryName String savedInstrumentationInterface = DCRuntime.instrumentation_interface; + try { + DCRuntime.instrumentation_interface = "daikon.dcomp.DCompInstrumented"; + ClassFile classFile = ClassFile.of(); + ClassModel sample = classFile.parse(classBytes(sampleClassName())); + byte[] annotated = + classFile.transformClass( + sample, + ClassTransform.transformingMethods( + method -> method.methodName().stringValue().equals(SMALL_METHOD), + MethodTransform.endHandler( + methodBuilder -> { + methodBuilder.with( + RuntimeVisibleAnnotationsAttribute.of( + Annotation.of(ClassDesc.of("java.lang.Deprecated")), + Annotation.of( + ClassDesc.of( + "jdk.internal.vm.annotation.IntrinsicCandidate")))); + }))); + + MethodModel instrumentedMethod = + instrumentedCopy(classFile.parse(instrumentAsJdkClass(annotated)), SMALL_METHOD); + Set annotations = new HashSet<>(); + for (Attribute attribute : instrumentedMethod.attributes()) { + if (attribute instanceof RuntimeVisibleAnnotationsAttribute rvaa) { + for (Annotation annotation : rvaa.annotations()) { + annotations.add(annotation.className().stringValue()); + } + } + } + assertEquals(Set.of("Ljava/lang/Deprecated;"), annotations); + } finally { + DCRuntime.instrumentation_interface = savedInstrumentationInterface; + } + } + + /** Name of the method that {@link #oversizedClassBytes} adds to {@link Sample}. */ + private static final @Identifier String OVERSIZED_METHOD = "tooBig"; + + /** Name of the {@link Sample} method that instruments normally. */ + private static final String SMALL_METHOD = "add"; + + /** + * Number of {@code iload_1; iconst_1; iadd; istore_1} groups in {@link #OVERSIZED_METHOD}. Each + * group is 4 bytes, so the uninstrumented method is well under the JVM's 64K code-size limit, but + * instrumentation adds several DCRuntime calls per group, which pushes the instrumented form over + * it. + */ + private static final int OVERSIZED_GROUPS = 6000; + + /** + * Number of {@code iload_1; iconst_1; iadd; istore_1} groups in {@link #OVERSIZED_METHOD} for + * {@link #testHugeMethodUsesForwardingStub}. The method is 4 * HUGE_GROUPS + 2 bytes long, which + * is just under the JVM's 64K code-size limit -- so close that even the handful of bytes of + * tag-stack bookkeeping that an oversized method is given does not fit. + */ + private static final int HUGE_GROUPS = 16382; + + /** + * Number of {@code iload_1; iconst_1; iadd; istore_1} groups in {@link #OVERSIZED_METHOD} for + * {@link #testOversizedJunitFallbackRebuildsStackMap}, which uses the {@code branching} form of + * {@link #oversizedClassBytes}. That form adds 16 bytes, so the method is 4 * + * HUGE_BRANCHING_GROUPS + 16 bytes long: just under the JVM's 64K code-size limit, and too close + * to it for the handful of bytes of tag-stack bookkeeping that an oversized method is given. + */ + private static final int HUGE_BRANCHING_GROUPS = 16379; + + /** + * Number of {@code iload_1; iconst_1; iadd; istore_1} groups in {@link #OVERSIZED_METHOD} for + * {@link #testHugeThrowingJunitMethodCleansUpTagStackOnException}, which uses {@link + * #throwingClassBytes}. That form adds {@link #THROWING_FIXED_BYTES} bytes, so the method is just + * under the JVM's 64K code-size limit, and too close to it for the handful of bytes of tag-stack + * bookkeeping that an oversized method is given. + */ + private static final int HUGE_THROWING_GROUPS = 16378; + + /** + * Number of {@code iload_3; iconst_1; iadd; istore_3} groups that {@link + * #widenedBranchClassBytes} places between a branch and its target. Each group is 4 bytes, so the + * branch spans 4 * WIDENING_GROUPS bytes, which fits in the 2-byte operand of a branch + * instruction. Adding a DCompMarker parameter moves local 3 to slot 4, which widens the two + * one-byte instructions of each group to two bytes apiece; the branch then spans 6 * + * WIDENING_GROUPS bytes, which does not fit. This is also enough groups that the fully + * instrumented method exceeds the JVM's 64K code-size limit, so the method is emitted by {@code + * copyOversizedMethod}. + */ + private static final int WIDENING_GROUPS = 6000; + + /** + * The number of groups in the method that {@link #trackedWidenedBranchClassBytes} builds, which + * is instrumented rather than copied. Instrumentation inflates each 4-byte group to about 30 + * bytes, so this is enough that the branch spans more than the 32767 bytes that fit in its + * operand, which makes {@code java.lang.classfile} discard the built code and run the handler a + * second time; see {@link daikon.chicory.MethodGen24#resetForCodeBuilder}. It is also few enough + * that the fully instrumented method still fits in the JVM's 64K limit, which the non-JDK path + * requires: {@link DCInstrument24#instrument} has no oversized-method fallback, so a method that + * did not fit would make it abandon the whole class. + */ + private static final int TRACKED_WIDENING_GROUPS = 2000; + + /** + * Returns the bytes of {@link Sample} with an added method, {@link #OVERSIZED_METHOD}, whose + * instrumented form exceeds the JVM's 64K code-size limit. The method is added to a real class + * rather than a synthetic one because DCInstrument24 resolves the class being instrumented, and + * its superclasses, from the classpath. + * + * @param groups the number of 4-byte instruction groups in the added method + * @return the bytes of {@link Sample} plus a method that is too large to instrument + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private static byte[] oversizedClassBytes(int groups) throws IOException { + return oversizedClassBytes(groups, false); + } + + /** + * Returns the bytes of {@link Sample} with an added method, {@link #OVERSIZED_METHOD}, whose + * instrumented form exceeds the JVM's 64K code-size limit. The method is added to a real class + * rather than a synthetic one because DCInstrument24 resolves the class being instrumented, and + * its superclasses, from the classpath. + * + * @param groups the number of 4-byte instruction groups in the added method + * @param branching if true, the added method uses locals beyond its parameters and contains a + * branch, so it has a StackMapTable and its instructions must be widened when a DCompMarker + * parameter displaces those locals; if false, the method is straight-line code that uses no + * local beyond its parameter + * @return the bytes of {@link Sample} plus a method that is too large to instrument + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private static byte[] oversizedClassBytes(int groups, boolean branching) throws IOException { + return oversizedClassBytes(groups, branching, 0); + } + + /** + * Returns the bytes of {@link Sample} with an added method, {@link #OVERSIZED_METHOD}, whose + * instrumented form exceeds the JVM's 64K code-size limit. The method is added to a real class + * rather than a synthetic one because DCInstrument24 resolves the class being instrumented, and + * its superclasses, from the classpath. + * + * @param groups the number of 4-byte instruction groups in the added method + * @param branching if true, the added method uses locals beyond its parameters and contains a + * branch, so it has a StackMapTable and its instructions must be widened when a DCompMarker + * parameter displaces those locals; if false, the method is straight-line code that uses no + * local beyond its parameter + * @param padding the number of one-byte {@code nop} instructions to add, which tunes the method's + * length to a byte where a group of four cannot + * @return the bytes of {@link Sample} plus a method that is too large to instrument + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private static byte[] oversizedClassBytes(int groups, boolean branching, int padding) + throws IOException { + ClassFile classFile = ClassFile.of(); + ClassModel classModel = classFile.parse(classBytes(sampleClassName())); + return classFile.transformClass( + classModel, + ClassTransform.endHandler( + classBuilder -> + classBuilder.withMethodBody( + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + if (branching) { + // Locals 2 and 3 are referenced by one-byte instructions; adding a + // DCompMarker parameter moves them to 3 and 4, which widens those + // instructions and thus shifts every later bytecode offset. The branch + // target needs a stack map frame, whose offset therefore shifts too. + // + // All of this precedes the groups, and the branch skips only a few bytes, + // because BCEL cannot represent a branch offset or a stack map offset + // beyond 32767. + Label target = codeBuilder.newLabel(); + codeBuilder.iconst_0(); + codeBuilder.istore(2); + codeBuilder.iconst_0(); + codeBuilder.istore(3); + codeBuilder.iload(3); + codeBuilder.ifge(target); + codeBuilder.iinc(3, 1); + codeBuilder.iinc(2, 1); + codeBuilder.labelBinding(target); + // Declare every local, including "this" and the parameter. Without a + // complete LocalVariableTable, BCEL's fixLocalVariableTable runs the stack + // verifier over the whole method to infer the live range of each + // undeclared local, which takes minutes on a method this large. + Label start = codeBuilder.startLabel(); + Label end = codeBuilder.endLabel(); + codeBuilder.localVariable( + 0, "this", ClassDesc.of(sampleClassName()), start, end); + codeBuilder.localVariable(1, "arg", CD_int, start, end); + codeBuilder.localVariable(2, "local2", CD_int, start, end); + codeBuilder.localVariable(3, "local3", CD_int, start, end); + } + for (int i = 0; i < groups; i++) { + codeBuilder.iload(1); + codeBuilder.iconst_1(); + codeBuilder.iadd(); + codeBuilder.istore(1); + } + for (int i = 0; i < padding; i++) { + codeBuilder.nop(); + } + codeBuilder.iload(1); + codeBuilder.ireturn(); + }))); + } + + /** + * Returns the given class with an {@code org.junit.Test} annotation on {@link #OVERSIZED_METHOD}, + * which is what makes DCInstrument treat the class as a JUnit test class. + * + * @param classBytes the bytes of a class that has an {@link #OVERSIZED_METHOD} method + * @return the same class, with that method annotated + */ + private static byte[] withJunitTestAnnotation(byte[] classBytes) { + ClassFile classFile = ClassFile.of(); + return classFile.transformClass( + classFile.parse(classBytes), + ClassTransform.transformingMethods( + method -> method.methodName().stringValue().equals(OVERSIZED_METHOD), + MethodTransform.endHandler( + methodBuilder -> + methodBuilder.with( + RuntimeVisibleAnnotationsAttribute.of( + Annotation.of(ClassDesc.of("org.junit.Test"))))))); + } + + /** Name of the {@link Sample} method that {@link #siblingCallClassBytes} calls. */ + private static final String SIBLING_METHOD = "combine"; + + /** + * Returns the bytes of {@link Sample} with an added method, {@link #OVERSIZED_METHOD}, whose + * instrumented form exceeds the JVM's 64K code-size limit and whose body calls {@link + * #SIBLING_METHOD} with two primitive arguments. + * + * @param groups the number of 4-byte instruction groups in the added method + * @return the bytes of {@link Sample} plus that method + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private static byte[] siblingCallClassBytes(int groups) throws IOException { + ClassFile classFile = ClassFile.of(); + ClassModel classModel = classFile.parse(classBytes(sampleClassName())); + return classFile.transformClass( + classModel, + ClassTransform.endHandler( + classBuilder -> + classBuilder.withMethodBody( + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + // An instrumented sibling method pops a tag for each of its two primitive + // parameters, but this method's body is emitted without instrumentation, so + // it pushes none. + codeBuilder.aload(0); + codeBuilder.iload(1); + codeBuilder.iconst_1(); + codeBuilder.invokevirtual( + ClassDesc.of(sampleClassName()), + SIBLING_METHOD, + MethodTypeDesc.of(CD_int, CD_int, CD_int)); + codeBuilder.istore(1); + for (int i = 0; i < groups; i++) { + codeBuilder.iload(1); + codeBuilder.iconst_1(); + codeBuilder.iadd(); + codeBuilder.istore(1); + } + codeBuilder.iload(1); + codeBuilder.ireturn(); + // Declare every local; see oversizedClassBytes for why. + Label start = codeBuilder.startLabel(); + Label end = codeBuilder.endLabel(); + codeBuilder.localVariable( + 0, "this", ClassDesc.of(sampleClassName()), start, end); + codeBuilder.localVariable(1, "arg", CD_int, start, end); + }))); + } + + /** The exception that the method built by {@link #throwingClassBytes} throws. */ + private static final ClassDesc THROWN_EXCEPTION = ClassDesc.of("java.lang.IllegalStateException"); + + /** + * The number of bytes of {@link #OVERSIZED_METHOD}, as built by {@link #throwingClassBytes}, that + * are not part of one of its 4-byte groups. + */ + private static final int THROWING_FIXED_BYTES = 21; + + /** + * Returns the bytes of {@link Sample} with an added method, {@link #OVERSIZED_METHOD}, that + * throws {@link #THROWN_EXCEPTION} when its argument is nonzero and returns normally when its + * argument is zero, and whose instrumented form exceeds the JVM's 64K code-size limit. + * + *

Before throwing, the method calls {@link #SIBLING_METHOD} and discards its result. When the + * sibling is instrumented, that call leaves a result tag on the tag stack that the uninstrumented + * body never pops, so the throw leaves both that tag and the body's marker for the + * exceptional-exit cleanup to remove. + * + *

The method is {@code 4 * groups} plus {@link #THROWING_FIXED_BYTES} bytes long. + * + * @param groups the number of 4-byte instruction groups in the added method + * @return the bytes of {@link Sample} plus that method + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private static byte[] throwingClassBytes(int groups) throws IOException { + ClassFile classFile = ClassFile.of(); + ClassModel classModel = classFile.parse(classBytes(sampleClassName())); + return classFile.transformClass( + classModel, + ClassTransform.endHandler( + classBuilder -> + classBuilder.withMethodBody( + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + // if (arg != 0) { combine(arg, 1); throw new IllegalStateException(); } + Label normal = codeBuilder.newLabel(); + codeBuilder.iload(1); // 1 byte + codeBuilder.ifeq(normal); // 3 bytes + codeBuilder.aload(0); // 1 byte + codeBuilder.iload(1); // 1 byte + codeBuilder.iconst_1(); // 1 byte + codeBuilder.invokevirtual( // 3 bytes + ClassDesc.of(sampleClassName()), + SIBLING_METHOD, + MethodTypeDesc.of(CD_int, CD_int, CD_int)); + codeBuilder.pop(); // 1 byte + codeBuilder.new_(THROWN_EXCEPTION); // 3 bytes + codeBuilder.dup(); // 1 byte + codeBuilder.invokespecial( // 3 bytes + THROWN_EXCEPTION, "", MethodTypeDesc.of(CD_void)); + codeBuilder.athrow(); // 1 byte + codeBuilder.labelBinding(normal); + for (int i = 0; i < groups; i++) { + codeBuilder.iload(1); + codeBuilder.iconst_1(); + codeBuilder.iadd(); + codeBuilder.istore(1); + } + codeBuilder.iload(1); // 1 byte + codeBuilder.ireturn(); // 1 byte + // Declare every local; see oversizedClassBytes for why. + Label start = codeBuilder.startLabel(); + Label end = codeBuilder.endLabel(); + codeBuilder.localVariable( + 0, "this", ClassDesc.of(sampleClassName()), start, end); + codeBuilder.localVariable(1, "arg", CD_int, start, end); + }))); + } + + /** + * Returns {@link Sample} plus a method whose instrumented form contains a branch that + * does not fit in its 2-byte operand, so that the code builder runs the handler twice. Unlike + * {@link #widenedBranchClassBytes}, the method here is small enough to be instrumented normally, + * because this fixture is for the non-JDK path where the method is tracked. + * + * @return the bytes of {@link Sample} plus that method + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private static byte[] trackedWidenedBranchClassBytes() throws IOException { + ClassFile classFile = ClassFile.of(); + ClassModel classModel = classFile.parse(classBytes(sampleClassName())); + return classFile.transformClass( + classModel, + ClassTransform.endHandler( + classBuilder -> + classBuilder.withMethodBody( + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + // The branch is always taken, so the groups it jumps over are dead at run + // time; they exist to put the branch target out of reach in the instrumented + // code, which is several times longer than what is written here. + Label target = codeBuilder.newLabel(); + codeBuilder.iconst_0(); + codeBuilder.istore(2); + codeBuilder.iload(2); + codeBuilder.ifge(target); + for (int i = 0; i < TRACKED_WIDENING_GROUPS; i++) { + codeBuilder.iload(2); + codeBuilder.iconst_1(); + codeBuilder.iadd(); + codeBuilder.istore(2); + } + codeBuilder.labelBinding(target); + codeBuilder.iload(1); + codeBuilder.ireturn(); + }))); + } + + /** + * Returns the bytes of {@link Sample} with an added method, {@link #OVERSIZED_METHOD}, whose + * instrumented form exceeds the JVM's 64K code-size limit and whose branch fits in a 2-byte + * operand only until a DCompMarker parameter renumbers the locals it jumps over. See {@link + * #WIDENING_GROUPS}. + * + * @return the bytes of {@link Sample} plus that method + * @throws IOException if the class file for {@link Sample} cannot be read + */ + private static byte[] widenedBranchClassBytes() throws IOException { + ClassFile classFile = ClassFile.of(); + ClassModel classModel = classFile.parse(classBytes(sampleClassName())); + return classFile.transformClass( + classModel, + ClassTransform.endHandler( + classBuilder -> + classBuilder.withMethodBody( + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + // The branch is always taken, so the groups it jumps over are dead at run + // time; they exist to put the branch target out of reach once the + // instructions between here and there are widened. + Label target = codeBuilder.newLabel(); + codeBuilder.iconst_0(); + codeBuilder.istore(3); + codeBuilder.iload(3); + codeBuilder.ifge(target); + for (int i = 0; i < WIDENING_GROUPS; i++) { + codeBuilder.iload(3); + codeBuilder.iconst_1(); + codeBuilder.iadd(); + codeBuilder.istore(3); + } + codeBuilder.labelBinding(target); + codeBuilder.iload(1); + codeBuilder.ireturn(); + Label start = codeBuilder.startLabel(); + Label end = codeBuilder.endLabel(); + codeBuilder.localVariable( + 0, "this", ClassDesc.of(sampleClassName()), start, end); + codeBuilder.localVariable(1, "arg", CD_int, start, end); + codeBuilder.localVariable(3, "local3", CD_int, start, end); + }))); + } + + /** + * Returns the binary name of {@link Sample}. + * + * @return the binary name of {@link Sample} + */ + @SuppressWarnings("signature:return") // the name of a nested class + private static @BinaryName String sampleClassName() { + return Sample.class.getName(); + } + + /** + * Tests that a method whose instrumented form exceeds the JVM's 64K code-size limit is emitted + * without instrumentation, rather than causing the whole class to be abandoned. {@code + * java.lang.classfile} does not report the oversized method until it serializes the class, so + * {@link DCInstrument24#instrument_jdk_class} has to rebuild the class from scratch; this test + * checks that the rebuild leaves the rest of the class instrumented, and that the oversized + * method is not reported as skipped. + * + *

Also checks that the emitted method still maintains the tag stack. It carries the + * DCompMarker parameter, so its callers use the calling convention for an instrumented method, + * and the emitted method must honor that convention even though it does no comparability + * tracking: it discards the tag pushed for the primitive argument and pushes one for the + * primitive result. The method retains its original body to avoid adding an observable forwarding + * frame. + */ + @Test + public void testOversizedMethodUsesOriginalBody() throws IOException { + byte[] original = oversizedClassBytes(OVERSIZED_GROUPS); + ClassFile classFile = ClassFile.of(); + ClassModel originalModel = classFile.parse(original); + // The premise of this test is that only the *instrumented* method is too large. + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " is already over the code-size limit", + codeLength(originalModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)) < 65536); + + ClassInfo classInfo = new ClassInfo(sampleClassName(), classLoader()); + DCInstrument24 dci = new DCInstrument24(classFile, classFile.parse(original), true); + @BinaryName String savedInstrumentationInterface = DCRuntime.instrumentation_interface; + // BuildJDK24 sets this static field before each class it instruments. + DCRuntime.instrumentation_interface = "daikon.dcomp.DCompInstrumented"; + byte[] instrumented; + try { + // Skipping the oversized method prints a warning; discard it. + instrumented = withDiagnosticsDiscarded(() -> dci.instrument_jdk_class(classInfo)); + } finally { + DCRuntime.instrumentation_interface = savedInstrumentationInterface; + } + + // The method is emitted with the DCompMarker signature, so it is still callable and does not + // belong on the skipped_methods list, which names methods that are missing from the + // instrumented class. DCInstrument treats oversized methods the same way. + assertFalse( + "oversized method reported as skipped: " + dci.get_skipped_methods(), + dci.get_skipped_methods().stream().anyMatch(m -> m.contains(OVERSIZED_METHOD))); + + ClassModel instrumentedModel = classFile.parse(instrumented); + // Exactly the tag-stack bookkeeping, and none of the instrumentation proper: no + // create_tag_frame, no enter/exit. OVERSIZED_METHOD takes one primitive argument and returns + // a primitive, so it owes the caller one discarded argument tag and one pushed result tag. + assertEquals( + "oversized method does not maintain the tag stack", + Set.of("discard_tag", "push_const"), + runtimeCalls(instrumentedModel, OVERSIZED_METHOD)); + assertFalse( + "oversized method adds an observable forwarding frame", + callsOwnMethod(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int))); + assertFalse( + "rest of the class was not instrumented", + runtimeCalls(instrumentedModel, SMALL_METHOD).isEmpty()); + } + + /** + * Tests that an oversized method whose branch has to be widened gets exactly one DCompMarker + * parameter. + * + *

java.lang.classfile runs a code-building handler a second time when the code the first run + * built contains a branch whose target does not fit in the branch instruction's 2-byte operand: + * it discards that code and runs the handler again, this time widening the branch. {@code + * copyOversizedMethod} adds the DCompMarker parameter from inside that handler, and for the + * method here it is adding the parameter -- which moves local 3 to slot 4 and so widens every + * instruction that references it -- that puts the branch target out of reach. The second run + * therefore starts from a MethodGen24 that already has the parameter. Adding it again would + * append a second DCompMarker to the parameter list and shift the locals a second time, emitting + * a body whose locals are a slot higher than the method's descriptor provides, which fails + * verification when the class is loaded below. + * + * @throws IOException if the class file for {@link Sample} cannot be read + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked + */ + @Test + public void testOversizedMethodWithWidenedBranch() + throws IOException, ReflectiveOperationException { + byte[] original = widenedBranchClassBytes(); + ClassFile classFile = ClassFile.of(); + ClassInfo classInfo = new ClassInfo(sampleClassName(), classLoader()); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + @BinaryName String savedInstrumentationInterface = DCRuntime.instrumentation_interface; + // BuildJDK24 sets these before each class it instruments. + Premain.jdk_instrumented = false; + DCRuntime.instrumentation_interface = "daikon.dcomp.DCompInstrumented"; + DCInstrument24 dci = new DCInstrument24(classFile, classFile.parse(original), true); + byte[] instrumented; + try { + // Skipping the oversized method prints a warning; discard it. + instrumented = withDiagnosticsDiscarded(() -> dci.instrument_jdk_class(classInfo)); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + DCRuntime.instrumentation_interface = savedInstrumentationInterface; + } + + ClassModel instrumentedModel = classFile.parse(instrumented); + // The premise of this test: the method kept its original body plus the tag-stack bookkeeping, + // rather than being fully instrumented or replaced by a forwarding stub. + assertEquals( + "oversized method does not maintain the tag stack", + Set.of("discard_tag", "push_const"), + runtimeCalls(instrumentedModel, OVERSIZED_METHOD)); + assertFalse( + "oversized method adds an observable forwarding frame", + callsOwnMethod(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int))); + + MethodModel body = instrumentedCopy(instrumentedModel, OVERSIZED_METHOD); + assertEquals( + "oversized method has the wrong parameters", + MethodTypeDesc.of(CD_int, CD_int, ClassDesc.of("daikon.dcomp.DCompMarker")), + body.methodTypeSymbol()); + long markerLocals = + body.code() + .orElseThrow() + .elementStream() + .filter(e -> e instanceof LocalVariable lv && lv.name().stringValue().equals("marker")) + .count(); + assertEquals("oversized method has the wrong number of marker locals", 1, markerLocals); + + // Loading the class runs the verifier over the emitted body. + @BinaryName String className = sampleClassName(); + Class generatedClass = + byteArrayClassLoader(Map.of(className, withShadowRuntimeRedirected(instrumentedModel))) + .loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + // The tag stack now holds only this method's marker. + int markerOnlySize = DCRuntime.tag_stack_size(); + DCRuntime.push_const(); // primitive argument tag + @SuppressWarnings({ + "nullness:argument", // The DCompMarker argument is always null. + "signedness:argument" // TODO + }) + Object result = + nonNullResult( + "oversized method returned null", + generatedClass + .getMethod(OVERSIZED_METHOD, int.class, DCompMarker.class) + .invoke(receiver, 1, null)); + assertEquals("oversized method returned the wrong value", 1, result); + // It consumed the argument tag and left exactly the result tag. + assertEquals( + "oversized method did not leave exactly the result tag", + markerOnlySize + 1, + DCRuntime.tag_stack_size()); + DCRuntime.discard_tag(1); + assertEquals("oversized method left a stale tag", markerOnlySize, DCRuntime.tag_stack_size()); + } finally { + DCRuntime.normal_exit(tagFrame); + } + } + + /** + * Returns the result of a reflective method call, which must not be null. Use this rather than + * {@link org.junit.Assert#assertNotNull}, which the Nullness Checker treats as requiring a + * non-null argument because JUnit 4 is not annotated. + * + * @param message the message to use if the result is null + * @param result the result of a reflective method call + * @return {@code result} + */ + private static Object nonNullResult(String message, @Nullable Object result) { + if (result == null) { + throw new AssertionError(message); + } + return result; + } + + /** + * Returns true if the instrumented copy of the named method -- the copy with a DCompMarker + * parameter -- invokes a method of its own class with the same name and the given descriptor. + * Used to check that an oversized method does not add a forwarding frame. + * + * @param classModel an instrumented class + * @param methodName the name of the method to examine + * @param target the descriptor of the method it should invoke + * @return true if the instrumented copy invokes that method + */ + private static boolean callsOwnMethod( + ClassModel classModel, String methodName, MethodTypeDesc target) { + String ownName = classModel.thisClass().asInternalName(); + for (CodeElement element : instrumentedCopy(classModel, methodName).code().orElseThrow()) { + if (element instanceof InvokeInstruction invoke + && invoke.owner().asInternalName().equals(ownName) + && invoke.name().stringValue().equals(methodName) + && invoke.typeSymbol().equals(target)) { + return true; + } + } + return false; + } + + /** + * Returns the instrumented copy of the named method, that is, the copy that has a DCompMarker + * parameter. + * + * @param classModel an instrumented class + * @param methodName the name of the method + * @return the instrumented copy of the named method + */ + private static MethodModel instrumentedCopy(ClassModel classModel, String methodName) { + for (MethodModel method : classModel.methods()) { + if (!method.methodName().stringValue().equals(methodName)) { + continue; + } + List params = method.methodTypeSymbol().parameterList(); + if (!params.isEmpty() && params.get(params.size() - 1).displayName().equals("DCompMarker")) { + return method; + } + } + throw new Error("no instrumented copy of " + methodName); + } + + /** + * Tests that a method that is too large for even the minimal tag-stack bookkeeping is emitted as + * a forwarding stub, rather than aborting the class. The bookkeeping is only a few bytes long, + * but the method that needs it is by definition close to the JVM's 64K code-size limit, so the + * copy that {@link DCInstrument24#copyOversizedMethod} makes can exceed the limit too. + * + *

The forwarding stub has the DCompMarker signature its callers expect, discards primitive + * argument tags, invokes the unchanged original body with virtual dispatch, and produces the + * primitive result tag. + * + * @throws IOException if the class file for {@link Sample} cannot be read + * @throws ReflectiveOperationException if the generated classes cannot be loaded or invoked + */ + @SuppressWarnings("signedness:argument") // TODO + @Test + public void testHugeMethodUsesForwardingStub() throws IOException, ReflectiveOperationException { + byte[] original = oversizedClassBytes(HUGE_GROUPS); + ClassFile classFile = ClassFile.of(); + ClassModel originalModel = classFile.parse(original); + // The premise of this test is that the uninstrumented method fits, but only just: adding the + // tag-stack bookkeeping to it would not. + int length = codeLength(originalModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + assertTrue("uninstrumented " + OVERSIZED_METHOD + " does not fit: " + length, length <= 65535); + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " has room for the bookkeeping: " + length, + length > 65535 - 8); + + ClassInfo classInfo = new ClassInfo(sampleClassName(), classLoader()); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + @BinaryName String savedInstrumentationInterface = DCRuntime.instrumentation_interface; + // BuildJDK24 sets this static field before each class it instruments. + Premain.jdk_instrumented = false; + DCRuntime.instrumentation_interface = "daikon.dcomp.DCompInstrumented"; + DCInstrument24 dci = new DCInstrument24(classFile, classFile.parse(original), true); + byte[] instrumented; + try { + // Skipping the oversized method prints a warning; discard it. + instrumented = withDiagnosticsDiscarded(() -> dci.instrument_jdk_class(classInfo)); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + DCRuntime.instrumentation_interface = savedInstrumentationInterface; + } + + ClassModel instrumentedModel = classFile.parse(instrumented); + assertEquals( + "huge method's forwarding stub does not maintain the tag stack", + Set.of("discard_tag", "push_const"), + runtimeCalls(instrumentedModel, OVERSIZED_METHOD)); + assertTrue( + "huge method has no forwarding stub", + callsOwnMethod(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int))); + assertEquals( + "huge method's forwarding stub does not preserve virtual dispatch", + INVOKEVIRTUAL, + ownMethodCallOpcode( + instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int))); + // The unchanged original body remains alongside the small DCompMarker forwarding overload. + assertEquals( + "huge method's body was changed", + length, + codeLength(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int))); + assertFalse( + "huge method reported as skipped: " + dci.get_skipped_methods(), + dci.get_skipped_methods().stream().anyMatch(m -> m.contains(OVERSIZED_METHOD))); + assertFalse( + "rest of the class was not instrumented", + runtimeCalls(instrumentedModel, SMALL_METHOD).isEmpty()); + + byte[] executable = withShadowRuntimeRedirected(instrumentedModel); + + @BinaryName String superclassName = sampleClassName(); + @SuppressWarnings("signature:assignment") // Appending a nested-class suffix preserves format. + @BinaryName String subclassName = superclassName + "$DispatchOverride"; + byte[] subclass = + classFile.build( + ClassDesc.of(subclassName), + classBuilder -> { + classBuilder.withFlags(ClassFile.ACC_PUBLIC | ClassFile.ACC_SUPER); + classBuilder.withSuperclass(ClassDesc.of(superclassName)); + classBuilder.withMethodBody( + "", + MethodTypeDesc.of(CD_void), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + codeBuilder.aload(0); + codeBuilder.invokespecial( + ClassDesc.of(superclassName), "", MethodTypeDesc.of(CD_void)); + codeBuilder.return_(); + }); + classBuilder.withMethodBody( + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + codeBuilder.bipush(42); + codeBuilder.ireturn(); + }); + }); + ClassLoader loader = + byteArrayClassLoader(Map.of(superclassName, executable, subclassName, subclass)); + Class superclass = loader.loadClass(superclassName); + Object receiver = loader.loadClass(subclassName).getConstructor().newInstance(); + + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + DCRuntime.push_const(); + @SuppressWarnings("nullness:argument") // The DCompMarker argument is always null. + Object result = + nonNullResult( + "forwarding stub returned null", + superclass + .getMethod(OVERSIZED_METHOD, int.class, DCompMarker.class) + .invoke(receiver, 1, null)); + assertEquals("forwarding stub bypassed the subclass override", 42, result); + DCRuntime.discard_tag(1); + } finally { + DCRuntime.normal_exit(tagFrame); + } + } + + /** + * Tests the final oversized-method fallback for a JUnit class in the legacy BCEL instrumenter. A + * JUnit method retains its original descriptor, so the fallback must put the bookkeeping in a + * wrapper with that descriptor and move the unchanged body to a private marker overload. + * + * @throws IOException if the generated class cannot be parsed + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked + */ + @SuppressWarnings("signedness:argument") // TODO + @Test + public void testHugeJunitMethodUsesForwardingStub() + throws IOException, ReflectiveOperationException { + ClassFile classFile = ClassFile.of(); + byte[] original = oversizedClassBytes(HUGE_GROUPS); + byte[] junitClass = withJunitTestAnnotation(original); + + @BinaryName String className = sampleClassName(); + JavaClass parsed = new ClassParser(new ByteArrayInputStream(junitClass), className).parse(); + boolean wasJunitClass = Premain.junitTestClasses.contains(className); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + List savedOmitPattern = Runtime.ppt_omit_pattern; + JavaClass instrumented; + try { + Premain.jdk_instrumented = false; + Runtime.ppt_omit_pattern = List.of(Pattern.compile(Pattern.quote(className))); + DCInstrument dci = new DCInstrument(parsed, false, classLoader()); + instrumented = withDiagnosticsDiscarded(dci::instrument); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + Runtime.ppt_omit_pattern = savedOmitPattern; + if (!wasJunitClass) { + Premain.junitTestClasses.remove(className); + } + } + + ClassModel instrumentedModel = classFile.parse(instrumented.getBytes()); + MethodModel wrapper = + methodWithType(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + MethodModel body = + methodWithType( + instrumentedModel, + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int, ClassDesc.of("daikon.dcomp.DCompMarker"))); + + assertEquals( + "JUnit forwarding stub does not maintain the tag stack", + // uninstrumented_exit is the catch-all handler's call; see + // testHugeThrowingJunitMethodCleansUpTagStackOnException. + Set.of("uninstrumented_enter", "uninstrumented_exit_primitive", "uninstrumented_exit"), + runtimeCalls(wrapper)); + assertEquals("unchanged body contains runtime calls", Set.of(), runtimeCalls(body)); + assertEquals( + "oversized body was changed", + codeLength(classFile.parse(original), OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)), + ((CodeAttribute) body.code().orElseThrow()).codeLength()); + assertTrue("oversized body is not private", body.flags().has(AccessFlag.PRIVATE)); + assertTrue("oversized body is not synthetic", body.flags().has(AccessFlag.SYNTHETIC)); + + Class generatedClass = + byteArrayClassLoader(Map.of(className, instrumented.getBytes())).loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + // The tag stack now holds only this method's marker. + int markerOnlySize = DCRuntime.tag_stack_size(); + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + Object result = + nonNullResult( + "forwarding stub returned null", + generatedClass.getMethod(OVERSIZED_METHOD, int.class).invoke(receiver, 1)); + assertEquals("forwarding stub returned the wrong value", HUGE_GROUPS + 1, result); + + // The stub consumed the argument tag and left exactly the result tag. + assertEquals( + "forwarding stub did not leave exactly the result tag", + markerOnlySize + 1, + DCRuntime.tag_stack_size()); + DCRuntime.discard_tag(1); + assertEquals("forwarding stub left a stale tag", markerOnlySize, DCRuntime.tag_stack_size()); + } finally { + DCRuntime.normal_exit(tagFrame); + } + } + + /** + * Tests that the calls made by an oversized JUnit method's uninstrumented body do not consume + * tags belonging to an outer frame. + * + *

A JUnit method keeps its original descriptor, so the emitted method replaces the + * instrumented version rather than sitting alongside it, and the calls its retained body makes + * reach instrumented methods. Those callees pop a tag for each primitive parameter, but an + * uninstrumented body pushes none, so without the {@code DRuntime.uninstrumented_enter} marker + * the callee would pop tags belonging to whatever frame is below -- eventually the method marker + * itself, which leaves the tag stack unusable. + * + * @throws IOException if the generated class cannot be parsed + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked + */ + @SuppressWarnings("signedness:argument") // TODO + @Test + public void testOversizedJunitMethodDoesNotStealSiblingTags() + throws IOException, ReflectiveOperationException { + ClassFile classFile = ClassFile.of(); + byte[] junitClass = withJunitTestAnnotation(siblingCallClassBytes(OVERSIZED_GROUPS)); + + @BinaryName String className = sampleClassName(); + JavaClass parsed = new ClassParser(new ByteArrayInputStream(junitClass), className).parse(); + boolean wasJunitClass = Premain.junitTestClasses.contains(className); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + List savedOmitPattern = Runtime.ppt_omit_pattern; + JavaClass instrumented; + try { + Premain.jdk_instrumented = false; + Runtime.ppt_omit_pattern = List.of(Pattern.compile(Pattern.quote(className))); + DCInstrument dci = new DCInstrument(parsed, false, classLoader()); + instrumented = withDiagnosticsDiscarded(dci::instrument); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + Runtime.ppt_omit_pattern = savedOmitPattern; + if (!wasJunitClass) { + Premain.junitTestClasses.remove(className); + } + } + + ClassModel instrumentedModel = classFile.parse(instrumented.getBytes()); + // A JUnit method keeps its original descriptor, so there is no DCompMarker copy of either + // method to look up. + MethodModel oversized = + methodWithType(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + MethodModel sibling = + methodWithType( + instrumentedModel, SIBLING_METHOD, MethodTypeDesc.of(CD_int, CD_int, CD_int)); + // The premise of this test is that the method was too large to instrument, so it kept its + // original body, bracketed by the uninstrumented-body bookkeeping and nothing else. + assertEquals( + "oversized method does not bracket its uninstrumented body", + // uninstrumented_exit is the catch-all handler's call; see + // testOversizedJunitMethodCleansUpTagStackOnException. + Set.of("uninstrumented_enter", "uninstrumented_exit_primitive", "uninstrumented_exit"), + runtimeCalls(oversized)); + // The sibling it calls was instrumented, so it pops a tag for each of its two parameters. + assertFalse("sibling method was not instrumented", runtimeCalls(sibling).isEmpty()); + + Class generatedClass = + byteArrayClassLoader(Map.of(className, instrumented.getBytes())).loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + // The tag stack now holds only this method's marker. + int markerOnlySize = DCRuntime.tag_stack_size(); + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + Object result = + nonNullResult( + "oversized method returned null", + generatedClass.getMethod(OVERSIZED_METHOD, int.class).invoke(receiver, 1)); + // combine(1, 1) returns 2, and each group adds 1 to it. + assertEquals("oversized method returned the wrong value", OVERSIZED_GROUPS + 2, result); + + // The method consumed the caller's tags and left exactly the result tag, and the tags its + // body's call left behind are gone. + assertEquals( + "oversized method did not leave exactly the result tag", + markerOnlySize + 1, + DCRuntime.tag_stack_size()); + DCRuntime.discard_tag(1); + assertEquals("oversized method left a stale tag", markerOnlySize, DCRuntime.tag_stack_size()); + } finally { + DCRuntime.normal_exit(tagFrame); + } + } + + /** + * Tests that the JUnit oversized-method fallback rebuilds the stack map of the body it moves to + * the private marker overload. Adding the DCompMarker parameter renumbers every local that + * follows the parameters, which widens the instructions that reference them and thereby shifts + * the bytecode offsets that the stack map records. If the body keeps the stack map it was parsed + * with, the class fails verification when it is loaded below. + * + *

This calls {@link DCInstrument#create_oversized_method} directly rather than instrumenting + * the class. Fully instrumenting a 64K method that has a stack map takes minutes, because BCEL + * rescans the instruction list for each instruction it rewrites, and the fully instrumented form + * is discarded as oversized anyway. + * + * @throws IOException if the generated class cannot be parsed + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked + */ + @SuppressWarnings("signedness:argument") // TODO + @Test + public void testOversizedJunitFallbackRebuildsStackMap() + throws IOException, ReflectiveOperationException { + ClassFile classFile = ClassFile.of(); + byte[] original = oversizedClassBytes(HUGE_BRANCHING_GROUPS, true); + // The premise of this test is that the uninstrumented method fits, but only just: adding the + // tag-stack bookkeeping to it would not, so the fallback is used. + int originalLength = + codeLength(classFile.parse(original), OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " does not fit: " + originalLength, + originalLength <= 65535); + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " has room for the bookkeeping: " + originalLength, + originalLength > 65535 - 7); + + @BinaryName String className = sampleClassName(); + JavaClass parsed = new ClassParser(new ByteArrayInputStream(original), className).parse(); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + JavaClass instrumented; + try { + Premain.jdk_instrumented = false; + DCInstrument dci = new DCInstrument(parsed, false, classLoader()); + Method huge = dci.classGen.containsMethod(OVERSIZED_METHOD, "(I)I"); + assert huge != null : "@AssumeAssertion(nullness): oversizedClassBytes added this method"; + // A JUnit method keeps its original descriptor, so no DCompMarker is added to it. + Method wrapper = + withDiagnosticsDiscarded( + () -> { + try { + return dci.create_oversized_method(huge, false); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + dci.classGen.replaceMethod(huge, wrapper); + instrumented = dci.classGen.getJavaClass(); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + } + + ClassModel instrumentedModel = classFile.parse(instrumented.getBytes()); + MethodModel body = + methodWithType( + instrumentedModel, + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int, ClassDesc.of("daikon.dcomp.DCompMarker"))); + // The body is unchanged except that the DCompMarker parameter displaces locals 2 and 3, which + // widens the two one-byte instructions that end up referencing slot 4. + assertEquals( + "oversized body was changed beyond renumbering its locals", + originalLength + 2, + ((CodeAttribute) body.code().orElseThrow()).codeLength()); + + // Loading the class verifies it, which is what checks the rebuilt stack map. + Class generatedClass = + byteArrayClassLoader(Map.of(className, instrumented.getBytes())).loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + Object result = + nonNullResult( + "forwarding stub returned null", + generatedClass.getMethod(OVERSIZED_METHOD, int.class).invoke(receiver, 1)); + assertEquals("forwarding stub returned the wrong value", HUGE_BRANCHING_GROUPS + 1, result); + DCRuntime.discard_tag(1); + } finally { + DCRuntime.normal_exit(tagFrame); + } + } + + /** + * Tests the last-resort form of the JUnit oversized-method fallback: a body that cannot be given + * the DCompMarker parameter at all, because adding it renumbers the locals and widens the + * instructions that reference them, which pushes a body that fit over the code-size limit. + * + *

BCEL does not reject an oversized code array -- the class file is simply written with a + * {@code code_length} that the JVM refuses to load -- so the fallback has to check the size + * itself and then distinguish the body from its wrapper by name, which leaves the body's code + * array byte-for-byte unchanged. + * + *

Like {@link #testOversizedJunitFallbackRebuildsStackMap}, this calls {@link + * DCInstrument#create_oversized_method} directly rather than instrumenting the class, because + * fully instrumenting a 64K method with a stack map takes minutes and its instrumented form is + * discarded as oversized anyway. + * + * @throws IOException if the generated class cannot be parsed + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked + */ + @SuppressWarnings("signedness:argument") // TODO + @Test + public void testOversizedJunitFallbackRenamesBodyThatCannotTakeTheMarker() + throws IOException, ReflectiveOperationException { + ClassFile classFile = ClassFile.of(); + // Two bytes of padding put the method close enough to the limit that the DCompMarker parameter + // does not fit; a group of four bytes could not. + byte[] original = oversizedClassBytes(HUGE_BRANCHING_GROUPS, true, 2); + int originalLength = + codeLength(classFile.parse(original), OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " does not fit: " + originalLength, + originalLength <= 65535); + // The premise of this test: adding the DCompMarker parameter widens the two one-byte + // instructions that end up referencing slot 4; see testOversizedJunitFallbackRebuildsStackMap. + assertTrue( + "the DCompMarker parameter still fits in " + OVERSIZED_METHOD + ": " + originalLength, + originalLength + 2 > 65535); + + @BinaryName String className = sampleClassName(); + JavaClass parsed = new ClassParser(new ByteArrayInputStream(original), className).parse(); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + JavaClass instrumented; + try { + Premain.jdk_instrumented = false; + DCInstrument dci = new DCInstrument(parsed, false, classLoader()); + Method huge = dci.classGen.containsMethod(OVERSIZED_METHOD, "(I)I"); + assert huge != null : "@AssumeAssertion(nullness): oversizedClassBytes added this method"; + // A JUnit method keeps its original descriptor, so no DCompMarker is added to it. + Method wrapper = + withDiagnosticsDiscarded( + () -> { + try { + return dci.create_oversized_method(huge, false); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + dci.classGen.replaceMethod(huge, wrapper); + instrumented = dci.classGen.getJavaClass(); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + } + + ClassModel instrumentedModel = classFile.parse(instrumented.getBytes()); + // The body kept its original descriptor and its code array; only its name changed. + MethodModel body = + methodWithType( + instrumentedModel, + DCInstrument.oversized_body_name(OVERSIZED_METHOD), + MethodTypeDesc.of(CD_int, CD_int)); + assertEquals( + "oversized body was changed", + originalLength, + ((CodeAttribute) body.code().orElseThrow()).codeLength()); + assertTrue("oversized body is not private", body.flags().has(AccessFlag.PRIVATE)); + assertTrue("oversized body is not synthetic", body.flags().has(AccessFlag.SYNTHETIC)); + + // Loading the class verifies it, which is what checks that the emitted body is well-formed. + Class generatedClass = + byteArrayClassLoader(Map.of(className, instrumented.getBytes())).loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + // The tag stack now holds only this method's marker. + int markerOnlySize = DCRuntime.tag_stack_size(); + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + Object result = + nonNullResult( + "forwarding stub returned null", + generatedClass.getMethod(OVERSIZED_METHOD, int.class).invoke(receiver, 1)); + assertEquals("forwarding stub returned the wrong value", HUGE_BRANCHING_GROUPS + 1, result); + assertEquals( + "forwarding stub did not leave exactly the result tag", + markerOnlySize + 1, + DCRuntime.tag_stack_size()); + DCRuntime.discard_tag(1); + } finally { + DCRuntime.normal_exit(tagFrame); + } } /** - * A superclass with a parameterized constructor, so that {@link TwoConstructors} can read a field - * in the argument expression of its {@code super()} call -- that is, before the superclass - * constructor has run. + * Returns the given class with an added method whose name is the one that the JUnit + * oversized-method fallback derives for the body it moves, {@link + * DCInstrument#oversized_body_name}, and whose descriptor is that of {@link #OVERSIZED_METHOD}. + * The added method returns -1, which distinguishes it from the moved body. + * + * @param classBytes the bytes of a class that has an {@link #OVERSIZED_METHOD} method + * @return the same class, plus a method that collides with the derived body name */ - public static class Base { + private static byte[] withBodyNameCollision(byte[] classBytes) { + ClassFile classFile = ClassFile.of(); + return classFile.transformClass( + classFile.parse(classBytes), + ClassTransform.endHandler( + classBuilder -> + classBuilder.withMethodBody( + DCInstrument.oversized_body_name(OVERSIZED_METHOD), + MethodTypeDesc.of(CD_int, CD_int), + ClassFile.ACC_PUBLIC, + codeBuilder -> { + codeBuilder.iconst_m1(); + codeBuilder.ireturn(); + }))); + } - /** An arbitrary value. */ - int base; + /** + * Tests that the last-resort JUnit oversized-method fallback does not emit a body whose name and + * descriptor duplicate those of a method the class already has. + * + *

That fallback distinguishes the body from its wrapper by name alone, so the body keeps the + * original method's descriptor; and in a JUnit test class every other method keeps its original + * descriptor too. A class that happens to declare a method with the derived name would therefore + * end up with two methods of the same name and descriptor, which {@code ClassGen.addMethod} does + * not check for and which makes the class unloadable. + * + *

Like {@link #testOversizedJunitFallbackRenamesBodyThatCannotTakeTheMarker}, this calls + * {@link DCInstrument#create_oversized_method} directly rather than instrumenting the class, + * because fully instrumenting a 64K method with a stack map takes minutes and its instrumented + * form is discarded as oversized anyway. + * + * @throws IOException if the generated class cannot be parsed + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked + */ + @SuppressWarnings("signedness:argument") // TODO + @Test + public void testOversizedJunitFallbackAvoidsBodyNameCollision() + throws IOException, ReflectiveOperationException { + ClassFile classFile = ClassFile.of(); + // Two bytes of padding put the method close enough to the limit that the DCompMarker parameter + // does not fit, which is what forces the fallback that renames the body; see + // testOversizedJunitFallbackRenamesBodyThatCannotTakeTheMarker. + byte[] original = withBodyNameCollision(oversizedClassBytes(HUGE_BRANCHING_GROUPS, true, 2)); + int originalLength = + codeLength(classFile.parse(original), OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " does not fit: " + originalLength, + originalLength <= 65535); + assertTrue( + "the DCompMarker parameter still fits in " + OVERSIZED_METHOD + ": " + originalLength, + originalLength + 2 > 65535); - /** - * Creates a new Base. - * - * @param base the value to store - */ - public Base(int base) { - this.base = base; + @BinaryName String className = sampleClassName(); + JavaClass parsed = new ClassParser(new ByteArrayInputStream(original), className).parse(); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + JavaClass instrumented; + @Identifier String bodyName; + try { + Premain.jdk_instrumented = false; + DCInstrument dci = new DCInstrument(parsed, false, classLoader()); + Method huge = dci.classGen.containsMethod(OVERSIZED_METHOD, "(I)I"); + assert huge != null : "@AssumeAssertion(nullness): oversizedClassBytes added this method"; + // Ask for the body name now, while the class holds only the colliding method, so that the + // test does not depend on how the collision is resolved. + bodyName = dci.unused_oversized_body_name(OVERSIZED_METHOD, "(I)I"); + // A JUnit method keeps its original descriptor, so no DCompMarker is added to it. + Method wrapper = + withDiagnosticsDiscarded( + () -> { + try { + return dci.create_oversized_method(huge, false); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + dci.classGen.replaceMethod(huge, wrapper); + instrumented = dci.classGen.getJavaClass(); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + } + + @Identifier String collidingName = DCInstrument.oversized_body_name(OVERSIZED_METHOD); + assertFalse( + "the derived body name was reused even though it was taken: " + bodyName, + bodyName.equals(collidingName)); + + ClassModel instrumentedModel = classFile.parse(instrumented.getBytes()); + // The body kept its original descriptor and its code array; only its name changed. + MethodModel body = + methodWithType(instrumentedModel, bodyName, MethodTypeDesc.of(CD_int, CD_int)); + assertEquals( + "oversized body was changed", + originalLength, + ((CodeAttribute) body.code().orElseThrow()).codeLength()); + assertTrue("oversized body is not private", body.flags().has(AccessFlag.PRIVATE)); + assertTrue("oversized body is not synthetic", body.flags().has(AccessFlag.SYNTHETIC)); + // The method that the derived name collided with is still there, and is still its own body. + assertEquals( + "the colliding method was displaced", + 2, + codeLength(instrumentedModel, collidingName, MethodTypeDesc.of(CD_int, CD_int))); + + // Defining the class is what rejects two methods with the same name and descriptor. + Class generatedClass = + byteArrayClassLoader(Map.of(className, instrumented.getBytes())).loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + Object result = + nonNullResult( + "forwarding stub returned null", + generatedClass.getMethod(OVERSIZED_METHOD, int.class).invoke(receiver, 1)); + assertEquals("forwarding stub returned the wrong value", HUGE_BRANCHING_GROUPS + 1, result); + DCRuntime.discard_tag(1); + assertEquals( + "the colliding method no longer returns its own value", + -1, + nonNullResult( + "colliding method returned null", + generatedClass.getMethod(collidingName, int.class).invoke(receiver, 1))); + } finally { + DCRuntime.normal_exit(tagFrame); } } /** - * A class with two constructors, the second of which reads a field before calling its superclass - * constructor. Used by {@link #constructorInitializedStateDoesNotLeakBetweenMethods}. + * Tests that an oversized JUnit method whose body throws leaves the tag stack as it found it and + * propagates the original throwable. + * + *

The emitted method brackets its retained body with {@code DCRuntime.uninstrumented_enter} + * and {@code DCRuntime.uninstrumented_exit}, but the exit call sits before each return, so a + * throw would skip it. The method's caller is JUnit's reflective invocation, which maintains no + * tag stack, so nothing else would remove the marker that {@code uninstrumented_enter} pushed or + * the tags that the body's calls left above it, and a later method would consume that garbage as + * its own argument tags. + * + * @throws IOException if the generated class cannot be parsed + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked */ - public static class TwoConstructors extends Base { + @SuppressWarnings("signedness:argument") // TODO + @Test + public void testOversizedJunitMethodCleansUpTagStackOnException() + throws IOException, ReflectiveOperationException { + ClassFile classFile = ClassFile.of(); + byte[] junitClass = withJunitTestAnnotation(throwingClassBytes(OVERSIZED_GROUPS)); - /** Creates a new TwoConstructors. This constructor reads no fields before {@code super()}. */ - public TwoConstructors() { - super(0); + @BinaryName String className = sampleClassName(); + JavaClass parsed = new ClassParser(new ByteArrayInputStream(junitClass), className).parse(); + boolean wasJunitClass = Premain.junitTestClasses.contains(className); + boolean savedJdkInstrumented = Premain.jdk_instrumented; + List savedOmitPattern = Runtime.ppt_omit_pattern; + JavaClass instrumented; + try { + Premain.jdk_instrumented = false; + Runtime.ppt_omit_pattern = List.of(Pattern.compile(Pattern.quote(className))); + DCInstrument dci = new DCInstrument(parsed, false, classLoader()); + instrumented = withDiagnosticsDiscarded(dci::instrument); + } finally { + Premain.jdk_instrumented = savedJdkInstrumented; + Runtime.ppt_omit_pattern = savedOmitPattern; + if (!wasJunitClass) { + Premain.junitTestClasses.remove(className); + } } - /** - * Creates a new TwoConstructors, reading {@code s.value} before {@code super()} runs. - * - * @param s supplies the value to store - */ - public TwoConstructors(Sample s) { - super(s.value); + ClassModel instrumentedModel = classFile.parse(instrumented.getBytes()); + // A JUnit method keeps its original descriptor, so there is no DCompMarker copy to look up. + MethodModel oversized = + methodWithType(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + MethodModel sibling = + methodWithType( + instrumentedModel, SIBLING_METHOD, MethodTypeDesc.of(CD_int, CD_int, CD_int)); + // The premise of this test is that the method was too large to instrument, so it kept its + // original body, bracketed on both its normal and its exceptional exit and nothing else. + assertEquals( + "oversized method does not bracket both exits from its uninstrumented body", + Set.of("uninstrumented_enter", "uninstrumented_exit_primitive", "uninstrumented_exit"), + runtimeCalls(oversized)); + // The sibling that the body calls before throwing was instrumented, so it leaves a result tag + // that the uninstrumented body never pops. + assertFalse("sibling method was not instrumented", runtimeCalls(sibling).isEmpty()); + + Class generatedClass = + byteArrayClassLoader(Map.of(className, instrumented.getBytes())).loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + java.lang.reflect.Method oversizedMethod = + generatedClass.getMethod(OVERSIZED_METHOD, int.class); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + // The tag stack now holds only this method's marker. + int markerOnlySize = DCRuntime.tag_stack_size(); + + // An argument of zero returns normally, which the exceptional-exit handling must not change. + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + Object result = + nonNullResult("oversized method returned null", oversizedMethod.invoke(receiver, 0)); + assertEquals("oversized method returned the wrong value", OVERSIZED_GROUPS, result); + assertEquals( + "oversized method did not leave exactly the result tag", + markerOnlySize + 1, + DCRuntime.tag_stack_size()); + DCRuntime.discard_tag(1); + assertEquals("oversized method left a stale tag", markerOnlySize, DCRuntime.tag_stack_size()); + + // A nonzero argument throws, after a call that leaves a tag above the body's marker. + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + assertEquals( + "oversized method threw the wrong exception", + IllegalStateException.class, + thrownCause(oversizedMethod, receiver, 1).getClass()); + // A throwing method produces no result tag, so the tag stack is back where it started. + assertEquals( + "exceptional exit from the oversized method left the tag stack dirty", + markerOnlySize, + DCRuntime.tag_stack_size()); + } finally { + DCRuntime.normal_exit(tagFrame); } } /** - * Tests that a class instrumented by {@link DCInstrument24#instrument_jdk_class} calls the shadow - * runtime class {@code java.lang.DCRuntime} rather than {@code daikon.dcomp.DCRuntime}. A class - * in a pre-instrumented {@code java.base} module may not refer to anything outside {@code - * java.base}. This must hold even when the DCInstrument24 constructor chose {@code - * daikon.dcomp.DCRuntime}, which it does whenever {@code Premain.jdk_instrumented} is false. + * Tests that the JUnit oversized-method forwarding stub leaves the tag stack as it found it and + * propagates the original throwable when the body it forwards to throws. The stub's {@code + * DCRuntime.uninstrumented_exit_primitive} call sits after the invocation of the body, so a throw + * would skip it; see {@link #testOversizedJunitMethodCleansUpTagStackOnException} for why nothing + * else would clean up. * - * @throws IOException if the class file for {@link Sample} cannot be read + *

Like {@link #testOversizedJunitFallbackRebuildsStackMap}, this calls {@link + * DCInstrument#create_oversized_method} directly rather than instrumenting the class, because + * fully instrumenting a 64K method with a stack map takes minutes and its instrumented form is + * discarded as oversized anyway. The sibling that the body calls is therefore uninstrumented and + * leaves no tag behind, so what this checks is the removal of the stub's own marker. + * + * @throws IOException if the generated class cannot be parsed + * @throws ReflectiveOperationException if the generated class cannot be loaded or invoked */ + @SuppressWarnings("signedness:argument") // TODO @Test - public void testJdkClassCallsShadowRuntime() throws IOException { + public void testHugeThrowingJunitMethodCleansUpTagStackOnException() + throws IOException, ReflectiveOperationException { + ClassFile classFile = ClassFile.of(); + byte[] original = throwingClassBytes(HUGE_THROWING_GROUPS); + int originalLength = + codeLength(classFile.parse(original), OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + assertEquals( + "unexpected length for " + OVERSIZED_METHOD, + 4 * HUGE_THROWING_GROUPS + THROWING_FIXED_BYTES, + originalLength); + // The premise of this test is that the uninstrumented method fits, but only just: adding the + // tag-stack bookkeeping to it would not, so the forwarding stub is used. + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " does not fit: " + originalLength, + originalLength <= 65535); + assertTrue( + "uninstrumented " + OVERSIZED_METHOD + " has room for the bookkeeping: " + originalLength, + originalLength > 65535 - 11); + + @BinaryName String className = sampleClassName(); + JavaClass parsed = new ClassParser(new ByteArrayInputStream(original), className).parse(); boolean savedJdkInstrumented = Premain.jdk_instrumented; - @BinaryName String savedInstrumentationInterface = DCRuntime.instrumentation_interface; - Premain.jdk_instrumented = false; - // BuildJDK24 sets this static field before each class it instruments. - DCRuntime.instrumentation_interface = "daikon.dcomp.DCompInstrumented"; - byte[] instrumented; + JavaClass instrumented; try { - instrumented = instrumentAsJdkClass(); + Premain.jdk_instrumented = false; + DCInstrument dci = new DCInstrument(parsed, false, classLoader()); + Method huge = dci.classGen.containsMethod(OVERSIZED_METHOD, "(I)I"); + assert huge != null : "@AssumeAssertion(nullness): throwingClassBytes added this method"; + // A JUnit method keeps its original descriptor, so no DCompMarker is added to it. + Method wrapper = + withDiagnosticsDiscarded( + () -> { + try { + return dci.create_oversized_method(huge, false); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + dci.classGen.replaceMethod(huge, wrapper); + instrumented = dci.classGen.getJavaClass(); } finally { Premain.jdk_instrumented = savedJdkInstrumented; - DCRuntime.instrumentation_interface = savedInstrumentationInterface; } - Set invoked = invokedClasses(instrumented); - assertTrue( - "instrumented class does not call java/lang/DCRuntime: " + invoked, - invoked.contains("java/lang/DCRuntime")); - assertFalse( - "instrumented class calls daikon/dcomp/DCRuntime: " + invoked, - invoked.contains("daikon/dcomp/DCRuntime")); + + ClassModel instrumentedModel = classFile.parse(instrumented.getBytes()); + MethodModel stub = + methodWithType(instrumentedModel, OVERSIZED_METHOD, MethodTypeDesc.of(CD_int, CD_int)); + MethodModel body = + methodWithType( + instrumentedModel, + OVERSIZED_METHOD, + MethodTypeDesc.of(CD_int, CD_int, ClassDesc.of("daikon.dcomp.DCompMarker"))); + assertEquals( + "JUnit forwarding stub does not maintain the tag stack on both exits", + Set.of("uninstrumented_enter", "uninstrumented_exit_primitive", "uninstrumented_exit"), + runtimeCalls(stub)); + assertEquals("unchanged body contains runtime calls", Set.of(), runtimeCalls(body)); + + // Loading the class verifies it, which is what checks the stack map frame that the stub's new + // exception handler needs. + Class generatedClass = + byteArrayClassLoader(Map.of(className, instrumented.getBytes())).loadClass(className); + Object receiver = generatedClass.getConstructor().newInstance(); + java.lang.reflect.Method oversizedMethod = + generatedClass.getMethod(OVERSIZED_METHOD, int.class); + Object[] tagFrame = DCRuntime.create_tag_frame("1"); + try { + // The tag stack now holds only this method's marker. + int markerOnlySize = DCRuntime.tag_stack_size(); + + // An argument of zero returns normally, which the exceptional-exit handling must not change. + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + Object result = + nonNullResult("forwarding stub returned null", oversizedMethod.invoke(receiver, 0)); + assertEquals("forwarding stub returned the wrong value", HUGE_THROWING_GROUPS, result); + assertEquals( + "forwarding stub did not leave exactly the result tag", + markerOnlySize + 1, + DCRuntime.tag_stack_size()); + DCRuntime.discard_tag(1); + assertEquals("forwarding stub left a stale tag", markerOnlySize, DCRuntime.tag_stack_size()); + + // A nonzero argument throws out of the body that the stub forwards to. + DCRuntime.push_const(); // primitive argument tag + DCRuntime.push_const(); // caller-produced primitive result tag + assertEquals( + "forwarding stub threw the wrong exception", + IllegalStateException.class, + thrownCause(oversizedMethod, receiver, 1).getClass()); + // A throwing method produces no result tag, so the tag stack is back where it started. + assertEquals( + "exceptional exit from the forwarding stub left the tag stack dirty", + markerOnlySize, + DCRuntime.tag_stack_size()); + } finally { + DCRuntime.normal_exit(tagFrame); + } } /** - * Instruments {@link Sample} as if it were a JDK class. + * Invokes the given method, which is expected to throw, and returns what it threw. * - * @return the instrumented bytes of {@link Sample} - * @throws IOException if the class file for {@link Sample} cannot be read + * @param method the method to invoke + * @param receiver the receiver to invoke it on + * @param arg the argument to pass + * @return the throwable that the method threw + * @throws ReflectiveOperationException if the method cannot be invoked */ - private byte[] instrumentAsJdkClass() throws IOException { - @SuppressWarnings("signature:assignment") // the name of a nested class - @BinaryName String classname = Sample.class.getName(); - InputStream sampleStream = - DCInstrumentTest24.class.getResourceAsStream("DCInstrumentTest24$Sample.class"); - if (sampleStream == null) { - throw new Error("cannot find the class file for " + classname); - } - byte[] original; - try (InputStream is = sampleStream) { - original = is.readAllBytes(); + private static Throwable thrownCause(java.lang.reflect.Method method, Object receiver, int arg) + throws ReflectiveOperationException { + try { + Object result = method.invoke(receiver, arg); + throw new AssertionError(method.getName() + " returned " + result + " instead of throwing"); + } catch (InvocationTargetException e) { + return nonNullThrowable(e.getCause()); } - ClassFile classFile = ClassFile.of(); - ClassModel classModel = classFile.parse(original); - ClassInfo classInfo = new ClassInfo(classname, DCInstrumentTest24.class.getClassLoader()); - DCInstrument24 dci = new DCInstrument24(classFile, classModel, true); - // instrument_jdk_class throws rather than returning null if it cannot instrument the class. - return dci.instrument_jdk_class(classInfo); } - /** Name of the method that {@link #oversizedClassBytes} adds to {@link Sample}. */ - private static final String OVERSIZED_METHOD = "tooBig"; - - /** Name of the {@link Sample} method that instruments normally. */ - private static final String SMALL_METHOD = "add"; - /** - * Number of {@code iload_0; iconst_1; iadd; istore_0} groups in {@link #OVERSIZED_METHOD}. Each - * group is 4 bytes, so the uninstrumented method is well under the JVM's 64K code-size limit, but - * instrumentation adds several DCRuntime calls per group, which pushes the instrumented form over - * it. + * Returns its argument, which must be non-null. + * + * @param cause the throwable that an invocation threw + * @return {@code cause} */ - private static final int OVERSIZED_GROUPS = 6000; + private static Throwable nonNullThrowable(@Nullable Throwable cause) { + if (cause == null) { + throw new AssertionError("the invocation failed with no cause"); + } + return cause; + } /** - * Returns the bytes of {@link Sample} with an added method, {@link #OVERSIZED_METHOD}, whose - * instrumented form exceeds the JVM's 64K code-size limit. The method is added to a real class - * rather than a synthetic one because DCInstrument24 resolves the class being instrumented, and - * its superclasses, from the classpath. + * Returns the given pre-instrumented-JDK class, made executable in this test JVM by redirecting + * its shadow runtime calls to the ordinary DynComp runtime. * - * @return the bytes of {@link Sample} plus a method that is too large to instrument - * @throws IOException if the class file for {@link Sample} cannot be read + * @param classModel a class instrumented by {@code instrument_jdk_class} + * @return the bytes of that class, calling {@code daikon.dcomp.DCRuntime} */ - private static byte[] oversizedClassBytes() throws IOException { - ClassFile classFile = ClassFile.of(); - ClassModel classModel = classFile.parse(classBytes(sampleClassName())); - return classFile.transformClass( - classModel, - ClassTransform.endHandler( - classBuilder -> - classBuilder.withMethodBody( - OVERSIZED_METHOD, - MethodTypeDesc.of(CD_int, CD_int), - ClassFile.ACC_PUBLIC | ClassFile.ACC_STATIC, - codeBuilder -> { - for (int i = 0; i < OVERSIZED_GROUPS; i++) { - codeBuilder.iload(0); - codeBuilder.iconst_1(); - codeBuilder.iadd(); - codeBuilder.istore(0); - } - codeBuilder.iload(0); - codeBuilder.ireturn(); - }))); + private static byte[] withShadowRuntimeRedirected(ClassModel classModel) { + ClassDesc runtimeClass = ClassDesc.of("daikon.dcomp.DCRuntime"); + return ClassFile.of() + .transformClass( + classModel, + ClassTransform.transformingMethodBodies( + (codeBuilder, element) -> { + if (element instanceof InvokeInstruction invoke + && invoke.owner().asInternalName().equals("java/lang/DCRuntime")) { + codeBuilder.invoke( + invoke.opcode(), + runtimeClass, + invoke.name().stringValue(), + invoke.typeSymbol(), + false); + } else { + codeBuilder.with(element); + } + })); } /** - * Returns the binary name of {@link Sample}. + * Returns a child-first class loader for the given class definitions. * - * @return the binary name of {@link Sample} + * @param definitions maps binary class names to class-file bytes + * @return the class loader */ - @SuppressWarnings("signature:return") // the name of a nested class - private static @BinaryName String sampleClassName() { - return Sample.class.getName(); + private static ClassLoader byteArrayClassLoader(Map<@BinaryName String, byte[]> definitions) { + return new ClassLoader(classLoader()) { + @Override + protected Class loadClass(@BinaryName String name, boolean resolve) + throws ClassNotFoundException { + if (!definitions.containsKey(name)) { + return super.loadClass(name, resolve); + } + Object lock = getClassLoadingLock(name); + synchronized (lock) { + Class result = findLoadedClass(name); + if (result == null) { + byte[] bytes = definitions.get(name); + assert bytes != null : "@AssumeAssertion(definitions.containsKey(name))"; + result = defineClass(name, bytes, 0, bytes.length); + } + if (resolve) { + resolveClass(result); + } + return result; + } + } + }; } /** - * Tests that a method whose instrumented form exceeds the JVM's 64K code-size limit is emitted - * without instrumentation, rather than causing the whole class to be abandoned. {@code - * java.lang.classfile} does not report the oversized method until it serializes the class, so - * {@link DCInstrument24#instrument_jdk_class} has to rebuild the class from scratch; this test - * checks that the rebuild leaves the rest of the class instrumented, and that the oversized - * method is not reported as skipped. + * Tests that a tracked method whose handler runs twice is registered only once. {@code + * java.lang.classfile} discards the code it built and runs the handler again when a branch does + * not fit in its 2-byte operand; see {@link daikon.chicory.MethodGen24#resetForCodeBuilder}. The + * second run must reuse the {@code MethodInfo} that the first one registered, or the method is + * added twice to {@code classInfo.method_infos} and to {@code DCRuntime.methods}, and the indices + * that {@code add_enter} and {@code add_exit} emit no longer agree with the runtime's list. + * + *

Registration happens only when {@code trackMethod && !in_jdk}, so this uses {@link + * DCInstrument24#instrument} rather than {@code instrument_jdk_class}: the tests that cover + * branch widening on the JDK path never reach the code this exercises. + * + * @throws IOException if the class file for {@link Sample} cannot be read */ @Test - public void testOversizedMethodIsSkipped() throws IOException { - byte[] original = oversizedClassBytes(); - ClassFile classFile = ClassFile.of(); - ClassModel originalModel = classFile.parse(original); - // The premise of this test is that only the *instrumented* method is too large. - assertTrue( - "uninstrumented " + OVERSIZED_METHOD + " is already over the code-size limit", - codeLength(originalModel, OVERSIZED_METHOD) < 65536); + public void testWidenedBranchRegistersTrackedMethodOnce() throws IOException { + byte[] original = trackedWidenedBranchClassBytes(); + ClassLoader loader = classLoader(); + ClassFile classFile = + ClassFile.of( + ClassFile.ClassHierarchyResolverOption.of( + ClassHierarchyResolver.ofResourceParsing(loader))); + @BinaryName String className = sampleClassName(); + ClassInfo classInfo = new ClassInfo(className, loader); - ClassInfo classInfo = new ClassInfo(sampleClassName(), classLoader()); - DCInstrument24 dci = new DCInstrument24(classFile, classFile.parse(original), true); + boolean savedJdkInstrumented = Premain.jdk_instrumented; @BinaryName String savedInstrumentationInterface = DCRuntime.instrumentation_interface; - // BuildJDK24 sets this static field before each class it instruments. + Premain.jdk_instrumented = false; DCRuntime.instrumentation_interface = "daikon.dcomp.DCompInstrumented"; + int methodsBefore = DCRuntime.methods.size(); byte[] instrumented; try { - // Skipping the oversized method prints a warning; discard it. - instrumented = withDiagnosticsDiscarded(() -> dci.instrument_jdk_class(classInfo)); + DCInstrument24 dci = new DCInstrument24(classFile, classFile.parse(original), false); + instrumented = dci.instrument(classInfo); } finally { + Premain.jdk_instrumented = savedJdkInstrumented; DCRuntime.instrumentation_interface = savedInstrumentationInterface; } - // The method is emitted with the DCompMarker signature, so it is still callable and does not - // belong on the skipped_methods list, which names methods that are missing from the - // instrumented class. DCInstrument treats oversized methods the same way. - assertFalse( - "oversized method reported as skipped: " + dci.get_skipped_methods(), - dci.get_skipped_methods().stream().anyMatch(m -> m.contains(OVERSIZED_METHOD))); + // instrument() returns null if anything goes wrong, including a method that does not fit; the + // fixture is sized so that it does. + assert instrumented != null : "@AssumeAssertion(nullness)"; + // The premise of this test: the instrumented method is long enough that a branch spanning it + // cannot fit in a 2-byte operand, which is what makes the code builder run the handler twice. ClassModel instrumentedModel = classFile.parse(instrumented); + int length = + ((CodeAttribute) instrumentedCopy(instrumentedModel, OVERSIZED_METHOD).code().orElseThrow()) + .codeLength(); assertTrue( - "oversized method was instrumented", - runtimeCalls(instrumentedModel, OVERSIZED_METHOD).isEmpty()); - assertFalse( - "rest of the class was not instrumented", - runtimeCalls(instrumentedModel, SMALL_METHOD).isEmpty()); + "instrumented " + OVERSIZED_METHOD + " is too short to widen a branch: " + length, + length > 32767); + + long registered = + classInfo.method_infos.stream() + .filter(mi -> mi.method_name.equals(OVERSIZED_METHOD)) + .count(); + assertEquals("tracked method was not registered exactly once", 1, registered); + assertEquals( + "DCRuntime.methods disagrees with classInfo.method_infos", + classInfo.method_infos.size(), + DCRuntime.methods.size() - methodsBefore); } /** @@ -576,9 +2171,13 @@ public DelegatingConstructor(int flag) { * not yet initialized; {@link DCInstrument24#tag_fields_ok} enforces that. The flag was set when * a constructor reached its {@code super()} call but never cleared, so in a class with more than * one constructor every constructor after the first was treated as initialized from its very - * first instruction. {@link DCInstrument24#instrument_jdk_class} made this worse: it may rebuild - * a class with the same instance, so a value left over from an abandoned attempt would make the - * retry emit different code than the first attempt. + * first instruction. Two forms of retry make this worse, so the flag is cleared in {@code + * instrumentCode}, which both of them re-enter: {@link DCInstrument24#instrument_jdk_class} may + * rebuild a class with the same instance, and {@code java.lang.classfile} may run a code-building + * handler a second time to widen a branch. Either way, a value left over from the first run would + * make the second run emit different code -- including a tag accessor for a field that a + * constructor touches before its {@code super()} call, which for an instance field is a call on + * an uninitialized {@code this} and so does not verify. * *

{@link TwoConstructors} reads {@code Sample.value} in the argument to its {@code super()} * call, so the read happens while {@code this} is still uninitialized and must use the {@code @@ -699,17 +2298,26 @@ private static T withDiagnosticsDiscarded(Supplier action) { /** * Returns the length, in bytes, of the code of the named method. * + *

An instrumented class can hold two methods of the same name, the unchanged original and the + * DCompMarker overload, so the descriptor selects between them rather than relying on the order + * in which they were emitted. + * * @param classModel the class containing the method * @param methodName the name of the method + * @param descriptor the descriptor of the method * @return the code length of the named method */ - private static int codeLength(ClassModel classModel, String methodName) { + private static int codeLength( + ClassModel classModel, String methodName, MethodTypeDesc descriptor) { for (MethodModel method : classModel.methods()) { - if (method.methodName().stringValue().equals(methodName)) { - return method.code().orElseThrow().elementList().size(); + if (method.methodName().stringValue().equals(methodName) + && method.methodTypeSymbol().equals(descriptor)) { + // A CodeModel that was parsed from a class file, as opposed to one being built, is a + // CodeAttribute, which knows the length of the code array. + return ((CodeAttribute) method.code().orElseThrow()).codeLength(); } } - throw new Error("no method named " + methodName); + throw new Error("no method " + methodName + descriptor.displayDescriptor()); } /** @@ -722,29 +2330,70 @@ private static int codeLength(ClassModel classModel, String methodName) { * @return the names of the DCRuntime methods that the instrumented copy invokes */ private static Set runtimeCalls(ClassModel classModel, String methodName) { + return runtimeCalls(instrumentedCopy(classModel, methodName)); + } + + /** + * Returns the DCRuntime methods invoked by the given method. + * + * @param method the method to examine + * @return the names of the DCRuntime methods that the method invokes + */ + private static Set runtimeCalls(MethodModel method) { Set result = new HashSet<>(); + method + .code() + .ifPresent( + code -> { + for (CodeElement element : code) { + if (element instanceof InvokeInstruction invoke + && invoke.owner().asInternalName().endsWith("/DCRuntime")) { + result.add(invoke.name().stringValue()); + } + } + }); + return result; + } + + /** + * Returns the method with the given name and type. + * + * @param classModel the class containing the method + * @param methodName the method name + * @param methodType the method type + * @return the matching method + */ + private static MethodModel methodWithType( + ClassModel classModel, String methodName, MethodTypeDesc methodType) { for (MethodModel method : classModel.methods()) { - if (!method.methodName().stringValue().equals(methodName)) { - continue; + if (method.methodName().stringValue().equals(methodName) + && method.methodTypeSymbol().equals(methodType)) { + return method; } - List params = method.methodTypeSymbol().parameterList(); - if (params.isEmpty() || !params.get(params.size() - 1).displayName().equals("DCompMarker")) { - // This is the uninstrumented copy of the method, which has no DCompMarker parameter. - continue; + } + throw new AssertionError("no method named " + methodName + " with type " + methodType); + } + + /** + * Returns the opcode used by the instrumented copy's call to its own original overload. + * + * @param classModel an instrumented class + * @param methodName the method to examine + * @param target the original overload's descriptor + * @return the invocation opcode + */ + private static java.lang.classfile.Opcode ownMethodCallOpcode( + ClassModel classModel, String methodName, MethodTypeDesc target) { + String ownName = classModel.thisClass().asInternalName(); + for (CodeElement element : instrumentedCopy(classModel, methodName).code().orElseThrow()) { + if (element instanceof InvokeInstruction invoke + && invoke.owner().asInternalName().equals(ownName) + && invoke.name().stringValue().equals(methodName) + && invoke.typeSymbol().equals(target)) { + return invoke.opcode(); } - method - .code() - .ifPresent( - code -> { - for (CodeElement element : code) { - if (element instanceof InvokeInstruction invoke - && invoke.owner().asInternalName().endsWith("/DCRuntime")) { - result.add(invoke.name().stringValue()); - } - } - }); } - return result; + throw new AssertionError("no call to the original " + methodName + " overload"); } /** diff --git a/java/daikon/dcomp/DCRuntime.java b/java/daikon/dcomp/DCRuntime.java index dba126448..b2725681b 100644 --- a/java/daikon/dcomp/DCRuntime.java +++ b/java/daikon/dcomp/DCRuntime.java @@ -719,10 +719,23 @@ public static Object[] create_tag_frame(String params) { for (int ii = 1; ii < params.length(); ii++) { int offset = params.charAt(ii) - '0'; // Character.digit (params.charAt(ii), Character.MAX_RADIX); - assert td.tag_stack.peek() != method_marker; - tag_frame[offset] = td.tag_stack.pop(); - if (debug_tag_frame) { - System.out.printf("popped %s into tag_frame[%d]%n", tag_frame[offset], offset); + if (td.tag_stack.isEmpty() || td.tag_stack.peek() == method_marker) { + // The caller left no argument tags on the tag stack. Either it is an uninstrumented + // method body reached through an instrumented calling convention (see + // uninstrumented_enter, which pushes the marker that stops this loop) or the call did not + // come from Java code at all, as when JUnit invokes a test method reflectively. Use a + // fresh tag, which makes the parameter comparable to nothing else, rather than consuming + // a tag that belongs to an outer frame. + tag_frame[offset] = new Constant(); + if (debug_tag_frame) { + System.out.printf( + "caller left no tag; created %s for tag_frame[%d]%n", tag_frame[offset], offset); + } + } else { + tag_frame[offset] = td.tag_stack.pop(); + if (debug_tag_frame) { + System.out.printf("popped %s into tag_frame[%d]%n", tag_frame[offset], offset); + } } } @@ -859,6 +872,93 @@ public static void normal_exit_primitive(Object[] tag_frame) { } } + /** + * Called on entry to an uninstrumented method body that is reached through an instrumented + * calling convention. That happens for a method whose instrumented form exceeds the JVM's 64K + * code-size limit; see {@code DCInstrument.create_oversized_method}. + * + *

Discards the tags that the caller left for this call, then pushes a method marker. The + * marker matters because an uninstrumented body pushes no argument tags for the calls it makes: + * without it, a callee that does maintain the tag stack would consume tags belonging to an outer + * frame. {@link #create_tag_frame} sees the marker and creates fresh tags instead. {@link + * #uninstrumented_exit} and {@link #uninstrumented_exit_primitive} remove the marker. If an + * exception propagates out of the body instead, a catch-all handler that DCInstrument added + * around the body calls {@code uninstrumented_exit} and rethrows; the enclosing method's {@code + * normal_exit} would not do it, because the body belongs to a JUnit test method whose caller is + * JUnit's reflective invocation rather than an instrumented frame. + * + * @param tagCount the number of tags the caller left on the tag stack for this call + */ + public static void uninstrumented_enter(int tagCount) { + if (debug) { + System.out.printf("%nEnter uninstrumented: %s%n", caller_name()); + } + + // This may be the first DCRuntime method called on this thread, so the per-thread data map + // must be checked, exactly as in create_tag_frame. + Thread t = Thread.currentThread(); + ThreadData td = thread_to_data.computeIfAbsent(t, __ -> new ThreadData()); + + while (--tagCount >= 0 && !td.tag_stack.isEmpty() && td.tag_stack.peek() != method_marker) { + td.tag_stack.pop(); + } + td.tag_stack.push(method_marker); + td.tag_stack_call_depth++; + if (debug_tag_frame) { + System.out.printf("tag stack call_depth: %d%n", td.tag_stack_call_depth); + System.out.printf("tag stack size: %d%n", td.tag_stack.size()); + } + } + + /** + * Called on return from an uninstrumented method body whose return type is not primitive; see + * {@link #uninstrumented_enter}. Discards everything the body left on the tag stack, including + * the marker that {@code uninstrumented_enter} pushed. + */ + public static void uninstrumented_exit() { + uninstrumented_exit(false); + } + + /** + * Called on return from an uninstrumented method body whose return type is primitive; see {@link + * #uninstrumented_enter}. Discards everything the body left on the tag stack, including the + * marker that {@code uninstrumented_enter} pushed, and then pushes the result tag that this + * method's caller expects. + */ + public static void uninstrumented_exit_primitive() { + uninstrumented_exit(true); + } + + /** + * Implements {@link #uninstrumented_exit} and {@link #uninstrumented_exit_primitive}. + * + * @param primitiveResult true if the method's return type is primitive, in which case a result + * tag is pushed for the caller + */ + private static void uninstrumented_exit(boolean primitiveResult) { + if (debug) { + System.out.printf("Exit uninstrumented: %s%n", caller_name()); + } + + ThreadData td = thread_to_data.get(Thread.currentThread()); + // Discard any tag the body's callees left behind, then the marker itself. The marker is + // missing only if something else has already unwound past it, which normal_exit also tolerates. + while (!td.tag_stack.isEmpty() && td.tag_stack.peek() != method_marker) { + td.tag_stack.pop(); + } + if (!td.tag_stack.isEmpty()) { + td.tag_stack.pop(); // discard marker + } + td.tag_stack_call_depth--; + if (primitiveResult) { + push_const(); + } + if (debug_tag_frame) { + System.out.printf("tag stack call_depth: %d%n", td.tag_stack_call_depth); + System.out.printf("tag stack size: %d%n", td.tag_stack.size()); + } + } + /** * Clean up the tag stack on an exception exit from a method. Pops items off of the tag stack * until the method marker is found. @@ -1003,6 +1103,18 @@ public static void discard_tag(int cnt) { } } + /** + * Returns the number of entries on the current thread's tag stack, counting the method markers. + * Intended for tests, which use it to verify that instrumented code leaves the tag stack as its + * callers expect. + * + * @return the size of the current thread's tag stack + */ + static int tag_stack_size() { + ThreadData td = thread_to_data.get(Thread.currentThread()); + return td == null ? 0 : td.tag_stack.size(); + } + /** * Manipulate the tags for an array store instruction. The tag at the top of stack is stored into * the tag storage for the array. Mark the array and the index as comparable. diff --git a/java/daikon/test/AllTestsSuite.java b/java/daikon/test/AllTestsSuite.java index dad8443e9..7d74e7e7a 100644 --- a/java/daikon/test/AllTestsSuite.java +++ b/java/daikon/test/AllTestsSuite.java @@ -6,6 +6,7 @@ /** All Daikon's unit tests. Does not include system tests. */ @RunWith(Suite.class) @Suite.SuiteClasses({ + daikon.chicory.RuntimeTest.class, daikon.test.TestClassOrInterfaceTypeDecorateVisitor.class, daikon.test.TestAst.class, daikon.test.config.ConfigurationTest.class, diff --git a/prek.toml b/prek.toml index 43154d75f..a24be9280 100644 --- a/prek.toml +++ b/prek.toml @@ -22,7 +22,7 @@ exclude = { glob = [ "**/COPYING", "src/main/resources/specifications/jdk/*", "src/test/resources/end-to-end/**/*", - "tests/kvasir-tests/povray/*", + "tests/kvasir-tests/povray/**/*", ] } [[repos]]