diff options
| -rw-r--r-- | openjdkjvmti/ti_breakpoint.cc | 13 | ||||
| -rw-r--r-- | openjdkjvmti/ti_breakpoint.h | 2 | ||||
| -rw-r--r-- | openjdkjvmti/ti_redefine.cc | 9 | ||||
| -rw-r--r-- | openjdkjvmti/ti_redefine.h | 2 | ||||
| -rw-r--r-- | test/1947-breakpoint-redefine-deopt/check_deopt.cc | 35 | ||||
| -rw-r--r-- | test/1947-breakpoint-redefine-deopt/expected.txt | 8 | ||||
| -rw-r--r-- | test/1947-breakpoint-redefine-deopt/info.txt | 5 | ||||
| -rwxr-xr-x | test/1947-breakpoint-redefine-deopt/run | 18 | ||||
| -rw-r--r-- | test/1947-breakpoint-redefine-deopt/src/Main.java | 104 | ||||
| -rw-r--r-- | test/1947-breakpoint-redefine-deopt/src/art/Breakpoint.java | 202 | ||||
| -rw-r--r-- | test/1947-breakpoint-redefine-deopt/src/art/Redefinition.java | 91 | ||||
| -rw-r--r-- | test/Android.bp | 1 |
12 files changed, 474 insertions, 16 deletions
diff --git a/openjdkjvmti/ti_breakpoint.cc b/openjdkjvmti/ti_breakpoint.cc index d5fffdf439..136b1d3e1a 100644 --- a/openjdkjvmti/ti_breakpoint.cc +++ b/openjdkjvmti/ti_breakpoint.cc @@ -78,16 +78,11 @@ void BreakpointUtil::RemoveBreakpointsInClass(ArtJvmTiEnv* env, art::mirror::Cla env->breakpoints.erase(it); } } - if (!to_remove.empty()) { - LOG(WARNING) << "Methods with breakpoints potentially not being un-deoptimized."; + DeoptManager* deopt = DeoptManager::Get(); + for (const Breakpoint& b : to_remove) { + // TODO It might be good to send these all at once instead. + deopt->RemoveMethodBreakpoint(b.GetMethod()); } - // TODO Figure out how to do this. - // DeoptManager* deopt = DeoptManager::Get(); - // for (const Breakpoint& b : to_remove) { - // // TODO It might be good to send these all at once instead. - // // deopt->RemoveMethodBreakpointSuspended(b.GetMethod()); - // LOG(WARNING) << "not un-deopting methods! :-0"; - // } } jvmtiError BreakpointUtil::SetBreakpoint(jvmtiEnv* jenv, jmethodID method, jlocation location) { diff --git a/openjdkjvmti/ti_breakpoint.h b/openjdkjvmti/ti_breakpoint.h index 9b08b425fa..7aa33aeec0 100644 --- a/openjdkjvmti/ti_breakpoint.h +++ b/openjdkjvmti/ti_breakpoint.h @@ -78,7 +78,7 @@ class BreakpointUtil { static jvmtiError ClearBreakpoint(jvmtiEnv* env, jmethodID method, jlocation location); // Used by class redefinition to remove breakpoints on redefined classes. static void RemoveBreakpointsInClass(ArtJvmTiEnv* env, art::mirror::Class* klass) - REQUIRES(art::Locks::mutator_lock_); + REQUIRES_SHARED(art::Locks::mutator_lock_); }; } // namespace openjdkjvmti diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc index c3fb946b9a..46734548a8 100644 --- a/openjdkjvmti/ti_redefine.cc +++ b/openjdkjvmti/ti_redefine.cc @@ -1215,7 +1215,9 @@ void Redefiner::ClassRedefinition::UnregisterJvmtiBreakpoints() { } void Redefiner::ClassRedefinition::UnregisterBreakpoints() { - DCHECK(art::Dbg::IsDebuggerActive()); + if (LIKELY(!art::Dbg::IsDebuggerActive())) { + return; + } art::JDWP::JdwpState* state = art::Dbg::GetJdwpState(); if (state != nullptr) { state->UnregisterLocationEventsOnClass(GetMirrorClass()); @@ -1223,11 +1225,9 @@ void Redefiner::ClassRedefinition::UnregisterBreakpoints() { } void Redefiner::UnregisterAllBreakpoints() { - if (LIKELY(!art::Dbg::IsDebuggerActive())) { - return; - } for (Redefiner::ClassRedefinition& redef : redefinitions_) { redef.UnregisterBreakpoints(); + redef.UnregisterJvmtiBreakpoints(); } } @@ -1356,7 +1356,6 @@ jvmtiError Redefiner::Run() { // TODO Rewrite so we don't do a stack walk for each and every class. redef.FindAndAllocateObsoleteMethods(klass); redef.UpdateClass(klass, data.GetNewDexCache(), data.GetOriginalDexFile()); - redef.UnregisterJvmtiBreakpoints(); } RestoreObsoleteMethodMapsIfUnneeded(holder); // TODO We should check for if any of the redefined methods are intrinsic methods here and, if any diff --git a/openjdkjvmti/ti_redefine.h b/openjdkjvmti/ti_redefine.h index c920707afd..778f87e68e 100644 --- a/openjdkjvmti/ti_redefine.h +++ b/openjdkjvmti/ti_redefine.h @@ -199,7 +199,7 @@ class Redefiner { void UnregisterBreakpoints() REQUIRES_SHARED(art::Locks::mutator_lock_); // This should be done with all threads suspended. - void UnregisterJvmtiBreakpoints() REQUIRES(art::Locks::mutator_lock_); + void UnregisterJvmtiBreakpoints() REQUIRES_SHARED(art::Locks::mutator_lock_); private: Redefiner* driver_; diff --git a/test/1947-breakpoint-redefine-deopt/check_deopt.cc b/test/1947-breakpoint-redefine-deopt/check_deopt.cc new file mode 100644 index 0000000000..b40b201c9c --- /dev/null +++ b/test/1947-breakpoint-redefine-deopt/check_deopt.cc @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "jni.h" +#include "art_method-inl.h" +#include "jni_internal.h" +#include "instrumentation.h" +#include "scoped_thread_state_change-inl.h" + +namespace art { + +extern "C" JNIEXPORT jboolean JNICALL Java_Main_isMethodDeoptimized(JNIEnv*, jclass, jobject m) { + ScopedObjectAccess soa(art::Thread::Current()); + ArtMethod* art_method = ArtMethod::FromReflectedMethod(soa, m); + return Runtime::Current()->GetInstrumentation()->IsDeoptimized(art_method); +} + +extern "C" JNIEXPORT jboolean JNICALL Java_Main_isInterpretOnly(JNIEnv*, jclass) { + return Runtime::Current()->GetInstrumentation()->IsForcedInterpretOnly(); +} + +} // namespace art diff --git a/test/1947-breakpoint-redefine-deopt/expected.txt b/test/1947-breakpoint-redefine-deopt/expected.txt new file mode 100644 index 0000000000..558e2a8036 --- /dev/null +++ b/test/1947-breakpoint-redefine-deopt/expected.txt @@ -0,0 +1,8 @@ +JNI_OnLoad called +Hello + Breakpoint reached: public void Main$Transform.sayHi() @ line=27 +Hello +Redefining transform! +Goodbye + Breakpoint reached: public void Main$Transform.sayHi() @ line=20 +Goodbye diff --git a/test/1947-breakpoint-redefine-deopt/info.txt b/test/1947-breakpoint-redefine-deopt/info.txt new file mode 100644 index 0000000000..210dea0471 --- /dev/null +++ b/test/1947-breakpoint-redefine-deopt/info.txt @@ -0,0 +1,5 @@ +Test basic JVMTI breakpoint functionality. + +This test ensures we can place breakpoints on particular lines of a method. It +sets breakpoints on each line in turn of a function with multiple execution +paths and then runs the function, receiving the breakpoint events. diff --git a/test/1947-breakpoint-redefine-deopt/run b/test/1947-breakpoint-redefine-deopt/run new file mode 100755 index 0000000000..51875a7e86 --- /dev/null +++ b/test/1947-breakpoint-redefine-deopt/run @@ -0,0 +1,18 @@ +#!/bin/bash +# +# Copyright 2017 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Ask for stack traces to be dumped to a file rather than to stdout. +./default-run "$@" --jvmti diff --git a/test/1947-breakpoint-redefine-deopt/src/Main.java b/test/1947-breakpoint-redefine-deopt/src/Main.java new file mode 100644 index 0000000000..c490779efb --- /dev/null +++ b/test/1947-breakpoint-redefine-deopt/src/Main.java @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.util.Arrays; +import java.lang.reflect.Executable; +import java.lang.reflect.Method; +import java.util.Base64; +import art.Breakpoint; +import art.Redefinition; + +public class Main { + static class Transform { + public void sayHi() { + System.out.println("Hello"); + } + } + + /** + * base64 encoded class/dex file for + * class Transform { + * public void sayHi() { + * System.out.println("Goodbye"); + * } + * } + */ + private static final byte[] DEX_BYTES = Base64.getDecoder().decode( + "ZGV4CjAzNQA7jFommHUzfbuvjq/I2cDcwdjqQk6KPfqYAwAAcAAAAHhWNBIAAAAAAAAAANQCAAAU" + + "AAAAcAAAAAkAAADAAAAAAgAAAOQAAAABAAAA/AAAAAQAAAAEAQAAAQAAACQBAABUAgAARAEAAJ4B" + + "AACmAQAArwEAAMEBAADJAQAA7QEAAA0CAAAkAgAAOAIAAEwCAABgAgAAawIAAHYCAAB5AgAAfQIA" + + "AIoCAACQAgAAlQIAAJ4CAAClAgAAAgAAAAMAAAAEAAAABQAAAAYAAAAHAAAACAAAAAkAAAAMAAAA" + + "DAAAAAgAAAAAAAAADQAAAAgAAACYAQAABwAEABAAAAAAAAAAAAAAAAAAAAASAAAABAABABEAAAAF" + + "AAAAAAAAAAAAAAAAAAAABQAAAAAAAAAKAAAAiAEAAMYCAAAAAAAAAgAAALcCAAC9AgAAAQABAAEA" + + "AACsAgAABAAAAHAQAwAAAA4AAwABAAIAAACxAgAACAAAAGIAAAAaAQEAbiACABAADgBEAQAAAAAA" + + "AAAAAAAAAAAAAQAAAAYABjxpbml0PgAHR29vZGJ5ZQAQTE1haW4kVHJhbnNmb3JtOwAGTE1haW47" + + "ACJMZGFsdmlrL2Fubm90YXRpb24vRW5jbG9zaW5nQ2xhc3M7AB5MZGFsdmlrL2Fubm90YXRpb24v" + + "SW5uZXJDbGFzczsAFUxqYXZhL2lvL1ByaW50U3RyZWFtOwASTGphdmEvbGFuZy9PYmplY3Q7ABJM" + + "amF2YS9sYW5nL1N0cmluZzsAEkxqYXZhL2xhbmcvU3lzdGVtOwAJTWFpbi5qYXZhAAlUcmFuc2Zv" + + "cm0AAVYAAlZMAAthY2Nlc3NGbGFncwAEbmFtZQADb3V0AAdwcmludGxuAAVzYXlIaQAFdmFsdWUA" + + "EgAHDgAUAAcOeAACAgETGAECAwIOBAgPFwsAAAEBAICABNACAQHoAhAAAAAAAAAAAQAAAAAAAAAB" + + "AAAAFAAAAHAAAAACAAAACQAAAMAAAAADAAAAAgAAAOQAAAAEAAAAAQAAAPwAAAAFAAAABAAAAAQB" + + "AAAGAAAAAQAAACQBAAADEAAAAQAAAEQBAAABIAAAAgAAAFABAAAGIAAAAQAAAIgBAAABEAAAAQAA" + + "AJgBAAACIAAAFAAAAJ4BAAADIAAAAgAAAKwCAAAEIAAAAgAAALcCAAAAIAAAAQAAAMYCAAAAEAAA" + + "AQAAANQCAAA="); + + public static void notifyBreakpointReached(Thread thr, Executable e, long loc) { + System.out.println( + "\tBreakpoint reached: " + e + " @ line=" + Breakpoint.locationToLine(e, loc)); + } + + public static void check(boolean b, String msg) { + if (!b) { + throw new Error("FAILED: " + msg); + } + } + public static void main(String[] args) throws Exception { + System.loadLibrary(args[0]); + // Set up breakpoints + Breakpoint.stopBreakpointWatch(Thread.currentThread()); + Breakpoint.startBreakpointWatch( + Main.class, + Main.class.getDeclaredMethod( + "notifyBreakpointReached", Thread.class, Executable.class, Long.TYPE), + Thread.currentThread()); + + Method targetMethod = Transform.class.getDeclaredMethod("sayHi"); + Transform t = new Transform(); + check(isInterpretOnly() || !isMethodDeoptimized(targetMethod), + "method should not be deoptimized"); + t.sayHi(); + + // Set a breakpoint at the start of the function. + Breakpoint.setBreakpoint(targetMethod, 0); + check(isInterpretOnly() || isMethodDeoptimized(targetMethod), + "method should be deoptimized"); + t.sayHi(); + + System.out.println("Redefining transform!"); + Redefinition.doCommonClassRedefinition(Transform.class, new byte[0], DEX_BYTES); + check(isInterpretOnly() || !isMethodDeoptimized(targetMethod), + "method should not be deoptimized"); + t.sayHi(); + + Breakpoint.setBreakpoint(targetMethod, 0); + check(isInterpretOnly() || isMethodDeoptimized(targetMethod), + "method should be deoptimized"); + t.sayHi(); + } + + static native boolean isMethodDeoptimized(Method m); + static native boolean isInterpretOnly(); +} diff --git a/test/1947-breakpoint-redefine-deopt/src/art/Breakpoint.java b/test/1947-breakpoint-redefine-deopt/src/art/Breakpoint.java new file mode 100644 index 0000000000..bbb89f707f --- /dev/null +++ b/test/1947-breakpoint-redefine-deopt/src/art/Breakpoint.java @@ -0,0 +1,202 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package art; + +import java.lang.reflect.Executable; +import java.util.HashSet; +import java.util.Set; +import java.util.Objects; + +public class Breakpoint { + public static class Manager { + public static class BP { + public final Executable method; + public final long location; + + public BP(Executable method) { + this(method, getStartLocation(method)); + } + + public BP(Executable method, long location) { + this.method = method; + this.location = location; + } + + @Override + public boolean equals(Object other) { + return (other instanceof BP) && + method.equals(((BP)other).method) && + location == ((BP)other).location; + } + + @Override + public String toString() { + return method.toString() + " @ " + getLine(); + } + + @Override + public int hashCode() { + return Objects.hash(method, location); + } + + public int getLine() { + try { + LineNumber[] lines = getLineNumberTable(method); + int best = -1; + for (LineNumber l : lines) { + if (l.location > location) { + break; + } else { + best = l.line; + } + } + return best; + } catch (Exception e) { + return -1; + } + } + } + + private Set<BP> breaks = new HashSet<>(); + + public void setBreakpoints(BP... bs) { + for (BP b : bs) { + if (breaks.add(b)) { + Breakpoint.setBreakpoint(b.method, b.location); + } + } + } + public void setBreakpoint(Executable method, long location) { + setBreakpoints(new BP(method, location)); + } + + public void clearBreakpoints(BP... bs) { + for (BP b : bs) { + if (breaks.remove(b)) { + Breakpoint.clearBreakpoint(b.method, b.location); + } + } + } + public void clearBreakpoint(Executable method, long location) { + clearBreakpoints(new BP(method, location)); + } + + public void clearAllBreakpoints() { + clearBreakpoints(breaks.toArray(new BP[0])); + } + } + + public static void startBreakpointWatch(Class<?> methodClass, + Executable breakpointReached, + Thread thr) { + startBreakpointWatch(methodClass, breakpointReached, false, thr); + } + + /** + * Enables the trapping of breakpoint events. + * + * If allowRecursive == true then breakpoints will be sent even if one is currently being handled. + */ + public static native void startBreakpointWatch(Class<?> methodClass, + Executable breakpointReached, + boolean allowRecursive, + Thread thr); + public static native void stopBreakpointWatch(Thread thr); + + public static final class LineNumber implements Comparable<LineNumber> { + public final long location; + public final int line; + + private LineNumber(long loc, int line) { + this.location = loc; + this.line = line; + } + + public boolean equals(Object other) { + return other instanceof LineNumber && ((LineNumber)other).line == line && + ((LineNumber)other).location == location; + } + + public int compareTo(LineNumber other) { + int v = Integer.valueOf(line).compareTo(Integer.valueOf(other.line)); + if (v != 0) { + return v; + } else { + return Long.valueOf(location).compareTo(Long.valueOf(other.location)); + } + } + } + + public static native void setBreakpoint(Executable m, long loc); + public static void setBreakpoint(Executable m, LineNumber l) { + setBreakpoint(m, l.location); + } + + public static native void clearBreakpoint(Executable m, long loc); + public static void clearBreakpoint(Executable m, LineNumber l) { + clearBreakpoint(m, l.location); + } + + private static native Object[] getLineNumberTableNative(Executable m); + public static LineNumber[] getLineNumberTable(Executable m) { + Object[] nativeTable = getLineNumberTableNative(m); + long[] location = (long[])(nativeTable[0]); + int[] lines = (int[])(nativeTable[1]); + if (lines.length != location.length) { + throw new Error("Lines and locations have different lengths!"); + } + LineNumber[] out = new LineNumber[lines.length]; + for (int i = 0; i < lines.length; i++) { + out[i] = new LineNumber(location[i], lines[i]); + } + return out; + } + + public static native long getStartLocation(Executable m); + + public static int locationToLine(Executable m, long location) { + try { + Breakpoint.LineNumber[] lines = Breakpoint.getLineNumberTable(m); + int best = -1; + for (Breakpoint.LineNumber l : lines) { + if (l.location > location) { + break; + } else { + best = l.line; + } + } + return best; + } catch (Exception e) { + return -1; + } + } + + public static long lineToLocation(Executable m, int line) throws Exception { + try { + Breakpoint.LineNumber[] lines = Breakpoint.getLineNumberTable(m); + for (Breakpoint.LineNumber l : lines) { + if (l.line == line) { + return l.location; + } + } + throw new Exception("Unable to find line " + line + " in " + m); + } catch (Exception e) { + throw new Exception("Unable to get line number info for " + m, e); + } + } +} + diff --git a/test/1947-breakpoint-redefine-deopt/src/art/Redefinition.java b/test/1947-breakpoint-redefine-deopt/src/art/Redefinition.java new file mode 100644 index 0000000000..56d2938a01 --- /dev/null +++ b/test/1947-breakpoint-redefine-deopt/src/art/Redefinition.java @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package art; + +import java.util.ArrayList; +// Common Redefinition functions. Placed here for use by CTS +public class Redefinition { + public static final class CommonClassDefinition { + public final Class<?> target; + public final byte[] class_file_bytes; + public final byte[] dex_file_bytes; + + public CommonClassDefinition(Class<?> target, byte[] class_file_bytes, byte[] dex_file_bytes) { + this.target = target; + this.class_file_bytes = class_file_bytes; + this.dex_file_bytes = dex_file_bytes; + } + } + + // A set of possible test configurations. Test should set this if they need to. + // This must be kept in sync with the defines in ti-agent/common_helper.cc + public static enum Config { + COMMON_REDEFINE(0), + COMMON_RETRANSFORM(1), + COMMON_TRANSFORM(2); + + private final int val; + private Config(int val) { + this.val = val; + } + } + + public static void setTestConfiguration(Config type) { + nativeSetTestConfiguration(type.val); + } + + private static native void nativeSetTestConfiguration(int type); + + // Transforms the class + public static native void doCommonClassRedefinition(Class<?> target, + byte[] classfile, + byte[] dexfile); + + public static void doMultiClassRedefinition(CommonClassDefinition... defs) { + ArrayList<Class<?>> classes = new ArrayList<>(); + ArrayList<byte[]> class_files = new ArrayList<>(); + ArrayList<byte[]> dex_files = new ArrayList<>(); + + for (CommonClassDefinition d : defs) { + classes.add(d.target); + class_files.add(d.class_file_bytes); + dex_files.add(d.dex_file_bytes); + } + doCommonMultiClassRedefinition(classes.toArray(new Class<?>[0]), + class_files.toArray(new byte[0][]), + dex_files.toArray(new byte[0][])); + } + + public static void addMultiTransformationResults(CommonClassDefinition... defs) { + for (CommonClassDefinition d : defs) { + addCommonTransformationResult(d.target.getCanonicalName(), + d.class_file_bytes, + d.dex_file_bytes); + } + } + + public static native void doCommonMultiClassRedefinition(Class<?>[] targets, + byte[][] classfiles, + byte[][] dexfiles); + public static native void doCommonClassRetransformation(Class<?>... target); + public static native void setPopRetransformations(boolean pop); + public static native void popTransformationFor(String name); + public static native void enableCommonRetransformation(boolean enable); + public static native void addCommonTransformationResult(String target_name, + byte[] class_bytes, + byte[] dex_bytes); +} diff --git a/test/Android.bp b/test/Android.bp index 72cdd41cfa..17a50421a8 100644 --- a/test/Android.bp +++ b/test/Android.bp @@ -438,6 +438,7 @@ cc_defaults { "667-jit-jni-stub/jit_jni_stub_test.cc", "674-hiddenapi/hiddenapi.cc", "708-jit-cache-churn/jit.cc", + "1947-breakpoint-redefine-deopt/check_deopt.cc", "common/runtime_state.cc", "common/stack_inspect.cc", ], |