8006005: Fix constant pool index validation and alignment trap for method parameter reflection

Summary: This patch addresses an alignment trap due to the storage format of method parameters data in constMethod.  It also adds code to validate constant pool indexes for method parameters data.
Reviewed-by: jrose, dholmes
Contributed-by: eric.mccorkle@oracle.com

src/share/vm/classfile/classFileParser.cpp

@@ -59,6 +59,7 @@
 #include "services/classLoadingService.hpp"
 #include "services/threadService.hpp"
 #include "utilities/array.hpp"
+#include "utilities/globalDefinitions.hpp"
 
 // We generally try to create the oops directly when parsing, rather than
 // allocating temporary data structures and copying the bytes twice. A
@@ -2148,9 +2149,21 @@ methodHandle ClassFileParser::parse_method(ClassLoaderData* loader_data,
                                      cp, CHECK_(nullHandle));
     } else if (method_attribute_name == vmSymbols::tag_method_parameters()) {
       method_parameters_length = cfs->get_u1_fast();
+      // Track the actual size (note: this is written for clarity; a
+      // decent compiler will CSE and constant-fold this into a single
+      // expression)
+      u2 actual_size = 1;
       method_parameters_data = cfs->get_u1_buffer();
+      actual_size += 2 * method_parameters_length;
       cfs->skip_u2_fast(method_parameters_length);
+      actual_size += 4 * method_parameters_length;
       cfs->skip_u4_fast(method_parameters_length);
+      // Enforce attribute length
+      if (method_attribute_length != actual_size) {
+        classfile_parse_error(
+          "Invalid MethodParameters method attribute length %u in class file %s",
+          method_attribute_length, CHECK_(nullHandle));
+      }
       // ignore this attribute if it cannot be reflected
       if (!SystemDictionary::Parameter_klass_loaded())
         method_parameters_length = 0;
@@ -2297,7 +2310,10 @@ methodHandle ClassFileParser::parse_method(ClassLoaderData* loader_data,
       elem[i].name_cp_index =
         Bytes::get_Java_u2(method_parameters_data);
       method_parameters_data += 2;
-      elem[i].flags = Bytes::get_Java_u4(method_parameters_data);
+      u4 flags = Bytes::get_Java_u4(method_parameters_data);
+      // This caused an alignment fault on Sparc, if flags was a u4
+      elem[i].flags_lo = extract_low_short_from_int(flags);
+      elem[i].flags_hi = extract_high_short_from_int(flags);
       method_parameters_data += 4;
     }
   }

src/share/vm/oops/constMethod.hpp

@@ -122,7 +122,12 @@ class ExceptionTableElement VALUE_OBJ_CLASS_SPEC {
 class MethodParametersElement VALUE_OBJ_CLASS_SPEC {
  public:
   u2 name_cp_index;
-  u4 flags;
+  // This has to happen, otherwise it will cause SIGBUS from a
+  // misaligned u4 on some architectures (ie SPARC)
+  // because MethodParametersElements are only aligned mod 2
+  // within the ConstMethod container  u2 flags_hi;
+  u2 flags_hi;
+  u2 flags_lo;
 };
 
 

src/share/vm/prims/jvm.cpp

@@ -1589,6 +1589,12 @@ JVM_ENTRY(jbyteArray, JVM_GetClassTypeAnnotations(JNIEnv *env, jclass cls))
   return NULL;
 JVM_END
 
+static void bounds_check(constantPoolHandle cp, jint index, TRAPS) {
+  if (!cp->is_within_bounds(index)) {
+    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Constant pool index out of bounds");
+  }
+}
+
 JVM_ENTRY(jobjectArray, JVM_GetMethodParameters(JNIEnv *env, jobject method))
 {
   JVMWrapper("JVM_GetMethodParameters");
@@ -1598,15 +1604,31 @@ JVM_ENTRY(jobjectArray, JVM_GetMethodParameters(JNIEnv *env, jobject method))
   Handle reflected_method (THREAD, JNIHandles::resolve_non_null(method));
   const int num_params = mh->method_parameters_length();
 
-  if(0 != num_params) {
+  if (0 != num_params) {
+    // make sure all the symbols are properly formatted
+    for (int i = 0; i < num_params; i++) {
+      MethodParametersElement* params = mh->method_parameters_start();
+      int index = params[i].name_cp_index;
+      bounds_check(mh->constants(), index, CHECK_NULL);
+
+      if (0 != index && !mh->constants()->tag_at(index).is_utf8()) {
+        THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(),
+                    "Wrong type at constant pool index");
+      }
+
+    }
+
     objArrayOop result_oop = oopFactory::new_objArray(SystemDictionary::reflect_Parameter_klass(), num_params, CHECK_NULL);
     objArrayHandle result (THREAD, result_oop);
 
-    for(int i = 0; i < num_params; i++) {
+    for (int i = 0; i < num_params; i++) {
       MethodParametersElement* params = mh->method_parameters_start();
-      Symbol* const sym = mh->constants()->symbol_at(params[i].name_cp_index);
+      // For a 0 index, give a NULL symbol
+      Symbol* const sym = 0 != params[i].name_cp_index ?
+        mh->constants()->symbol_at(params[i].name_cp_index) : NULL;
+      int flags = build_int_from_shorts(params[i].flags_lo, params[i].flags_hi);
       oop param = Reflection::new_parameter(reflected_method, i, sym,
-                                            params[i].flags, CHECK_NULL);
+                                            flags, CHECK_NULL);
       result->obj_at_put(i, param);
     }
     return (jobjectArray)JNIHandles::make_local(env, result());
@@ -1830,13 +1852,6 @@ JVM_ENTRY(jint, JVM_ConstantPoolGetSize(JNIEnv *env, jobject obj, jobject unused
 JVM_END
 
 
-static void bounds_check(constantPoolHandle cp, jint index, TRAPS) {
-  if (!cp->is_within_bounds(index)) {
-    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Constant pool index out of bounds");
-  }
-}
-
-
 JVM_ENTRY(jclass, JVM_ConstantPoolGetClassAt(JNIEnv *env, jobject obj, jobject unused, jint index))
 {
   JVMWrapper("JVM_ConstantPoolGetClassAt");
@@ -1851,7 +1866,6 @@ JVM_ENTRY(jclass, JVM_ConstantPoolGetClassAt(JNIEnv *env, jobject obj, jobject u
 }
 JVM_END
 
-
 JVM_ENTRY(jclass, JVM_ConstantPoolGetClassAtIfLoaded(JNIEnv *env, jobject obj, jobject unused, jint index))
 {
   JVMWrapper("JVM_ConstantPoolGetClassAtIfLoaded");

src/share/vm/runtime/reflection.cpp

@@ -862,7 +862,15 @@ oop Reflection::new_field(fieldDescriptor* fd, bool intern_name, TRAPS) {
 
 oop Reflection::new_parameter(Handle method, int index, Symbol* sym,
                               int flags, TRAPS) {
-  Handle name = java_lang_String::create_from_symbol(sym, CHECK_NULL);
+  Handle name;
+
+  // A null symbol here translates to the empty string
+  if(NULL != sym) {
+    name = java_lang_String::create_from_symbol(sym, CHECK_NULL);
+  } else {
+    name = java_lang_String::create_from_str("", CHECK_NULL);
+  }
+
   Handle rh = java_lang_reflect_Parameter::create(CHECK_NULL);
   java_lang_reflect_Parameter::set_name(rh(), name());
   java_lang_reflect_Parameter::set_modifiers(rh(), flags);