8173381: osName/osArch/osVersion is missing in ModuleDescriptor created by SystemModules
authormchung
Mon, 30 Jan 2017 11:00:18 -0800
changeset 43336 be9fca030f91
parent 43335 911fcebb04d2
child 43337 e107e6921357
child 43518 362db50cabe6
8173381: osName/osArch/osVersion is missing in ModuleDescriptor created by SystemModules Reviewed-by: alanb
jdk/src/java.base/share/classes/jdk/internal/module/ModuleInfoExtender.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ModuleSorter.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
jdk/test/tools/jlink/plugins/SystemModuleDescriptors/SystemModulesTest.java
jdk/test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java
jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m1/p1/Main.java
jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m4/p4/Main.java
jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m4/p4/dummy/dummy.properties
--- a/jdk/src/java.base/share/classes/jdk/internal/module/ModuleInfoExtender.java	Mon Jan 30 13:11:56 2017 +0300
+++ b/jdk/src/java.base/share/classes/jdk/internal/module/ModuleInfoExtender.java	Mon Jan 30 11:00:18 2017 -0800
@@ -53,21 +53,21 @@
     // the input stream to read the original module-info.class
     private final InputStream in;
 
-    // the packages in the Packages attribute
+    // the packages in the ModulePackages attribute
     private Set<String> packages;
 
-    // the value of the Version attribute
+    // the value of the module_version in Module attribute
     private Version version;
 
-    // the value of the MainClass attribute
+    // the value of the ModuleMainClass attribute
     private String mainClass;
 
-    // the values for the TargetPlatform attribute
+    // the values for the ModuleTarget attribute
     private String osName;
     private String osArch;
     private String osVersion;
 
-    // the hashes for the Hashes attribute
+    // the hashes for the ModuleHashes attribute
     private ModuleHashes hashes;
 
     // the value of the ModuleResolution attribute
@@ -78,7 +78,7 @@
     }
 
     /**
-     * Sets the set of packages for the Packages attribute
+     * Sets the set of packages for the ModulePackages attribute
      */
     public ModuleInfoExtender packages(Set<String> packages) {
         this.packages = Collections.unmodifiableSet(packages);
@@ -86,7 +86,7 @@
     }
 
     /**
-     * Sets the value of the Version attribute.
+     * Sets the value of the module_version in Module attribute.
      */
     public ModuleInfoExtender version(Version version) {
         this.version = version;
@@ -94,7 +94,7 @@
     }
 
     /**
-     * Sets the value of the MainClass attribute.
+     * Sets the value of the ModuleMainClass attribute.
      */
     public ModuleInfoExtender mainClass(String mainClass) {
         this.mainClass = mainClass;
@@ -102,7 +102,7 @@
     }
 
     /**
-     * Sets the values for the TargetPlatform attribute.
+     * Sets the values for the ModuleTarget attribute.
      */
     public ModuleInfoExtender targetPlatform(String osName,
                                              String osArch,
@@ -114,7 +114,7 @@
     }
 
     /**
-     * The Hashes attribute will be emitted to the module-info with
+     * The ModuleHashes attribute will be emitted to the module-info with
      * the hashes encapsulated in the given {@code ModuleHashes}
      * object.
      */
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ModuleSorter.java	Mon Jan 30 13:11:56 2017 +0300
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ModuleSorter.java	Mon Jan 30 11:00:18 2017 -0800
@@ -25,10 +25,12 @@
 package jdk.tools.jlink.internal;
 
 import jdk.tools.jlink.plugin.PluginException;
+import jdk.tools.jlink.plugin.ResourcePoolEntry;
 import jdk.tools.jlink.plugin.ResourcePoolModule;
 import jdk.tools.jlink.plugin.ResourcePoolModuleView;
 
 import java.lang.module.ModuleDescriptor;
+import java.nio.ByteBuffer;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -53,10 +55,19 @@
         moduleView.modules().forEach(this::addModule);
     }
 
+    private ModuleDescriptor readModuleDescriptor(ResourcePoolModule module) {
+        String p = "/" + module.name() + "/module-info.class";
+        ResourcePoolEntry content = module.findEntry(p).orElseThrow(() ->
+            new PluginException("module-info.class not found for " +
+                module.name() + " module")
+        );
+        ByteBuffer bb = ByteBuffer.wrap(content.contentBytes());
+        return ModuleDescriptor.read(bb);
+    }
+
     private ModuleSorter addModule(ResourcePoolModule module) {
-        ModuleDescriptor descriptor = module.descriptor();
         addNode(module);
-        descriptor.requires().stream()
+        readModuleDescriptor(module).requires().stream()
             .forEach(req -> {
                 String dm = req.name();
                 ResourcePoolModule dep = moduleView.findModule(dm)
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Mon Jan 30 13:11:56 2017 +0300
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Mon Jan 30 11:00:18 2017 -0800
@@ -29,7 +29,11 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.module.ModuleDescriptor;
-import java.lang.module.ModuleDescriptor.*;
+import java.lang.module.ModuleDescriptor.Exports;
+import java.lang.module.ModuleDescriptor.Opens;
+import java.lang.module.ModuleDescriptor.Provides;
+import java.lang.module.ModuleDescriptor.Requires;
+import java.lang.module.ModuleDescriptor.Version;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.EnumSet;
@@ -41,8 +45,6 @@
 import java.util.TreeSet;
 import java.util.function.IntSupplier;
 
-import jdk.internal.misc.JavaLangModuleAccess;
-import jdk.internal.misc.SharedSecrets;
 import jdk.internal.module.Checks;
 import jdk.internal.module.ModuleHashes;
 import jdk.internal.module.ModuleInfo.Attributes;
@@ -55,6 +57,7 @@
 
 import static jdk.internal.org.objectweb.asm.Opcodes.*;
 
+import jdk.tools.jlink.internal.ModuleSorter;
 import jdk.tools.jlink.plugin.PluginException;
 import jdk.tools.jlink.plugin.ResourcePool;
 import jdk.tools.jlink.plugin.Plugin;
@@ -63,7 +66,7 @@
 
 /**
  * Jlink plugin to reconstitute module descriptors for system modules.
- * It will extend module-info.class with Packages attribute,
+ * It will extend module-info.class with ModulePackages attribute,
  * if not present. It also determines the number of packages of
  * the boot layer at link time.
  *
@@ -73,16 +76,15 @@
  * @see SystemModules
  */
 public final class SystemModulesPlugin implements Plugin {
-    private static final JavaLangModuleAccess JLMA =
-        SharedSecrets.getJavaLangModuleAccess();
-
     private static final String NAME = "system-modules";
     private static final String DESCRIPTION =
         PluginsResourceBundle.getDescription(NAME);
 
     private boolean enabled;
+    private boolean retainModuleTarget;
     public SystemModulesPlugin() {
         this.enabled = true;
+        this.retainModuleTarget = false;
     }
 
     @Override
@@ -102,9 +104,19 @@
     }
 
     @Override
+    public boolean hasArguments() {
+        return true;
+    }
+
+    @Override
     public void configure(Map<String, String> config) {
-        if (config.containsKey(NAME)) {
-            enabled = false;
+        String arg = config.get(NAME);
+        if (arg != null) {
+            if (arg.equals("retainModuleTarget")) {
+                retainModuleTarget = true;
+            } else {
+                throw new IllegalArgumentException(NAME + ": " + arg);
+            }
         }
     }
 
@@ -114,12 +126,15 @@
             throw new PluginException(NAME + " was set");
         }
 
-        SystemModulesClassGenerator generator = new SystemModulesClassGenerator();
+        SystemModulesClassGenerator generator =
+            new SystemModulesClassGenerator(retainModuleTarget);
 
         // generate the byte code to create ModuleDescriptors
-        // skip parsing module-info.class and skip name check
-        in.moduleView().modules().forEach(module -> {
+        // such that the modules linked in the image would skip parsing
+        // of module-info.class and also skip name check
 
+        // Sort modules in the topological order so that java.base is always first.
+        new ModuleSorter(in.moduleView()).sorted().forEach(module -> {
             ResourcePoolEntry data = module.findEntry("module-info.class").orElseThrow(
                 // automatic module not supported yet
                 () ->  new PluginException("module-info.class not found for " +
@@ -128,19 +143,10 @@
 
             assert module.name().equals(data.moduleName());
             try {
-                ModuleInfo moduleInfo = new ModuleInfo(data.contentBytes(), module.packages());
-                generator.addModule(moduleInfo);
+                // validate the module and add to system modules
+                data = generator.buildModuleInfo(data, module.packages());
 
-                // link-time validation
-                moduleInfo.validateNames();
-                // check if any exported or open package is not present
-                moduleInfo.validatePackages();
-
-                // Packages attribute needs update
-                if (moduleInfo.shouldAddPackagesAttribute()) {
-                    // replace with the overridden version
-                    data = data.copyWithContent(moduleInfo.getBytes());
-                }
+                // add resource pool entry
                 out.add(data);
             } catch (IOException e) {
                 throw new PluginException(e);
@@ -164,29 +170,55 @@
         return out.build();
     }
 
-    class ModuleInfo {
-        final ModuleDescriptor descriptor;
-        final ModuleHashes recordedHashes;
-        final ModuleResolution moduleResolution;
-        final Set<String> packages;
-        final ByteArrayInputStream bain;
+    static class ModuleInfo {
+        private final Attributes attrs;
+        private final Set<String> packages;
+        private final ByteArrayInputStream bain;
+        private final boolean dropModuleTarget;
+        private ModuleDescriptor descriptor;  // may be different that the original one
 
-        ModuleInfo(byte[] bytes, Set<String> packages) throws IOException {
+        ModuleInfo(byte[] bytes, Set<String> packages, boolean dropModuleTarget)
+            throws IOException
+        {
             this.bain = new ByteArrayInputStream(bytes);
             this.packages = packages;
 
-            Attributes attrs = jdk.internal.module.ModuleInfo.read(bain, null);
+            this.attrs = jdk.internal.module.ModuleInfo.read(bain, null);
             this.descriptor = attrs.descriptor();
-            this.recordedHashes = attrs.recordedHashes();
-            this.moduleResolution = attrs.moduleResolution();
-
             if (descriptor.isAutomatic()) {
                 throw new InternalError("linking automatic module is not supported");
             }
+
+            if (dropModuleTarget) {
+                // drop target attribute only if any OS property is present
+                this.dropModuleTarget =
+                    descriptor.osName().isPresent() ||
+                    descriptor.osArch().isPresent() ||
+                    descriptor.osVersion().isPresent();
+            } else {
+                this.dropModuleTarget = false;
+            }
         }
 
         String moduleName() {
-            return descriptor.name();
+            return attrs.descriptor().name();
+        }
+
+        ModuleDescriptor descriptor() {
+            return descriptor;
+        }
+
+
+        Set<String> packages() {
+            return packages;
+        }
+
+        ModuleHashes recordedHashes() {
+            return attrs.recordedHashes();
+        }
+
+        ModuleResolution moduleResolution() {
+            return attrs.moduleResolution();
         }
 
         /**
@@ -217,6 +249,9 @@
             for (String pn : descriptor.packages()) {
                 Checks.requirePackageName(pn);
             }
+            for (String pn : packages) {
+                Checks.requirePackageName(pn);
+            }
         }
 
 
@@ -242,22 +277,47 @@
         }
 
         /**
-         * Returns true if the PackagesAttribute should be written
+         * 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 shouldAddPackagesAttribute() {
-            return descriptor.packages().isEmpty() && packages.size() > 0;
+        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());
         }
 
         /**
-         * Returns the bytes for the module-info.class with PackagesAttribute
+         * Returns the bytes for the module-info.class with ModulePackages
          * if it contains at least one package
          */
         byte[] getBytes() throws IOException {
             bain.reset();
 
-            // add Packages attribute if not exist
-            if (shouldAddPackagesAttribute()) {
-                return new ModuleInfoRewriter(bain, packages).getBytes();
+            // 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();
             }
@@ -265,16 +325,23 @@
 
         class ModuleInfoRewriter extends ByteArrayOutputStream {
             final ModuleInfoExtender extender;
-            ModuleInfoRewriter(InputStream in, Set<String> packages) throws IOException {
+            ModuleInfoRewriter(InputStream in) {
                 this.extender = ModuleInfoExtender.newExtender(in);
-                // Add Packages attribute
-                if (packages.size() > 0) {
-                    this.extender.packages(packages);
-                }
-                this.extender.write(this);
             }
 
-            byte[] getBytes() {
+            void addModulePackages(Set<String> packages) {
+                // Add ModulePackages attribute
+                if (packages.size() > 0) {
+                    extender.packages(packages);
+                }
+            }
+
+            void dropModuleTarget() {
+                extender.targetPlatform("", "", "");
+            }
+
+            byte[] getBytes() throws IOException {
+                extender.write(this);
                 return buf;
             }
         }
@@ -316,6 +383,7 @@
         private int nextLocalVar         = 2;  // index to next local variable
 
         private final ClassWriter cw;
+        private boolean dropModuleTarget;
 
         // Method visitor for generating the SystemModules::modules() method
         private MethodVisitor mv;
@@ -329,9 +397,10 @@
         private final DedupSetBuilder dedupSetBuilder
             = new DedupSetBuilder(this::getNextLocalVar);
 
-        public SystemModulesClassGenerator() {
+        public SystemModulesClassGenerator(boolean retainModuleTarget) {
             this.cw = new ClassWriter(ClassWriter.COMPUTE_MAXS +
                                       ClassWriter.COMPUTE_FRAMES);
+            this.dropModuleTarget = !retainModuleTarget;
         }
 
         private int getNextLocalVar() {
@@ -395,14 +464,52 @@
         }
 
         /*
-         * Adds the given ModuleDescriptor to the system module list, and
-         * prepares mapping from various Sets to SetBuilders to emit an
-         * optimized number of sets during build.
+         * Adds the given ModuleDescriptor to the system module list.
+         * It performs link-time validation and prepares mapping from various
+         * Sets to SetBuilders to emit an optimized number of sets during build.
          */
-        public void addModule(ModuleInfo moduleInfo) {
-            ModuleDescriptor md = moduleInfo.descriptor;
+        public ResourcePoolEntry buildModuleInfo(ResourcePoolEntry entry,
+                                                 Set<String> packages)
+            throws IOException
+        {
+            if (moduleInfos.isEmpty() && !entry.moduleName().equals("java.base")) {
+                throw new InternalError("java.base must be the first module to process");
+            }
+
+            ModuleInfo moduleInfo;
+            if (entry.moduleName().equals("java.base")) {
+                moduleInfo = new ModuleInfo(entry.contentBytes(), packages, false);
+                ModuleDescriptor md = moduleInfo.descriptor;
+                // drop Moduletarget attribute only if java.base has all OS properties
+                // otherwise, retain it
+                if (dropModuleTarget &&
+                        md.osName().isPresent() && md.osArch().isPresent() &&
+                        md.osVersion().isPresent()) {
+                    dropModuleTarget = true;
+                } else {
+                    dropModuleTarget = false;
+                }
+            } else {
+                moduleInfo = new ModuleInfo(entry.contentBytes(), packages, dropModuleTarget);
+            }
+
+            // link-time validation
+            moduleInfo.validateNames();
+            // check if any exported or open package is not present
+            moduleInfo.validatePackages();
+
+            // module-info.class may be overridden for optimization
+            // 1. update ModuleTarget attribute to drop osName, osArch, osVersion
+            // 2. add/update ModulePackages attribute
+            if (moduleInfo.shouldRewrite()) {
+                entry = entry.copyWithContent(moduleInfo.getBytes());
+            }
             moduleInfos.add(moduleInfo);
+            dedups(moduleInfo.descriptor());
+            return entry;
+        }
 
+        private void dedups(ModuleDescriptor md) {
             // exports
             for (Exports e : md.exports()) {
                 dedupSetBuilder.stringSet(e.targets());
@@ -466,8 +573,8 @@
 
             for (int index = 0; index < moduleInfos.size(); index++) {
                 ModuleInfo minfo = moduleInfos.get(index);
-                new ModuleDescriptorBuilder(minfo.descriptor,
-                                            minfo.packages,
+                new ModuleDescriptorBuilder(minfo.descriptor(),
+                                            minfo.packages(),
                                             index).build();
             }
             mv.visitVarInsn(ALOAD, MD_VAR);
@@ -494,8 +601,8 @@
 
             for (int index = 0; index < moduleInfos.size(); index++) {
                 ModuleInfo minfo = moduleInfos.get(index);
-                if (minfo.recordedHashes != null) {
-                    new ModuleHashesBuilder(minfo.recordedHashes,
+                if (minfo.recordedHashes() != null) {
+                    new ModuleHashesBuilder(minfo.recordedHashes(),
                                             index,
                                             hmv).build();
                 }
@@ -525,12 +632,12 @@
 
             for (int index=0; index < moduleInfos.size(); index++) {
                 ModuleInfo minfo = moduleInfos.get(index);
-                if (minfo.moduleResolution != null) {
+                if (minfo.moduleResolution() != null) {
                     mresmv.visitVarInsn(ALOAD, 0);
                     pushInt(mresmv, index);
                     mresmv.visitTypeInsn(NEW, MODULE_RESOLUTION_CLASSNAME);
                     mresmv.visitInsn(DUP);
-                    mresmv.visitLdcInsn(minfo.moduleResolution.value());
+                    mresmv.visitLdcInsn(minfo.moduleResolution().value());
                     mresmv.visitMethodInsn(INVOKESPECIAL,
                                            MODULE_RESOLUTION_CLASSNAME,
                                            "<init>",
@@ -644,6 +751,11 @@
                 // main class
                 md.mainClass().ifPresent(this::mainClass);
 
+                // os name, arch, version
+                targetPlatform(md.osName().orElse(null),
+                               md.osArch().orElse(null),
+                               md.osVersion().orElse(null));
+
                 putModuleDescriptor();
             }
 
@@ -937,6 +1049,33 @@
                     "version", STRING_SIG, false);
                 mv.visitInsn(POP);
             }
+
+            /*
+             * Invoke Builder.osName(String name)
+             *        Builder.osArch(String arch)
+             *        Builder.osVersion(String version)
+             */
+            void targetPlatform(String osName, String osArch, String osVersion) {
+                if (osName != null) {
+                    invokeBuilderMethod("osName", osName);
+                }
+
+                if (osArch != null) {
+                    invokeBuilderMethod("osArch", osArch);
+                }
+
+                if (osVersion != null) {
+                    invokeBuilderMethod("osVersion", osVersion);
+                }
+            }
+
+            void invokeBuilderMethod(String methodName, String value) {
+                mv.visitVarInsn(ALOAD, BUILDER_VAR);
+                mv.visitLdcInsn(value);
+                mv.visitMethodInsn(INVOKEVIRTUAL, MODULE_DESCRIPTOR_BUILDER,
+                    methodName, STRING_SIG, false);
+                mv.visitInsn(POP);
+            }
         }
 
         class ModuleHashesBuilder {
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties	Mon Jan 30 13:11:56 2017 +0300
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties	Mon Jan 30 11:00:18 2017 -0800
@@ -81,6 +81,8 @@
 Takes a file hinting to jlink what java.lang.invoke classes to pre-generate. If\n\
 this flag is not specified a default set of classes will be generated.
 
+system-modules.argument=retainModuleTarget
+
 system-modules.description=Fast loading of module descriptors (always enabled)
 
 onoff.argument=<on|off>
--- a/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/SystemModulesTest.java	Mon Jan 30 13:11:56 2017 +0300
+++ b/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/SystemModulesTest.java	Mon Jan 30 11:00:18 2017 -0800
@@ -25,6 +25,9 @@
 import java.lang.module.ModuleDescriptor.*;
 import java.lang.module.ModuleFinder;
 import java.lang.module.ModuleReference;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -36,6 +39,7 @@
 
 /**
  * @test
+ * @bug 8142968 8173381
  * @modules java.base/jdk.internal.misc
  * @run testng SystemModulesTest
  * @summary Verify the properties of ModuleDescriptor created
@@ -43,7 +47,72 @@
  */
 
 public class SystemModulesTest {
-    private static final JavaLangModuleAccess jlma = SharedSecrets.getJavaLangModuleAccess();
+    private static final JavaLangModuleAccess JLMA =
+        SharedSecrets.getJavaLangModuleAccess();
+    private static final String OS_NAME = System.getProperty("os.name");
+    private static final String OS_ARCH = System.getProperty("os.arch");
+    //  system modules containing no package
+    private static final Set<String> EMPTY_MODULES =
+        Set.of("java.se", "java.se.ee", "jdk.jdwp.agent", "jdk.pack");
+
+    @Test
+    public void testSystemModules() {
+        Path jimage = Paths.get(System.getProperty("java.home"), "lib", "modules");
+        if (Files.notExists(jimage))
+            return;
+
+        ModuleFinder.ofSystem().findAll().stream()
+                    .map(ModuleReference::descriptor)
+                    .forEach(this::checkAttributes);
+    }
+
+    // JMOD files are created with osName and osArch that may be different
+    // than os.name and os.arch system property
+    private boolean checkOSName(String name) {
+        if (name.equals(OS_NAME))
+            return true;
+
+        if (OS_NAME.equals("Mac OS X")) {
+            return name.equals("Darwin");
+        } else if (OS_NAME.startsWith("Windows")) {
+            return name.startsWith("Windows");
+        } else {
+            System.err.println("ERROR: " + name + " but expected: " + OS_NAME);
+            return false;
+        }
+    }
+
+    private boolean checkOSArch(String name) {
+        if (name.equals(OS_ARCH))
+            return true;
+
+        switch (OS_ARCH) {
+            case "i386":
+            case "x86":
+                return name.equals("i586");
+            default:
+                System.err.println("ERROR: " + name + " but expected: " + OS_ARCH);
+                return false;
+        }
+    }
+
+    private void checkAttributes(ModuleDescriptor md) {
+        System.out.format("%s %s %s %s%n", md.name(),
+                          md.osName(), md.osArch(), md.osVersion());
+
+        if (md.name().equals("java.base")) {
+            assertTrue(checkOSName(md.osName().get()));
+            assertTrue(checkOSArch(md.osArch().get()));
+            assertTrue(md.osVersion().isPresent());
+        } else {
+            // target platform attribute is dropped by jlink plugin
+            assertFalse(md.osName().isPresent());
+            assertFalse(md.osArch().isPresent());
+            assertFalse(md.osVersion().isPresent());
+            assertTrue(md.packages().size() > 0
+                || EMPTY_MODULES.contains(md.name()), md.name());
+        }
+    }
 
     /**
      * Verify ModuleDescriptor contains unmodifiable sets
@@ -59,18 +128,19 @@
     private void testModuleDescriptor(ModuleDescriptor md) {
         assertUnmodifiable(md.packages(), "package");
         assertUnmodifiable(md.requires(),
-                           jlma.newRequires(Set.of(Requires.Modifier.TRANSITIVE), "require", null));
+                           JLMA.newRequires(Set.of(Requires.Modifier.TRANSITIVE),
+                                            "require", null));
         for (Requires req : md.requires()) {
             assertUnmodifiable(req.modifiers(), Requires.Modifier.TRANSITIVE);
         }
 
-        assertUnmodifiable(md.exports(), jlma.newExports(Set.of(), "export", Set.of()));
+        assertUnmodifiable(md.exports(), JLMA.newExports(Set.of(), "export", Set.of()));
         for (Exports exp : md.exports()) {
             assertUnmodifiable(exp.modifiers(), Exports.Modifier.SYNTHETIC);
             assertUnmodifiable(exp.targets(), "target");
         }
 
-        assertUnmodifiable(md.opens(), jlma.newOpens(Set.of(), "open", Set.of()));
+        assertUnmodifiable(md.opens(), JLMA.newOpens(Set.of(), "open", Set.of()));
         for (Opens opens : md.opens()) {
             assertUnmodifiable(opens.modifiers(), Opens.Modifier.SYNTHETIC);
             assertUnmodifiable(opens.targets(), "target");
@@ -79,7 +149,7 @@
         assertUnmodifiable(md.uses(), "use");
 
         assertUnmodifiable(md.provides(),
-                           jlma.newProvides("provide", List.of("provide")));
+                           JLMA.newProvides("provide", List.of("provide")));
         for (Provides provides : md.provides()) {
             assertUnmodifiable(provides.providers(), "provide");
         }
--- a/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java	Mon Jan 30 13:11:56 2017 +0300
+++ b/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java	Mon Jan 30 11:00:18 2017 -0800
@@ -22,13 +22,20 @@
  */
 
 import java.io.File;
+import java.io.IOException;
+import java.lang.module.ModuleDescriptor;
+import java.lang.reflect.Layer;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Arrays;
+import java.util.Set;
+import java.util.spi.ToolProvider;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import jdk.testlibrary.FileUtils;
+
 import static jdk.testlibrary.ProcessTools.*;
 
 
@@ -38,6 +45,7 @@
 
 /**
  * @test
+ * @bug 8142968 8173381
  * @library /lib/testlibrary
  * @modules jdk.compiler jdk.jlink
  * @build UserModuleTest CompilerUtils jdk.testlibrary.FileUtils jdk.testlibrary.ProcessTools
@@ -50,8 +58,9 @@
 
     private static final Path SRC_DIR = Paths.get(TEST_SRC, "src");
     private static final Path MODS_DIR = Paths.get("mods");
+    private static final Path JMODS_DIR = Paths.get("jmods");
+
     private static final Path IMAGE = Paths.get("image");
-    private static final Path JMODS = Paths.get(JAVA_HOME, "jmods");
     private static final String MAIN_MID = "m1/p1.Main";
 
     // the names of the modules in this test
@@ -59,7 +68,7 @@
 
 
     private static boolean hasJmods() {
-        if (!Files.exists(JMODS)) {
+        if (!Files.exists(Paths.get(JAVA_HOME, "jmods"))) {
             System.err.println("Test skipped. NO jmods directory");
             return false;
         }
@@ -75,25 +84,17 @@
 
         for (String mn : modules) {
             Path msrc = SRC_DIR.resolve(mn);
-            assertTrue(CompilerUtils.compile(msrc, MODS_DIR, "--module-source-path", SRC_DIR.toString()));
+            assertTrue(CompilerUtils.compile(msrc, MODS_DIR,
+                "--module-source-path", SRC_DIR.toString()));
         }
 
         if (Files.exists(IMAGE)) {
             FileUtils.deleteFileTreeUnchecked(IMAGE);
         }
 
-        createImage(IMAGE, "java.base", "m1", "m3");
-    }
+        createImage(IMAGE, "m1", "m3");
 
-    private void createImage(Path outputDir, String... modules) throws Throwable {
-        Path jlink = Paths.get(JAVA_HOME, "bin", "jlink");
-        String mp = JMODS.toString() + File.pathSeparator + MODS_DIR.toString();
-        assertTrue(executeProcess(jlink.toString(), "--output", outputDir.toString(),
-                        "--add-modules", Arrays.stream(modules).collect(Collectors.joining(",")),
-                        "--module-path", mp)
-                        .outputTo(System.out)
-                        .errorTo(System.out)
-                        .getExitValue() == 0);
+        createJmods("m1", "m4");
     }
 
     /*
@@ -120,9 +121,9 @@
 
         Path java = IMAGE.resolve("bin").resolve("java");
         assertTrue(executeProcess(java.toString(), "-m", "m3/p3.Main")
-            .outputTo(System.out)
-            .errorTo(System.out)
-            .getExitValue() == 0);
+                        .outputTo(System.out)
+                        .errorTo(System.out)
+                        .getExitValue() == 0);
     }
 
     /*
@@ -150,12 +151,114 @@
     public void testDedupSet() throws Throwable {
         if (!hasJmods()) return;
 
-        Path dir = Paths.get("newImage");
-        createImage(dir, "java.base", "m1", "m2", "m3", "m4");
+        Path dir = Paths.get("dedupSetTest");
+        createImage(dir, "m1", "m2", "m3", "m4");
         Path java = dir.resolve("bin").resolve("java");
         assertTrue(executeProcess(java.toString(), "-m", MAIN_MID)
                         .outputTo(System.out)
                         .errorTo(System.out)
                         .getExitValue() == 0);
     }
+
+    private void createJmods(String... modules) throws IOException {
+        // use the same target platform as in java.base
+        ModuleDescriptor md = Layer.boot().findModule("java.base").get()
+                                   .getDescriptor();
+        String osName = md.osName().get();
+        String osArch = md.osArch().get();
+
+        // create JMOD files
+        Files.createDirectories(JMODS_DIR);
+        Stream.of(modules).forEach(mn ->
+            assertTrue(jmod("create",
+                "--class-path", MODS_DIR.resolve(mn).toString(),
+                "--os-name", osName,
+                "--os-arch", osArch,
+                "--main-class", mn.replace('m', 'p') + ".Main",
+                JMODS_DIR.resolve(mn + ".jmod").toString()) == 0)
+        );
+    }
+
+
+    /**
+     * Verify the module descriptor if package p4.dummy is excluded at link time.
+     */
+    @Test
+    public void testModulePackagesAttribute() throws Throwable {
+        if (!hasJmods()) return;
+
+        // create an image using JMOD files
+        Path dir = Paths.get("packagesTest");
+        String mp = Paths.get(JAVA_HOME, "jmods").toString() +
+            File.pathSeparator + JMODS_DIR.toString();
+
+        Set<String> modules = Set.of("m1", "m4");
+        assertTrue(JLINK_TOOL.run(System.out, System.out,
+            "--output", dir.toString(),
+            "--exclude-resources", "m4/p4/dummy/*",
+            "--add-modules", modules.stream().collect(Collectors.joining(",")),
+            "--module-path", mp) == 0);
+
+        // verify ModuleDescriptor
+        Path java = dir.resolve("bin").resolve("java");
+        assertTrue(executeProcess(java.toString(),
+                        "--add-modules=m1", "-m", "m4")
+            .outputTo(System.out)
+            .errorTo(System.out)
+            .getExitValue() == 0);
+    }
+
+    /**
+     * Verify the plugin to retain ModuleTarget attribute
+     */
+    @Test
+    public void testRetainModuleTarget() throws Throwable {
+        if (!hasJmods()) return;
+
+        // create an image using JMOD files
+        Path dir = Paths.get("retainModuleTargetTest");
+        String mp = Paths.get(JAVA_HOME, "jmods").toString() +
+            File.pathSeparator + JMODS_DIR.toString();
+
+        Set<String> modules = Set.of("m1", "m4");
+        assertTrue(JLINK_TOOL.run(System.out, System.out,
+            "--output", dir.toString(),
+            "--system-modules", "retainModuleTarget",
+            "--exclude-resources", "m4/p4/dummy/*",
+            "--add-modules", modules.stream().collect(Collectors.joining(",")),
+            "--module-path", mp) == 0);
+
+        // verify ModuleDescriptor
+        Path java = dir.resolve("bin").resolve("java");
+        assertTrue(executeProcess(java.toString(),
+                        "--add-modules=m1", "-m", "m4", "retainModuleTarget")
+            .outputTo(System.out)
+            .errorTo(System.out)
+            .getExitValue() == 0);
+    }
+
+    static final ToolProvider JLINK_TOOL = ToolProvider.findFirst("jlink")
+        .orElseThrow(() ->
+            new RuntimeException("jlink tool not found")
+        );
+
+    static final ToolProvider JMOD_TOOL = ToolProvider.findFirst("jmod")
+        .orElseThrow(() ->
+            new RuntimeException("jmod tool not found")
+        );
+
+    static final String MODULE_PATH = Paths.get(JAVA_HOME, "jmods").toString()
+        + File.pathSeparator + MODS_DIR.toString();
+
+    private void createImage(Path outputDir, String... modules) throws Throwable {
+        assertTrue(JLINK_TOOL.run(System.out, System.out,
+            "--output", outputDir.toString(),
+            "--add-modules", Arrays.stream(modules).collect(Collectors.joining(",")),
+            "--module-path", MODULE_PATH) == 0);
+    }
+
+    private static int jmod(String... options) {
+        System.out.println("jmod " + Arrays.asList(options));
+        return JMOD_TOOL.run(System.out, System.out, options);
+    }
 }
--- a/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m1/p1/Main.java	Mon Jan 30 13:11:56 2017 +0300
+++ b/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m1/p1/Main.java	Mon Jan 30 11:00:18 2017 -0800
@@ -23,7 +23,10 @@
 
 package p1;
 
+import java.io.IOException;
 import java.lang.module.ModuleDescriptor;
+import java.lang.reflect.Layer;
+import java.lang.reflect.Module;
 import java.net.URI;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
@@ -37,25 +40,48 @@
         // load another package
         p2.T.test();
 
-        // check the module descriptor of a system module
-        validate(Main.class.getModule().getDescriptor());
+        // validate the module descriptor
+        validate(Main.class.getModule());
+
+        // validate the Moduletarget attribute for java.base
+        ModuleDescriptor md = Layer.boot().findModule("java.base").get()
+                                   .getDescriptor();
+        if (!md.osName().isPresent() || !md.osArch().isPresent() ||
+                !md.osVersion().isPresent()) {
+            throw new RuntimeException("java.base: " + md.osName() + " " +
+                    md.osArch() + " " + md.osVersion());
+        }
+    }
+
+    static void validate(Module module) throws IOException {
+        ModuleDescriptor md = module.getDescriptor();
 
         // read m1/module-info.class
         FileSystem fs = FileSystems.newFileSystem(URI.create("jrt:/"),
                                                   Collections.emptyMap());
-        Path path = fs.getPath("/", "modules", "m1", "module-info.class");
-        validate(ModuleDescriptor.read(Files.newInputStream(path)));
-    }
+        Path path = fs.getPath("/", "modules", module.getName(), "module-info.class");
+        ModuleDescriptor md1 = ModuleDescriptor.read(Files.newInputStream(path));
+
 
-    static void validate(ModuleDescriptor md) {
+        // check the module descriptor of a system module and read from jimage
         checkPackages(md.packages(), "p1", "p2");
+        checkPackages(md1.packages(), "p1", "p2");
+
+        // check ModuleTarget attribute
+        checkModuleTargetAttribute(md);
+        checkModuleTargetAttribute(md1);
     }
 
     static void checkPackages(Set<String> pkgs, String... expected) {
-        for (String pn : expected) {
-            if (!pkgs.contains(pn)) {
-                throw new RuntimeException(pn + " missing in " + pkgs);
-            }
+        if (!pkgs.equals(Set.of(expected))) {
+            throw new RuntimeException(pkgs + " expected: " + Set.of(expected));
+        }
+    }
+
+    static void checkModuleTargetAttribute(ModuleDescriptor md) {
+        if (md.osName().isPresent() || md.osArch().isPresent() ||
+                md.osVersion().isPresent()) {
+            throw new RuntimeException(md.osName() + " " + md.osArch() + " " + md.osVersion());
         }
     }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m4/p4/Main.java	Mon Jan 30 11:00:18 2017 -0800
@@ -0,0 +1,114 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package p4;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.module.ModuleDescriptor;
+import java.lang.module.ModuleFinder;
+import java.lang.reflect.Layer;
+import java.net.URI;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.Set;
+
+public class Main {
+    // the system module plugin by default drops ModuleTarget attribute
+    private static boolean expectModuleTarget = false;
+    public static void main(String... args) throws IOException {
+        if (args.length > 0) {
+            if (!args[0].equals("retainModuleTarget")) {
+                throw new IllegalArgumentException(args[0]);
+            }
+
+            expectModuleTarget = true;
+        }
+
+        // java.base is packaged with osName/osArch/osVersion
+        ModuleDescriptor md = Layer.boot().findModule("java.base").get()
+                                   .getDescriptor();
+        if (!md.osName().isPresent() ||
+                !md.osArch().isPresent() ||
+                !md.osVersion().isPresent()) {
+            throw new RuntimeException("osName/osArch/osVersion is missing: " +
+                md.osName() + " " + md.osArch() + " " + md.osVersion());
+        }
+
+        // verify module-info.class for m1 and m4
+        checkModule("m1", "p1", "p2");
+        checkModule("m4", "p4");
+    }
+
+    private static void checkModule(String mn, String... packages) throws IOException {
+        // verify ModuleDescriptor from the runtime module
+        ModuleDescriptor md = Layer.boot().findModule(mn).get()
+                                   .getDescriptor();
+        checkModuleDescriptor(md, packages);
+
+        // verify ModuleDescriptor from module-info.class read from ModuleReader
+        try (InputStream in = ModuleFinder.ofSystem().find(mn).get()
+            .open().open("module-info.class").get()) {
+            checkModuleDescriptor(ModuleDescriptor.read(in), packages);
+        }
+
+        // verify ModuleDescriptor from module-info.class read from jimage
+        FileSystem fs = FileSystems.newFileSystem(URI.create("jrt:/"),
+            Collections.emptyMap());
+        Path path = fs.getPath("/", "modules", mn, "module-info.class");
+        checkModuleDescriptor(ModuleDescriptor.read(Files.newInputStream(path)), packages);
+    }
+
+    static void checkModuleDescriptor(ModuleDescriptor md, String... packages) {
+        String mainClass = md.name().replace('m', 'p') + ".Main";
+        if (!md.mainClass().get().equals(mainClass)) {
+            throw new RuntimeException(md.mainClass().toString());
+        }
+
+        if (expectModuleTarget) {
+            // ModuleTarget attribute is retained
+            if (!md.osName().isPresent() || !md.osArch().isPresent()) {
+                throw new RuntimeException("osName or osArch is missing: " +
+                    md.osName() + " " + md.osArch());
+            }
+        } else {
+            // by default ModuleTarget attribute is dropped
+            if (md.osName().isPresent() || md.osArch().isPresent()) {
+                throw new RuntimeException("osName and osArch should not be set: " +
+                    md.osName() + " " + md.osArch());
+            }
+        }
+
+        if (md.osVersion().isPresent()) {
+            throw new RuntimeException("Expected no osVersion set: " + md.osVersion());
+        }
+
+        Set<String> pkgs = md.packages();
+        if (!pkgs.equals(Set.of(packages))) {
+            throw new RuntimeException(pkgs + " expected: " + Set.of(packages));
+        }
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/tools/jlink/plugins/SystemModuleDescriptors/src/m4/p4/dummy/dummy.properties	Mon Jan 30 11:00:18 2017 -0800
@@ -0,0 +1,1 @@
+key=dummy