Reland "Fix incorrect accessibility check in inliner"
This reverts commit ed88cc4517b42a5291f7313bd8ccda3cd4067826.
Change-Id: I5286796768aa1e5adc83afb6d0a0ff3e153f3687
diff --git a/src/main/java/com/android/tools/r8/graph/AccessControl.java b/src/main/java/com/android/tools/r8/graph/AccessControl.java
index 854c5e7..788b358 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessControl.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessControl.java
@@ -38,6 +38,14 @@
public static OptionalBool isMethodAccessible(
DexEncodedMethod method,
DexClass holder,
+ ProgramMethod context,
+ AppView<? extends AppInfoWithClassHierarchy> appView) {
+ return isMethodAccessible(method, holder, context.getHolder(), appView.appInfo());
+ }
+
+ public static OptionalBool isMethodAccessible(
+ DexEncodedMethod method,
+ DexClass holder,
DexProgramClass context,
AppInfoWithClassHierarchy appInfo) {
return isMemberAccessible(method.accessFlags, holder, context, appInfo);
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java
index 978646c..41f5dc8 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java
@@ -388,7 +388,7 @@
return false;
}
ResolutionResult resolutionResult =
- appView.appInfo().resolveMethodOn(superType, method, instruction.itf);
+ appView.appInfo().resolveMethodOn(superType, method, instruction.isInterface);
if (!resolutionResult.isSingleResolution()) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
index abaf1c3..0f736a0 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
@@ -29,15 +29,15 @@
public class InvokeDirect extends InvokeMethodWithReceiver {
- private final boolean itf;
+ private final boolean isInterface;
public InvokeDirect(DexMethod target, Value result, List<Value> arguments) {
this(target, result, arguments, false);
}
- public InvokeDirect(DexMethod target, Value result, List<Value> arguments, boolean itf) {
+ public InvokeDirect(DexMethod target, Value result, List<Value> arguments, boolean isInterface) {
super(target, result, arguments);
- this.itf = itf;
+ this.isInterface = isInterface;
// invoke-direct <init> should have no out value.
assert !target.name.toString().equals(Constants.INSTANCE_INITIALIZER_NAME)
|| result == null;
@@ -48,8 +48,9 @@
return Opcodes.INVOKE_DIRECT;
}
- public boolean isInterface() {
- return itf;
+ @Override
+ public boolean getInterfaceBit() {
+ return isInterface;
}
@Override
@@ -145,7 +146,8 @@
@Override
public void buildCf(CfBuilder builder) {
- builder.add(new CfInvoke(org.objectweb.asm.Opcodes.INVOKESPECIAL, getInvokedMethod(), itf));
+ builder.add(
+ new CfInvoke(org.objectweb.asm.Opcodes.INVOKESPECIAL, getInvokedMethod(), isInterface));
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java b/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java
index 3ef66ca..e95be77 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java
@@ -31,6 +31,11 @@
}
@Override
+ public boolean getInterfaceBit() {
+ return true;
+ }
+
+ @Override
public int opcode() {
return Opcodes.INVOKE_INTERFACE;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
index 8c059d9..e0e00ac 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
@@ -41,6 +41,8 @@
this.method = target;
}
+ public abstract boolean getInterfaceBit();
+
@Override
public DexType getReturnType() {
return method.proto.returnType;
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokePolymorphic.java b/src/main/java/com/android/tools/r8/ir/code/InvokePolymorphic.java
index 6a1e8a2..824a283 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokePolymorphic.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokePolymorphic.java
@@ -36,6 +36,11 @@
}
@Override
+ public boolean getInterfaceBit() {
+ return false;
+ }
+
+ @Override
public int opcode() {
return Opcodes.INVOKE_POLYMORPHIC;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
index 7cd9b1e..7f5836a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
@@ -43,6 +43,11 @@
}
@Override
+ public boolean getInterfaceBit() {
+ return isInterface;
+ }
+
+ @Override
public int opcode() {
return Opcodes.INVOKE_STATIC;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeSuper.java b/src/main/java/com/android/tools/r8/ir/code/InvokeSuper.java
index 0e1faf4..4815431 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeSuper.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeSuper.java
@@ -24,11 +24,16 @@
public class InvokeSuper extends InvokeMethodWithReceiver {
- public final boolean itf;
+ public final boolean isInterface;
- public InvokeSuper(DexMethod target, Value result, List<Value> arguments, boolean itf) {
+ public InvokeSuper(DexMethod target, Value result, List<Value> arguments, boolean isInterface) {
super(target, result, arguments);
- this.itf = itf;
+ this.isInterface = isInterface;
+ }
+
+ @Override
+ public boolean getInterfaceBit() {
+ return isInterface;
}
@Override
@@ -77,7 +82,8 @@
@Override
public void buildCf(CfBuilder builder) {
- builder.add(new CfInvoke(org.objectweb.asm.Opcodes.INVOKESPECIAL, getInvokedMethod(), itf));
+ builder.add(
+ new CfInvoke(org.objectweb.asm.Opcodes.INVOKESPECIAL, getInvokedMethod(), isInterface));
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java b/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
index 6dafcd0..2d95115 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
@@ -32,6 +32,11 @@
}
@Override
+ public boolean getInterfaceBit() {
+ return false;
+ }
+
+ @Override
public int opcode() {
return Opcodes.INVOKE_VIRTUAL;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 5893a3b..c4bf62f 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
@@ -113,6 +114,7 @@
@Override
public boolean passesInliningConstraints(
InvokeMethod invoke,
+ SingleResolutionResult resolutionResult,
ProgramMethod singleTarget,
Reason reason,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
@@ -167,7 +169,7 @@
}
// Abort inlining attempt if method -> target access is not right.
- if (!inliner.hasInliningAccess(method, singleTarget)) {
+ if (resolutionResult.isAccessibleFrom(method, appView.appInfo()).isPossiblyFalse()) {
whyAreYouNotInliningReporter.reportInaccessible();
return false;
}
@@ -254,6 +256,7 @@
@Override
public InlineAction computeInlining(
InvokeMethod invoke,
+ SingleResolutionResult resolutionResult,
ProgramMethod singleTarget,
ProgramMethod context,
ClassInitializationAnalysis classInitializationAnalysis,
@@ -277,7 +280,8 @@
return null;
}
- if (!passesInliningConstraints(invoke, singleTarget, reason, whyAreYouNotInliningReporter)) {
+ if (!passesInliningConstraints(
+ invoke, resolutionResult, singleTarget, reason, whyAreYouNotInliningReporter)) {
return null;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
index a48dece..eea6b7c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
@@ -42,6 +43,7 @@
@Override
public boolean passesInliningConstraints(
InvokeMethod invoke,
+ SingleResolutionResult resolutionResult,
ProgramMethod candidate,
Reason reason,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
@@ -60,15 +62,18 @@
@Override
public InlineAction computeInlining(
InvokeMethod invoke,
+ SingleResolutionResult resolutionResult,
ProgramMethod singleTarget,
ProgramMethod context,
ClassInitializationAnalysis classInitializationAnalysis,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
- return computeForInvoke(invoke, whyAreYouNotInliningReporter);
+ return computeForInvoke(invoke, resolutionResult, whyAreYouNotInliningReporter);
}
private InlineAction computeForInvoke(
- InvokeMethod invoke, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
+ InvokeMethod invoke,
+ SingleResolutionResult resolutionResult,
+ WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
Inliner.InliningInfo info = invokesToInline.get(invoke);
if (info == null) {
return null;
@@ -80,7 +85,7 @@
// with neverInline() flag.
assert !info.target.getDefinition().getOptimizationInfo().neverInline();
assert passesInliningConstraints(
- invoke, info.target, Reason.FORCE, whyAreYouNotInliningReporter);
+ invoke, resolutionResult, info.target, Reason.FORCE, whyAreYouNotInliningReporter);
return new InlineAction(info.target, invoke, Reason.FORCE);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 2589e6a..17cea8b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.NestMemberClassAttribute;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.analysis.proto.ProtoInliningReasonStrategy;
import com.android.tools.r8.ir.analysis.type.Nullability;
@@ -199,29 +200,6 @@
return false;
}
- boolean hasInliningAccess(ProgramMethod context, ProgramMethod target) {
- if (!isVisibleWithFlags(target.getHolderType(), context, target.getDefinition().accessFlags)) {
- return false;
- }
- // The class needs also to be visible for us to have access.
- return isVisibleWithFlags(target.getHolderType(), context, target.getHolder().accessFlags);
- }
-
- private boolean isVisibleWithFlags(DexType target, ProgramMethod context, AccessFlags<?> flags) {
- if (flags.isPublic()) {
- return true;
- }
- if (flags.isPrivate()) {
- return NestUtils.sameNest(target, context.getHolderType(), appView);
- }
- if (flags.isProtected()) {
- return appView.appInfo().isSubtype(context.getHolderType(), target)
- || target.isSamePackage(context.getHolderType());
- }
- // package-private
- return target.isSamePackage(context.getHolderType());
- }
-
public synchronized boolean isDoubleInlineSelectedTarget(ProgramMethod method) {
return doubleInlineSelectedTargets.contains(method);
}
@@ -975,6 +953,17 @@
if (current.isInvokeMethod()) {
InvokeMethod invoke = current.asInvokeMethod();
// TODO(b/142116551): This should be equivalent to invoke.lookupSingleTarget()!
+
+ SingleResolutionResult resolutionResult =
+ appView
+ .appInfo()
+ .resolveMethod(invoke.getInvokedMethod(), invoke.getInterfaceBit())
+ .asSingleResolution();
+ if (resolutionResult == null) {
+ continue;
+ }
+
+ // TODO(b/156853206): Should not duplicate resolution.
ProgramMethod singleTarget = oracle.lookupSingleTarget(invoke, context);
if (singleTarget == null) {
WhyAreYouNotInliningReporter.handleInvokeWithUnknownTarget(invoke, appView, context);
@@ -989,6 +978,7 @@
InlineAction action =
oracle.computeInlining(
invoke,
+ resolutionResult,
singleTarget,
context,
classInitializationAnalysis,
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningOracle.java
index a9e4736..5518ab7 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningOracle.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.optimize;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
@@ -23,12 +24,14 @@
boolean passesInliningConstraints(
InvokeMethod invoke,
+ SingleResolutionResult resolutionResult,
ProgramMethod candidate,
Reason reason,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter);
InlineAction computeInlining(
InvokeMethod invoke,
+ SingleResolutionResult resolutionResult,
ProgramMethod singleTarget,
ProgramMethod context,
ClassInitializationAnalysis classInitializationAnalysis,
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/NestUtils.java b/src/main/java/com/android/tools/r8/ir/optimize/NestUtils.java
index 3cd05c5..9db947a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/NestUtils.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/NestUtils.java
@@ -57,7 +57,7 @@
assert encodedMethod.isPrivateMethod();
// Call to private method which has now to be interface/virtual
// (Now call to nest member private method).
- if (invoke.isInterface()) {
+ if (invoke.getInterfaceBit()) {
iterator.replaceCurrentInstruction(
new InvokeInterface(method, invoke.outValue(), invoke.inValues()));
} else {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 1355477..da8c4cc 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.ResolutionResult;
+import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
import com.android.tools.r8.ir.analysis.type.Nullability;
@@ -282,6 +283,16 @@
if (user.isInvokeMethod()) {
InvokeMethod invokeMethod = user.asInvokeMethod();
+ SingleResolutionResult resolutionResult =
+ appView
+ .appInfo()
+ .resolveMethod(invokeMethod.getInvokedMethod(), invokeMethod.getInterfaceBit())
+ .asSingleResolution();
+ if (resolutionResult == null) {
+ return user; // Not eligible.
+ }
+
+ // TODO(b/156853206): Avoid duplicating resolution.
DexEncodedMethod singleTargetMethod = invokeMethod.lookupSingleTarget(appView, method);
if (singleTargetMethod == null) {
return user; // Not eligible.
@@ -321,7 +332,7 @@
InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
InliningInfo inliningInfo =
isEligibleDirectVirtualMethodCall(
- invoke, singleTarget, indirectUsers, defaultOracle);
+ invoke, resolutionResult, singleTarget, indirectUsers, defaultOracle);
if (inliningInfo != null) {
methodCallsOnInstance.put(invoke, inliningInfo);
continue;
@@ -879,6 +890,7 @@
private InliningInfo isEligibleDirectVirtualMethodCall(
InvokeMethodWithReceiver invoke,
+ SingleResolutionResult resolutionResult,
ProgramMethod singleTarget,
Set<Instruction> indirectUsers,
Supplier<InliningOracle> defaultOracle) {
@@ -896,7 +908,11 @@
if (singleTarget.getDefinition().isLibraryMethodOverride().isTrue()) {
InliningOracle inliningOracle = defaultOracle.get();
if (!inliningOracle.passesInliningConstraints(
- invoke, singleTarget, Reason.SIMPLE, NopWhyAreYouNotInliningReporter.getInstance())) {
+ invoke,
+ resolutionResult,
+ singleTarget,
+ Reason.SIMPLE,
+ NopWhyAreYouNotInliningReporter.getInstance())) {
return null;
}
}
@@ -920,13 +936,16 @@
Pair<Type, DexMethod> invokeInfo = eligibility.callsReceiver.get(0);
Type invokeType = invokeInfo.getFirst();
DexMethod indirectlyInvokedMethod = invokeInfo.getSecond();
- ResolutionResult resolutionResult =
- appView.appInfo().resolveMethodOn(eligibleClass, indirectlyInvokedMethod);
- if (!resolutionResult.isSingleResolution()) {
+ SingleResolutionResult indirectResolutionResult =
+ appView
+ .appInfo()
+ .resolveMethodOn(eligibleClass, indirectlyInvokedMethod)
+ .asSingleResolution();
+ if (indirectResolutionResult == null) {
return null;
}
ProgramMethod indirectSingleTarget =
- resolutionResult.asSingleResolution().getResolutionPair().asProgramMethod();
+ indirectResolutionResult.getResolutionPair().asProgramMethod();
if (!isEligibleIndirectVirtualMethodCall(
indirectlyInvokedMethod, invokeType, indirectSingleTarget)) {
return null;
@@ -1195,24 +1214,28 @@
return false;
}
+ SingleResolutionResult resolutionResult =
+ appView
+ .appInfo()
+ .resolveMethod(invoke.getInvokedMethod(), invoke.getInterfaceBit())
+ .asSingleResolution();
+ if (resolutionResult == null) {
+ return false;
+ }
+
// Check if the method is inline-able by standard inliner.
- DexEncodedMethod singleTarget = invoke.lookupSingleTarget(appView, method);
+ // TODO(b/156853206): Should not duplicate resolution.
+ ProgramMethod singleTarget = invoke.lookupSingleProgramTarget(appView, method);
if (singleTarget == null) {
return false;
}
- DexProgramClass holder = asProgramClassOrNull(appView.definitionForHolder(singleTarget));
- if (holder == null) {
- return false;
- }
-
- ProgramMethod singleTargetMethod = new ProgramMethod(holder, singleTarget);
-
InliningOracle oracle = defaultOracle.get();
InlineAction inlineAction =
oracle.computeInlining(
invoke,
- singleTargetMethod,
+ resolutionResult,
+ singleTarget,
method,
ClassInitializationAnalysis.trivial(),
NopWhyAreYouNotInliningReporter.getInstance());
