Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insufficient Proguard configuration rules #1564

Closed
christofferqa opened this issue Sep 23, 2019 · 4 comments
Closed

Insufficient Proguard configuration rules #1564

christofferqa opened this issue Sep 23, 2019 · 4 comments

Comments

@christofferqa
Copy link

If a field is never written, R8 will replace all reads of that field by the default value of the field. Therefore it is important that the Proguard configuration for kotlinx.coroutines correctly specifies which fields are written using reflection.

In https://issuetracker.google.com/issues/141416122, a small project is shared that breaks due to missing Proguard configuration rules in kotlinx.coroutines. I found that adding the following two keep rules to the project solves the issue.

-keep,allowobfuscation class kotlinx.coroutines.io.ByteBufferChannel { kotlin.coroutines.Continuation writeOp; }
-keep,allowobfuscation class kotlinx.io.pool.DefaultPool { long top; }

I then found that the Proguard configuration rules for kotlinx.coroutines contain the following rule (https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/jvm/resources/META-INF/proguard/coroutines.pro#L8):

-keepclassmembernames class kotlinx.** {
    volatile <fields>;
}

This should be updated to:

-keepclassmembers class kotlinx.** {
    volatile <fields>;
}

In addition to specifying that the names of these fields cannot be changed, this also specifies that these fields are assigned using reflection (in this case, AtomicReferenceFieldUpdater).

Originally filed at https://issuetracker.google.com/issues/141416122 and ktorio/ktor#1354.

@cy6erGn0m
Copy link
Contributor

cy6erGn0m commented Sep 23, 2019

According to the documentation

-keepclassmembernames class_specification
Short for-keepclassmembers,allowshrinkingclass_specification Specifies class members whose names are to be preserved, if they aren't removed in the shrinking phase.

-keepclassmembers [,modifier,...] class_specification
Specifies class members to be preserved, if their classes are preserved as well.

Not sure we can replace keepclassmembernames with keepclassmembers because it's not enough to keep field, we need to keep names as well.

@christofferqa
Copy link
Author

The keepclassmembers directive also preserves the name of the field.

@cy6erGn0m
Copy link
Contributor

@christofferqa I don't see where it is documented

@christofferqa
Copy link
Author

According to the documentation, -keepclassmembernames is an alias for -keepclassmembers,allowshrinking. Thus, the only difference between -keepclassmembernames and -keepclassmembers is that the latter rule does not allow matched fields to be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants