Skip to content

Commit c42d51d

Browse files
antonisclaudemarkushisentry-junior[bot]
authored
fix(build): Strip MethodParameters so AGP 8.0 and older can dex the SDK (#6356)
* fix(build): Strip MethodParameters so AGP 8.0 and older can dex the SDK javac 21 writes nameless MethodParameters entries for synthetic and mandated parameters (JDK-8292275), which crash D8 older than 8.1 with a NullPointerException. Remove the attribute from the main Java classes of published modules after compiling, restoring the javac 17 output. Fixes #6355 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * changelog * fix(build): Strip MethodParameters from every module, not only published main code * fix(build): Strip MethodParameters only in published modules again * fix(build): Strip MethodParameters only for methods with an unnamed parameter * build: Verify published jars and aars have no unnamed MethodParameters entries --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Markus Hintersteiner <markus.hintersteiner@sentry.io> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
1 parent 72bcb18 commit c42d51d

7 files changed

Lines changed: 212 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
### Fixes
1111

12+
- 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))
1213
- 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))
1314
- Prevent Reactor pool maintenance tasks from retaining unbounded Sentry scope chains ([#6283](https://github.com/getsentry/sentry-java/pull/6283))
1415

‎build-logic/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ plugins {
55
dependencies {
66
implementation(libs.animalsniffer.gradle.plugin)
77
implementation(libs.spotlessLib)
8+
implementation(libs.asm)
89
compileOnly(libs.kotlin.gradle.plugin)
910
compileOnly(libs.android.gradle.plugin)
1011
}

‎build-logic/src/main/kotlin/io/sentry/gradle/JdkTargetConvention.kt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.sentry.gradle
22

3+
import com.android.build.api.artifact.SingleArtifact
4+
import com.android.build.api.variant.LibraryAndroidComponentsExtension
35
import com.android.build.gradle.BaseExtension
46
import org.gradle.api.JavaVersion
57
import org.gradle.api.Plugin
@@ -27,6 +29,8 @@ private val PUBLISHED_JAVA_VERSION = JavaVersion.VERSION_1_8
2729
* - sample apps target the build JDK (21)
2830
* - unit test code will target the build JDK (21) (required for Mockito 5)
2931
* - published JVM modules main code pins the Java 1.8 release, for Java and for Kotlin
32+
* - published modules Java code drops the MethodParameters attribute javac 21 adds, and their
33+
* archives are checked for it
3034
* - published Android modules main code targets Java 1.8
3135
* - minSdk is set for Android libraries
3236
*/
@@ -39,6 +43,30 @@ class JdkTargetConvention : Plugin<Project> {
3943
}
4044

4145
project.plugins.withId("com.vanniktech.maven.publish") {
46+
// Published Java classes must stay dexable by older D8 versions, see StripMethodParameters
47+
project.tasks.withType(JavaCompile::class.java).configureEach {
48+
doLast(StripMethodParameters())
49+
}
50+
val verifyMethodParameters =
51+
project.tasks.register("verifyMethodParameters", VerifyMethodParameters::class.java) {
52+
description = "Checks the published archives for MethodParameters entries without a name"
53+
group = "verification"
54+
report.set(project.layout.buildDirectory.file("reports/method-parameters.txt"))
55+
}
56+
project.tasks.matching { it.name == "check" }.configureEach {
57+
dependsOn(verifyMethodParameters)
58+
}
59+
project.plugins.withId("java-library") {
60+
verifyMethodParameters.configure { archives.from(project.tasks.named("jar")) }
61+
}
62+
project.plugins.withId("com.android.library") {
63+
project.extensions.getByType(LibraryAndroidComponentsExtension::class.java).onVariants {
64+
variant ->
65+
verifyMethodParameters.configure {
66+
archives.from(variant.artifacts.get(SingleArtifact.AAR))
67+
}
68+
}
69+
}
4270
// Only works for non-android modules
4371
// See https://issuetracker.google.com/issues/278800528
4472
project.plugins.withId("java-library") {

‎build-logic/src/main/kotlin/io/sentry/gradle/MethodParameters.kt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package io.sentry.gradle
2+
3+
import org.objectweb.asm.ClassReader
4+
import org.objectweb.asm.ClassVisitor
5+
import org.objectweb.asm.MethodVisitor
6+
import org.objectweb.asm.Opcodes
7+
8+
/**
9+
* Returns `name + descriptor` of every method of the class that has a MethodParameters entry
10+
* without a name. Only those entries make the D8 of AGP 8.0 and older fail, see
11+
* [StripMethodParameters].
12+
*/
13+
internal fun ClassReader.methodsWithUnnamedParameters(): Set<String> {
14+
val methods = mutableSetOf<String>()
15+
val scanner =
16+
object : ClassVisitor(Opcodes.ASM9) {
17+
override fun visitMethod(
18+
access: Int,
19+
name: String?,
20+
descriptor: String?,
21+
signature: String?,
22+
exceptions: Array<out String>?,
23+
): MethodVisitor =
24+
object : MethodVisitor(Opcodes.ASM9) {
25+
override fun visitParameter(parameterName: String?, access: Int) {
26+
if (parameterName == null) {
27+
methods.add(name + descriptor)
28+
}
29+
}
30+
}
31+
}
32+
// no SKIP_DEBUG here, ASM counts MethodParameters as debug info and then skips it
33+
accept(scanner, ClassReader.SKIP_CODE)
34+
return methods
35+
}

‎build-logic/src/main/kotlin/io/sentry/gradle/StripMethodParameters.kt‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package io.sentry.gradle
2+
3+
import org.gradle.api.Action
4+
import org.gradle.api.Task
5+
import org.gradle.api.tasks.compile.JavaCompile
6+
import org.objectweb.asm.ClassReader
7+
import org.objectweb.asm.ClassVisitor
8+
import org.objectweb.asm.ClassWriter
9+
import org.objectweb.asm.MethodVisitor
10+
import org.objectweb.asm.Opcodes
11+
12+
/**
13+
* Removes the
14+
* [MethodParameters](https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.24)
15+
* attribute from the methods of a JavaCompile task that have a parameter without a name.
16+
*
17+
* Since JDK 21 (JDK-8292275) javac writes MethodParameters for every method with synthetic or
18+
* mandated parameters, e.g. bridge methods, even without `-parameters`. Those entries carry flags
19+
* but no name, and the D8 of AGP 8.0 and older fails the build while dexing. See the
20+
* [R8 fix](https://r8.googlesource.com/r8/+/9da58a26e22f195d8187492fac350a90e3315e92) for more
21+
* details. An alternative would be adding the `-parameters` compile flag, but this could lead to
22+
* other binary incompatibility issues along the road.
23+
*
24+
* `parameters_count` must match the parameters of the method descriptor, so a single entry cannot
25+
* be removed. The attribute goes away for the whole method instead, as soon as one of its entries
26+
* has no name. Methods whose entries all have a name, e.g. the canonical constructor of a record
27+
* or anything compiled with `-parameters`, keep the attribute and their names.
28+
*/
29+
internal class StripMethodParameters : Action<Task> {
30+
override fun execute(task: Task) {
31+
val classesDir = (task as JavaCompile).destinationDirectory.get().asFile
32+
classesDir
33+
.walkTopDown()
34+
.filter { it.isFile && it.extension == "class" }
35+
.forEach { file ->
36+
val reader = ClassReader(file.readBytes())
37+
val methods = reader.methodsWithUnnamedParameters()
38+
if (methods.isNotEmpty()) {
39+
val writer = ClassWriter(reader, 0)
40+
reader.accept(MethodParametersStripper(writer, methods), 0)
41+
file.writeBytes(writer.toByteArray())
42+
}
43+
}
44+
}
45+
46+
private class MethodParametersStripper(next: ClassVisitor, private val methods: Set<String>) :
47+
ClassVisitor(Opcodes.ASM9, next) {
48+
49+
override fun visitMethod(
50+
access: Int,
51+
name: String?,
52+
descriptor: String?,
53+
signature: String?,
54+
exceptions: Array<out String>?,
55+
): MethodVisitor? {
56+
val next = super.visitMethod(access, name, descriptor, signature, exceptions)
57+
// every other method keeps the byte for byte copy ASM does for an untouched method
58+
if (name + descriptor !in methods) {
59+
return next
60+
}
61+
return object : MethodVisitor(Opcodes.ASM9, next) {
62+
override fun visitParameter(name: String?, access: Int) = Unit
63+
}
64+
}
65+
}
66+
}

‎build-logic/src/main/kotlin/io/sentry/gradle/VerifyMethodParameters.kt‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package io.sentry.gradle
2+
3+
import java.io.ByteArrayInputStream
4+
import java.util.zip.ZipInputStream
5+
import org.gradle.api.DefaultTask
6+
import org.gradle.api.GradleException
7+
import org.gradle.api.file.ConfigurableFileCollection
8+
import org.gradle.api.file.RegularFileProperty
9+
import org.gradle.api.tasks.CacheableTask
10+
import org.gradle.api.tasks.InputFiles
11+
import org.gradle.api.tasks.OutputFile
12+
import org.gradle.api.tasks.PathSensitive
13+
import org.gradle.api.tasks.PathSensitivity
14+
import org.gradle.api.tasks.TaskAction
15+
import org.objectweb.asm.ClassReader
16+
17+
/**
18+
* Fails the build if an archive still holds a MethodParameters entry without a name, the entries
19+
* [StripMethodParameters] removes. It reads the final jar and aar of a module, so it also covers
20+
* classes that no JavaCompile task of this build produced, e.g. repackaged ones.
21+
*/
22+
@CacheableTask
23+
abstract class VerifyMethodParameters : DefaultTask() {
24+
25+
/** The jar and aar files to read. An aar is opened down to its nested jars. */
26+
@get:InputFiles
27+
@get:PathSensitive(PathSensitivity.RELATIVE)
28+
abstract val archives: ConfigurableFileCollection
29+
30+
@get:OutputFile abstract val report: RegularFileProperty
31+
32+
@TaskAction
33+
fun verify() {
34+
val offenders = mutableListOf<String>()
35+
var classes = 0
36+
archives.files
37+
.filter { it.isFile }
38+
.forEach { archive ->
39+
archive.inputStream().use { stream ->
40+
classes += scan(ZipInputStream(stream), archive.name, offenders)
41+
}
42+
}
43+
44+
val summary =
45+
if (offenders.isEmpty()) "$classes classes checked, no unnamed MethodParameters entry"
46+
else offenders.joinToString("\n")
47+
val reportFile = report.get().asFile
48+
reportFile.parentFile.mkdirs()
49+
reportFile.writeText(summary + "\n")
50+
51+
if (offenders.isNotEmpty()) {
52+
throw GradleException(
53+
"${offenders.size} method(s) keep a MethodParameters entry without a name, which makes " +
54+
"the D8 of AGP 8.0 and older fail. See StripMethodParameters.\n$summary"
55+
)
56+
}
57+
logger.info("verifyMethodParameters: $summary")
58+
}
59+
60+
/** Adds `archive!/entry method` for every offending method and returns the classes read. */
61+
private fun scan(zip: ZipInputStream, path: String, offenders: MutableList<String>): Int {
62+
var classes = 0
63+
while (true) {
64+
val entry = zip.nextEntry ?: break
65+
val name = entry.name
66+
when {
67+
name.endsWith(".class") -> {
68+
val reader = ClassReader(zip.readBytes())
69+
reader.methodsWithUnnamedParameters().forEach { offenders.add("$path!/$name $it") }
70+
classes++
71+
}
72+
name.endsWith(".jar") -> {
73+
val nested = ZipInputStream(ByteArrayInputStream(zip.readBytes()))
74+
classes += scan(nested, "$path!/$name", offenders)
75+
}
76+
}
77+
}
78+
return classes
79+
}
80+
}

‎gradle/libs.versions.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ lottie-compose = { module = "com.airbnb.android:lottie-compose", version = "6.7.
161161
logback-classic = { module = "ch.qos.logback:logback-classic", version.ref = "logback" }
162162
kotlin-gradle-plugin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" }
163163
android-gradle-plugin = { module = "com.android.tools.build:gradle", version.ref = "agp" }
164+
asm = { module = "org.ow2.asm:asm", version = "9.9" }
164165
nopen-annotations = { module = "com.jakewharton.nopen:nopen-annotations", version.ref = "nopen" }
165166
nopen-checker = { module = "com.jakewharton.nopen:nopen-checker", version.ref = "nopen" }
166167
nullaway = { module = "com.uber.nullaway:nullaway", version = "0.14.2" }

0 commit comments

Comments
 (0)