RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Jini George
Hi all,

Requesting reviews for: https://bugs.openjdk.java.net/browse/JDK-8184042

Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/

Problem gist: The deprecated ptrace() command, PT_ATTACH was changed to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals) to be delivered via mach messages.
This caused SA to hang at waitpid() waiting for a signal, which does not arrive.

Solution in a nutshell: The solution is to make the required changes to handle mach 'soft signal' exceptions in the form of mach messages instead of signals, while attaching to and detaching from the target process. The detailed steps are outlined in JBS.

The changes appear huge due to the inclusion of pre-generated mach exception handling files (mach_exc*). Since this is an integration blocker, it would be great to get quick reviews on this.

Thank you,
Jini.



 


Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

David Holmes
Hi Jini,

Just reading the bug report and your description below this seems like a
major change to try and use a facility (mach exceptions) that no one
seems to have any experience with! That isn't something to be rushed.
Even if PT_ATTACH has been deprecated restoring its use may be the quick
way forward instead of trying to rush in something like this.

Just my 2c.

Cheers,
David

On 18/08/2017 8:00 PM, Jini George wrote:

> Hi all,
>
> Requesting reviews for: https://bugs.openjdk.java.net/browse/JDK-8184042
>
> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>
> Problem gist: The deprecated ptrace() command, PT_ATTACH was changed to
> PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals) to be
> delivered via mach messages.This caused SA to hang at waitpid() waiting
> for a signal, which does not arrive.
>
> Solution in a nutshell: The solution is to make the required changes to
> handle mach 'soft signal' exceptions in the form of mach messages
> instead of signals, while attaching to and detaching from the target
> process. The detailed steps are outlined in JBS.
>
> The changes appear huge due to the inclusion of pre-generated mach
> exception handling files (mach_exc*). Since this is an integration
> blocker, it would be great to get quick reviews on this.
>
> Thank you,
> Jini.
>
>
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Jini George
Thank you, David. Makes sense to not rush through the review. Since this
is not an integration blocker now, I will wait for the reviews for this
fix to go through, and not revert to PT_ATTACH (which causes the
build-time deprecation warnings).

Regards,
Jini.


On 8/18/2017 4:30 PM, David Holmes wrote:

> Hi Jini,
>
> Just reading the bug report and your description below this seems like
> a major change to try and use a facility (mach exceptions) that no one
> seems to have any experience with! That isn't something to be rushed.
> Even if PT_ATTACH has been deprecated restoring its use may be the
> quick way forward instead of trying to rush in something like this.
>
> Just my 2c.
>
> Cheers,
> David
>
> On 18/08/2017 8:00 PM, Jini George wrote:
>> Hi all,
>>
>> Requesting reviews for: https://bugs.openjdk.java.net/browse/JDK-8184042
>>
>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>
>> Problem gist: The deprecated ptrace() command, PT_ATTACH was changed
>> to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals)
>> to be delivered via mach messages.This caused SA to hang at waitpid()
>> waiting for a signal, which does not arrive.
>>
>> Solution in a nutshell: The solution is to make the required changes
>> to handle mach 'soft signal' exceptions in the form of mach messages
>> instead of signals, while attaching to and detaching from the target
>> process. The detailed steps are outlined in JBS.
>>
>> The changes appear huge due to the inclusion of pre-generated mach
>> exception handling files (mach_exc*). Since this is an integration
>> blocker, it would be great to get quick reviews on this.
>>
>> Thank you,
>> Jini.
>>
>>
>>
>>
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

serguei.spitsyn@oracle.com
In reply to this post by David Holmes
Hi Jini,


On 8/18/17 04:00, David Holmes wrote:
> Hi Jini,
>
> Just reading the bug report and your description below this seems like
> a major change to try and use a facility (mach exceptions) that no one
> seems to have any experience with! That isn't something to be rushed.

> Even if PT_ATTACH has been deprecated restoring its use may be the
> quick way forward instead of trying to rush in something like this.

This approach looks reasonable to me.
Otherwise, it would be nice to hear why it is not good.
How much would it break the fix of the JDK-8182299?

Thanks,
Serguei

>
> Just my 2c.
>
> Cheers,
> David
>
> On 18/08/2017 8:00 PM, Jini George wrote:
>> Hi all,
>>
>> Requesting reviews for: https://bugs.openjdk.java.net/browse/JDK-8184042
>>
>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>
>> Problem gist: The deprecated ptrace() command, PT_ATTACH was changed
>> to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals)
>> to be delivered via mach messages.This caused SA to hang at waitpid()
>> waiting for a signal, which does not arrive.
>>
>> Solution in a nutshell: The solution is to make the required changes
>> to handle mach 'soft signal' exceptions in the form of mach messages
>> instead of signals, while attaching to and detaching from the target
>> process. The detailed steps are outlined in JBS.
>>
>> The changes appear huge due to the inclusion of pre-generated mach
>> exception handling files (mach_exc*). Since this is an integration
>> blocker, it would be great to get quick reviews on this.
>>
>> Thank you,
>> Jini.
>>
>>
>>
>>
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Dmitry Samersoff-2
Jini,

1.
As a quick fix we probably can disable deprecation warning, i.e. add
something like

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
...
#pragma clang diagnostic pop

around offending code.

2.
As a long term your fix looks reasonable for me, but I would prefer to
generate exception stub during a build (i.e. add mig to the build
system) rather than check-in pre-generated code.

i.e. It might be better to fix JDK-8184042 and JDK-8186427 in one shot.

-Dmitry

On 25.08.2017 12:24, [hidden email] wrote:

> Hi Jini,
>
>
> On 8/18/17 04:00, David Holmes wrote:
>> Hi Jini,
>>
>> Just reading the bug report and your description below this seems like
>> a major change to try and use a facility (mach exceptions) that no one
>> seems to have any experience with! That isn't something to be rushed.
>
>> Even if PT_ATTACH has been deprecated restoring its use may be the
>> quick way forward instead of trying to rush in something like this.
>
> This approach looks reasonable to me.
> Otherwise, it would be nice to hear why it is not good.
> How much would it break the fix of the JDK-8182299?
>
> Thanks,
> Serguei
>
>>
>> Just my 2c.
>>
>> Cheers,
>> David
>>
>> On 18/08/2017 8:00 PM, Jini George wrote:
>>> Hi all,
>>>
>>> Requesting reviews for: https://bugs.openjdk.java.net/browse/JDK-8184042
>>>
>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>
>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was changed
>>> to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals)
>>> to be delivered via mach messages.This caused SA to hang at waitpid()
>>> waiting for a signal, which does not arrive.
>>>
>>> Solution in a nutshell: The solution is to make the required changes
>>> to handle mach 'soft signal' exceptions in the form of mach messages
>>> instead of signals, while attaching to and detaching from the target
>>> process. The detailed steps are outlined in JBS.
>>>
>>> The changes appear huge due to the inclusion of pre-generated mach
>>> exception handling files (mach_exc*). Since this is an integration
>>> blocker, it would be great to get quick reviews on this.
>>>
>>> Thank you,
>>> Jini.
>>>
>>>
>>>
>>>
>>>
>>>
>


signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

serguei.spitsyn@oracle.com
In reply to this post by serguei.spitsyn@oracle.com
On 8/25/17 02:24, [hidden email] wrote:

> Hi Jini,
>
>
> On 8/18/17 04:00, David Holmes wrote:
>> Hi Jini,
>>
>> Just reading the bug report and your description below this seems
>> like a major change to try and use a facility (mach exceptions) that
>> no one seems to have any experience with! That isn't something to be
>> rushed.
>
>> Even if PT_ATTACH has been deprecated restoring its use may be the
>> quick way forward instead of trying to rush in something like this.
>
> This approach looks reasonable to me.

I've just realized that my statement might sound incorrectly.
I meant that the David's suggestion to restore the use of the deprecated
PT_ATTACH looks reasonable.

Sorry, if it caused any confusion.

Thanks,
Serguei


> Otherwise, it would be nice to hear why it is not good.
> How much would it break the fix of the JDK-8182299?
>
> Thanks,
> Serguei
>
>>
>> Just my 2c.
>>
>> Cheers,
>> David
>>
>> On 18/08/2017 8:00 PM, Jini George wrote:
>>> Hi all,
>>>
>>> Requesting reviews for:
>>> https://bugs.openjdk.java.net/browse/JDK-8184042
>>>
>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>
>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was changed
>>> to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals)
>>> to be delivered via mach messages.This caused SA to hang at
>>> waitpid() waiting for a signal, which does not arrive.
>>>
>>> Solution in a nutshell: The solution is to make the required changes
>>> to handle mach 'soft signal' exceptions in the form of mach messages
>>> instead of signals, while attaching to and detaching from the target
>>> process. The detailed steps are outlined in JBS.
>>>
>>> The changes appear huge due to the inclusion of pre-generated mach
>>> exception handling files (mach_exc*). Since this is an integration
>>> blocker, it would be great to get quick reviews on this.
>>>
>>> Thank you,
>>> Jini.
>>>
>>>
>>>
>>>
>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

serguei.spitsyn@oracle.com
On 9/7/17 14:39, [hidden email] wrote:

> On 8/25/17 02:24, [hidden email] wrote:
>> Hi Jini,
>>
>>
>> On 8/18/17 04:00, David Holmes wrote:
>>> Hi Jini,
>>>
>>> Just reading the bug report and your description below this seems
>>> like a major change to try and use a facility (mach exceptions) that
>>> no one seems to have any experience with! That isn't something to be
>>> rushed.
>>
>>> Even if PT_ATTACH has been deprecated restoring its use may be the
>>> quick way forward instead of trying to rush in something like this.
>>
>> This approach looks reasonable to me.
>
> I've just realized that my statement might sound incorrectly.
> I meant that the David's suggestion to restore the use of the
> deprecated PT_ATTACH looks reasonable.

Forgot to send a link.
Not sure, if an approach of replacing ptrace usage could be reasonable:
   http://uninformed.org/index.cgi?v=4&a=3&p=14


Thanks,
Serguei


>
> Sorry, if it caused any confusion.
>
> Thanks,
> Serguei
>
>
>> Otherwise, it would be nice to hear why it is not good.
>> How much would it break the fix of the JDK-8182299?
>>
>> Thanks,
>> Serguei
>>
>>>
>>> Just my 2c.
>>>
>>> Cheers,
>>> David
>>>
>>> On 18/08/2017 8:00 PM, Jini George wrote:
>>>> Hi all,
>>>>
>>>> Requesting reviews for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8184042
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>>
>>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was
>>>> changed to PT_ATTACHEXC, which causes mach exceptions (and not UNIX
>>>> signals) to be delivered via mach messages.This caused SA to hang
>>>> at waitpid() waiting for a signal, which does not arrive.
>>>>
>>>> Solution in a nutshell: The solution is to make the required
>>>> changes to handle mach 'soft signal' exceptions in the form of mach
>>>> messages instead of signals, while attaching to and detaching from
>>>> the target process. The detailed steps are outlined in JBS.
>>>>
>>>> The changes appear huge due to the inclusion of pre-generated mach
>>>> exception handling files (mach_exc*). Since this is an integration
>>>> blocker, it would be great to get quick reviews on this.
>>>>
>>>> Thank you,
>>>> Jini.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Jini George
Hi Serguei, David,

Thank you very much for the link and your responses. I agree that the
restoration of PT_ATTACH would have been a good very-short term solution
if it were an integration blocker. But given that the restoration would
be more or less a hack since PT_ATTACH has been deprecated and replaced
by PT_ATTACHEXC, and since we have time, I think the right solution is
to move to PT_ATTACHEXC. Sooner or later, we would have to do the switch
anyways. Here is a snippet from the man page of ptrace() from Mac OSX
version 10.11.6.


      PT_ATTACH     This call has been replaced with PT_ATTACHEXC.

      PT_ATTACHEXC  This request allows a process to gain control of an
otherwise unrelated process and begin
                    tracing it.  It does not need any cooperation from
the to-be-traced process.  In this case,
                    pid specifies the process ID of the to-be-traced
process, and the other two arguments are
                    ignored.  This request requires that the target
process must have the same real UID as the
                    tracing process, and that it must not be executing a
setuid or setgid executable.  (If the
                    tracing process is running as root, these
restrictions do not apply.)  The tracing process
                    will see the newly-traced process stop and may then
control it as if it had been traced all
                    along. Note that this call differs from the prior
call ( PT_ATTACH) in that signals from
                    the child are delivered to the parent as Mach
exceptions (see EXC_SOFT_SIGNAL).


Moreover other debuggers like gdb and lldb have switched to using
PT_ATTACHEXC. As pointed out in the link also, ptrace() on Apple has
always been bare-bones. We need to supplement it with acquiring port
rights to an exception port and waiting on that port where we can
receive the Mach exception messages. My modifications are along the
above lines.

Thank you,
Jini.

On 9/8/2017 3:33 AM, [hidden email] wrote:

> On 9/7/17 14:39, [hidden email] wrote:
>> On 8/25/17 02:24, [hidden email] wrote:
>>> Hi Jini,
>>>
>>>
>>> On 8/18/17 04:00, David Holmes wrote:
>>>> Hi Jini,
>>>>
>>>> Just reading the bug report and your description below this seems
>>>> like a major change to try and use a facility (mach exceptions)
>>>> that no one seems to have any experience with! That isn't something
>>>> to be rushed.
>>>
>>>> Even if PT_ATTACH has been deprecated restoring its use may be the
>>>> quick way forward instead of trying to rush in something like this.
>>>
>>> This approach looks reasonable to me.
>>
>> I've just realized that my statement might sound incorrectly.
>> I meant that the David's suggestion to restore the use of the
>> deprecated PT_ATTACH looks reasonable.
>
> Forgot to send a link.
> Not sure, if an approach of replacing ptrace usage could be reasonable:
>   http://uninformed.org/index.cgi?v=4&a=3&p=14
>
>
> Thanks,
> Serguei
>
>
>>
>> Sorry, if it caused any confusion.
>>
>> Thanks,
>> Serguei
>>
>>
>>> Otherwise, it would be nice to hear why it is not good.
>>> How much would it break the fix of the JDK-8182299?
>>>
>>> Thanks,
>>> Serguei
>>>
>>>>
>>>> Just my 2c.
>>>>
>>>> Cheers,
>>>> David
>>>>
>>>> On 18/08/2017 8:00 PM, Jini George wrote:
>>>>> Hi all,
>>>>>
>>>>> Requesting reviews for:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8184042
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>>>
>>>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was
>>>>> changed to PT_ATTACHEXC, which causes mach exceptions (and not
>>>>> UNIX signals) to be delivered via mach messages.This caused SA to
>>>>> hang at waitpid() waiting for a signal, which does not arrive.
>>>>>
>>>>> Solution in a nutshell: The solution is to make the required
>>>>> changes to handle mach 'soft signal' exceptions in the form of
>>>>> mach messages instead of signals, while attaching to and detaching
>>>>> from the target process. The detailed steps are outlined in JBS.
>>>>>
>>>>> The changes appear huge due to the inclusion of pre-generated mach
>>>>> exception handling files (mach_exc*). Since this is an integration
>>>>> blocker, it would be great to get quick reviews on this.
>>>>>
>>>>> Thank you,
>>>>> Jini.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Daniel D. Daugherty
In reply to this post by Jini George
On 8/18/17 4:00 AM, Jini George wrote:
Hi all,
 
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m
    L832:   result = mach_port_insert_right (mach_task_self(),
        Nit - extra blank before '('

    L860-863 - nit - indents don't match the other func calls.

    I read through all the changes and looked at logic flow, error
    returns, variable usage, etc. The problem is I'm not conversant
    in Mach exception programming so there could easily be things
    that I missed here.

    You really need to have a MacOS X person look at these changes
    also.

hotspot/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/BsdDebuggerLocal.java
    No comments.

hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_exc.h
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excServer.c
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excUser.c
    These files are generated by a tool and will be replaced
    by build changes. I didn't review them.

hotspot/test/serviceability/sa/TestAttachDetach.java
    L35: import java.util.ArrayList;
    L36: import java.util.List;
    L37: import java.util.Arrays;
    L38: import java.util.Optional;
    L40: import jdk.test.lib.Utils;
    L41: import jdk.test.lib.process.OutputAnalyzer;
    L42: import jdk.test.lib.process.ProcessTools;
        These imports do not seem to be needed.

    L45: import jdk.test.lib.JDKToolLauncher;
        Duplicate import.

    L98:                             "The String 'Attaching to process' appears " +
    L99:                              numberOfMatches + " number of times");
        Would be helpful to include the expected value also.

jdk/test/ProblemList.txt
    Thanks for remembering to update the ProblemList.txt file!

I don't have any comments other than nits. However, I don't
know mach exceptions so I can't really do a functional review.

We need someone that knows MacOS X mach exceptions to chime
in on this thread. I'm adding Gerard Z to this review thread
in the hopes that he can help...

Dan



Problem gist: The deprecated ptrace() command, PT_ATTACH was changed to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals) to be delivered via mach messages.
This caused SA to hang at waitpid() waiting for a signal, which does not arrive.

Solution in a nutshell: The solution is to make the required changes to handle mach 'soft signal' exceptions in the form of mach messages instead of signals, while attaching to and detaching from the target process. The detailed steps are outlined in JBS.

The changes appear huge due to the inclusion of pre-generated mach exception handling files (mach_exc*). Since this is an integration blocker, it would be great to get quick reviews on this.

Thank you,
Jini.



 



Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Jini George
Thanks much, Dan.

-Jini.

On 9/15/2017 4:18 AM, Daniel D. Daugherty wrote:
On 8/18/17 4:00 AM, Jini George wrote:
Hi all,
 
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m
    L832:   result = mach_port_insert_right (mach_task_self(),
        Nit - extra blank before '('

    L860-863 - nit - indents don't match the other func calls.

    I read through all the changes and looked at logic flow, error
    returns, variable usage, etc. The problem is I'm not conversant
    in Mach exception programming so there could easily be things
    that I missed here.

    You really need to have a MacOS X person look at these changes
    also.

hotspot/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/BsdDebuggerLocal.java
    No comments.

hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_exc.h
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excServer.c
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excUser.c
    These files are generated by a tool and will be replaced
    by build changes. I didn't review them.

hotspot/test/serviceability/sa/TestAttachDetach.java
    L35: import java.util.ArrayList;
    L36: import java.util.List;
    L37: import java.util.Arrays;
    L38: import java.util.Optional;
    L40: import jdk.test.lib.Utils;
    L41: import jdk.test.lib.process.OutputAnalyzer;
    L42: import jdk.test.lib.process.ProcessTools;
        These imports do not seem to be needed.

    L45: import jdk.test.lib.JDKToolLauncher;
        Duplicate import.

    L98:                             "The String 'Attaching to process' appears " +
    L99:                              numberOfMatches + " number of times");
        Would be helpful to include the expected value also.

jdk/test/ProblemList.txt
    Thanks for remembering to update the ProblemList.txt file!

I don't have any comments other than nits. However, I don't
know mach exceptions so I can't really do a functional review.

We need someone that knows MacOS X mach exceptions to chime
in on this thread. I'm adding Gerard Z to this review thread
in the hopes that he can help...

Dan



Problem gist: The deprecated ptrace() command, PT_ATTACH was changed to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals) to be delivered via mach messages.
This caused SA to hang at waitpid() waiting for a signal, which does not arrive.

Solution in a nutshell: The solution is to make the required changes to handle mach 'soft signal' exceptions in the form of mach messages instead of signals, while attaching to and detaching from the target process. The detailed steps are outlined in JBS.

The changes appear huge due to the inclusion of pre-generated mach exception handling files (mach_exc*). Since this is an integration blocker, it would be great to get quick reviews on this.

Thank you,
Jini.



 




Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Gerard Ziemski
In reply to this post by Daniel D. Daugherty
Sorry, I’m not familiar with mach exceptions.


> On Sep 14, 2017, at 5:48 PM, Daniel D. Daugherty <[hidden email]> wrote:
>
> We need someone that knows MacOS X mach exceptions to chime
> in on this thread. I'm adding Gerard Z to this review thread
> in the hopes that he can help...

Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Poonam Parhar
In reply to this post by Jini George
Hello Jini,

I don't know much about the mach exceptions either. I read through your wiki page and looked at the relevant gdb and lldb code to understand how it works.

A few points regarding the changes for this CR:

File: src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m

1. In the lldb code, a separate thread is being used to catch and handle the mach exceptions. I am assuming we don't need to do that since we use the mach exceptions mechanism only during the attach process, and after which we suspend all the threads in the target process. Is that correct?

2.
 823   if (result != KERN_SUCCESS) {
 824     print_error("attach: mach_port_allocate() failed: '%s' (%d)\n",
 825                 mach_error_string(result), result);
For all the mach calls in this function, would it make sense to print the exception_port as well while reporting errors to make is easier to debug the attach failures.

3.
843 result = task_get_exception_ports(gTask, 844 EXC_MASK_ALL, 845 saved_masks, 846 &saved_exception_types_count, 847 saved_ports, 848 saved_behaviors, 849 saved_flavors);

A comment for this call explaining why we are getting and saving the exceptions info here would be good. e.g. we need to restore the exceptions state for the process when we detach from it.

4. It would make the code look cleaner to have all these exception details that we need to save as the fields of a structure.

5. tgt_exception_port and saved_exception_types_count are saved on the Java side, but saved_masks, saved_ports, saved_behaviors and saved_flavors are defined as static on the native side. All of these values are process specific, and are needed when we detach from a process. Why are these being treated differently?

Thanks,
Poonam


On 9/14/2017 8:17 PM, Jini George wrote:
Thanks much, Dan.

-Jini.

On 9/15/2017 4:18 AM, Daniel D. Daugherty wrote:
On 8/18/17 4:00 AM, Jini George wrote:
Hi all,
 
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m
    L832:   result = mach_port_insert_right (mach_task_self(),
        Nit - extra blank before '('

    L860-863 - nit - indents don't match the other func calls.

    I read through all the changes and looked at logic flow, error
    returns, variable usage, etc. The problem is I'm not conversant
    in Mach exception programming so there could easily be things
    that I missed here.

    You really need to have a MacOS X person look at these changes
    also.

hotspot/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/BsdDebuggerLocal.java
    No comments.

hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_exc.h
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excServer.c
hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excUser.c
    These files are generated by a tool and will be replaced
    by build changes. I didn't review them.

hotspot/test/serviceability/sa/TestAttachDetach.java
    L35: import java.util.ArrayList;
    L36: import java.util.List;
    L37: import java.util.Arrays;
    L38: import java.util.Optional;
    L40: import jdk.test.lib.Utils;
    L41: import jdk.test.lib.process.OutputAnalyzer;
    L42: import jdk.test.lib.process.ProcessTools;
        These imports do not seem to be needed.

    L45: import jdk.test.lib.JDKToolLauncher;
        Duplicate import.

    L98:                             "The String 'Attaching to process' appears " +
    L99:                              numberOfMatches + " number of times");
        Would be helpful to include the expected value also.

jdk/test/ProblemList.txt
    Thanks for remembering to update the ProblemList.txt file!

I don't have any comments other than nits. However, I don't
know mach exceptions so I can't really do a functional review.

We need someone that knows MacOS X mach exceptions to chime
in on this thread. I'm adding Gerard Z to this review thread
in the hopes that he can help...

Dan



Problem gist: The deprecated ptrace() command, PT_ATTACH was changed to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals) to be delivered via mach messages.
This caused SA to hang at waitpid() waiting for a signal, which does not arrive.

Solution in a nutshell: The solution is to make the required changes to handle mach 'soft signal' exceptions in the form of mach messages instead of signals, while attaching to and detaching from the target process. The detailed steps are outlined in JBS.

The changes appear huge due to the inclusion of pre-generated mach exception handling files (mach_exc*). Since this is an integration blocker, it would be great to get quick reviews on this.

Thank you,
Jini.



 





Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Jini George
Thanks very much, Poonam, for this.

My comments inline.

On 9/16/2017 1:18 AM, Poonam Parhar wrote:

> Hello Jini,
>
> I don't know much about the mach exceptions either. I read through your
> wiki page and looked at the relevant gdb and lldb code to understand how
> it works.
>
> A few points regarding the changes for this CR:
>
> File: src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m
>
> 1. In the lldb code, a separate thread is being used to catch and handle
> the mach exceptions. I am assuming we don't need to do that since we use
> the mach exceptions mechanism only during the attach process, and after
> which we suspend all the threads in the target process.Is that correct?

Yes.

>
> 823 if (result != KERN_SUCCESS) {
> 824 print_error("attach: mach_port_allocate() failed: '%s' (%d)\n",
> 825 mach_error_string(result), result);
>
> For all the mach calls in this function, would it make sense to print
> the exception_port as well while reporting errors to make is easier to
> debug the attach failures.

Will do.

>
> 3.
> 843 result = task_get_exception_ports(gTask, 844 EXC_MASK_ALL, 845
> saved_masks, 846 &saved_exception_types_count, 847 saved_ports, 848
> saved_behaviors, 849 saved_flavors);
>
> A comment for this call explaining why we are getting and saving the
> exceptions info here would be good. e.g. we need to restore the
> exceptions state for the process when we detach from it.

Makes sense. Will do.


> 4. It would make the code look cleaner to have all these exception
> details that we need to save as the fields of a structure.

Will do.

>
> 5. /tgt_exception_port/ and /saved_exception_types_count/ are saved on
> the Java side, but /saved_masks, //saved_ports, //saved_behaviors and
> //saved_flavors/ are defined as static on the native side. All of these
> values are process specific, and are needed when we detach from a
> process. Why are these being treated differently?

"tgt_exception_port" and "saved_exception_types_count" can also be moved
to the native side -- will do this. Looks like this would apply to
"symbolicator" and "task" too.

Thanks,
Jini.



>
> Thanks,
> Poonam
>
>
> On 9/14/2017 8:17 PM, Jini George wrote:
>> Thanks much, Dan.
>>
>> -Jini.
>>
>> On 9/15/2017 4:18 AM, Daniel D. Daugherty wrote:
>>> On 8/18/17 4:00 AM, Jini George wrote:
>>>> Hi all,
>>>>
>>>> Requesting reviews for: https://bugs.openjdk.java.net/browse/JDK-8184042
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>
>>> hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m
>>>     L832:   result = mach_port_insert_right (mach_task_self(),
>>>         Nit - extra blank before '('
>>>
>>>     L860-863 - nit - indents don't match the other func calls.
>>>
>>>     I read through all the changes and looked at logic flow, error
>>>     returns, variable usage, etc. The problem is I'm not conversant
>>>     in Mach exception programming so there could easily be things
>>>     that I missed here.
>>>
>>>     You really need to have a MacOS X person look at these changes
>>>     also.
>>>
>>> hotspot/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/BsdDebuggerLocal.java
>>>     No comments.
>>>
>>> hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_exc.h
>>> hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excServer.c
>>> hotspot/src/jdk.hotspot.agent/macosx/native/libsaproc/mach_excUser.c
>>>     These files are generated by a tool and will be replaced
>>>     by build changes. I didn't review them.
>>>
>>> hotspot/test/serviceability/sa/TestAttachDetach.java
>>>     L35: import java.util.ArrayList;
>>>     L36: import java.util.List;
>>>     L37: import java.util.Arrays;
>>>     L38: import java.util.Optional;
>>>     L40: import jdk.test.lib.Utils;
>>>     L41: import jdk.test.lib.process.OutputAnalyzer;
>>>     L42: import jdk.test.lib.process.ProcessTools;
>>>         These imports do not seem to be needed.
>>>
>>>     L45: import jdk.test.lib.JDKToolLauncher;
>>>         Duplicate import.
>>>
>>>     L98:                             "The String 'Attaching to
>>> process' appears " +
>>>     L99:                              numberOfMatches + " number of
>>> times");
>>>         Would be helpful to include the expected value also.
>>>
>>> jdk/test/ProblemList.txt
>>>     Thanks for remembering to update the ProblemList.txt file!
>>>
>>> I don't have any comments other than nits. However, I don't
>>> know mach exceptions so I can't really do a functional review.
>>>
>>> We need someone that knows MacOS X mach exceptions to chime
>>> in on this thread. I'm adding Gerard Z to this review thread
>>> in the hopes that he can help...
>>>
>>> Dan
>>>
>>>
>>>>
>>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was changed
>>>> to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals)
>>>> to be delivered via mach messages.This caused SA to hang at
>>>> waitpid() waiting for a signal, which does not arrive.
>>>>
>>>> Solution in a nutshell: The solution is to make the required changes
>>>> to handle mach 'soft signal' exceptions in the form of mach messages
>>>> instead of signals, while attaching to and detaching from the target
>>>> process. The detailed steps are outlined in JBS.
>>>>
>>>> The changes appear huge due to the inclusion of pre-generated mach
>>>> exception handling files (mach_exc*). Since this is an integration
>>>> blocker, it would be great to get quick reviews on this.
>>>>
>>>> Thank you,
>>>> Jini.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Jini George
In reply to this post by serguei.spitsyn@oracle.com
Hi all,

I have created a webrev restoring the PT_ATTACH:

http://cr.openjdk.java.net/~jgeorge/8184042/webrev.01/

Have included Dmitry's comments on disabling the the deprecation
warning. I would like to request for reviews for this.

Thank you,
Jini.


On 9/8/2017 3:09 AM, [hidden email] wrote:

> On 8/25/17 02:24, [hidden email] wrote:
>> Hi Jini,
>>
>>
>> On 8/18/17 04:00, David Holmes wrote:
>>> Hi Jini,
>>>
>>> Just reading the bug report and your description below this seems
>>> like a major change to try and use a facility (mach exceptions) that
>>> no one seems to have any experience with! That isn't something to be
>>> rushed.
>>
>>> Even if PT_ATTACH has been deprecated restoring its use may be the
>>> quick way forward instead of trying to rush in something like this.
>>
>> This approach looks reasonable to me.
>
> I've just realized that my statement might sound incorrectly.
> I meant that the David's suggestion to restore the use of the deprecated
> PT_ATTACH looks reasonable.
>
> Sorry, if it caused any confusion.
>
> Thanks,
> Serguei
>
>
>> Otherwise, it would be nice to hear why it is not good.
>> How much would it break the fix of the JDK-8182299?
>>
>> Thanks,
>> Serguei
>>
>>>
>>> Just my 2c.
>>>
>>> Cheers,
>>> David
>>>
>>> On 18/08/2017 8:00 PM, Jini George wrote:
>>>> Hi all,
>>>>
>>>> Requesting reviews for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8184042
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>>
>>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was changed
>>>> to PT_ATTACHEXC, which causes mach exceptions (and not UNIX signals)
>>>> to be delivered via mach messages.This caused SA to hang at
>>>> waitpid() waiting for a signal, which does not arrive.
>>>>
>>>> Solution in a nutshell: The solution is to make the required changes
>>>> to handle mach 'soft signal' exceptions in the form of mach messages
>>>> instead of signals, while attaching to and detaching from the target
>>>> process. The detailed steps are outlined in JBS.
>>>>
>>>> The changes appear huge due to the inclusion of pre-generated mach
>>>> exception handling files (mach_exc*). Since this is an integration
>>>> blocker, it would be great to get quick reviews on this.
>>>>
>>>> Thank you,
>>>> Jini.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

David Holmes
Thanks for your patience on this one Jini! The change looks good.

Thanks,
David

On 10/10/2017 2:15 AM, Jini George wrote:

> Hi all,
>
> I have created a webrev restoring the PT_ATTACH:
>
> http://cr.openjdk.java.net/~jgeorge/8184042/webrev.01/
>
> Have included Dmitry's comments on disabling the the deprecation
> warning. I would like to request for reviews for this.
>
> Thank you,
> Jini.
>
>
> On 9/8/2017 3:09 AM, [hidden email] wrote:
>> On 8/25/17 02:24, [hidden email] wrote:
>>> Hi Jini,
>>>
>>>
>>> On 8/18/17 04:00, David Holmes wrote:
>>>> Hi Jini,
>>>>
>>>> Just reading the bug report and your description below this seems
>>>> like a major change to try and use a facility (mach exceptions) that
>>>> no one seems to have any experience with! That isn't something to be
>>>> rushed.
>>>
>>>> Even if PT_ATTACH has been deprecated restoring its use may be the
>>>> quick way forward instead of trying to rush in something like this.
>>>
>>> This approach looks reasonable to me.
>>
>> I've just realized that my statement might sound incorrectly.
>> I meant that the David's suggestion to restore the use of the
>> deprecated PT_ATTACH looks reasonable.
>>
>> Sorry, if it caused any confusion.
>>
>> Thanks,
>> Serguei
>>
>>
>>> Otherwise, it would be nice to hear why it is not good.
>>> How much would it break the fix of the JDK-8182299?
>>>
>>> Thanks,
>>> Serguei
>>>
>>>>
>>>> Just my 2c.
>>>>
>>>> Cheers,
>>>> David
>>>>
>>>> On 18/08/2017 8:00 PM, Jini George wrote:
>>>>> Hi all,
>>>>>
>>>>> Requesting reviews for:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8184042
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>>>
>>>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was
>>>>> changed to PT_ATTACHEXC, which causes mach exceptions (and not UNIX
>>>>> signals) to be delivered via mach messages.This caused SA to hang
>>>>> at waitpid() waiting for a signal, which does not arrive.
>>>>>
>>>>> Solution in a nutshell: The solution is to make the required
>>>>> changes to handle mach 'soft signal' exceptions in the form of mach
>>>>> messages instead of signals, while attaching to and detaching from
>>>>> the target process. The detailed steps are outlined in JBS.
>>>>>
>>>>> The changes appear huge due to the inclusion of pre-generated mach
>>>>> exception handling files (mach_exc*). Since this is an integration
>>>>> blocker, it would be great to get quick reviews on this.
>>>>>
>>>>> Thank you,
>>>>> Jini.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

serguei.spitsyn@oracle.com
Hi Jini,

+1

Thanks,
Serguei


On 10/9/17 20:31, David Holmes wrote:

> Thanks for your patience on this one Jini! The change looks good.
>
> Thanks,
> David
>
> On 10/10/2017 2:15 AM, Jini George wrote:
>> Hi all,
>>
>> I have created a webrev restoring the PT_ATTACH:
>>
>> http://cr.openjdk.java.net/~jgeorge/8184042/webrev.01/
>>
>> Have included Dmitry's comments on disabling the the deprecation
>> warning. I would like to request for reviews for this.
>>
>> Thank you,
>> Jini.
>>
>>
>> On 9/8/2017 3:09 AM, [hidden email] wrote:
>>> On 8/25/17 02:24, [hidden email] wrote:
>>>> Hi Jini,
>>>>
>>>>
>>>> On 8/18/17 04:00, David Holmes wrote:
>>>>> Hi Jini,
>>>>>
>>>>> Just reading the bug report and your description below this seems
>>>>> like a major change to try and use a facility (mach exceptions)
>>>>> that no one seems to have any experience with! That isn't
>>>>> something to be rushed.
>>>>
>>>>> Even if PT_ATTACH has been deprecated restoring its use may be the
>>>>> quick way forward instead of trying to rush in something like this.
>>>>
>>>> This approach looks reasonable to me.
>>>
>>> I've just realized that my statement might sound incorrectly.
>>> I meant that the David's suggestion to restore the use of the
>>> deprecated PT_ATTACH looks reasonable.
>>>
>>> Sorry, if it caused any confusion.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>> Otherwise, it would be nice to hear why it is not good.
>>>> How much would it break the fix of the JDK-8182299?
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>>
>>>>> Just my 2c.
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>> On 18/08/2017 8:00 PM, Jini George wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Requesting reviews for:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8184042
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>>>>
>>>>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was
>>>>>> changed to PT_ATTACHEXC, which causes mach exceptions (and not
>>>>>> UNIX signals) to be delivered via mach messages.This caused SA to
>>>>>> hang at waitpid() waiting for a signal, which does not arrive.
>>>>>>
>>>>>> Solution in a nutshell: The solution is to make the required
>>>>>> changes to handle mach 'soft signal' exceptions in the form of
>>>>>> mach messages instead of signals, while attaching to and
>>>>>> detaching from the target process. The detailed steps are
>>>>>> outlined in JBS.
>>>>>>
>>>>>> The changes appear huge due to the inclusion of pre-generated
>>>>>> mach exception handling files (mach_exc*). Since this is an
>>>>>> integration blocker, it would be great to get quick reviews on this.
>>>>>>
>>>>>> Thank you,
>>>>>> Jini.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: SA: MacOS X: 8184042: several serviceability/sa tests timed out on MacOS X

Jini George
Thank you, David and Serguei.

- Jini.

On 10/10/2017 9:48 AM, [hidden email] wrote:

> Hi Jini,
>
> +1
>
> Thanks,
> Serguei
>
>
> On 10/9/17 20:31, David Holmes wrote:
>> Thanks for your patience on this one Jini! The change looks good.
>>
>> Thanks,
>> David
>>
>> On 10/10/2017 2:15 AM, Jini George wrote:
>>> Hi all,
>>>
>>> I have created a webrev restoring the PT_ATTACH:
>>>
>>> http://cr.openjdk.java.net/~jgeorge/8184042/webrev.01/
>>>
>>> Have included Dmitry's comments on disabling the the deprecation
>>> warning. I would like to request for reviews for this.
>>>
>>> Thank you,
>>> Jini.
>>>
>>>
>>> On 9/8/2017 3:09 AM, [hidden email] wrote:
>>>> On 8/25/17 02:24, [hidden email] wrote:
>>>>> Hi Jini,
>>>>>
>>>>>
>>>>> On 8/18/17 04:00, David Holmes wrote:
>>>>>> Hi Jini,
>>>>>>
>>>>>> Just reading the bug report and your description below this seems
>>>>>> like a major change to try and use a facility (mach exceptions)
>>>>>> that no one seems to have any experience with! That isn't
>>>>>> something to be rushed.
>>>>>
>>>>>> Even if PT_ATTACH has been deprecated restoring its use may be the
>>>>>> quick way forward instead of trying to rush in something like this.
>>>>>
>>>>> This approach looks reasonable to me.
>>>>
>>>> I've just realized that my statement might sound incorrectly.
>>>> I meant that the David's suggestion to restore the use of the
>>>> deprecated PT_ATTACH looks reasonable.
>>>>
>>>> Sorry, if it caused any confusion.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>> Otherwise, it would be nice to hear why it is not good.
>>>>> How much would it break the fix of the JDK-8182299?
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>>
>>>>>> Just my 2c.
>>>>>>
>>>>>> Cheers,
>>>>>> David
>>>>>>
>>>>>> On 18/08/2017 8:00 PM, Jini George wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Requesting reviews for:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8184042
>>>>>>>
>>>>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/
>>>>>>>
>>>>>>> Problem gist: The deprecated ptrace() command, PT_ATTACH was
>>>>>>> changed to PT_ATTACHEXC, which causes mach exceptions (and not
>>>>>>> UNIX signals) to be delivered via mach messages.This caused SA to
>>>>>>> hang at waitpid() waiting for a signal, which does not arrive.
>>>>>>>
>>>>>>> Solution in a nutshell: The solution is to make the required
>>>>>>> changes to handle mach 'soft signal' exceptions in the form of
>>>>>>> mach messages instead of signals, while attaching to and
>>>>>>> detaching from the target process. The detailed steps are
>>>>>>> outlined in JBS.
>>>>>>>
>>>>>>> The changes appear huge due to the inclusion of pre-generated
>>>>>>> mach exception handling files (mach_exc*). Since this is an
>>>>>>> integration blocker, it would be great to get quick reviews on this.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Jini.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>
>