Optimizing: Improve generated code for field access...
... to resolved fields that are package-private or protected
and/or defined in a package-private class but still known to
be accessible from the unresolved compiling class.
Test: Update test 727-checker-unresolved-class
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 161898207
Change-Id: I3c90253014da209fd6a665cc230ac375b5ef1a9b
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 1993fa2..1aef100 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -2023,9 +2023,29 @@
// Check access.
Handle<mirror::Class> compiling_class = dex_compilation_unit_->GetCompilingClass();
if (compiling_class == nullptr) {
- // For unresolved compiling class, handle only the simple case of a public field
- // in a public class and use a slow runtime call for all other cases.
- if (!resolved_field->IsPublic() || !resolved_field->GetDeclaringClass()->IsPublic()) {
+ // Check if the declaring class or referencing class is accessible.
+ SamePackageCompare same_package(*dex_compilation_unit_);
+ ObjPtr<mirror::Class> declaring_class = resolved_field->GetDeclaringClass();
+ bool declaring_class_accessible = declaring_class->IsPublic() || same_package(declaring_class);
+ if (!declaring_class_accessible) {
+ // It is possible to access members from an inaccessible superclass
+ // by referencing them through an accessible subclass.
+ ObjPtr<mirror::Class> referenced_class = class_linker->LookupResolvedType(
+ dex_compilation_unit_->GetDexFile()->GetFieldId(field_idx).class_idx_,
+ dex_compilation_unit_->GetDexCache().Get(),
+ class_loader.Get());
+ DCHECK(referenced_class != nullptr); // Must have been resolved when resolving the field.
+ if (!referenced_class->IsPublic() && !same_package(referenced_class)) {
+ return nullptr;
+ }
+ }
+ // Check whether the field itself is accessible.
+ // Since the referrer is unresolved but the field is resolved, it cannot be
+ // inside the same class, so a private field is known to be inaccessible.
+ // And without a resolved referrer, we cannot check for protected member access
+ // in superlass, so we handle only access to public member or within the package.
+ if (resolved_field->IsPrivate() ||
+ (!resolved_field->IsPublic() && !declaring_class_accessible)) {
return nullptr;
}
} else if (!compiling_class->CanAccessResolvedField(resolved_field->GetDeclaringClass(),
diff --git a/test/608-checker-unresolved-lse/src-dex2oat-unresolved/MissingSuperClass.java b/test/608-checker-unresolved-lse/src-dex2oat-unresolved/MissingInterface.java
similarity index 94%
rename from test/608-checker-unresolved-lse/src-dex2oat-unresolved/MissingSuperClass.java
rename to test/608-checker-unresolved-lse/src-dex2oat-unresolved/MissingInterface.java
index b11b9be..307f7be 100644
--- a/test/608-checker-unresolved-lse/src-dex2oat-unresolved/MissingSuperClass.java
+++ b/test/608-checker-unresolved-lse/src-dex2oat-unresolved/MissingInterface.java
@@ -14,5 +14,5 @@
* limitations under the License.
*/
-public class MissingSuperClass {
+public interface MissingInterface {
}
diff --git a/test/608-checker-unresolved-lse/src/Main.java b/test/608-checker-unresolved-lse/src/Main.java
index a39dd51..2e4910f 100644
--- a/test/608-checker-unresolved-lse/src/Main.java
+++ b/test/608-checker-unresolved-lse/src/Main.java
@@ -14,10 +14,7 @@
* limitations under the License.
*/
-// We make Main extend an unresolved super class. This will lead to an
-// unresolved access to Foo.field, as we won't know if Main can access
-// a package private field.
-public class Main extends MissingSuperClass {
+public class Main {
public static void main(String[] args) {
instanceFieldTest();
@@ -25,10 +22,7 @@
instanceFieldTest2();
}
- /// CHECK-START: void Main.instanceFieldTest() inliner (before)
- /// CHECK-NOT: InstanceFieldSet
-
- /// CHECK-START: void Main.instanceFieldTest() inliner (after)
+ /// CHECK-START: void Main.instanceFieldTest() load_store_elimination (before)
/// CHECK: InstanceFieldSet
/// CHECK: UnresolvedInstanceFieldGet
@@ -42,17 +36,15 @@
/// CHECK: InstanceFieldSet
/// CHECK: UnresolvedInstanceFieldGet
public static void instanceFieldTest() {
- Foo f = new Foo();
- if (f.iField != 42) {
+ SubFoo sf = new SubFoo();
+ Foo f = sf;
+ f.iField = 42;
+ if (sf.iField != 42) {
throw new Error("Expected 42, got " + f.iField);
}
}
- /// CHECK-START: void Main.instanceFieldTest2() inliner (before)
- /// CHECK-NOT: InstanceFieldSet
- /// CHECK-NOT: InstanceFieldGet
-
- /// CHECK-START: void Main.instanceFieldTest2() inliner (after)
+ /// CHECK-START: void Main.instanceFieldTest2() load_store_elimination (before)
/// CHECK: InstanceFieldSet
/// CHECK: InstanceFieldGet
/// CHECK: UnresolvedInstanceFieldSet
@@ -69,19 +61,18 @@
/// CHECK: UnresolvedInstanceFieldSet
/// CHECK: InstanceFieldGet
public static void instanceFieldTest2() {
- Foo f = new Foo();
- int a = f.$inline$GetInstanceField();
- f.iField = 43;
- a = f.$inline$GetInstanceField();
+ SubFoo sf = new SubFoo();
+ Foo f = sf;
+ f.iField = 42;
+ int a = f.iField;
+ sf.iField = 43;
+ a = f.iField;
if (a != 43) {
throw new Error("Expected 43, got " + a);
}
}
- /// CHECK-START: void Main.staticFieldTest() inliner (before)
- /// CHECK-NOT: StaticFieldSet
-
- /// CHECK-START: void Main.staticFieldTest() inliner (after)
+ /// CHECK-START: void Main.staticFieldTest() load_store_elimination (before)
/// CHECK: StaticFieldSet
/// CHECK: StaticFieldSet
/// CHECK: UnresolvedStaticFieldGet
@@ -90,37 +81,21 @@
/// CHECK: StaticFieldSet
/// CHECK: UnresolvedStaticFieldGet
public static void staticFieldTest() {
- // Ensure Foo is initialized.
- Foo f = new Foo();
- f.$inline$StaticSet42();
- f.$inline$StaticSet43();
- if (Foo.sField != 43) {
- throw new Error("Expected 43, got " + Foo.sField);
+ Foo.sField = 42;
+ Foo.sField = 43;
+ if (SubFoo.sField != 43) {
+ throw new Error("Expected 43, got " + SubFoo.sField);
}
}
}
class Foo {
- // field needs to be package-private to make the access in Main.main
- // unresolved.
- int iField;
- static int sField;
+ public int iField;
+ public static int sField;
+}
- public void $inline$StaticSet42() {
- sField = 42;
- }
-
- public void $inline$StaticSet43() {
- sField = 43;
- }
-
- public int $inline$GetInstanceField() {
- return iField;
- }
-
- // Constructor needs to be public to get it resolved in Main.main
- // and therefore inlined.
- public Foo() {
- iField = 42;
- }
+// We make SubFoo implement an unresolved interface, so the SubFoo
+// shall be unresolved and all field accesses through SubFoo shall
+// yield unresolved field access HIR.
+class SubFoo extends Foo implements MissingInterface {
}
diff --git a/test/727-checker-unresolved-class/src-ex/resolved/SubclassOfUnresolvedClass2.java b/test/727-checker-unresolved-class/src-ex/resolved/SubclassOfUnresolvedClass2.java
index 41b5e8b..8469cf7 100644
--- a/test/727-checker-unresolved-class/src-ex/resolved/SubclassOfUnresolvedClass2.java
+++ b/test/727-checker-unresolved-class/src-ex/resolved/SubclassOfUnresolvedClass2.java
@@ -53,12 +53,11 @@
}
/// CHECK-START: void resolved.SubclassOfUnresolvedClass2.$noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK: UnresolvedStaticFieldSet
+ /// CHECK: StaticFieldSet
/// CHECK-START: void resolved.SubclassOfUnresolvedClass2.$noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: UnresolvedStaticFieldSet
static void $noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() {
- // TODO: Use StaticFieldSet.
ResolvedPublicSubclassOfPackagePrivateClass.publicIntField = 42;
}
diff --git a/test/727-checker-unresolved-class/src/resolved/SubclassOfUnresolvedClass.java b/test/727-checker-unresolved-class/src/resolved/SubclassOfUnresolvedClass.java
index fb8327c..d767411 100644
--- a/test/727-checker-unresolved-class/src/resolved/SubclassOfUnresolvedClass.java
+++ b/test/727-checker-unresolved-class/src/resolved/SubclassOfUnresolvedClass.java
@@ -38,22 +38,20 @@
}
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPublicFieldInResolvedPackagePrivateClass() builder (after)
- /// CHECK: UnresolvedStaticFieldSet
+ /// CHECK: StaticFieldSet
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPublicFieldInResolvedPackagePrivateClass() builder (after)
- /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: UnresolvedStaticFieldSet
static void $noinline$testPublicFieldInResolvedPackagePrivateClass() {
- // TODO: Use StaticFieldSet.
ResolvedPackagePrivateClass.publicIntField = 42;
}
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK: UnresolvedStaticFieldSet
+ /// CHECK: StaticFieldSet
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: UnresolvedStaticFieldSet
static void $noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() {
- // TODO: Use StaticFieldSet.
ResolvedPublicSubclassOfPackagePrivateClass.publicIntField = 42;
}
@@ -82,22 +80,20 @@
}
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPackagePrivateFieldInResolvedPackagePrivateClass() builder (after)
- /// CHECK: UnresolvedStaticFieldSet
+ /// CHECK: StaticFieldSet
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPackagePrivateFieldInResolvedPackagePrivateClass() builder (after)
- /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: UnresolvedStaticFieldSet
static void $noinline$testPackagePrivateFieldInResolvedPackagePrivateClass() {
- // TODO: Use StaticFieldSet.
ResolvedPackagePrivateClass.intField = 42;
}
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPackagePrivateFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK: UnresolvedStaticFieldSet
+ /// CHECK: StaticFieldSet
/// CHECK-START: void resolved.SubclassOfUnresolvedClass.$noinline$testPackagePrivateFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: UnresolvedStaticFieldSet
static void $noinline$testPackagePrivateFieldInPackagePrivateClassViaResolvedPublicSubclass() {
- // TODO: Use StaticFieldSet.
ResolvedPublicSubclassOfPackagePrivateClass.intField = 42;
}
diff --git a/test/727-checker-unresolved-class/src/unresolved/UnresolvedClass.java b/test/727-checker-unresolved-class/src/unresolved/UnresolvedClass.java
index 7311069..eaa9493 100644
--- a/test/727-checker-unresolved-class/src/unresolved/UnresolvedClass.java
+++ b/test/727-checker-unresolved-class/src/unresolved/UnresolvedClass.java
@@ -51,12 +51,11 @@
}
/// CHECK-START: void unresolved.UnresolvedClass.$noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK: UnresolvedStaticFieldSet
+ /// CHECK: StaticFieldSet
/// CHECK-START: void unresolved.UnresolvedClass.$noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() builder (after)
- /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: UnresolvedStaticFieldSet
static void $noinline$testPublicFieldInPackagePrivateClassViaResolvedPublicSubclass() {
- // TODO: Use StaticFieldSet when the referenced class is public.
ResolvedPublicSubclassOfPackagePrivateClass.publicIntField = 42;
}