diff options
18 files changed, 647 insertions, 57 deletions
diff --git a/core/java/android/app/OWNERS b/core/java/android/app/OWNERS index eea607bb0362..5894803ac194 100644 --- a/core/java/android/app/OWNERS +++ b/core/java/android/app/OWNERS @@ -54,6 +54,9 @@ per-file IBackupAgent.aidl = file:/services/backup/OWNERS per-file Broadcast* = file:/BROADCASTS_OWNERS per-file ReceiverInfo* = file:/BROADCASTS_OWNERS +# KeyguardManager +per-file KeyguardManager.java = file:/services/core/java/com/android/server/locksettings/OWNERS + # LocaleManager per-file *Locale* = file:/services/core/java/com/android/server/locales/OWNERS @@ -93,7 +96,5 @@ per-file *Task* = file:/services/core/java/com/android/server/wm/OWNERS per-file Window* = file:/services/core/java/com/android/server/wm/OWNERS per-file ConfigurationController.java = file:/services/core/java/com/android/server/wm/OWNERS -# TODO(b/174932174): determine the ownership of KeyguardManager.java - # Zygote per-file *Zygote* = file:/ZYGOTE_OWNERS diff --git a/core/java/android/security/net/config/SystemCertificateSource.java b/core/java/android/security/net/config/SystemCertificateSource.java index 13f7e5d4232b..3a254c1d92fc 100644 --- a/core/java/android/security/net/config/SystemCertificateSource.java +++ b/core/java/android/security/net/config/SystemCertificateSource.java @@ -39,9 +39,13 @@ public final class SystemCertificateSource extends DirectoryCertificateSource { } private static File getDirectory() { - // TODO(miguelaranda): figure out correct code path. + if ((System.getProperty("system.certs.enabled") != null) + && (System.getProperty("system.certs.enabled")).equals("true")) { + return new File(System.getenv("ANDROID_ROOT") + "/etc/security/cacerts"); + } File updatable_dir = new File("/apex/com.android.conscrypt/cacerts"); - if (updatable_dir.exists() && !(updatable_dir.list().length == 0)) { + if (updatable_dir.exists() + && !(updatable_dir.list().length == 0)) { return updatable_dir; } return new File(System.getenv("ANDROID_ROOT") + "/etc/security/cacerts"); diff --git a/core/java/android/widget/Toast.java b/core/java/android/widget/Toast.java index ca57c84a1631..fceee4e01799 100644 --- a/core/java/android/widget/Toast.java +++ b/core/java/android/widget/Toast.java @@ -189,6 +189,9 @@ public class Toast { /** * Show the view for the specified duration. + * + * <p>Note that toasts being sent from the background are rate limited, so avoid sending such + * toasts in quick succession. */ public void show() { if (Compatibility.isChangeEnabled(CHANGE_TEXT_TOASTS_IN_THE_SYSTEM)) { diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java index 076e4e118e66..1505ccce97a1 100644 --- a/core/java/com/android/internal/os/ZygoteInit.java +++ b/core/java/com/android/internal/os/ZygoteInit.java @@ -185,8 +185,13 @@ public class ZygoteInit { private static void preloadSharedLibraries() { Log.i(TAG, "Preloading shared libraries..."); System.loadLibrary("android"); - System.loadLibrary("compiler_rt"); System.loadLibrary("jnigraphics"); + + // TODO(b/206676167): This library is only used for renderscript today. When renderscript is + // removed, this load can be removed as well. + if (!SystemProperties.getBoolean("config.disable_renderscript", false)) { + System.loadLibrary("compiler_rt"); + } } native private static void nativePreloadAppProcessHALs(); diff --git a/media/java/android/media/tv/OWNERS b/media/java/android/media/tv/OWNERS index fa0429350a25..0b022e514662 100644 --- a/media/java/android/media/tv/OWNERS +++ b/media/java/android/media/tv/OWNERS @@ -1,6 +1,7 @@ quxiangfang@google.com shubang@google.com hgchen@google.com +qingxun@google.com # For android remote service per-file ITvRemoteServiceInput.aidl = file:/media/lib/tvremote/OWNERS diff --git a/tools/locked_region_code_injection/Android.bp b/tools/locked_region_code_injection/Android.bp index a0cc446cd42d..954b816a52bf 100644 --- a/tools/locked_region_code_injection/Android.bp +++ b/tools/locked_region_code_injection/Android.bp @@ -19,3 +19,20 @@ java_binary_host { "ow2-asm-tree", ], } + +java_library_host { + name: "lockedregioncodeinjection_input", + manifest: "test/manifest.txt", + srcs: ["test/*/*.java"], + static_libs: [ + "guava", + "ow2-asm", + "ow2-asm-analysis", + "ow2-asm-commons", + "ow2-asm-tree", + "hamcrest-library", + "hamcrest", + "platform-test-annotations", + "junit", + ], +} diff --git a/tools/locked_region_code_injection/OWNERS b/tools/locked_region_code_injection/OWNERS new file mode 100644 index 000000000000..bd43f1736ca5 --- /dev/null +++ b/tools/locked_region_code_injection/OWNERS @@ -0,0 +1,4 @@ +# Everyone in frameworks/base is included by default +shayba@google.com +shombert@google.com +timmurray@google.com diff --git a/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockFindingClassVisitor.java b/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockFindingClassVisitor.java index 81a077324e6c..2067bb4ef2fe 100644 --- a/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockFindingClassVisitor.java +++ b/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockFindingClassVisitor.java @@ -13,37 +13,51 @@ */ package lockedregioncodeinjection; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.LinkedList; -import java.util.List; +import static com.google.common.base.Preconditions.checkElementIndex; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; import org.objectweb.asm.commons.TryCatchBlockSorter; import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.InsnList; +import org.objectweb.asm.tree.InsnNode; import org.objectweb.asm.tree.LabelNode; +import org.objectweb.asm.tree.LineNumberNode; import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.TypeInsnNode; import org.objectweb.asm.tree.TryCatchBlockNode; import org.objectweb.asm.tree.analysis.Analyzer; import org.objectweb.asm.tree.analysis.AnalyzerException; import org.objectweb.asm.tree.analysis.BasicValue; import org.objectweb.asm.tree.analysis.Frame; -import static com.google.common.base.Preconditions.checkElementIndex; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedList; +import java.util.List; /** - * This visitor does two things: + * This visitor operates on two kinds of targets. For a legacy target, it does the following: * - * 1. Finds all the MONITOR_ENTER / MONITOR_EXIT in the byte code and insert the corresponding pre + * 1. Finds all the MONITOR_ENTER / MONITOR_EXIT in the byte code and inserts the corresponding pre * and post methods calls should it matches one of the given target type in the Configuration. * * 2. Find all methods that are synchronized and insert pre method calls in the beginning and post * method calls just before all return instructions. + * + * For a scoped target, it does the following: + * + * 1. Finds all the MONITOR_ENTER instructions in the byte code. If the target of the opcode is + * named in a --scope switch, then the pre method is invoked ON THE TARGET immediately after + * MONITOR_ENTER opcode completes. + * + * 2. Finds all the MONITOR_EXIT instructions in the byte code. If the target of the opcode is + * named in a --scope switch, then the post method is invoked ON THE TARGET immediately before + * MONITOR_EXIT opcode completes. */ class LockFindingClassVisitor extends ClassVisitor { private String className = null; @@ -73,12 +87,16 @@ class LockFindingClassVisitor extends ClassVisitor { class LockFindingMethodVisitor extends MethodVisitor { private String owner; private MethodVisitor chain; + private final String className; + private final String methodName; public LockFindingMethodVisitor(String owner, MethodNode mn, MethodVisitor chain) { super(Utils.ASM_VERSION, mn); assert owner != null; this.owner = owner; this.chain = chain; + className = owner; + methodName = mn.name; } @SuppressWarnings("unchecked") @@ -93,6 +111,12 @@ class LockFindingClassVisitor extends ClassVisitor { for (LockTarget t : targets) { if (t.getTargetDesc().equals("L" + owner + ";")) { ownerMonitor = t; + if (ownerMonitor.getScoped()) { + final String emsg = String.format( + "scoped targets do not support synchronized methods in %s.%s()", + className, methodName); + throw new RuntimeException(emsg); + } } } } @@ -118,9 +142,11 @@ class LockFindingClassVisitor extends ClassVisitor { AbstractInsnNode s = instructions.getFirst(); MethodInsnNode call = new MethodInsnNode(Opcodes.INVOKESTATIC, ownerMonitor.getPreOwner(), ownerMonitor.getPreMethod(), "()V", false); - insertMethodCallBefore(mn, frameMap, handlersMap, s, 0, call); + insertMethodCallBeforeSync(mn, frameMap, handlersMap, s, 0, call); } + boolean anyDup = false; + for (int i = 0; i < instructions.size(); i++) { AbstractInsnNode s = instructions.get(i); @@ -131,9 +157,15 @@ class LockFindingClassVisitor extends ClassVisitor { LockTargetState state = (LockTargetState) operand; for (int j = 0; j < state.getTargets().size(); j++) { LockTarget target = state.getTargets().get(j); - MethodInsnNode call = new MethodInsnNode(Opcodes.INVOKESTATIC, - target.getPreOwner(), target.getPreMethod(), "()V", false); - insertMethodCallAfter(mn, frameMap, handlersMap, s, i, call); + MethodInsnNode call = methodCall(target, true); + if (target.getScoped()) { + TypeInsnNode cast = typeCast(target); + i += insertInvokeAcquire(mn, frameMap, handlersMap, s, i, + call, cast); + anyDup = true; + } else { + i += insertMethodCallBefore(mn, frameMap, handlersMap, s, i, call); + } } } } @@ -144,8 +176,9 @@ class LockFindingClassVisitor extends ClassVisitor { if (operand instanceof LockTargetState) { LockTargetState state = (LockTargetState) operand; for (int j = 0; j < state.getTargets().size(); j++) { - // The instruction after a monitor_exit should be a label for the end of the implicit - // catch block that surrounds the synchronized block to call monitor_exit when an exception + // The instruction after a monitor_exit should be a label for + // the end of the implicit catch block that surrounds the + // synchronized block to call monitor_exit when an exception // occurs. checkState(instructions.get(i + 1).getType() == AbstractInsnNode.LABEL, "Expected to find label after monitor exit"); @@ -161,9 +194,16 @@ class LockFindingClassVisitor extends ClassVisitor { "Expected label to be the end of monitor exit's try block"); LockTarget target = state.getTargets().get(j); - MethodInsnNode call = new MethodInsnNode(Opcodes.INVOKESTATIC, - target.getPostOwner(), target.getPostMethod(), "()V", false); - insertMethodCallAfter(mn, frameMap, handlersMap, label, labelIndex, call); + MethodInsnNode call = methodCall(target, false); + if (target.getScoped()) { + TypeInsnNode cast = typeCast(target); + i += insertInvokeRelease(mn, frameMap, handlersMap, s, i, + call, cast); + anyDup = true; + } else { + insertMethodCallAfter(mn, frameMap, handlersMap, label, + labelIndex, call); + } } } } @@ -174,16 +214,116 @@ class LockFindingClassVisitor extends ClassVisitor { MethodInsnNode call = new MethodInsnNode(Opcodes.INVOKESTATIC, ownerMonitor.getPostOwner(), ownerMonitor.getPostMethod(), "()V", false); - insertMethodCallBefore(mn, frameMap, handlersMap, s, i, call); + insertMethodCallBeforeSync(mn, frameMap, handlersMap, s, i, call); i++; // Skip ahead. Otherwise, we will revisit this instruction again. } } + + if (anyDup) { + mn.maxStack++; + } + super.visitEnd(); mn.accept(chain); } + + // Insert a call to a monitor pre handler. The node and the index identify the + // monitorenter call itself. Insert DUP immediately prior to the MONITORENTER. + // Insert the typecast and call (in that order) after the MONITORENTER. + public int insertInvokeAcquire(MethodNode mn, List<Frame> frameMap, + List<List<TryCatchBlockNode>> handlersMap, AbstractInsnNode node, int index, + MethodInsnNode call, TypeInsnNode cast) { + InsnList instructions = mn.instructions; + + // Insert a DUP right before MONITORENTER, to capture the object being locked. + // Note that the object will be typed as java.lang.Object. + instructions.insertBefore(node, new InsnNode(Opcodes.DUP)); + frameMap.add(index, frameMap.get(index)); + handlersMap.add(index, handlersMap.get(index)); + + // Insert the call right after the MONITORENTER. These entries are pushed after + // MONITORENTER so they are inserted in reverse order. MONITORENTER should be + // the target of a try/catch block, which means it must be immediately + // followed by a label (which is part of the try/catch block definition). + // Move forward past the label so the invocation in inside the proper block. + // Throw an error if the next instruction is not a label. + node = node.getNext(); + if (!(node instanceof LabelNode)) { + throw new RuntimeException(String.format("invalid bytecode sequence in %s.%s()", + className, methodName)); + } + node = node.getNext(); + index = instructions.indexOf(node); + + instructions.insertBefore(node, cast); + frameMap.add(index, frameMap.get(index)); + handlersMap.add(index, handlersMap.get(index)); + + instructions.insertBefore(node, call); + frameMap.add(index, frameMap.get(index)); + handlersMap.add(index, handlersMap.get(index)); + + return 3; + } + + // Insert instructions completely before the current opcode. This is slightly + // different from insertMethodCallBefore(), which inserts the call before MONITOREXIT + // but inserts the start and end labels after MONITOREXIT. + public int insertInvokeRelease(MethodNode mn, List<Frame> frameMap, + List<List<TryCatchBlockNode>> handlersMap, AbstractInsnNode node, int index, + MethodInsnNode call, TypeInsnNode cast) { + InsnList instructions = mn.instructions; + + instructions.insertBefore(node, new InsnNode(Opcodes.DUP)); + frameMap.add(index, frameMap.get(index)); + handlersMap.add(index, handlersMap.get(index)); + + instructions.insertBefore(node, cast); + frameMap.add(index, frameMap.get(index)); + handlersMap.add(index, handlersMap.get(index)); + + instructions.insertBefore(node, call); + frameMap.add(index, frameMap.get(index)); + handlersMap.add(index, handlersMap.get(index)); + + return 3; + } + } + + public static MethodInsnNode methodCall(LockTarget target, boolean pre) { + String spec = "()V"; + if (!target.getScoped()) { + if (pre) { + return new MethodInsnNode( + Opcodes.INVOKESTATIC, target.getPreOwner(), target.getPreMethod(), spec); + } else { + return new MethodInsnNode( + Opcodes.INVOKESTATIC, target.getPostOwner(), target.getPostMethod(), spec); + } + } else { + if (pre) { + return new MethodInsnNode( + Opcodes.INVOKEVIRTUAL, target.getPreOwner(), target.getPreMethod(), spec); + } else { + return new MethodInsnNode( + Opcodes.INVOKEVIRTUAL, target.getPostOwner(), target.getPostMethod(), spec); + } + } } - public static void insertMethodCallBefore(MethodNode mn, List<Frame> frameMap, + public static TypeInsnNode typeCast(LockTarget target) { + if (!target.getScoped()) { + return null; + } else { + // preOwner and postOwner return the same string for scoped targets. + return new TypeInsnNode(Opcodes.CHECKCAST, target.getPreOwner()); + } + } + + /** + * Insert a method call before the beginning or end of a synchronized method. + */ + public static void insertMethodCallBeforeSync(MethodNode mn, List<Frame> frameMap, List<List<TryCatchBlockNode>> handlersMap, AbstractInsnNode node, int index, MethodInsnNode call) { List<TryCatchBlockNode> handlers = handlersMap.get(index); @@ -226,6 +366,22 @@ class LockFindingClassVisitor extends ClassVisitor { updateCatchHandler(mn, handlers, start, end, handlersMap); } + // Insert instructions completely before the current opcode. This is slightly different from + // insertMethodCallBeforeSync(), which inserts the call before MONITOREXIT but inserts the + // start and end labels after MONITOREXIT. + public int insertMethodCallBefore(MethodNode mn, List<Frame> frameMap, + List<List<TryCatchBlockNode>> handlersMap, AbstractInsnNode node, int index, + MethodInsnNode call) { + InsnList instructions = mn.instructions; + + instructions.insertBefore(node, call); + frameMap.add(index, frameMap.get(index)); + handlersMap.add(index, handlersMap.get(index)); + + return 1; + } + + @SuppressWarnings("unchecked") public static void updateCatchHandler(MethodNode mn, List<TryCatchBlockNode> handlers, LabelNode start, LabelNode end, List<List<TryCatchBlockNode>> handlersMap) { diff --git a/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTarget.java b/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTarget.java index c5e59e3dc64d..5f6240327a7f 100644 --- a/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTarget.java +++ b/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTarget.java @@ -21,14 +21,28 @@ package lockedregioncodeinjection; public class LockTarget { public static final LockTarget NO_TARGET = new LockTarget("", null, null); + // The lock which must be instrumented, in Java internal form (L<path>;). private final String targetDesc; + // The methods to be called when the lock is taken (released). For non-scoped locks, + // these are fully qualified static methods. For scoped locks, these are the + // unqualified names of a member method of the target lock. private final String pre; private final String post; + // If true, the pre and post methods are virtual on the target class. The pre and post methods + // are both called while the lock is held. If this field is false then the pre and post methods + // take no parameters and the post method is called after the lock is released. This is legacy + // behavior. + private final boolean scoped; - public LockTarget(String targetDesc, String pre, String post) { + public LockTarget(String targetDesc, String pre, String post, boolean scoped) { this.targetDesc = targetDesc; this.pre = pre; this.post = post; + this.scoped = scoped; + } + + public LockTarget(String targetDesc, String pre, String post) { + this(targetDesc, pre, post, false); } public String getTargetDesc() { @@ -40,7 +54,11 @@ public class LockTarget { } public String getPreOwner() { - return pre.substring(0, pre.lastIndexOf('.')); + if (scoped) { + return targetDesc.substring(1, targetDesc.length() - 1); + } else { + return pre.substring(0, pre.lastIndexOf('.')); + } } public String getPreMethod() { @@ -52,10 +70,23 @@ public class LockTarget { } public String getPostOwner() { - return post.substring(0, post.lastIndexOf('.')); + if (scoped) { + return targetDesc.substring(1, targetDesc.length() - 1); + } else { + return post.substring(0, post.lastIndexOf('.')); + } } public String getPostMethod() { return post.substring(post.lastIndexOf('.') + 1); } + + public boolean getScoped() { + return scoped; + } + + @Override + public String toString() { + return targetDesc + ":" + pre + ":" + post; + } } diff --git a/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTargetState.java b/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTargetState.java index 99d841829132..5df0160abd8c 100644 --- a/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTargetState.java +++ b/tools/locked_region_code_injection/src/lockedregioncodeinjection/LockTargetState.java @@ -13,10 +13,11 @@ */ package lockedregioncodeinjection; -import java.util.List; import org.objectweb.asm.Type; import org.objectweb.asm.tree.analysis.BasicValue; +import java.util.List; + public class LockTargetState extends BasicValue { private final List<LockTarget> lockTargets; @@ -31,4 +32,10 @@ public class LockTargetState extends BasicValue { public List<LockTarget> getTargets() { return lockTargets; } + + @Override + public String toString() { + return "LockTargetState(" + getType().getDescriptor() + + ", " + lockTargets.size() + ")"; + } } diff --git a/tools/locked_region_code_injection/src/lockedregioncodeinjection/Main.java b/tools/locked_region_code_injection/src/lockedregioncodeinjection/Main.java index 828cce72dda9..d22ea2338ff7 100644 --- a/tools/locked_region_code_injection/src/lockedregioncodeinjection/Main.java +++ b/tools/locked_region_code_injection/src/lockedregioncodeinjection/Main.java @@ -21,7 +21,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.Collections; +import java.util.ArrayList; import java.util.Enumeration; import java.util.List; import java.util.zip.ZipEntry; @@ -36,6 +36,7 @@ public class Main { String legacyTargets = null; String legacyPreMethods = null; String legacyPostMethods = null; + List<LockTarget> targets = new ArrayList<>(); for (int i = 0; i < args.length; i++) { if ("-i".equals(args[i].trim())) { i++; @@ -52,23 +53,25 @@ public class Main { } else if ("--post".equals(args[i].trim())) { i++; legacyPostMethods = args[i].trim(); + } else if ("--scoped".equals(args[i].trim())) { + i++; + targets.add(Utils.getScopedTarget(args[i].trim())); } - } - // TODO(acleung): Better help message than asserts. - assert inJar != null; - assert outJar != null; + if (inJar == null) { + throw new RuntimeException("missing input jar path"); + } + if (outJar == null) { + throw new RuntimeException("missing output jar path"); + } assert legacyTargets == null || (legacyPreMethods != null && legacyPostMethods != null); ZipFile zipSrc = new ZipFile(inJar); ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(outJar)); - List<LockTarget> targets = null; if (legacyTargets != null) { - targets = Utils.getTargetsFromLegacyJackConfig(legacyTargets, legacyPreMethods, - legacyPostMethods); - } else { - targets = Collections.emptyList(); + targets.addAll(Utils.getTargetsFromLegacyJackConfig(legacyTargets, legacyPreMethods, + legacyPostMethods)); } Enumeration<? extends ZipEntry> srcEntries = zipSrc.entries(); diff --git a/tools/locked_region_code_injection/src/lockedregioncodeinjection/Utils.java b/tools/locked_region_code_injection/src/lockedregioncodeinjection/Utils.java index b44e8b42f052..bfef10611e6c 100644 --- a/tools/locked_region_code_injection/src/lockedregioncodeinjection/Utils.java +++ b/tools/locked_region_code_injection/src/lockedregioncodeinjection/Utils.java @@ -44,4 +44,27 @@ public class Utils { return config; } + + /** + * Returns a single {@link LockTarget} from a string. The target is a comma-separated list of + * the target class, the request method, the release method, and a boolean which is true if this + * is a scoped target and false if this is a legacy target. The boolean is optional and + * defaults to true. + */ + public static LockTarget getScopedTarget(String arg) { + String[] c = arg.split(","); + if (c.length == 3) { + return new LockTarget(c[0], c[1], c[2], true); + } else if (c.length == 4) { + if (c[3].equals("true")) { + return new LockTarget(c[0], c[1], c[2], true); + } else if (c[3].equals("false")) { + return new LockTarget(c[0], c[1], c[2], false); + } else { + System.err.println("illegal target parameter \"" + c[3] + "\""); + } + } + // Fall through + throw new RuntimeException("invalid scoped target format"); + } } diff --git a/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestMain.java b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestMain.java index 31fa0bf63416..28f00b9c897a 100644 --- a/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestMain.java +++ b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestMain.java @@ -17,7 +17,10 @@ import org.junit.Assert; import org.junit.Test; /** - * To run the unit tests: + * To run the unit tests, first build the two necessary artifacts. Do this explicitly as they are + * not generally retained by a normal "build all". After lunching a target: + * m lockedregioncodeinjection + * m lockedregioncodeinjection_input * * <pre> * <code> @@ -29,31 +32,25 @@ import org.junit.Test; * mkdir -p out * rm -fr out/* * - * # Make booster - * javac -cp lib/asm-7.0_BETA.jar:lib/asm-commons-7.0_BETA.jar:lib/asm-tree-7.0_BETA.jar:lib/asm-analysis-7.0_BETA.jar:lib/guava-21.0.jar src/*/*.java -d out/ - * pushd out - * jar cfe lockedregioncodeinjection.jar lockedregioncodeinjection.Main */*.class - * popd - * - * # Make unit tests. - * javac -cp lib/junit-4.12.jar test/*/*.java -d out/ - * - * pushd out - * jar cfe test_input.jar lockedregioncodeinjection.Test */*.class - * popd + * # Paths to the build artifacts. These assume linux-x86; YMMV. + * ROOT=$TOP/out/host/linux-x86 + * EXE=$ROOT/bin/lockedregioncodeinjection + * INPUT=$ROOT/frameworkd/lockedregioncodeinjection_input.jar * * # Run tool on unit tests. - * java -ea -cp lib/asm-7.0_BETA.jar:lib/asm-commons-7.0_BETA.jar:lib/asm-tree-7.0_BETA.jar:lib/asm-analysis-7.0_BETA.jar:lib/guava-21.0.jar:out/lockedregioncodeinjection.jar \ - * lockedregioncodeinjection.Main \ - * -i out/test_input.jar -o out/test_output.jar \ + * $EXE -i $INPUT -o out/test_output.jar \ * --targets 'Llockedregioncodeinjection/TestTarget;' \ * --pre 'lockedregioncodeinjection/TestTarget.boost' \ * --post 'lockedregioncodeinjection/TestTarget.unboost' * * # Run unit tests. - * java -ea -cp lib/hamcrest-core-1.3.jar:lib/junit-4.12.jar:out/test_output.jar \ + * java -ea -cp out/test_output.jar \ * org.junit.runner.JUnitCore lockedregioncodeinjection.TestMain * </code> + * OR + * <code> + * bash test/unit-test.sh + * </code> * </pre> */ public class TestMain { @@ -64,7 +61,9 @@ public class TestMain { Assert.assertEquals(TestTarget.boostCount, 0); Assert.assertEquals(TestTarget.unboostCount, 0); - Assert.assertEquals(TestTarget.unboostCount, 0); + Assert.assertEquals(TestTarget.invokeCount, 0); + Assert.assertEquals(TestTarget.boostCountLocked, 0); + Assert.assertEquals(TestTarget.unboostCountLocked, 0); synchronized (t) { Assert.assertEquals(TestTarget.boostCount, 1); @@ -75,6 +74,8 @@ public class TestMain { Assert.assertEquals(TestTarget.boostCount, 1); Assert.assertEquals(TestTarget.unboostCount, 1); Assert.assertEquals(TestTarget.invokeCount, 1); + Assert.assertEquals(TestTarget.boostCountLocked, 0); + Assert.assertEquals(TestTarget.unboostCountLocked, 0); } @Test @@ -84,12 +85,16 @@ public class TestMain { Assert.assertEquals(TestTarget.boostCount, 0); Assert.assertEquals(TestTarget.unboostCount, 0); + Assert.assertEquals(TestTarget.boostCountLocked, 0); + Assert.assertEquals(TestTarget.unboostCountLocked, 0); t.synchronizedCall(); Assert.assertEquals(TestTarget.boostCount, 1); Assert.assertEquals(TestTarget.unboostCount, 1); Assert.assertEquals(TestTarget.invokeCount, 1); + Assert.assertEquals(TestTarget.boostCountLocked, 0); + Assert.assertEquals(TestTarget.unboostCountLocked, 0); } @Test @@ -99,12 +104,16 @@ public class TestMain { Assert.assertEquals(TestTarget.boostCount, 0); Assert.assertEquals(TestTarget.unboostCount, 0); + Assert.assertEquals(TestTarget.boostCountLocked, 0); + Assert.assertEquals(TestTarget.unboostCountLocked, 0); t.synchronizedCallReturnInt(); Assert.assertEquals(TestTarget.boostCount, 1); Assert.assertEquals(TestTarget.unboostCount, 1); Assert.assertEquals(TestTarget.invokeCount, 1); + Assert.assertEquals(TestTarget.boostCountLocked, 0); + Assert.assertEquals(TestTarget.unboostCountLocked, 0); } @Test @@ -253,4 +262,125 @@ public class TestMain { Assert.assertEquals(TestTarget.invokeCount, 1); } + @Test + public void testScopedTarget() { + TestScopedTarget target = new TestScopedTarget(); + Assert.assertEquals(0, target.scopedLock().mLevel); + + synchronized (target.scopedLock()) { + Assert.assertEquals(1, target.scopedLock().mLevel); + } + Assert.assertEquals(0, target.scopedLock().mLevel); + + synchronized (target.scopedLock()) { + synchronized (target.scopedLock()) { + Assert.assertEquals(2, target.scopedLock().mLevel); + } + } + Assert.assertEquals(0, target.scopedLock().mLevel); + } + + @Test + public void testScopedExceptionHandling() { + TestScopedTarget target = new TestScopedTarget(); + Assert.assertEquals(0, target.scopedLock().mLevel); + + boolean handled; + + // 1: an exception inside the block properly releases the lock. + handled = false; + try { + synchronized (target.scopedLock()) { + Assert.assertEquals(true, Thread.holdsLock(target.scopedLock())); + Assert.assertEquals(1, target.scopedLock().mLevel); + throw new RuntimeException(); + } + } catch (RuntimeException e) { + Assert.assertEquals(0, target.scopedLock().mLevel); + handled = true; + } + Assert.assertEquals(0, target.scopedLock().mLevel); + Assert.assertEquals(true, handled); + // Just verify that the lock can still be taken + Assert.assertEquals(false, Thread.holdsLock(target.scopedLock())); + + // 2: An exception inside the monitor enter function + handled = false; + target.throwOnEnter(true); + try { + synchronized (target.scopedLock()) { + // The exception was thrown inside monitorEnter(), so the code should + // never reach this point. + Assert.assertEquals(0, 1); + } + } catch (RuntimeException e) { + Assert.assertEquals(0, target.scopedLock().mLevel); + handled = true; + } + Assert.assertEquals(0, target.scopedLock().mLevel); + Assert.assertEquals(true, handled); + // Just verify that the lock can still be taken + Assert.assertEquals(false, Thread.holdsLock(target.scopedLock())); + + // 3: An exception inside the monitor exit function + handled = false; + target.throwOnEnter(true); + try { + synchronized (target.scopedLock()) { + Assert.assertEquals(true, Thread.holdsLock(target.scopedLock())); + Assert.assertEquals(1, target.scopedLock().mLevel); + } + } catch (RuntimeException e) { + Assert.assertEquals(0, target.scopedLock().mLevel); + handled = true; + } + Assert.assertEquals(0, target.scopedLock().mLevel); + Assert.assertEquals(true, handled); + // Just verify that the lock can still be taken + Assert.assertEquals(false, Thread.holdsLock(target.scopedLock())); + } + + // Provide an in-class type conversion for the scoped target. + private Object untypedLock(TestScopedTarget target) { + return target.scopedLock(); + } + + @Test + public void testScopedLockTyping() { + TestScopedTarget target = new TestScopedTarget(); + Assert.assertEquals(target.scopedLock().mLevel, 0); + + // Scoped lock injection works on the static type of an object. In general, it is + // a very bad idea to do type conversion on scoped locks, but the general rule is + // that conversions within a single method are recognized by the lock injection + // tool and injection occurs. Conversions outside a single method are not + // recognized and injection does not occur. + + // 1. Conversion occurs outside the class. The visible type of the lock is Object + // in this block, so no injection takes place on 'untypedLock', even though the + // dynamic type is TestScopedLock. + synchronized (target.untypedLock()) { + Assert.assertEquals(0, target.scopedLock().mLevel); + Assert.assertEquals(true, target.untypedLock() instanceof TestScopedLock); + Assert.assertEquals(true, Thread.holdsLock(target.scopedLock())); + } + + // 2. Conversion occurs inside the class but in another method. The visible type + // of the lock is Object in this block, so no injection takes place on + // 'untypedLock', even though the dynamic type is TestScopedLock. + synchronized (untypedLock(target)) { + Assert.assertEquals(0, target.scopedLock().mLevel); + Assert.assertEquals(true, target.untypedLock() instanceof TestScopedLock); + Assert.assertEquals(true, Thread.holdsLock(target.scopedLock())); + } + + // 3. Conversion occurs inside the method. The compiler can determine the type of + // the lock within a single function, so injection does take place here. + Object untypedLock = target.scopedLock(); + synchronized (untypedLock) { + Assert.assertEquals(1, target.scopedLock().mLevel); + Assert.assertEquals(true, untypedLock instanceof TestScopedLock); + Assert.assertEquals(true, Thread.holdsLock(target.scopedLock())); + } + } } diff --git a/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestScopedLock.java b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestScopedLock.java new file mode 100644 index 000000000000..7441d2b1da48 --- /dev/null +++ b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestScopedLock.java @@ -0,0 +1,38 @@ +/* + * 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 lockedregioncodeinjection; + +public class TestScopedLock { + public int mEntered = 0; + public int mExited = 0; + + public int mLevel = 0; + private final TestScopedTarget mTarget; + + TestScopedLock(TestScopedTarget target) { + mTarget = target; + } + + void monitorEnter() { + mLevel++; + mEntered++; + mTarget.enter(mLevel); + } + + void monitorExit() { + mLevel--; + mExited++; + mTarget.exit(mLevel); + } +} diff --git a/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestScopedTarget.java b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestScopedTarget.java new file mode 100644 index 000000000000..c80975e9c6b3 --- /dev/null +++ b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestScopedTarget.java @@ -0,0 +1,56 @@ +/* + * 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 lockedregioncodeinjection; + +public class TestScopedTarget { + + public final TestScopedLock mLock;; + + private boolean mNextEnterThrows = false; + private boolean mNextExitThrows = false; + + TestScopedTarget() { + mLock = new TestScopedLock(this); + } + + TestScopedLock scopedLock() { + return mLock; + } + + Object untypedLock() { + return mLock; + } + + void enter(int level) { + if (mNextEnterThrows) { + mNextEnterThrows = false; + throw new RuntimeException(); + } + } + + void exit(int level) { + if (mNextExitThrows) { + mNextExitThrows = false; + throw new RuntimeException(); + } + } + + void throwOnEnter(boolean b) { + mNextEnterThrows = b; + } + + void throwOnExit(boolean b) { + mNextExitThrows = b; + } +} diff --git a/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestTarget.java b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestTarget.java index d1c8f340a598..e3ba6a77e3b7 100644 --- a/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestTarget.java +++ b/tools/locked_region_code_injection/test/lockedregioncodeinjection/TestTarget.java @@ -19,8 +19,17 @@ public class TestTarget { public static int invokeCount = 0; public static boolean nextUnboostThrows = false; + // If this is not null, then this is the lock under test. The lock must not be held when boost() + // or unboost() are called. + public static Object mLock = null; + public static int boostCountLocked = 0; + public static int unboostCountLocked = 0; + public static void boost() { boostCount++; + if (mLock != null && Thread.currentThread().holdsLock(mLock)) { + boostCountLocked++; + } } public static void unboost() { @@ -29,6 +38,9 @@ public class TestTarget { throw new RuntimeException(); } unboostCount++; + if (mLock != null && Thread.currentThread().holdsLock(mLock)) { + unboostCountLocked++; + } } public static void invoke() { diff --git a/tools/locked_region_code_injection/test/manifest.txt b/tools/locked_region_code_injection/test/manifest.txt new file mode 100644 index 000000000000..2314c188721c --- /dev/null +++ b/tools/locked_region_code_injection/test/manifest.txt @@ -0,0 +1 @@ +Main-Class: org.junit.runner.JUnitCore diff --git a/tools/locked_region_code_injection/test/unit-test.sh b/tools/locked_region_code_injection/test/unit-test.sh new file mode 100755 index 000000000000..9fa6f39af14a --- /dev/null +++ b/tools/locked_region_code_injection/test/unit-test.sh @@ -0,0 +1,98 @@ +#! /bin/bash +# + +# Copyright (C) 2023 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. + +# This script runs the tests for the lockedregioninjectioncode. See +# TestMain.java for the invocation. The script expects that a full build has +# already been done and artifacts are in $TOP/out. + +# Compute the default top of the workspace. The following code copies the +# strategy of croot. (croot cannot be usd directly because it is a function and +# functions are not carried over into subshells.) This gives the correct answer +# if run from inside a workspace. If run from outside a workspace, supply TOP +# on the command line. +TOPFILE=build/make/core/envsetup.mk +TOP=$(dirname $(realpath $0)) +while [[ ! $TOP = / && ! -f $TOP/$TOPFILE ]]; do + TOP=$(dirname $TOP) +done +# TOP is "/" if this script is located outside a workspace. + +# If the user supplied a top directory, use it instead +if [[ -n $1 ]]; then + TOP=$1 + shift +fi +if [[ -z $TOP || $TOP = / ]]; then + echo "usage: $0 <workspace-root>" + exit 1 +elif [[ ! -d $TOP ]]; then + echo "$TOP is not a directory" + exit 1 +elif [[ ! -d $TOP/prebuilts/misc/common ]]; then + echo "$TOP does not look like w workspace" + exit 1 +fi +echo "Using workspace $TOP" + +# Pick up the current java compiler. The lunch target is not very important, +# since most, if not all, will use the same host binaries. +pushd $TOP > /dev/null +. build/envsetup.sh > /dev/null 2>&1 +lunch redfin-userdebug > /dev/null 2>&1 +popd > /dev/null + +# Bail on any error +set -o pipefail +trap 'exit 1' ERR + +# Create the two sources +pushd $TOP > /dev/null +m lockedregioncodeinjection +m lockedregioncodeinjection_input +popd > /dev/null + +# Create a temporary directory outside of the workspace. +OUT=$TOP/out/host/test/lockedregioncodeinjection +echo + +# Clean the directory +if [[ -d $OUT ]]; then rm -r $OUT; fi +mkdir -p $OUT + +ROOT=$TOP/out/host/linux-x86 +EXE=$ROOT/bin/lockedregioncodeinjection +INP=$ROOT/framework/lockedregioncodeinjection_input.jar + +# Run tool on unit tests. +$EXE \ + -i $INP -o $OUT/test_output.jar \ + --targets 'Llockedregioncodeinjection/TestTarget;' \ + --pre 'lockedregioncodeinjection/TestTarget.boost' \ + --post 'lockedregioncodeinjection/TestTarget.unboost' \ + --scoped 'Llockedregioncodeinjection/TestScopedLock;,monitorEnter,monitorExit' + +# Run unit tests. +java -ea -cp $OUT/test_output.jar \ + org.junit.runner.JUnitCore lockedregioncodeinjection.TestMain + +# Extract the class files and decompile them for possible post-analysis. +pushd $OUT > /dev/null +jar -x --file test_output.jar lockedregioncodeinjection +for class in lockedregioncodeinjection/*.class; do + javap -c -v $class > ${class%.class}.asm +done +popd > /dev/null + +echo "artifacts are in $OUT" |