diff --git a/CHANGELOG.md b/CHANGELOG.md index 465da8d957..097d29cda6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Fix a D8 crash (`NullPointerException: Cannot invoke "String.length()"`) when dexing the SDK with AGP 8.0 and older, a regression from 8.61.1 ([#6356](https://github.com/getsentry/sentry-java/pull/6356)) - Always remove the first-draw listener after the first frame, even if no layout pass follows it ([#6152](https://github.com/getsentry/sentry-java/pull/6152)) ## 8.61.1 diff --git a/build-logic/build.gradle.kts b/build-logic/build.gradle.kts index d02ab58488..60a340c9b8 100644 --- a/build-logic/build.gradle.kts +++ b/build-logic/build.gradle.kts @@ -5,6 +5,7 @@ plugins { dependencies { implementation(libs.animalsniffer.gradle.plugin) implementation(libs.spotlessLib) + implementation(libs.asm) compileOnly(libs.kotlin.gradle.plugin) compileOnly(libs.android.gradle.plugin) } diff --git a/build-logic/src/main/kotlin/io/sentry/gradle/JdkTargetConvention.kt b/build-logic/src/main/kotlin/io/sentry/gradle/JdkTargetConvention.kt index c92c83bbac..ca2999f878 100644 --- a/build-logic/src/main/kotlin/io/sentry/gradle/JdkTargetConvention.kt +++ b/build-logic/src/main/kotlin/io/sentry/gradle/JdkTargetConvention.kt @@ -1,5 +1,7 @@ package io.sentry.gradle +import com.android.build.api.artifact.SingleArtifact +import com.android.build.api.variant.LibraryAndroidComponentsExtension import com.android.build.gradle.BaseExtension import org.gradle.api.JavaVersion import org.gradle.api.Plugin @@ -27,6 +29,8 @@ private val PUBLISHED_JAVA_VERSION = JavaVersion.VERSION_1_8 * - sample apps target the build JDK (21) * - unit test code will target the build JDK (21) (required for Mockito 5) * - published JVM modules main code pins the Java 1.8 release, for Java and for Kotlin + * - published modules Java code drops the MethodParameters attribute javac 21 adds, and their + * archives are checked for it * - published Android modules main code targets Java 1.8 * - minSdk is set for Android libraries */ @@ -39,6 +43,30 @@ class JdkTargetConvention : Plugin { } project.plugins.withId("com.vanniktech.maven.publish") { + // Published Java classes must stay dexable by older D8 versions, see StripMethodParameters + project.tasks.withType(JavaCompile::class.java).configureEach { + doLast(StripMethodParameters()) + } + val verifyMethodParameters = + project.tasks.register("verifyMethodParameters", VerifyMethodParameters::class.java) { + description = "Checks the published archives for MethodParameters entries without a name" + group = "verification" + report.set(project.layout.buildDirectory.file("reports/method-parameters.txt")) + } + project.tasks.matching { it.name == "check" }.configureEach { + dependsOn(verifyMethodParameters) + } + project.plugins.withId("java-library") { + verifyMethodParameters.configure { archives.from(project.tasks.named("jar")) } + } + project.plugins.withId("com.android.library") { + project.extensions.getByType(LibraryAndroidComponentsExtension::class.java).onVariants { + variant -> + verifyMethodParameters.configure { + archives.from(variant.artifacts.get(SingleArtifact.AAR)) + } + } + } // Only works for non-android modules // See https://issuetracker.google.com/issues/278800528 project.plugins.withId("java-library") { diff --git a/build-logic/src/main/kotlin/io/sentry/gradle/MethodParameters.kt b/build-logic/src/main/kotlin/io/sentry/gradle/MethodParameters.kt new file mode 100644 index 0000000000..49662c59c7 --- /dev/null +++ b/build-logic/src/main/kotlin/io/sentry/gradle/MethodParameters.kt @@ -0,0 +1,35 @@ +package io.sentry.gradle + +import org.objectweb.asm.ClassReader +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.Opcodes + +/** + * Returns `name + descriptor` of every method of the class that has a MethodParameters entry + * without a name. Only those entries make the D8 of AGP 8.0 and older fail, see + * [StripMethodParameters]. + */ +internal fun ClassReader.methodsWithUnnamedParameters(): Set { + val methods = mutableSetOf() + val scanner = + object : ClassVisitor(Opcodes.ASM9) { + override fun visitMethod( + access: Int, + name: String?, + descriptor: String?, + signature: String?, + exceptions: Array?, + ): MethodVisitor = + object : MethodVisitor(Opcodes.ASM9) { + override fun visitParameter(parameterName: String?, access: Int) { + if (parameterName == null) { + methods.add(name + descriptor) + } + } + } + } + // no SKIP_DEBUG here, ASM counts MethodParameters as debug info and then skips it + accept(scanner, ClassReader.SKIP_CODE) + return methods +} diff --git a/build-logic/src/main/kotlin/io/sentry/gradle/StripMethodParameters.kt b/build-logic/src/main/kotlin/io/sentry/gradle/StripMethodParameters.kt new file mode 100644 index 0000000000..94460086a9 --- /dev/null +++ b/build-logic/src/main/kotlin/io/sentry/gradle/StripMethodParameters.kt @@ -0,0 +1,66 @@ +package io.sentry.gradle + +import org.gradle.api.Action +import org.gradle.api.Task +import org.gradle.api.tasks.compile.JavaCompile +import org.objectweb.asm.ClassReader +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.ClassWriter +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.Opcodes + +/** + * Removes the + * [MethodParameters](https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.24) + * attribute from the methods of a JavaCompile task that have a parameter without a name. + * + * Since JDK 21 (JDK-8292275) javac writes MethodParameters for every method with synthetic or + * mandated parameters, e.g. bridge methods, even without `-parameters`. Those entries carry flags + * but no name, and the D8 of AGP 8.0 and older fails the build while dexing. See the + * [R8 fix](https://r8.googlesource.com/r8/+/9da58a26e22f195d8187492fac350a90e3315e92) for more + * details. An alternative would be adding the `-parameters` compile flag, but this could lead to + * other binary incompatibility issues along the road. + * + * `parameters_count` must match the parameters of the method descriptor, so a single entry cannot + * be removed. The attribute goes away for the whole method instead, as soon as one of its entries + * has no name. Methods whose entries all have a name, e.g. the canonical constructor of a record + * or anything compiled with `-parameters`, keep the attribute and their names. + */ +internal class StripMethodParameters : Action { + override fun execute(task: Task) { + val classesDir = (task as JavaCompile).destinationDirectory.get().asFile + classesDir + .walkTopDown() + .filter { it.isFile && it.extension == "class" } + .forEach { file -> + val reader = ClassReader(file.readBytes()) + val methods = reader.methodsWithUnnamedParameters() + if (methods.isNotEmpty()) { + val writer = ClassWriter(reader, 0) + reader.accept(MethodParametersStripper(writer, methods), 0) + file.writeBytes(writer.toByteArray()) + } + } + } + + private class MethodParametersStripper(next: ClassVisitor, private val methods: Set) : + ClassVisitor(Opcodes.ASM9, next) { + + override fun visitMethod( + access: Int, + name: String?, + descriptor: String?, + signature: String?, + exceptions: Array?, + ): MethodVisitor? { + val next = super.visitMethod(access, name, descriptor, signature, exceptions) + // every other method keeps the byte for byte copy ASM does for an untouched method + if (name + descriptor !in methods) { + return next + } + return object : MethodVisitor(Opcodes.ASM9, next) { + override fun visitParameter(name: String?, access: Int) = Unit + } + } + } +} diff --git a/build-logic/src/main/kotlin/io/sentry/gradle/VerifyMethodParameters.kt b/build-logic/src/main/kotlin/io/sentry/gradle/VerifyMethodParameters.kt new file mode 100644 index 0000000000..48ff0664e8 --- /dev/null +++ b/build-logic/src/main/kotlin/io/sentry/gradle/VerifyMethodParameters.kt @@ -0,0 +1,80 @@ +package io.sentry.gradle + +import java.io.ByteArrayInputStream +import java.util.zip.ZipInputStream +import org.gradle.api.DefaultTask +import org.gradle.api.GradleException +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.RegularFileProperty +import org.gradle.api.tasks.CacheableTask +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.OutputFile +import org.gradle.api.tasks.PathSensitive +import org.gradle.api.tasks.PathSensitivity +import org.gradle.api.tasks.TaskAction +import org.objectweb.asm.ClassReader + +/** + * Fails the build if an archive still holds a MethodParameters entry without a name, the entries + * [StripMethodParameters] removes. It reads the final jar and aar of a module, so it also covers + * classes that no JavaCompile task of this build produced, e.g. repackaged ones. + */ +@CacheableTask +abstract class VerifyMethodParameters : DefaultTask() { + + /** The jar and aar files to read. An aar is opened down to its nested jars. */ + @get:InputFiles + @get:PathSensitive(PathSensitivity.RELATIVE) + abstract val archives: ConfigurableFileCollection + + @get:OutputFile abstract val report: RegularFileProperty + + @TaskAction + fun verify() { + val offenders = mutableListOf() + var classes = 0 + archives.files + .filter { it.isFile } + .forEach { archive -> + archive.inputStream().use { stream -> + classes += scan(ZipInputStream(stream), archive.name, offenders) + } + } + + val summary = + if (offenders.isEmpty()) "$classes classes checked, no unnamed MethodParameters entry" + else offenders.joinToString("\n") + val reportFile = report.get().asFile + reportFile.parentFile.mkdirs() + reportFile.writeText(summary + "\n") + + if (offenders.isNotEmpty()) { + throw GradleException( + "${offenders.size} method(s) keep a MethodParameters entry without a name, which makes " + + "the D8 of AGP 8.0 and older fail. See StripMethodParameters.\n$summary" + ) + } + logger.info("verifyMethodParameters: $summary") + } + + /** Adds `archive!/entry method` for every offending method and returns the classes read. */ + private fun scan(zip: ZipInputStream, path: String, offenders: MutableList): Int { + var classes = 0 + while (true) { + val entry = zip.nextEntry ?: break + val name = entry.name + when { + name.endsWith(".class") -> { + val reader = ClassReader(zip.readBytes()) + reader.methodsWithUnnamedParameters().forEach { offenders.add("$path!/$name $it") } + classes++ + } + name.endsWith(".jar") -> { + val nested = ZipInputStream(ByteArrayInputStream(zip.readBytes())) + classes += scan(nested, "$path!/$name", offenders) + } + } + } + return classes + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e5c2f9e217..0e8b0c790d 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -157,6 +157,7 @@ lottie-compose = { module = "com.airbnb.android:lottie-compose", version = "6.7. logback-classic = { module = "ch.qos.logback:logback-classic", version.ref = "logback" } kotlin-gradle-plugin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" } android-gradle-plugin = { module = "com.android.tools.build:gradle", version.ref = "agp" } +asm = { module = "org.ow2.asm:asm", version = "9.9" } nopen-annotations = { module = "com.jakewharton.nopen:nopen-annotations", version.ref = "nopen" } nopen-checker = { module = "com.jakewharton.nopen:nopen-checker", version.ref = "nopen" } nullaway = { module = "com.uber.nullaway:nullaway", version = "0.14.2" }