Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8292166

POSIX signal handling issues with pc access

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Won't Fix
    • Icon: P4 P4
    • tbd
    • 20
    • hotspot

      JDK-8283326 made some changes to the POSIX signal handling code. Some of these issue might have pre-dated that.

      The signal handler has an assert that the ucVoid argument is non-NULL. Later, when extracting the pc from the ucontext, it only does so if uc (obtained from ucVoid) is non-NULL. But it can't be NULL because of the assertion.

      The pc is extracted from the context even if it isn't needed. The signal may have already been handled by handle_assert_poison_fault. The pre-8283326 checked signal_was_handled. (Also, see below regarding signal_was_handled.)

      The static variant of handle_safefetch ignores the pc argument and refetches it from the ucontext. The setjmp variant of handle_safefetch also ignores the pc argument (naming it ignored1). Maybe these functions should just not take the pc as an argument. That would push the extraction code further down in the signal handler. Alternatively, the static variant could use the passed in argument rather than re-extracting it. (The static variant of handle_safefetch now uses the passed pc argument. The fix was made in JDK-8294053.)

      Extraction of the pc involves some special cases. The static variant of handle_safefetch doesn't deal with those cases. (They don't seem to apply, other than for ZERO, so okay.) There is also similar but (inconsistently) different code in the crash_handler function in vmError_posix.cpp. Maybe those places could share code.

      crash_handler also checks for a the ucVoid argument being NULL; is that actually possible here?

      It seems like the POSIX signal handler would be simpler if all places that set signal_was_handled instead checked the assigned-from value and returned true if that value is true. That would eliminate the signal_was_handled variable and all the downstream checks. But maybe this should be a separate issue.

            stuefe Thomas Stuefe
            kbarrett Kim Barrett
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: