Hi John,
Please review the amended patch, based on our discussions.
http://cr.openjdk.java.net/~ksrini/8151901/webrev.01/
Highlights:
* adjust the ordering of the InnerClass and BootStrapMethods computations
* enabled the test, and disabled memory leak check, which causes
intermittent
failures, because of GC's ergonomics etc.
* added the two test files to golden.jar, just in case.
Thanks
Kumar
Good.
Please change the comment:
s/null, no tuple change, force recomputation/null, drop ICs attribute, force CP
recomputation/
Reordering BSMs and ICs should work.
The BSMs may need extra ICs, but ICs cannot depend on BSMs.
I wonder why we did the other order first? I guess because that is what the
spec. says.
Oh, there's a problem with the attribute insertion order logic; it's buggy that
we unconditionally
insert a BSMs attr. and then delete it later. (That goes wrong when change<0
and we recompute
from scratch.)
Suggested amended patch enclosed.
--- John
On Mar 21, 2016, at 12:49 PM, Kumar Srinivasan <kumar.x.sriniva...@oracle.com>
wrote:
Hi John,
This patch contains fixes for two bugs:
8151901: test/tools/pack200/Pack200Test fails on verifying native unpacked JAR
8062335: Pack200 Java implementation differs from C version, outputs unused
UTF8 for BootstrapMethods Note: The test case exhibits both of the above.
With this the classes produced by java and the native implementation are
congruent,
though the JDK image's classes exhibit these issues, as a precaution I have
extracted the
minimal set of classes to reproduce the issues, into the golden jar.
The webrev is at:
http://cr.openjdk.java.net/~ksrini/8151901/webrev.00/
Thanks
Kumar
diff --git
a/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java
b/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java
--- a/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java
+++ b/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java
@@ -312,6 +312,12 @@
void setBootstrapMethods(Collection<BootstrapMethodEntry> bsms) {
assert(bootstrapMethods == null); // do not do this twice
bootstrapMethods = new ArrayList<>(bsms);
+ // Edit the attribute list, if necessary.
+ Attribute a = getAttribute(attrBootstrapMethodsEmpty);
+ if (bootstrapMethods != null && a == null)
+ addAttribute(attrBootstrapMethodsEmpty.canonicalInstance());
+ else if (bootstrapMethods == null && a != null)
+ removeAttribute(a);
}
boolean hasInnerClasses() {
diff --git
a/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java
b/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java
--- a/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java
+++ b/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java
@@ -1193,16 +1193,25 @@
cls.visitRefs(VRM_CLASSIC, cpRefs);
ArrayList<BootstrapMethodEntry> bsms = new ArrayList<>();
- /*
- * BootstrapMethod(BSMs) are added here before InnerClasses(ICs),
- * so as to ensure the order. Noting that the BSMs may be
- * removed if they are not found in the CP, after the ICs expansion.
- */
-
cls.addAttribute(Package.attrBootstrapMethodsEmpty.canonicalInstance());
// flesh out the local constant pool
ConstantPool.completeReferencesIn(cpRefs, true, bsms);
+ // Add the BSMs and references as required.
+ if (!bsms.isEmpty()) {
+ // Add the attirbute name to the CP (visitRefs would have wanted
this).
+ cpRefs.add(Package.getRefString("BootstrapMethods"));
+ // The pack-spec requires that BootstrapMethods precedes the
InnerClasses attribute.
+ // So add BSMs now before running expandLocalICs.
+ // But first delete any local InnerClasses attr. (This is rare.)
+ List<InnerClass> localICs = cls.getInnerClasses();
+ cls.setInnerClasses(null);
+ Collections.sort(bsms);
+ cls.setBootstrapMethods(bsms);
+ // Re-add the ICs, so the attribute lists are in the required
order.
+ cls.setInnerClasses(localICs);
+ }
+
// Now that we know all our local class references,
// compute the InnerClasses attribute.
int changed = cls.expandLocalICs();
@@ -1221,16 +1230,6 @@
ConstantPool.completeReferencesIn(cpRefs, true, bsms);
}
- // remove the attr previously set, otherwise add the bsm and
- // references as required
- if (bsms.isEmpty()) {
-
cls.attributes.remove(Package.attrBootstrapMethodsEmpty.canonicalInstance());
- } else {
- cpRefs.add(Package.getRefString("BootstrapMethods"));
- Collections.sort(bsms);
- cls.setBootstrapMethods(bsms);
- }
-
// construct a local constant pool
int numDoubles = 0;
for (Entry e : cpRefs) {
