6889255: javac MethodSymbol throws NPE if ClassReader does not read parameter names correctly

Reviewed-by: darcy

src/share/classes/com/sun/tools/javac/code/Symbol.java

@@ -1212,25 +1212,58 @@ public abstract class Symbol implements Element {
         public List<VarSymbol> params() {
             owner.complete();
             if (params == null) {
-                List<Name> names = savedParameterNames;
+                // If ClassReader.saveParameterNames has been set true, then
+                // savedParameterNames will be set to a list of names that
+                // matches the types in type.getParameterTypes().  If any names
+                // were not found in the class file, those names in the list will
+                // be set to the empty name.
+                // If ClassReader.saveParameterNames has been set false, then
+                // savedParameterNames will be null.
+                List<Name> paramNames = savedParameterNames;
                 savedParameterNames = null;
-                if (names == null) {
-                    names = List.nil();
-                    int i = 0;
-                    for (Type t : type.getParameterTypes())
-                        names = names.prepend(name.table.fromString("arg" + i++));
-                    names = names.reverse();
-                }
+                // discard the provided names if the list of names is the wrong size.
+                if (paramNames == null || paramNames.size() != type.getParameterTypes().size())
+                    paramNames = List.nil();
                 ListBuffer<VarSymbol> buf = new ListBuffer<VarSymbol>();
+                List<Name> remaining = paramNames;
+                // assert: remaining and paramNames are both empty or both
+                // have same cardinality as type.getParameterTypes()
+                int i = 0;
                 for (Type t : type.getParameterTypes()) {
-                    buf.append(new VarSymbol(PARAMETER, names.head, t, this));
-                    names = names.tail;
+                    Name paramName;
+                    if (remaining.isEmpty()) {
+                        // no names for any parameters available
+                        paramName = createArgName(i, paramNames);
+                    } else {
+                        paramName = remaining.head;
+                        remaining = remaining.tail;
+                        if (paramName.isEmpty()) {
+                            // no name for this specific parameter
+                            paramName = createArgName(i, paramNames);
+                        }
+                    }
+                    buf.append(new VarSymbol(PARAMETER, paramName, t, this));
+                    i++;
                 }
                 params = buf.toList();
             }
             return params;
         }
 
+        // Create a name for the argument at position 'index' that is not in
+        // the exclude list. In normal use, either no names will have been
+        // provided, in which case the exclude list is empty, or all the names
+        // will have been provided, in which case this method will not be called.
+        private Name createArgName(int index, List<Name> exclude) {
+            String prefix = "arg";
+            while (true) {
+                Name argName = name.table.fromString(prefix + index);
+                if (!exclude.contains(argName))
+                    return argName;
+                prefix += "$";
+            }
+        }
+
         public Symbol asMemberOf(Type site, Types types) {
             return new MethodSymbol(flags_field, name, types.memberType(site, this), owner);
         }

src/share/classes/com/sun/tools/javac/jvm/ClassReader.java

@@ -29,6 +29,7 @@ import java.io.*;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.CharBuffer;
+import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
@@ -191,6 +192,16 @@ public class ClassReader implements Completer {
      */
     boolean debugJSR308;
 
+    /** A table to hold the constant pool indices for method parameter
+     * names, as given in LocalVariableTable attributes.
+     */
+    int[] parameterNameIndices;
+
+    /**
+     * Whether or not any parameter names have been found.
+     */
+    boolean haveParameterNameIndices;
+
     /** Get the ClassReader instance for this invocation. */
     public static ClassReader instance(Context context) {
         ClassReader instance = context.get(classReaderKey);
@@ -922,32 +933,33 @@ public class ClassReader implements Completer {
                 void read(Symbol sym, int attrLen) {
                     int newbp = bp + attrLen;
                     if (saveParameterNames) {
-                        // pick up parameter names from the variable table
-                        List<Name> parameterNames = List.nil();
-                        int firstParam = ((sym.flags() & STATIC) == 0) ? 1 : 0;
-                        int endParam = firstParam + Code.width(sym.type.getParameterTypes());
+                        // Pick up parameter names from the variable table.
+                        // Parameter names are not explicitly identified as such,
+                        // but all parameter name entries in the LocalVariableTable
+                        // have a start_pc of 0.  Therefore, we record the name
+                        // indicies of all slots with a start_pc of zero in the
+                        // parameterNameIndicies array.
+                        // Note that this implicitly honors the JVMS spec that
+                        // there may be more than one LocalVariableTable, and that
+                        // there is no specified ordering for the entries.
                         int numEntries = nextChar();
-                        for (int i=0; i<numEntries; i++) {
+                        for (int i = 0; i < numEntries; i++) {
                             int start_pc = nextChar();
                             int length = nextChar();
                             int nameIndex = nextChar();
                             int sigIndex = nextChar();
                             int register = nextChar();
-                            if (start_pc == 0 &&
-                                firstParam <= register &&
-                                register < endParam) {
-                                int index = firstParam;
-                                for (Type t : sym.type.getParameterTypes()) {
-                                    if (index == register) {
-                                        parameterNames = parameterNames.prepend(readName(nameIndex));
-                                        break;
-                                    }
-                                    index += Code.width(t);
+                            if (start_pc == 0) {
+                                // ensure array large enough
+                                if (register >= parameterNameIndices.length) {
+                                    int newSize = Math.max(register, parameterNameIndices.length + 8);
+                                    parameterNameIndices =
+                                            Arrays.copyOf(parameterNameIndices, newSize);
                                 }
+                                parameterNameIndices[register] = nameIndex;
+                                haveParameterNameIndices = true;
                             }
                         }
-                        parameterNames = parameterNames.reverse();
-                        ((MethodSymbol)sym).savedParameterNames = parameterNames;
                     }
                     bp = newbp;
                 }
@@ -1839,6 +1851,8 @@ public class ClassReader implements Completer {
                                       syms.methodClass);
         }
         MethodSymbol m = new MethodSymbol(flags, name, type, currentOwner);
+        if (saveParameterNames)
+            initParameterNames(m);
         Symbol prevOwner = currentOwner;
         currentOwner = m;
         try {
@@ -1846,9 +1860,90 @@ public class ClassReader implements Completer {
         } finally {
             currentOwner = prevOwner;
         }
+        if (saveParameterNames)
+            setParameterNames(m, type);
         return m;
     }
 
+    /**
+     * Init the parameter names array.
+     * Parameter names are currently inferred from the names in the
+     * LocalVariableTable attributes of a Code attribute.
+     * (Note: this means parameter names are currently not available for
+     * methods without a Code attribute.)
+     * This method initializes an array in which to store the name indexes
+     * of parameter names found in LocalVariableTable attributes. It is
+     * slightly supersized to allow for additional slots with a start_pc of 0.
+     */
+    void initParameterNames(MethodSymbol sym) {
+        // make allowance for synthetic parameters.
+        final int excessSlots = 4;
+        int expectedParameterSlots =
+                Code.width(sym.type.getParameterTypes()) + excessSlots;
+        if (parameterNameIndices == null
+                || parameterNameIndices.length < expectedParameterSlots) {
+            parameterNameIndices = new int[expectedParameterSlots];
+        } else
+            Arrays.fill(parameterNameIndices, 0);
+        haveParameterNameIndices = false;
+    }
+
+    /**
+     * Set the parameter names for a symbol from the name index in the
+     * parameterNameIndicies array. The type of the symbol may have changed
+     * while reading the method attributes (see the Signature attribute).
+     * This may be because of generic information or because anonymous
+     * synthetic parameters were added.   The original type (as read from
+     * the method descriptor) is used to help guess the existence of
+     * anonymous synthetic parameters.
+     * On completion, sym.savedParameter names will either be null (if
+     * no parameter names were found in the class file) or will be set to a
+     * list of names, one per entry in sym.type.getParameterTypes, with
+     * any missing names represented by the empty name.
+     */
+    void setParameterNames(MethodSymbol sym, Type jvmType) {
+        // if no names were found in the class file, there's nothing more to do
+        if (!haveParameterNameIndices)
+            return;
+
+        int firstParam = ((sym.flags() & STATIC) == 0) ? 1 : 0;
+        // the code in readMethod may have skipped the first parameter when
+        // setting up the MethodType. If so, we make a corresponding allowance
+        // here for the position of the first parameter.  Note that this
+        // assumes the skipped parameter has a width of 1 -- i.e. it is not
+        // a double width type (long or double.)
+        if (sym.name == names.init && currentOwner.hasOuterInstance()) {
+            // Sometimes anonymous classes don't have an outer
+            // instance, however, there is no reliable way to tell so
+            // we never strip this$n
+            if (!currentOwner.name.isEmpty())
+                firstParam += 1;
+        }
+
+        if (sym.type != jvmType) {
+            // reading the method attributes has caused the symbol's type to
+            // be changed. (i.e. the Signature attribute.)  This may happen if
+            // there are hidden (synthetic) parameters in the descriptor, but
+            // not in the Signature.  The position of these hidden parameters
+            // is unspecified; for now, assume they are at the beginning, and
+            // so skip over them. The primary case for this is two hidden
+            // parameters passed into Enum constructors.
+            int skip = Code.width(jvmType.getParameterTypes())
+                    - Code.width(sym.type.getParameterTypes());
+            firstParam += skip;
+        }
+        List<Name> paramNames = List.nil();
+        int index = firstParam;
+        for (Type t: sym.type.getParameterTypes()) {
+            int nameIdx = (index < parameterNameIndices.length
+                    ? parameterNameIndices[index] : 0);
+            Name name = nameIdx == 0 ? names.empty : readName(nameIdx);
+            paramNames = paramNames.prepend(name);
+            index += Code.width(t);
+        }
+        sym.savedParameterNames = paramNames.reverse();
+    }
+
     /** Skip a field or method
      */
     void skipMember() {