8162340: Better class stream parsing
authoracorn
Wed, 27 Jul 2016 08:31:48 -0400
changeset 40016 bf6fcd467a7b
parent 40013 943cf01a6b82
child 40017 cfe045eb7da9
8162340: Better class stream parsing Summary: Check platform and boot loader for java/* packages Reviewed-by: lfoltan, coleenp, dholmes
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/classfile/classFileParser.hpp
hotspot/src/share/vm/classfile/classLoader.cpp
hotspot/src/share/vm/classfile/klassFactory.cpp
hotspot/src/share/vm/classfile/klassFactory.hpp
hotspot/src/share/vm/classfile/systemDictionary.cpp
hotspot/src/share/vm/classfile/systemDictionary.hpp
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/src/share/vm/oops/instanceKlass.hpp
hotspot/src/share/vm/oops/symbol.hpp
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Wed Jul 27 08:31:48 2016 -0400
@@ -5406,7 +5406,6 @@
                                  Symbol* name,
                                  ClassLoaderData* loader_data,
                                  Handle protection_domain,
-                                 TempNewSymbol* parsed_name,
                                  const Klass* host_klass,
                                  GrowableArray<Handle>* cp_patches,
                                  Publicity pub_level,
@@ -5416,7 +5415,6 @@
   _loader_data(loader_data),
   _host_klass(host_klass),
   _cp_patches(cp_patches),
-  _parsed_name(parsed_name),
   _super_klass(),
   _cp(NULL),
   _fields(NULL),
@@ -5657,15 +5655,6 @@
   Symbol* const class_name_in_cp = cp->klass_name_at(_this_class_index);
   assert(class_name_in_cp != NULL, "class_name can't be null");
 
-  if (_parsed_name != NULL) {
-    // It's important to set parsed_name *before* resolving the super class.
-    // (it's used for cleanup by the caller if parsing fails)
-    *_parsed_name = class_name_in_cp;
-    // parsed_name is returned and can be used if there's an error, so add to
-    // its reference count.  Caller will decrement the refcount.
-    (*_parsed_name)->increment_refcount();
-  }
-
   // Update _class_name which could be null previously
   // to reflect the name in the constant pool
   _class_name = class_name_in_cp;
@@ -5692,6 +5681,10 @@
     return;
   }
 
+  // Verification prevents us from creating names with dots in them, this
+  // asserts that that's the case.
+  assert(is_internal_format(_class_name), "external class name format used internally");
+
   if (!is_internal()) {
     if (log_is_enabled(Debug, class, preorder)){
       ResourceMark rm(THREAD);
@@ -5900,3 +5893,20 @@
 
   return _stream->clone();
 }
+// ----------------------------------------------------------------------------
+// debugging
+
+#ifdef ASSERT
+
+// return true if class_name contains no '.' (internal format is '/')
+bool ClassFileParser::is_internal_format(Symbol* class_name) {
+  if (class_name != NULL) {
+    ResourceMark rm;
+    char* name = class_name->as_C_string();
+    return strchr(name, '.') == NULL;
+  } else {
+    return true;
+  }
+}
+
+#endif
--- a/hotspot/src/share/vm/classfile/classFileParser.hpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classFileParser.hpp	Wed Jul 27 08:31:48 2016 -0400
@@ -81,7 +81,6 @@
   mutable ClassLoaderData* _loader_data;
   const Klass* _host_klass;
   GrowableArray<Handle>* _cp_patches; // overrides for CP entries
-  TempNewSymbol* _parsed_name;
 
   // Metadata created before the instance klass is created.  Must be deallocated
   // if not transferred to the InstanceKlass upon successful class loading
@@ -475,7 +474,6 @@
                   Symbol* name,
                   ClassLoaderData* loader_data,
                   Handle protection_domain,
-                  TempNewSymbol* parsed_name,
                   const Klass* host_klass,
                   GrowableArray<Handle>* cp_patches,
                   Publicity pub_level,
@@ -514,6 +512,11 @@
   bool is_internal() const { return INTERNAL == _pub_level; }
 
   static bool verify_unqualified_name(const char* name, unsigned int length, int type);
+
+#ifdef ASSERT
+  static bool is_internal_format(Symbol* class_name);
+#endif
+
 };
 
 #endif // SHARE_VM_CLASSFILE_CLASSFILEPARSER_HPP
--- a/hotspot/src/share/vm/classfile/classLoader.cpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classLoader.cpp	Wed Jul 27 08:31:48 2016 -0400
@@ -1503,7 +1503,6 @@
                                                                 protection_domain,
                                                                 NULL, // host_klass
                                                                 NULL, // cp_patches
-                                                                NULL, // parsed_name
                                                                 THREAD);
   if (HAS_PENDING_EXCEPTION) {
     if (DumpSharedSpaces) {
--- a/hotspot/src/share/vm/classfile/klassFactory.cpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/klassFactory.cpp	Wed Jul 27 08:31:48 2016 -0400
@@ -96,7 +96,6 @@
                                                      Handle protection_domain,
                                                      const Klass* host_klass,
                                                      GrowableArray<Handle>* cp_patches,
-                                                     TempNewSymbol* parsed_name,
                                                      TRAPS) {
 
   assert(stream != NULL, "invariant");
@@ -123,7 +122,6 @@
                          name,
                          loader_data,
                          protection_domain,
-                         parsed_name,
                          host_klass,
                          cp_patches,
                          ClassFileParser::BROADCAST, // publicity level
--- a/hotspot/src/share/vm/classfile/klassFactory.hpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/klassFactory.hpp	Wed Jul 27 08:31:48 2016 -0400
@@ -1,5 +1,5 @@
 /*
-* Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2015, 2016, 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
@@ -74,7 +74,6 @@
                                                 Handle protection_domain,
                                                 const Klass* host_klass,
                                                 GrowableArray<Handle>* cp_patches,
-                                                TempNewSymbol* parsed_name,
                                                 TRAPS);
 };
 
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp	Wed Jul 27 08:31:48 2016 -0400
@@ -70,7 +70,6 @@
 #include "services/threadService.hpp"
 #include "trace/traceMacros.hpp"
 #include "utilities/macros.hpp"
-#include "utilities/stringUtils.hpp"
 #include "utilities/ticks.hpp"
 #if INCLUDE_CDS
 #include "classfile/sharedClassUtil.hpp"
@@ -140,24 +139,6 @@
 }
 
 // ----------------------------------------------------------------------------
-// debugging
-
-#ifdef ASSERT
-
-// return true if class_name contains no '.' (internal format is '/')
-bool SystemDictionary::is_internal_format(Symbol* class_name) {
-  if (class_name != NULL) {
-    ResourceMark rm;
-    char* name = class_name->as_C_string();
-    return strchr(name, '.') == NULL;
-  } else {
-    return true;
-  }
-}
-
-#endif
-
-// ----------------------------------------------------------------------------
 // Parallel class loading check
 
 bool SystemDictionary::is_parallelCapable(Handle class_loader) {
@@ -335,6 +316,10 @@
 // Must be called, even if superclass is null, since this is
 // where the placeholder entry is created which claims this
 // thread is loading this class/classloader.
+// Be careful when modifying this code: once you have run
+// placeholders()->find_and_add(PlaceholderTable::LOAD_SUPER),
+// you need to find_and_remove it before returning.
+// So be careful to not exit with a CHECK_ macro betweeen these calls.
 Klass* SystemDictionary::resolve_super_or_fail(Symbol* child_name,
                                                  Symbol* class_name,
                                                  Handle class_loader,
@@ -399,6 +384,7 @@
       }
     }
     if (!throw_circularity_error) {
+      // Be careful not to exit resolve_super
       PlaceholderEntry* newprobe = placeholders()->find_and_add(p_index, p_hash, child_name, loader_data, PlaceholderTable::LOAD_SUPER, class_name, THREAD);
     }
   }
@@ -669,7 +655,10 @@
 #endif // INCLUDE_TRACE
 }
 
-
+// Be careful when modifying this code: once you have run
+// placeholders()->find_and_add(PlaceholderTable::LOAD_INSTANCE),
+// you need to find_and_remove it before returning.
+// So be careful to not exit with a CHECK_ macro betweeen these calls.
 Klass* SystemDictionary::resolve_instance_class_or_null(Symbol* name,
                                                         Handle class_loader,
                                                         Handle protection_domain,
@@ -1031,8 +1020,9 @@
 }
 
 // Note: this method is much like resolve_from_stream, but
-// updates no supplemental data structures.
-// TODO consolidate the two methods with a helper routine?
+// does not publish the classes via the SystemDictionary.
+// Handles unsafe_DefineAnonymousClass and redefineclasses
+// RedefinedClasses do not add to the class hierarchy
 Klass* SystemDictionary::parse_stream(Symbol* class_name,
                                       Handle class_loader,
                                       Handle protection_domain,
@@ -1069,8 +1059,7 @@
                                                            protection_domain,
                                                            host_klass,
                                                            cp_patches,
-                                                           NULL, // parsed_name
-                                                           THREAD);
+                                                           CHECK_NULL);
 
   if (host_klass != NULL && k.not_null()) {
     // If it's anonymous, initialize it now, since nobody else will.
@@ -1141,8 +1130,6 @@
   // already be present in the SystemDictionary, otherwise we would not
   // throw potential ClassFormatErrors.
   //
-  // Note: "parsed_name" is updated.
-  TempNewSymbol parsed_name = NULL;
 
  instanceKlassHandle k;
 
@@ -1154,9 +1141,7 @@
                                                  CHECK_NULL);
 #endif
 
-  if (k.not_null()) {
-    parsed_name = k->name();
-  } else {
+  if (k.is_null()) {
     if (st->buffer() == NULL) {
       return NULL;
     }
@@ -1166,64 +1151,28 @@
                                          protection_domain,
                                          NULL, // host_klass
                                          NULL, // cp_patches
-                                         &parsed_name,
-                                         THREAD);
+                                         CHECK_NULL);
   }
 
-  const char* pkg = "java/";
-  if (!HAS_PENDING_EXCEPTION &&
-      !class_loader.is_null() &&
-      !SystemDictionary::is_platform_class_loader(class_loader) &&
-      parsed_name != NULL &&
-      !strncmp((const char*)parsed_name->bytes(), pkg, strlen(pkg))) {
-    // It is illegal to define classes in the "java." package from
-    // JVM_DefineClass or jni_DefineClass unless you're the bootclassloader
-    ResourceMark rm(THREAD);
-    TempNewSymbol pkg_name = InstanceKlass::package_from_name(parsed_name, CHECK_NULL);
-    assert(pkg_name != NULL, "Error in parsing package name starting with 'java/'");
-    char* name = pkg_name->as_C_string();
-    StringUtils::replace_no_expand(name, "/", ".");
-    const char* msg_text = "Prohibited package name: ";
-    size_t len = strlen(msg_text) + strlen(name) + 1;
-    char* message = NEW_RESOURCE_ARRAY(char, len);
-    jio_snprintf(message, len, "%s%s", msg_text, name);
-    Exceptions::_throw_msg(THREAD_AND_LOCATION,
-      vmSymbols::java_lang_SecurityException(), message);
-  }
+  assert(k.not_null(), "no klass created");
+  Symbol* h_name = k->name();
+  assert(class_name == NULL || class_name == h_name, "name mismatch");
 
-  if (!HAS_PENDING_EXCEPTION) {
-    assert(parsed_name != NULL, "Sanity");
-    assert(class_name == NULL || class_name == parsed_name, "name mismatch");
-    // Verification prevents us from creating names with dots in them, this
-    // asserts that that's the case.
-    assert(is_internal_format(parsed_name),
-           "external class name format used internally");
-
-    // Add class just loaded
-    // If a class loader supports parallel classloading handle parallel define requests
-    // find_or_define_instance_class may return a different InstanceKlass
-    if (is_parallelCapable(class_loader)) {
-      k = find_or_define_instance_class(class_name, class_loader, k, THREAD);
-    } else {
-      define_instance_class(k, THREAD);
-    }
+  // Add class just loaded
+  // If a class loader supports parallel classloading handle parallel define requests
+  // find_or_define_instance_class may return a different InstanceKlass
+  if (is_parallelCapable(class_loader)) {
+    k = find_or_define_instance_class(h_name, class_loader, k, CHECK_NULL);
+  } else {
+    define_instance_class(k, CHECK_NULL);
   }
 
   // Make sure we have an entry in the SystemDictionary on success
   debug_only( {
-    if (!HAS_PENDING_EXCEPTION) {
-      assert(parsed_name != NULL, "parsed_name is still null?");
-      Symbol*  h_name    = k->name();
-      ClassLoaderData *defining_loader_data = k->class_loader_data();
-
-      MutexLocker mu(SystemDictionary_lock, THREAD);
+    MutexLocker mu(SystemDictionary_lock, THREAD);
 
-      Klass* check = find_class(parsed_name, loader_data);
-      assert(check == k(), "should be present in the dictionary");
-
-      Klass* check2 = find_class(h_name, defining_loader_data);
-      assert(check == check2, "name inconsistancy in SystemDictionary");
-    }
+    Klass* check = find_class(h_name, k->class_loader_data());
+    assert(check == k(), "should be present in the dictionary");
   } );
 
   return k();
@@ -1425,6 +1374,8 @@
       Handle lockObject = compute_loader_lock_object(class_loader, THREAD);
       check_loader_lock_contention(lockObject, THREAD);
       ObjectLocker ol(lockObject, THREAD, true);
+      // prohibited package check assumes all classes loaded from archive call
+      // restore_unshareable_info which calls ik->set_package()
       ik->restore_unshareable_info(loader_data, protection_domain, CHECK_(nh));
     }
 
@@ -1710,6 +1661,10 @@
 // findClass(), i.e. FindLoadedClass/DefineClassIfAbsent or they
 // potentially waste time reading and parsing the bytestream.
 // Note: VM callers should ensure consistency of k/class_name,class_loader
+// Be careful when modifying this code: once you have run
+// placeholders()->find_and_add(PlaceholderTable::DEFINE_CLASS),
+// you need to find_and_remove it before returning.
+// So be careful to not exit with a CHECK_ macro betweeen these calls.
 instanceKlassHandle SystemDictionary::find_or_define_instance_class(Symbol* class_name, Handle class_loader, instanceKlassHandle k, TRAPS) {
 
   instanceKlassHandle nh = instanceKlassHandle(); // null Handle
--- a/hotspot/src/share/vm/classfile/systemDictionary.hpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/systemDictionary.hpp	Wed Jul 27 08:31:48 2016 -0400
@@ -281,6 +281,7 @@
 
   // Parse new stream. This won't update the system dictionary or
   // class hierarchy, simply parse the stream. Used by JVMTI RedefineClasses.
+  // Also used by Unsafe_DefineAnonymousClass
   static Klass* parse_stream(Symbol* class_name,
                              Handle class_loader,
                              Handle protection_domain,
@@ -413,10 +414,6 @@
   // Verification
   static void verify();
 
-#ifdef ASSERT
-  static bool is_internal_format(Symbol* class_name);
-#endif
-
   // Initialization
   static void initialize(TRAPS);
 
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Wed Jul 27 08:31:48 2016 -0400
@@ -67,6 +67,7 @@
 #include "services/threadService.hpp"
 #include "utilities/dtrace.hpp"
 #include "utilities/macros.hpp"
+#include "utilities/stringUtils.hpp"
 #include "logging/log.hpp"
 #ifdef COMPILER1
 #include "c1/c1_Compiler.hpp"
@@ -2225,9 +2226,14 @@
 }
 
 void InstanceKlass::set_package(ClassLoaderData* loader_data, TRAPS) {
+
+  // ensure java/ packages only loaded by boot or platform builtin loaders
+  check_prohibited_package(name(), loader_data->class_loader(), CHECK);
+
   TempNewSymbol pkg_name = package_from_name(name(), CHECK);
 
   if (pkg_name != NULL && loader_data != NULL) {
+
     // Find in class loader's package entry table.
     _package_entry = loader_data->packages()->lookup_only(pkg_name);
 
@@ -2376,6 +2382,31 @@
 }
 */
 
+// Only boot and platform class loaders can define classes in "java/" packages.
+void InstanceKlass::check_prohibited_package(Symbol* class_name,
+                                                Handle class_loader,
+                                                TRAPS) {
+  const char* javapkg = "java/";
+  ResourceMark rm(THREAD);
+  if (!class_loader.is_null() &&
+      !SystemDictionary::is_platform_class_loader(class_loader) &&
+      class_name != NULL &&
+      strncmp(class_name->as_C_string(), javapkg, strlen(javapkg)) == 0) {
+    TempNewSymbol pkg_name = InstanceKlass::package_from_name(class_name, CHECK);
+    assert(pkg_name != NULL, "Error in parsing package name starting with 'java/'");
+    char* name = pkg_name->as_C_string();
+    const char* class_loader_name = InstanceKlass::cast(class_loader()->klass())->name()->as_C_string();
+    StringUtils::replace_no_expand(name, "/", ".");
+    const char* msg_text1 = "Class loader (instance of): ";
+    const char* msg_text2 = " tried to load prohibited package name: ";
+    size_t len = strlen(msg_text1) + strlen(class_loader_name) + strlen(msg_text2) + strlen(name) + 1;
+    char* message = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char, len);
+    jio_snprintf(message, len, "%s%s%s%s", msg_text1, class_loader_name, msg_text2, name);
+    THROW_MSG(vmSymbols::java_lang_SecurityException(), message);
+  }
+  return;
+}
+
 // tell if two classes have the same enclosing class (at package level)
 bool InstanceKlass::is_same_package_member_impl(const InstanceKlass* class1,
                                                 const Klass* class2,
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp	Wed Jul 27 08:31:48 2016 -0400
@@ -470,6 +470,12 @@
   static bool find_inner_classes_attr(instanceKlassHandle k,
                                       int* ooff, int* noff, TRAPS);
 
+ private:
+  // Check prohibited package ("java/" only loadable by boot or platform loaders)
+  static void check_prohibited_package(Symbol* class_name,
+                                       Handle class_loader,
+                                       TRAPS);
+ public:
   // tell if two classes have the same enclosing class (at package level)
   bool is_same_package_member(const Klass* class2, TRAPS) const {
     return is_same_package_member_impl(this, class2, THREAD);
--- a/hotspot/src/share/vm/oops/symbol.hpp	Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/oops/symbol.hpp	Wed Jul 27 08:31:48 2016 -0400
@@ -91,12 +91,6 @@
 // The allocation (or lookup) of K increments the reference count for K
 // and the destructor decrements the reference count.
 //
-// Another example of TempNewSymbol usage is parsed_name used in
-// ClassFileParser::parseClassFile() where parsed_name is used in the cleanup
-// after a failed attempt to load a class.  Here parsed_name is a
-// TempNewSymbol (passed in as a parameter) so the reference count on its symbol
-// will be decremented when it goes out of scope.
-
 // This cannot be inherited from ResourceObj because it cannot have a vtable.
 // Since sometimes this is allocated from Metadata, pick a base allocation
 // type without virtual functions.