From 8e5557635cbe270fa3ba75a0e3bda641c3f78eac Mon Sep 17 00:00:00 2001
From: Sophia <git@sophia.gg>
Date: Fri, 16 Jul 2021 01:33:39 +0100
Subject: [PATCH] Use protected as the minimum visibilty in open classes

The current implementation of VisibiltyTransformer determines visibility
using the owning classes at the access sites. This is insufficient in
the unusual case where a parent class accesses one of its own members
via one of its child classes.

An example of this is in client build 550,
client!mo.a(Ljava/awt/Component;BILsignlink!pm;I)Lclient!tj;

To correct this, the minimum visibility for members in an open class is
now protected, not private. In many cases this results in members having
a wider access than necessary. A more sophisticated approach would be to
determine the type (or possible types) used at the access site, so only
members that are accessed in this unusual manner need to have their
visibility weakened. This requires type analysis of everything on the
stack.

This change also introduces an implicit dependency from
VisibilityTransformer to FinalTransformer, as running the latter first
will minimise the set of non-final classes.

Signed-off-by: Sophia <git@sophia.gg>
---
 .../bytecode/BytecodeDeobfuscatorModule.kt     |  2 +-
 .../transform/VisibilityTransformer.kt         | 18 ++++++++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/BytecodeDeobfuscatorModule.kt b/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/BytecodeDeobfuscatorModule.kt
index 701a8d64..d74455cd 100644
--- a/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/BytecodeDeobfuscatorModule.kt
+++ b/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/BytecodeDeobfuscatorModule.kt
@@ -70,8 +70,8 @@ public object BytecodeDeobfuscatorModule : AbstractModule() {
         binder.addBinding().to(ResetTransformer::class.java)
         binder.addBinding().to(EmptyClassTransformer::class.java)
         binder.addBinding().to(MethodOrderTransformer::class.java)
-        binder.addBinding().to(VisibilityTransformer::class.java)
         binder.addBinding().to(FinalTransformer::class.java)
+        binder.addBinding().to(VisibilityTransformer::class.java)
         binder.addBinding().to(FinalFieldTransformer::class.java)
         binder.addBinding().to(OverrideTransformer::class.java)
         binder.addBinding().to(RedundantGotoTransformer::class.java)
diff --git a/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/transform/VisibilityTransformer.kt b/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/transform/VisibilityTransformer.kt
index 7132c2b3..57f50349 100644
--- a/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/transform/VisibilityTransformer.kt
+++ b/deob-bytecode/src/main/kotlin/org/openrs2/deob/bytecode/transform/VisibilityTransformer.kt
@@ -54,7 +54,8 @@ public class VisibilityTransformer @Inject constructor(private val profile: Prof
         references: Multimap<DisjointSet.Partition<MemberRef>, String>,
         disjointSet: DisjointSet<MemberRef>,
         member: MemberRef,
-        access: Int
+        access: Int,
+        classAccess: Int
     ): Int {
         val method = Type.getType(member.desc).sort == Type.METHOD
         if (method) {
@@ -72,6 +73,7 @@ public class VisibilityTransformer @Inject constructor(private val profile: Prof
         val overridable = method && member.name != "<init>"
         val hasOverride = overridable && partition.count { classPath[it.owner]!!.methods.contains(MemberDesc(it)) } > 1
         val abstract = method && access and Opcodes.ACC_ABSTRACT != 0
+        val finalClass = classAccess and Opcodes.ACC_FINAL != 0
         val partitionReferences = references[partition]
         val partitionOwners = partition.mapTo(mutableSetOf(), MemberRef::owner)
 
@@ -82,7 +84,13 @@ public class VisibilityTransformer @Inject constructor(private val profile: Prof
 
         // pick the weakest access level based on references in our own code
         val visibility = when {
-            partitionReferences.all { it == member.owner } && !hasOverride && !abstract -> Opcodes.ACC_PRIVATE
+            !hasOverride && !abstract && partitionReferences.all { it == member.owner } -> {
+                if (finalClass) {
+                    Opcodes.ACC_PRIVATE
+                } else {
+                    Opcodes.ACC_PROTECTED
+                }
+            }
             partitionReferences.all { partitionOwners.contains(it) } -> Opcodes.ACC_PROTECTED
             else -> Opcodes.ACC_PUBLIC
         }
@@ -118,7 +126,8 @@ public class VisibilityTransformer @Inject constructor(private val profile: Prof
                         fieldReferences,
                         inheritedFieldSets,
                         MemberRef(clazz, field),
-                        access
+                        access,
+                        clazz.access
                     )
                     field.access = (access and VISIBILITY_FLAGS.inv()) or visibility
 
@@ -135,7 +144,8 @@ public class VisibilityTransformer @Inject constructor(private val profile: Prof
                         methodReferences,
                         inheritedMethodSets,
                         MemberRef(clazz, method),
-                        access
+                        access,
+                        clazz.access
                     )
                     method.access = (access and VISIBILITY_FLAGS.inv()) or visibility
 
-- 
2.30.2