RFR: [8u] JDK-6424123: "JVM crashes on failed 'strdup' call"

[Thomas Stüfe]

Hi Andrew,

some small issues (not a 8u reviewer though):

----

os_aix.cpp:
   size_t data_page_size = SIZE_4K;
   {
-    void* p = ::malloc(SIZE_16M);
+    void* p = os::malloc(SIZE_16M, mtInternal);
     guarantee(p != NULL, "malloc failed");
     data_page_size = os::Aix::query_pagesize(p);
-    ::free(p);
+    os::free(p);
   }

This seems wrong. We should explicitly use raw malloc here.

We do so in head. I see that we rolled this part back with 8075505 which
was a larger change revamping AIX memory handling. I would leave this part
out.

----

 #ifdef SPARC
-          _masm->_verify_oop(r->as_Register(), strdup(st.as_string()),
__FILE__, __LINE__);
+          _masm->_verify_oop(r->as_Register(), os::strdup(st.as_string(),
mtCompiler), __FILE__, __LINE__);
 #else

This seems to be a memory leak, but it has been one before that change.

---

classLoader.cpp

@@ -536,11 +540,11 @@
         }

         default:
         {
           if (!skipCurrentJar && cur_entry != NULL) {
-            char* new_name = strdup(package_name);
+            char* new_name = os::strdup_check_oom(package_name);
             boot_class_path_packages.append(new_name);
           }
         }
       }
     }

I believe this leaks too, and did so before.

---

share/vm/compiler/compilerOracle.cpp

@@ -217,11 +218,11 @@
                            Symbol* method_name, Mode method_mode,
                            Symbol* signature, const char* opt,
                            const T value,  MethodMatcher* next) :
     MethodMatcher(class_name, class_mode, method_name, method_mode,
signature, next),
                   _type(get_type_for<T>()), _value(copy_value<T>(value)) {
-    _option = strdup(opt);
+    _option = os::strdup_check_oom(opt);
   }

   ~TypedMethodOptionMatcher() {
     free((void*)_option);
   }

Unmatched free. free must be os::free.

---

Cheers, Thomas

On Thu, Apr 2, 2020 at 6:47 AM Andrew Hughes <gnu.andrew at redhat.com> wrote:

> Bug: https://bugs.openjdk.java.net/browse/JDK-6424123
> Webrev: https://cr.openjdk.java.net/~andrew/openjdk8/6424123/webrev.01
>
> This patch cleans up some cases where strings aren't freed and also
> introduces strdup_check_oom to more elegantly handle cases where strdup
> fails.  That function is later used by JDK-8076475 [0].
>
> The differences in the backport are mainly due to later changes that
> have already been backported to 8u:
>
> * src/cpu/ppc/vm/vm_version_ppc.cpp
>   - Context changes due to later updates to the supported feature set
>
> * src/cpu/sparc/vm/vm_version_sparc.cpp
>   - Same as for vm_version_ppc.cpp
>
> * src/cpu/x86/vm/vm_version_x86.cpp
>   - Same again
>
> * src/os/aix/vm/porting_aix.cpp
>   - Header context is different as 8u doesn't import allocation.hpp
>
> * src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp
>   - Changes dropped as code was completely refactored by JDK-8134119 and
> now uses os::malloc & os::free already
>
> * src/share/vm/classfile/classLoader.cpp
>   - Context of signature of LazyClassPathEntry::LazyClassPathEntry was
> different, due to introduction of throw_exception. Changes to
> setup_bootstrap_search_path were dropped as JDK-8056971 changed
> sys_class_path to be used read only and not duplicated.
>
> * src/share/vm/classfile/classLoader.hpp
>   - Change to LazyClassPathEntry::_path (char* -> const char*) in
> JDK-8056971 had to be reverted so it could be passed to os::free. Some
> context differences due to changes in class methods (addition of
> open_entry).
>
> * src/share/vm/compiler/compilerOracle.cpp
>   - strdup is already present, thanks to JDK-8055286, so is replaced
> with strdup_check_oom. Destructor and removal of strdup in call to
> add_option_string are also already present, thank to that change.
>
> * src/share/vm/runtime/arguments.cpp
>   - Some context differences due to process_java_launcher_argument
> having a special case for "gamma" from JDK-7022037. This was removed by
> JDK-8027113 in OpenJDK 9.
>
> Ok for 8u?
>
> [0]
> https://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-April/011514.html
>
> Thanks,
> --
> Andrew :)
>
> Senior Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
>
>