Skip to content

Commit 206018b

Browse files
committed
Add ParameterAnnotationFixer
This fixes RuntimeVisibleParameterAnnotations and RuntimeInvisibleParameterAnnotations attributes to repair a case where ProGuard adds extra values for synthetic parameters (where the correct number of parameters is poorly defined). See MinecraftForge/ForgeGradle#472.
1 parent 5ab530d commit 206018b

File tree

2 files changed

+201
-0
lines changed

2 files changed

+201
-0
lines changed

src/main/java/de/oceanlabs/mcp/mcinjector/MCInjectorImpl.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import de.oceanlabs.mcp.mcinjector.adaptors.LVTFernflower;
4949
import de.oceanlabs.mcp.mcinjector.adaptors.LVTLvt;
5050
import de.oceanlabs.mcp.mcinjector.adaptors.LVTStrip;
51+
import de.oceanlabs.mcp.mcinjector.adaptors.ParameterAnnotationFixer;
5152
import de.oceanlabs.mcp.mcinjector.adaptors.ReadMarker;
5253

5354
public class MCInjectorImpl
@@ -638,6 +639,8 @@ public byte[] processClass(byte[] cls, boolean readOnly)
638639
}
639640

640641
ca = new AccessFixer(ca, this);
642+
643+
ca = new ParameterAnnotationFixer(ca, this);
641644
}
642645
ca = new AccessReader(ca, this);
643646

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
package de.oceanlabs.mcp.mcinjector.adaptors;
2+
3+
import static org.objectweb.asm.Opcodes.*;
4+
5+
import java.util.Arrays;
6+
import java.util.List;
7+
import java.util.logging.Logger;
8+
9+
import org.objectweb.asm.ClassVisitor;
10+
import org.objectweb.asm.Opcodes;
11+
import org.objectweb.asm.Type;
12+
import org.objectweb.asm.tree.AnnotationNode;
13+
import org.objectweb.asm.tree.ClassNode;
14+
import org.objectweb.asm.tree.InnerClassNode;
15+
import org.objectweb.asm.tree.MethodNode;
16+
17+
import de.oceanlabs.mcp.mcinjector.MCInjectorImpl;
18+
19+
public class ParameterAnnotationFixer extends ClassVisitor
20+
{
21+
private static final Logger LOGGER = Logger.getLogger("MCInjector");
22+
23+
public ParameterAnnotationFixer(ClassVisitor cn, MCInjectorImpl mci)
24+
{
25+
super(Opcodes.ASM5, cn);
26+
}
27+
28+
@Override
29+
public void visitEnd()
30+
{
31+
super.visitEnd();
32+
33+
ClassNode cls = MCInjectorImpl.getClassNode(cv);
34+
Type[] syntheticParams = getExpectedSyntheticParams(cls);
35+
if (syntheticParams != null)
36+
{
37+
for (MethodNode mn : cls.methods)
38+
{
39+
if (mn.name.equals("<init>"))
40+
{
41+
processConstructor(cls, mn, syntheticParams);
42+
}
43+
}
44+
}
45+
}
46+
47+
/**
48+
* Checks if the given class might have synthetic parameters in the
49+
* constructor. There are two cases where this might happen:
50+
* <ol>
51+
* <li>If the given class is an inner class, the first parameter is the
52+
* instance of the outer class.</li>
53+
* <li>If the given class is an enum, the first parameter is the enum
54+
* constant name and the second parameter is its ordinal.</li>
55+
* </ol>
56+
*
57+
* @return An array of types for synthetic parameters if the class can have
58+
* synthetic parameters, otherwise null.
59+
*/
60+
private Type[] getExpectedSyntheticParams(ClassNode cls)
61+
{
62+
// Check for enum
63+
// http://hg.openjdk.java.net/jdk8/jdk8/langtools/file/1ff9d5118aae/src/share/classes/com/sun/tools/javac/comp/Lower.java#l2866
64+
if ((cls.access & ACC_ENUM) != 0)
65+
{
66+
LOGGER.fine(" Considering " + cls.name
67+
+ " for extra parameter annotations as it is an enum");
68+
return new Type[] { Type.getObjectType("java/lang/String"), Type.INT_TYPE };
69+
}
70+
71+
// Check for inner class
72+
InnerClassNode info = null;
73+
for (InnerClassNode node : cls.innerClasses) // note: cls.innerClasses is never null
74+
{
75+
if (node.name.equals(cls.name))
76+
{
77+
info = node;
78+
break;
79+
}
80+
}
81+
// http://hg.openjdk.java.net/jdk8/jdk8/langtools/file/1ff9d5118aae/src/share/classes/com/sun/tools/javac/code/Symbol.java#l398
82+
if (info == null)
83+
{
84+
LOGGER.fine(" Not considering " + cls.name
85+
+ " for extra parameter annotations as it is not an inner class");
86+
return null; // It's not an inner class
87+
}
88+
if ((info.access & (ACC_STATIC | ACC_INTERFACE)) != 0)
89+
{
90+
LOGGER.fine(" Not considering " + cls.name
91+
+ " for extra parameter annotations as is an interface or static");
92+
return null; // It's static or can't have a constructor
93+
}
94+
95+
// http://hg.openjdk.java.net/jdk8/jdk8/langtools/file/1ff9d5118aae/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java#l2011
96+
if (info.innerName == null)
97+
{
98+
LOGGER.fine(" Not considering " + cls.name
99+
+ " for extra parameter annotations as it is annonymous");
100+
return null; // It's an anonymous class
101+
}
102+
103+
LOGGER.fine(" Considering " + cls.name
104+
+ " for extra parameter annotations as it is an inner class of "
105+
+ info.outerName);
106+
107+
return new Type[] { Type.getObjectType(info.outerName) };
108+
}
109+
110+
/**
111+
* Removes the parameter annotations for the given synthetic parameters,
112+
* if there are parameter annotations and the synthetic parameters exist.
113+
*/
114+
private void processConstructor(ClassNode cls, MethodNode mn, Type[] syntheticParams) {
115+
String methodInfo = mn.name + mn.desc + " in " + cls.name;
116+
Type[] params = Type.getArgumentTypes(mn.desc);
117+
118+
if (beginsWith(params, syntheticParams))
119+
{
120+
mn.visibleParameterAnnotations = process(methodInfo,
121+
"RuntimeVisibleParameterAnnotations", params.length,
122+
syntheticParams.length, mn.visibleParameterAnnotations);
123+
mn.invisibleParameterAnnotations = process(methodInfo,
124+
"RuntimeInvisibleParameterAnnotations", params.length,
125+
syntheticParams.length, mn.invisibleParameterAnnotations);
126+
}
127+
else
128+
{
129+
LOGGER.warning("Unexpected lack of synthetic args to the constructor: expected "
130+
+ Arrays.toString(syntheticParams) + " at the start of " + methodInfo);
131+
}
132+
}
133+
134+
private boolean beginsWith(Type[] values, Type[] prefix)
135+
{
136+
if (values.length < prefix.length)
137+
{
138+
return false;
139+
}
140+
for (int i = 0; i < prefix.length; i++)
141+
{
142+
if (!values[i].equals(prefix[i]))
143+
{
144+
return false;
145+
}
146+
}
147+
return true;
148+
}
149+
150+
/**
151+
* Removes annotation nodes corresponding to synthetic parameters, after
152+
* the existence of synthetic parameters has already been checked.
153+
*
154+
* @param methodInfo
155+
* A description of the method, for logging
156+
* @param attributeName
157+
* The name of the attribute, for logging
158+
* @param numParams
159+
* The number of parameters in the method
160+
* @param numSynthetic
161+
* The number of synthetic parameters (should not be 0)
162+
* @param annotations
163+
* The current array of annotation nodes, may be null
164+
* @return The new array of annotation nodes, may be null
165+
*/
166+
private List<AnnotationNode>[] process(String methodInfo,
167+
String attributeName, int numParams, int numSynthetic,
168+
List<AnnotationNode>[] annotations)
169+
{
170+
if (annotations == null)
171+
{
172+
LOGGER.finer(" " + methodInfo + " does not have a "
173+
+ attributeName + " attribute");
174+
return null;
175+
}
176+
177+
int numAnnotations = annotations.length;
178+
if (numParams == numAnnotations)
179+
{
180+
LOGGER.info("Found extra " + attributeName + " entries in "
181+
+ methodInfo + ": removing " + numSynthetic);
182+
return Arrays.copyOfRange(annotations, numSynthetic,
183+
numAnnotations);
184+
}
185+
else if (numParams == numAnnotations - numSynthetic)
186+
{
187+
LOGGER.info("Number of " + attributeName + " entries in "
188+
+ methodInfo + " is already as we want");
189+
return annotations;
190+
}
191+
else
192+
{
193+
LOGGER.warning("Unexpected number of " + attributeName
194+
+ " entries in " + methodInfo + ": " + numAnnotations);
195+
return annotations;
196+
}
197+
}
198+
}

0 commit comments

Comments
 (0)