OpenJDK / jdk / hs

changeset 43732:358d327a8220

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8174740: RuntimeException: Module m's descriptor returns inconsistent package set Reviewed-by: alanb
author mchung
date Sun, 12 Feb 2017 16:45:00 -0800
parents bc7110b230c1
children 25b25148d346
files jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java jdk/test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m1/module-info.java
diffstat 3 files changed, 78 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Sat Feb 11 21:31:43 2017 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Sun Feb 12 16:45:00 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -46,11 +46,16 @@
 import java.util.function.IntSupplier;
 
 import jdk.internal.module.Checks;
+import jdk.internal.module.ClassFileAttributes;
+import jdk.internal.module.ClassFileConstants;
 import jdk.internal.module.ModuleHashes;
 import jdk.internal.module.ModuleInfo.Attributes;
 import jdk.internal.module.ModuleInfoExtender;
 import jdk.internal.module.ModuleResolution;
 import jdk.internal.module.SystemModules;
+import jdk.internal.org.objectweb.asm.Attribute;
+import jdk.internal.org.objectweb.asm.ClassReader;
+import jdk.internal.org.objectweb.asm.ClassVisitor;
 import jdk.internal.org.objectweb.asm.ClassWriter;
 import jdk.internal.org.objectweb.asm.MethodVisitor;
 import jdk.internal.org.objectweb.asm.Opcodes;
@@ -109,6 +114,11 @@
     }
 
     @Override
+    public String getArgumentsDescription() {
+        return PluginsResourceBundle.getArgument(NAME);
+    }
+
+    @Override
     public void configure(Map<String, String> config) {
         String arg = config.get(NAME);
         if (arg != null) {
@@ -171,10 +181,11 @@
     }
 
     static class ModuleInfo {
+        private final ByteArrayInputStream bain;
         private final Attributes attrs;
         private final Set<String> packages;
-        private final ByteArrayInputStream bain;
         private final boolean dropModuleTarget;
+        private final boolean addModulePackages;
         private ModuleDescriptor descriptor;  // may be different that the original one
 
         ModuleInfo(byte[] bytes, Set<String> packages, boolean dropModuleTarget)
@@ -182,15 +193,21 @@
         {
             this.bain = new ByteArrayInputStream(bytes);
             this.packages = packages;
-
             this.attrs = jdk.internal.module.ModuleInfo.read(bain, null);
+            // If ModulePackages attribute is present, the packages from this
+            // module descriptor returns the packages in that attribute.
+            // If it's not present, ModuleDescriptor::packages only contains
+            // the exported and open packages from module-info.class
             this.descriptor = attrs.descriptor();
             if (descriptor.isAutomatic()) {
                 throw new InternalError("linking automatic module is not supported");
             }
 
+            // add ModulePackages attribute if this module contains some packages
+            // and ModulePackages is not present
+            this.addModulePackages = packages.size() > 0 && !hasModulePackages();
+            // drop target attribute only if any OS property is present
             if (dropModuleTarget) {
-                // drop target attribute only if any OS property is present
                 this.dropModuleTarget =
                     descriptor.osName().isPresent() ||
                     descriptor.osArch().isPresent() ||
@@ -276,51 +293,69 @@
             }
         }
 
+        boolean hasModulePackages() throws IOException {
+            Set<String> attrTypes = new HashSet<>();
+            ClassVisitor cv = new ClassVisitor(Opcodes.ASM5) {
+                @Override
+                public void visitAttribute(Attribute attr) {
+                    attrTypes.add(attr.type);
+                }
+            };
+
+            // prototype of attributes that should be parsed
+            Attribute[] attrs = new Attribute[] {
+                new ClassFileAttributes.ModulePackagesAttribute()
+            };
+
+            try (InputStream in = getInputStream()) {
+                // parse module-info.class
+                ClassReader cr = new ClassReader(in);
+                cr.accept(cv, attrs, 0);
+                return attrTypes.contains(ClassFileConstants.MODULE_PACKAGES);
+            }
+        }
+
         /**
          * Returns true if module-info.class should be written
          * 1. add ModulePackages attribute if not present; or
          * 2. drop ModuleTarget attribute except java.base
          */
         boolean shouldRewrite() {
-            return shouldAddModulePackages() || shouldDropModuleTarget();
-        }
-
-        boolean shouldAddModulePackages() {
-            return (descriptor.packages().isEmpty() && packages.size() > 0);
-        }
-
-        boolean shouldDropModuleTarget() {
-            return dropModuleTarget &&
-                        (descriptor.osName().isPresent() ||
-                         descriptor.osArch().isPresent() ||
-                         descriptor.osVersion().isPresent());
+            return addModulePackages || dropModuleTarget;
         }
 
         /**
          * Returns the bytes for the module-info.class with ModulePackages
-         * if it contains at least one package
+         * attribute added and/or with ModuleTarget attribute dropped.
          */
         byte[] getBytes() throws IOException {
+            try (InputStream in = getInputStream()) {
+                if (shouldRewrite()) {
+                    ModuleInfoRewriter rewriter = new ModuleInfoRewriter(in);
+                    if (addModulePackages) {
+                        rewriter.addModulePackages(packages);
+                    }
+                    if (dropModuleTarget) {
+                        rewriter.dropModuleTarget();
+                    }
+                    // rewritten module descriptor
+                    byte[] bytes = rewriter.getBytes();
+                    try (ByteArrayInputStream bain = new ByteArrayInputStream(bytes)) {
+                        this.descriptor = ModuleDescriptor.read(bain);
+                    }
+                    return bytes;
+                } else {
+                    return in.readAllBytes();
+                }
+            }
+        }
+
+        /*
+         * Returns the input stream of the module-info.class
+         */
+        InputStream getInputStream() {
             bain.reset();
-
-            // add ModulePackages attribute if not exist
-            if (shouldRewrite()) {
-                ModuleInfoRewriter rewriter = new ModuleInfoRewriter(bain);
-                if (shouldAddModulePackages()) {
-                    rewriter.addModulePackages(packages);
-                }
-                if (shouldDropModuleTarget()) {
-                    rewriter.dropModuleTarget();
-                }
-                // rewritten module descriptor
-                byte[] bytes = rewriter.getBytes();
-                try (ByteArrayInputStream bain = new ByteArrayInputStream(bytes)) {
-                     this.descriptor = ModuleDescriptor.read(bain);
-                }
-                return bytes;
-            } else {
-                return bain.readAllBytes();
-            }
+            return bain;
         }
 
         class ModuleInfoRewriter extends ByteArrayOutputStream {
--- a/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java	Sat Feb 11 21:31:43 2017 -0800
+++ b/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java	Sun Feb 12 16:45:00 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -45,7 +45,7 @@
 
 /**
  * @test
- * @bug 8142968 8173381
+ * @bug 8142968 8173381 8174740
  * @library /lib/testlibrary
  * @modules jdk.compiler jdk.jlink
  * @build UserModuleTest CompilerUtils jdk.testlibrary.FileUtils jdk.testlibrary.ProcessTools
--- a/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m1/module-info.java	Sat Feb 11 21:31:43 2017 -0800
+++ b/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m1/module-info.java	Sun Feb 12 16:45:00 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -21,5 +21,9 @@
  * questions.
  */
 
+/*
+ * m1 has an exported package and also internal package
+ */
 module m1 {
+    exports p1;
 }
© 2007, 2022 Oracle and/or its affiliates
Terms of Use · Privacy · Trademarks