Skip to content

Commit f82eacf

Browse files
shai-almogshai-almog
committed
Fix BytecodeMethod equality null safety
1 parent 455d610 commit f82eacf

File tree

3 files changed

+205
-36
lines changed

3 files changed

+205
-36
lines changed

‎vm/ByteCodeTranslator/src/com/codename1/tools/translator/BytecodeMethod.java‎

Lines changed: 71 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@
6161
*
6262
* @author Shai Almog
6363
*/
64-
public class BytecodeMethod implements SignatureSet {
64+
public class BytecodeMethod implements SignatureSet {
65+
private static MethodDependencyGraph dependencyGraph;
6566

6667
/**
6768
* @return the acceptStaticOnEquals
@@ -76,6 +77,10 @@ public static boolean isAcceptStaticOnEquals() {
7677
public static void setAcceptStaticOnEquals(boolean aAcceptStaticOnEquals) {
7778
acceptStaticOnEquals = aAcceptStaticOnEquals;
7879
}
80+
81+
public static void setDependencyGraph(MethodDependencyGraph dependencyGraph) {
82+
BytecodeMethod.dependencyGraph = dependencyGraph;
83+
}
7984
private List<ByteCodeMethodArg> arguments = new ArrayList<ByteCodeMethodArg>();
8085
private Set<LocalVariable> localVariables = new HashSet<LocalVariable>();
8186
private ByteCodeMethodArg returnType;
@@ -238,6 +243,10 @@ public BytecodeMethod(String clsName, int access, String name, String desc, Stri
238243
}
239244
currentArrayDim = 0;
240245
}
246+
247+
if (dependencyGraph != null) {
248+
dependencyGraph.registerMethod(this);
249+
}
241250
}
242251

243252
// use this instead of isMethodUsed to compare traditional with new results
@@ -350,20 +359,40 @@ public boolean isMethodUsedByNative(String[] nativeSources, ByteCodeClass cls) {
350359

351360
private Set<String> usedMethods;
352361
public boolean isMethodUsedOldWay(BytecodeMethod bm) {
353-
if(usedMethods == null) {
354-
usedMethods = new TreeSet<String>();
355-
for(Instruction ins : instructions) {
356-
String s = ins.getMethodUsed();
357-
if(s != null && !usedMethods.contains(s)) {
358-
usedMethods.add(s);
359-
}
360-
}
361-
}
362+
ensureUsedMethodsInitialized();
362363
if(bm.methodName.equals("__INIT__")) {
363364
return usedMethods.contains(bm.desc + ".<init>");
364365
}
365366
return usedMethods.contains(bm.desc + "." + bm.methodName);
366367
}
368+
369+
public Set<String> getCalledMethodSignatures() {
370+
ensureUsedMethodsInitialized();
371+
return usedMethods;
372+
}
373+
374+
public String getLookupSignature() {
375+
if(methodName.equals("__INIT__")) {
376+
return desc + ".<init>";
377+
}
378+
if(methodName.equals("__CLINIT__")) {
379+
return desc + ".<clinit>";
380+
}
381+
return desc + "." + methodName;
382+
}
383+
384+
private void ensureUsedMethodsInitialized() {
385+
if(usedMethods != null) {
386+
return;
387+
}
388+
usedMethods = new TreeSet<String>();
389+
for(Instruction ins : instructions) {
390+
String s = ins.getMethodUsed();
391+
if(s != null && !usedMethods.contains(s)) {
392+
usedMethods.add(s);
393+
}
394+
}
395+
}
367396

368397
public void findWritableFields(Set<String> outSet) {
369398
int len = instructions.size();
@@ -950,32 +979,48 @@ public String getMethodName() {
950979
}
951980

952981
public boolean equals(Object o) {
982+
if (this == o) {
983+
return true;
984+
}
985+
if (!(o instanceof BytecodeMethod)) {
986+
return false;
987+
}
988+
953989
BytecodeMethod bm = (BytecodeMethod)o;
954-
int val = bm.methodName.compareTo(methodName);
955-
if(val != 0) {
990+
991+
if (!methodName.equals(bm.methodName)) {
956992
return false;
957993
}
958-
if(acceptStaticOnEquals) {
959-
if(bm.arguments.size() != arguments.size()) {
994+
if (acceptStaticOnEquals) {
995+
if (bm.arguments.size() != arguments.size()) {
960996
return false;
961-
}
997+
}
962998
} else {
963-
if(staticMethod || bm.staticMethod || bm.arguments.size() != arguments.size()) {
999+
if (staticMethod || bm.staticMethod || bm.arguments.size() != arguments.size()) {
9641000
return false;
9651001
}
9661002
}
967-
for(int iter = 0 ; iter < arguments.size() ; iter++) {
1003+
1004+
for (int iter = 0; iter < arguments.size(); iter++) {
9681005
ByteCodeMethodArg arg1 = arguments.get(iter);
9691006
ByteCodeMethodArg arg2 = bm.arguments.get(iter);
970-
if(!arg1.equals(arg2)) {
1007+
if (!arg1.equals(arg2)) {
9711008
return false;
9721009
}
9731010
}
1011+
1012+
if (returnType == null) {
1013+
return bm.returnType == null;
1014+
}
9741015
return returnType.equals(bm.returnType);
9751016
}
976-
1017+
9771018
public int hashCode() {
978-
return methodName.hashCode();
1019+
int result = methodName == null ? 0 : methodName.hashCode();
1020+
result = 31 * result + arguments.size();
1021+
result = 31 * result + (acceptStaticOnEquals || !staticMethod ? 0 : 1);
1022+
result = 31 * result + (returnType == null ? 0 : returnType.hashCode());
1023+
return result;
9791024
}
9801025

9811026
public boolean isStatic() {
@@ -1049,6 +1094,12 @@ public void setMaxes(int maxStack, int maxLocals) {
10491094
private void addInstruction(Instruction i) {
10501095
instructions.add(i);
10511096
i.addDependencies(dependentClasses);
1097+
if (dependencyGraph != null) {
1098+
String methodUsed = i.getMethodUsed();
1099+
if (methodUsed != null) {
1100+
dependencyGraph.recordMethodCall(this, methodUsed);
1101+
}
1102+
}
10521103
}
10531104

10541105
public void addVariableOperation(int opcode, int var) {

‎vm/ByteCodeTranslator/src/com/codename1/tools/translator/MethodDependencyGraph.java‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/*
2+
* Copyright (c) 2012, Codename One and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
* This code is free software; you can redistribute it and/or modify it
5+
* under the terms of the GNU General Public License version 2 only, as
6+
* published by the Free Software Foundation. Codename One designates this
7+
* particular file as subject to the "Classpath" exception as provided
8+
* by Oracle in the LICENSE file that accompanied this code.
9+
*
10+
* This code is distributed in the hope that it will be useful, but WITHOUT
11+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
12+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
13+
* version 2 for more details (a copy is included in the LICENSE file that
14+
* accompanied this code).
15+
*
16+
* You should have received a copy of the GNU General Public License version
17+
* 2 along with this work; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
19+
*
20+
* Please contact Codename One through http://www.codenameone.com/ if you
21+
* need additional information or have any questions.
22+
*/
23+
24+
package com.codename1.tools.translator;
25+
26+
import java.util.ArrayList;
27+
import java.util.HashMap;
28+
import java.util.HashSet;
29+
import java.util.List;
30+
import java.util.Map;
31+
import java.util.Set;
32+
33+
/**
34+
* Maintains a hierarchical dependency index between classes and methods so we
35+
* can cheaply look up callers and prune eliminated elements without rescanning
36+
* bytecode.
37+
*/
38+
public class MethodDependencyGraph {
39+
private final Map<String, Set<BytecodeMethod>> callersByLookupSignature = new HashMap<String, Set<BytecodeMethod>>();
40+
private final Map<BytecodeMethod, Set<String>> methodToCalls = new HashMap<BytecodeMethod, Set<String>>();
41+
private final Map<String, Set<BytecodeMethod>> methodsByClass = new HashMap<String, Set<BytecodeMethod>>();
42+
43+
public void clear() {
44+
callersByLookupSignature.clear();
45+
methodToCalls.clear();
46+
methodsByClass.clear();
47+
}
48+
49+
public void registerMethod(BytecodeMethod method) {
50+
Set<BytecodeMethod> byClass = methodsByClass.get(method.getClsName());
51+
if (byClass == null) {
52+
byClass = new HashSet<BytecodeMethod>();
53+
methodsByClass.put(method.getClsName(), byClass);
54+
}
55+
byClass.add(method);
56+
}
57+
58+
public void recordMethodCall(BytecodeMethod caller, String calleeSignature) {
59+
Set<BytecodeMethod> callers = callersByLookupSignature.get(calleeSignature);
60+
if (callers == null) {
61+
callers = new HashSet<BytecodeMethod>();
62+
callersByLookupSignature.put(calleeSignature, callers);
63+
}
64+
callers.add(caller);
65+
66+
Set<String> calls = methodToCalls.get(caller);
67+
if (calls == null) {
68+
calls = new HashSet<String>();
69+
methodToCalls.put(caller, calls);
70+
}
71+
calls.add(calleeSignature);
72+
}
73+
74+
public List<BytecodeMethod> getCallers(String calleeSignature) {
75+
Set<BytecodeMethod> callers = callersByLookupSignature.get(calleeSignature);
76+
if (callers == null) {
77+
return new ArrayList<BytecodeMethod>();
78+
}
79+
return new ArrayList<BytecodeMethod>(callers);
80+
}
81+
82+
public void removeMethod(BytecodeMethod method) {
83+
Set<String> calls = methodToCalls.remove(method);
84+
if (calls != null) {
85+
for (String call : calls) {
86+
Set<BytecodeMethod> callers = callersByLookupSignature.get(call);
87+
if (callers != null) {
88+
callers.remove(method);
89+
if (callers.isEmpty()) {
90+
callersByLookupSignature.remove(call);
91+
}
92+
}
93+
}
94+
}
95+
96+
Set<BytecodeMethod> byClass = methodsByClass.get(method.getClsName());
97+
if (byClass != null) {
98+
byClass.remove(method);
99+
if (byClass.isEmpty()) {
100+
methodsByClass.remove(method.getClsName());
101+
}
102+
}
103+
}
104+
105+
public void removeClass(String clsName) {
106+
Set<BytecodeMethod> methods = methodsByClass.remove(clsName);
107+
if (methods != null) {
108+
for (BytecodeMethod method : new ArrayList<BytecodeMethod>(methods)) {
109+
removeMethod(method);
110+
}
111+
}
112+
}
113+
}

‎vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,20 @@ public class Parser extends ClassVisitor {
5050
private String clsName;
5151
private static String[] nativeSources;
5252
private static List<ByteCodeClass> classes = new ArrayList<ByteCodeClass>();
53+
private static MethodDependencyGraph dependencyGraph = new MethodDependencyGraph();
5354
private int lambdaCounter;
5455
public static void cleanup() {
55-
nativeSources = null;
56-
classes.clear();
57-
LabelInstruction.cleanup();
56+
nativeSources = null;
57+
classes.clear();
58+
dependencyGraph.clear();
59+
BytecodeMethod.setDependencyGraph(null);
60+
LabelInstruction.cleanup();
5861
}
5962
public static void parse(File sourceFile) throws Exception {
6063
if(ByteCodeTranslator.verbose) {
6164
System.out.println("Parsing: " + sourceFile.getAbsolutePath());
6265
}
66+
BytecodeMethod.setDependencyGraph(dependencyGraph);
6367
ClassReader r = new ClassReader(new FileInputStream(sourceFile));
6468
/*if(ByteCodeTranslator.verbose) {
6569
System.out.println("Class: " + r.getClassName() + " derives from: " + r.getSuperName() + " interfaces: " + Arrays.asList(r.getInterfaces()));
@@ -409,8 +413,8 @@ public static void writeOutput(File outputDirectory) throws Exception {
409413
}
410414
}
411415
}
412-
413-
// load the native sources (including user native code)
416+
417+
// load the native sources (including user native code)
414418
// We need to load native sources before we clear any unmarked classes
415419
// because native source may be the only thing referencing a class,
416420
// and the class may be purged before it even has a shot.
@@ -499,8 +503,8 @@ private static int eliminateUnusedMethods(boolean forceFound, int depth) {
499503
}
500504

501505
private static int cullMethods() {
502-
int nfound = 0;
503-
for(ByteCodeClass bc : classes) {
506+
int nfound = 0;
507+
for(ByteCodeClass bc : classes) {
504508
bc.unmark();
505509
if(bc.isIsInterface() || bc.getBaseClass() == null) {
506510
continue;
@@ -519,14 +523,15 @@ private static int cullMethods() {
519523
continue;
520524
}
521525
mtd.setEliminated(true);
526+
dependencyGraph.removeMethod(mtd);
522527
nfound++;
523528
}
524529
}
525530

526531
}
527532
return nfound;
528533
}
529-
534+
530535
private static boolean isMethodUsedByBaseClassOrInterface(BytecodeMethod mtd, ByteCodeClass cls) {
531536
boolean b = checkMethodUsedByBaseClassOrInterface(mtd, cls.getBaseClassObject());
532537
if(b) {
@@ -596,6 +601,7 @@ private static int cullClasses(boolean found, int depth) {
596601
int nfound = 0;
597602
for (ByteCodeClass cls : removedClasses) {
598603
nfound += cls.setEliminated(true);
604+
dependencyGraph.removeClass(cls.getClsName());
599605
}
600606
classes = tmp;
601607
return nfound + eliminateUnusedMethods(nfound > 0, depth + 1);
@@ -613,14 +619,13 @@ private static boolean isMethodUsed(BytecodeMethod m, ByteCodeClass cls) {
613619
if (!m.isEliminated() && m.isMethodUsedByNative(nativeSources, cls)) {
614620
return true;
615621
}
616-
for(ByteCodeClass bc : classes) {
617-
for(BytecodeMethod mtd : bc.getMethods()) {
618-
if(mtd.isEliminated() || mtd == m) {
619-
continue;
620-
}
621-
if(mtd.isMethodUsed(m)) {
622-
return true;
623-
}
622+
List<BytecodeMethod> callers = dependencyGraph.getCallers(m.getLookupSignature());
623+
for (BytecodeMethod caller : callers) {
624+
if(caller.isEliminated() || caller == m) {
625+
continue;
626+
}
627+
if(caller.isMethodUsed(m)) {
628+
return true;
624629
}
625630
}
626631
return false;

0 commit comments

Comments
 (0)