RFR(XXS): 8207913: +EnableTracing crashes on jdk8u

[David Holmes]

Hi,

On 21/08/2018 5:14 PM, Liu Xin wrote:
> 
> hi, Dave,
> 
> Thank you reviewing it. Indeed, I intend to submit to jdk8u-dev.
> 
> I backtrace the change. here is the original bug.
> https://bugs.openjdk.java.net/browse/JDK-8032250

Okay. So what you're actually looking for is a backport of this bug 
fixed in 9:

"8145788: JVM crashes with -XX:+EnableTracing"

https://bugs.openjdk.java.net/browse/JDK-8145788

Can you try the patch in 8u and see if that fixes the issue:

http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/2abdc525b6b4

Thanks,
David


> I found a person mentioned -XX:+EnableTracing was broken for OpenJdk8u 
> in 2014. He said jdk7 is okay.
> http://hllvm.group.iteye.com/group/topic/42367
> 
> You are right. I found some events do need ResourceMark. It's because 
> they need to use NEW_RESOURCE_ARRAY in TraceStream. Actually, only 2 
> cases really need to NEW_RESOURCE_ARRAY, Klass * and Method*.
> 
> Is that okay that I use a fixed buffer on stack here?  By using it, I 
> can safely eliminate the ResourceMark in traceEventClasses.cpp.
> 
> diff --git a/src/hotspot/src/share/vm/trace/traceStream.hpp 
> b/src/hotspot/src/share/vm/trace/traceStream.hpp
> index 89911ea2..8d4a732f 100644
> --- a/src/hotspot/src/share/vm/trace/traceStream.hpp
> +++ b/src/hotspot/src/share/vm/trace/traceStream.hpp
> @@ -32,6 +32,8 @@
>   #include "oops/symbol.hpp"
>   #include "utilities/ostream.hpp"
> 
> +const int MAX_LOCAL_BUFFER = 256;
> +
>   class TraceStream : public StackObj {
>    private:
>     outputStream& _st;
> @@ -79,29 +81,24 @@ class TraceStream : public StackObj {
>       _st.print("%s = %f", label, val);
>     }
> 
> -  // Caller is machine generated code located in traceEventClasses.hpp
> -  // Event<TraceId>::writeEvent() (pseudocode) contains the
> -  // necessary ResourceMark for the resource allocations below.
> -  // See traceEventClasses.xsl for details.
>     void print_val(const char* label, const Klass* const val) {
> +    char buf[MAX_LOCAL_BUFFER];
>       const char* description = "NULL";
> +
>       if (val != NULL) {
>         Symbol* name = val->name();
>         if (name != NULL) {
> -        description = name->as_C_string();
> +        description = name->as_C_string(buf, MAX_LOCAL_BUFFER);
>         }
>       }
>       _st.print("%s = %s", label, description);
>     }
> 
> -  // Caller is machine generated code located in traceEventClasses.hpp
> -  // Event<TraceId>::writeEvent() (pseudocode) contains the
> -  // necessary ResourceMark for the resource allocations below.
> -  // See traceEventClasses.xsl for details.
>     void print_val(const char* label, const Method* const val) {
> +    char buf[MAX_LOCAL_BUFFER];
>       const char* description = "NULL";
>       if (val != NULL) {
> -      description = val->name_and_sig_as_C_string();
> +      description = val->name_and_sig_as_C_string(buf, MAX_LOCAL_BUFFER);
>       }
>       _st.print("%s = %s", label, description);
>     }
> thanks,
> --lx
> 
> 
> 
> 
> On Mon, Aug 20, 2018 at 7:04 PM David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     Hi,
> 
>     I dropped jdk8-dev at openjdk.java.net
>     <mailto:jdk8-dev at openjdk.java.net> and added
>     jdk8u-dev at openjdk.java.net <mailto:jdk8u-dev at openjdk.java.net> to
>     the cc line.
> 
>     On 21/08/2018 8:38 AM, Liu, Xin wrote:
>      > Hi, Runtime develop,
>      >
>      > Could you review this change basing on jdk8u?
>      > Problem: https://bugs.openjdk.java.net/browse/JDK-8207913
>      > Webrev: http://cr.openjdk.java.net/~phh/8207913/webrev.01/
>     <http://cr.openjdk.java.net/%7Ephh/8207913/webrev.01/>
>      >
>      > JDK8u has this problem because Thread::current() is NULL when it
>     handles global arguments at startup. OracleJDK8 doesn’t suffer from
>     this issue.
>      > I found jdk12’s counterpart jfrEventClasses.hpp doesn’t have
>     ResourceMark.  Removing it works for both with/without ttyLocker.
> 
>     The ResourceMark has been there since the file was introduced in JDK 8.
>     What has changed to cause this problem and when? If we remove the RM
>     then we have to be sure no events that get traced will in fact rely on
>     there being a RM present.
> 
>      > We will check in jdk8u/hotspot if reviewers approve it.
> 
>     There is a specific approval process that has to be followed after the
>     change has been Reviewed.
> 
>     http://openjdk.java.net/projects/jdk8u/approval-template.html
> 
>     Thanks,
>     David
> 
>      > Thanks,
>      > --lx
>      >
>