Skip to content

Commit

Permalink
Do not transform invokespecial instructions produced for direct insta…
Browse files Browse the repository at this point in the history
…ntiations.

The instruction invokespecial is also used to invoke constructors and perform object instantiation. This CL fixes Hilt's transform to not change the owner of such instructions even if the owner is the older superclass since they are instantiating an object that happens to be the of the same type as the 'old' superclass.

Fixes #2662

RELNOTES=Fix an issue in Hilt's bytecode transform that would cause classes to fail validation if they contained an instantiation of an object whose type is the same as the superclass of the @androidentrypoint annotated class.
PiperOrigin-RevId: 378500741
  • Loading branch information
danysantiago authored and Dagger Team committed Jun 9, 2021
1 parent 38077ee commit 839a849
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 8 deletions.
Expand Up @@ -189,6 +189,12 @@ internal class AndroidEntryPointClassTransformer(
if (currentClassRef != oldSuperclassName) {
return@forEachInstruction
}
// If the method reference of the instruction is a constructor, then we should not
// rewrite it since its an instantiation and not a `super()` call.
val methodRefName = constantPool.getMethodrefName(methodRef)
if (methodRefName == "<init>") {
return@forEachInstruction
}
val nameAndTypeRef = constantPool.getMethodrefNameAndType(methodRef)
val newSuperclassRef = constantPool.addClassInfo(newSuperclassName)
val newMethodRef = constantPool.addMethodrefInfo(newSuperclassRef, nameAndTypeRef)
Expand Down
Expand Up @@ -17,18 +17,19 @@ import org.objectweb.asm.Opcodes
* ASM Adapter that transforms @AndroidEntryPoint-annotated classes to extend the Hilt
* generated android class, including the @HiltAndroidApp application class.
*/
@Suppress("UnstableApiUsage")
class AndroidEntryPointClassVisitor(
private val apiVersion: Int,
nextClassVisitor: ClassVisitor,
private val additionalClasses: File
) : ClassVisitor(apiVersion, nextClassVisitor) {

@Suppress("UnstableApiUsage") // ASM Pipeline APIs
interface AndroidEntryPointParams : InstrumentationParameters {
@get:Input
val additionalClassesDir: Property<File>
}

@Suppress("UnstableApiUsage") // ASM Pipeline APIs
abstract class Factory : AsmClassVisitorFactory<AndroidEntryPointParams> {
override fun createClassVisitor(
classContext: ClassContext,
Expand Down Expand Up @@ -80,7 +81,11 @@ class AndroidEntryPointClassVisitor(
exceptions: Array<out String>?
): MethodVisitor {
val nextMethodVisitor = super.visitMethod(access, name, descriptor, signature, exceptions)
val invokeSpecialVisitor = InvokeSpecialAdapter(apiVersion, nextMethodVisitor)
val invokeSpecialVisitor = InvokeSpecialAdapter(
apiVersion = apiVersion,
nextClassVisitor = nextMethodVisitor,
isConstructor = name == "<init>"
)
if (name == ON_RECEIVE_METHOD_NAME &&
descriptor == ON_RECEIVE_METHOD_DESCRIPTOR &&
hasOnReceiveBytecodeInjectionMarker()
Expand All @@ -104,16 +109,22 @@ class AndroidEntryPointClassVisitor(
* However, it has been observed that on APIs 19 to 22 the Android Runtime (ART) jumps over the
* direct superclass and into the method reference class, causing unexpected behaviours.
* Therefore, this method performs the additional transformation to rewrite direct super call
* invocations to use a method reference whose class in the pool is the new superclass. Note that
* this is not necessary for constructor calls since the Javassist library takes care of those.
* invocations to use a method reference whose class in the pool is the new superclass.
*
* @see: https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-6.html#jvms-6.5.invokespecial
* @see: https://source.android.com/devices/tech/dalvik/dalvik-bytecode
*/
inner class InvokeSpecialAdapter(
apiVersion: Int,
nextClassVisitor: MethodVisitor
nextClassVisitor: MethodVisitor,
private val isConstructor: Boolean
) : MethodVisitor(apiVersion, nextClassVisitor) {

// Flag to know that we have visited the first invokespecial instruction in a constructor call
// which corresponds to the `super()` constructor call required as the first statement of an
// overridden constructor body.
private var visitedSuperConstructorInvokeSpecial = false

override fun visitMethodInsn(
opcode: Int,
owner: String,
Expand All @@ -122,13 +133,29 @@ class AndroidEntryPointClassVisitor(
isInterface: Boolean
) {
if (opcode == Opcodes.INVOKESPECIAL && owner == oldSuperclassName) {
// Update the owner of all INVOKESPECIAL instructions, including those found in
// constructors.
super.visitMethodInsn(opcode, newSuperclassName, name, descriptor, isInterface)
// Update the owner of INVOKESPECIAL instructions, including those found in constructors.
super.visitMethodInsn(opcode, getAdaptedOwner(name) ?: owner, name, descriptor, isInterface)
} else {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface)
}
}

// Gets the updated owner of an INVOKESPECIAL found in the method being visited.
private fun getAdaptedOwner(methodRefName: String): String? {
// If the method reference is a constructor and we are visiting a constructor then only the
// first INVOKESPECIAL instruction found should be transformed since that correponds to the
// super constructor call.
if (methodRefName == "<init>" && isConstructor && !visitedSuperConstructorInvokeSpecial) {
visitedSuperConstructorInvokeSpecial = true
return newSuperclassName
}
// If the method reference is not a constructor then the instruction for a super call that
// should be transformed.
if (methodRefName != "<init>") {
return newSuperclassName
}
return null
}
}

/**
Expand Down
Expand Up @@ -25,6 +25,10 @@
android:name=".Injection2Test$TestActivity"
android:theme="@style/Theme.AppCompat.Light"
android:exported="false"/>
<activity
android:name=".InvokeSpecialTransformTest$TestActivity"
android:theme="@style/Theme.AppCompat.Light"
android:exported="false"/>
<activity
android:name=".ActivityScenarioRuleTest$TestActivity"
android:theme="@style/Theme.AppCompat.Light"
Expand Down
@@ -0,0 +1,77 @@
/*
* Copyright (C) 2020 The Dagger Authors.
*
* 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 dagger.hilt.android.simple;

import android.content.Context;
import android.os.Bundle;
import android.widget.FrameLayout;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.appcompat.app.AppCompatActivity;
import androidx.lifecycle.Lifecycle.State;
import androidx.test.core.app.ActivityScenario;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import dagger.hilt.android.AndroidEntryPoint;
import dagger.hilt.android.testing.HiltAndroidRule;
import dagger.hilt.android.testing.HiltAndroidTest;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

/** Test that verifies edge cases of invokespecial instructions transformation. */
@HiltAndroidTest
@RunWith(AndroidJUnit4.class)
public class InvokeSpecialTransformTest {

@Rule
public HiltAndroidRule rule = new HiltAndroidRule(this);

@Test
public void constructorCallOfOldSuperclass() {
try (ActivityScenario<TestActivity> scenario = ActivityScenario.launch(TestActivity.class)) {
scenario.moveToState(State.DESTROYED);
}
}

/** A test activity */
@AndroidEntryPoint
public static final class TestActivity extends AppCompatActivity {

@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(new CustomView(this).createBrother());
}
}

/** A custom view for testing */
@AndroidEntryPoint
public static class CustomView extends FrameLayout {

public CustomView(@NonNull Context context) {
// This super call is an invokespecial that should be transformed
super(context);
// This invokespecial that should not be transformed.
FrameLayout secondInvokeSpecial = new FrameLayout(getContext());
}

FrameLayout createBrother() {
// This invokespecial that should not be transformed.
return new FrameLayout(getContext());
}
}
}

0 comments on commit 839a849

Please sign in to comment.