Properly link omrsig and enable OMRPORT_OMRSIG_SUPPORT globally

[babsingh]

1. Properly link omrsig when building constgen and j9ddrgen

   When building constgen and j9ddrgen, omrsig should be linked when
   J9VM_PORT_OMRSIG_SUPPORT is enabled.

2. Remove OMRPORT_OMRSIG_SUPPORT definitions

   Currently, OMRPORT_OMRSIG_SUPPORT is only defined while building the
   port library. But, OMRPORT_OMRSIG_SUPPORT should be defined globally.
   This will allow omrsig macros such as OMRSIG_SIGNAL and OMRSIG_SIGACTION
   to be used consistently across OpenJ9.

   Removing localized definitions of OMRPORT_OMRSIG_SUPPORT before enabling
   it globally. This will prevent redundant definitions of
   OMRPORT_OMRSIG_SUPPORT when it is enabled globally.

3. Enable OMRPORT_OMRSIG_SUPPORT globally in OpenJ9

   Currently, OMRPORT_OMRSIG_SUPPORT is only defined while building the
   port library. But, OMRPORT_OMRSIG_SUPPORT should be defined globally.
   This will allow omrsig macros such as OMRSIG_SIGNAL and OMRSIG_SIGACTION
   to be used consistently across OpenJ9. Thus, OMRPORT_OMRSIG_SUPPORT is
   enabled globally.

4. Fix indentation in hyvm/module.xml

5. Link omrsig to port and vm only if J9VM_PORT_OMRSIG_SUPPORT is enabled

   Currently, omrsig is linked to port and vm unconditionally. omrsig
   should be linked to port and vm only if J9VM_PORT_OMRSIG_SUPPORT is
   enabled.

Closes: #1889

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

[babsingh]

This PR depends on eclipse-omr/omr#2855 and eclipse-omr/omr#2865.

[babsingh]

+@dnakamura to point out if any CMake dependencies were missed?

[DanHeidinga]

@babsingh I'm not sure I understand the issue here well enough yet to review the code. My naive thought is that the macros are working as designed.

They expand to sigaction/signal everywhere except the lowest layer that has intercepted them.

Can you clarify why you believe the usage is incorrect or problematic?

[babsingh]

They expand to sigaction/signal everywhere except the lowest layer that has intercepted them.

This is true. In omrsig.cpp, signal and sigaction are overwritten:

sighandler_t
signal(int signum, sighandler_t handler) __THROW
#endif /* defined(OMR_OS_WINDOWS) */
{
	return omrsig_signal_internal(signum, handler);
}

int
sigaction(int signum, const struct sigaction *act, struct sigaction *oldact) __THROW
{
	return omrsig_sigaction_internal(signum, act, oldact, false);
}

OMRSIG_SIGNAL and OMRSIG_SIGACTION macros are defined in omrport.h:

#if defined(OMRPORT_OMRSIG_SUPPORT)
#define OMRSIG_SIGNAL(signum, handler) omrsig_primary_signal(signum, handler)
#define OMRSIG_SIGACTION(signum, act, oldact) omrsig_primary_sigaction(signum, act, oldact)
#else /* defined(OMRPORT_OMRSIG_SUPPORT) */
#define OMRSIG_SIGNAL(signum, handler) signal(signum, handler)
#define OMRSIG_SIGACTION(signum, act, oldact) sigaction(signum, act, oldact)
#endif /* defined(OMRPORT_OMRSIG_SUPPORT) */

Calls to omrsig_handler are also surrounded by OMRPORT_OMRSIG_SUPPORT:

#if defined(J9VM_PORT_OMRSIG_SUPPORT)
	/* Chain to application handler */
	if ( !vmThread || (vmThread && (cachedVM->sigFlags & J9_SIG_NO_SIG_CHAIN) == 0) ) {
		omrsig_handler(sig, 0, 0);
	}
#endif /* defined(J9VM_PORT_OMRSIG_SUPPORT) */

omrsig_handler is needed to invoke the application signal handler.

Currently, OMRPORT_OMRSIG_SUPPORT is only enabled when building the port library. So, the above macros will expand to omrsig_primary_signal/omrsig_primary_sigaction within the port library. Outside the port library, the above macros will expand to signal/sigaction.

Within port library, omrsig_handler will be called. Outside port library, omrsig_handler won't be called. omrsig_handler is not called from dmpsup.c. Thus, the application signal handler is not invoked for SIGABRT. OMRPORT_OMRSIG_SUPPORT should be enabled throughout OpenJ9 for signal chaining to work consistently.

Does it matter if we call omrsig_primary_signal/omrsig_primary_sigaction directly in the JVM? If omrsig is not linked, can omrsig_primary_signal/omrsig_primary_sigaction [used in the JVM] and signal/sigaction [used in C/C++ app] be used together? After testing, I found that JVM's signal handling functionality isn't impacted if omrsig_primary_signal/omrsig_primary_sigaction are directly invoked in the JVM.

After OMRPORT_OMRSIG_SUPPORT is consistently enabled in OpenJ9, I will change the above macros as follows:

#define OMRSIG_SIGNAL(signum, handler) signal(signum, handler)
#define OMRSIG_SIGACTION(signum, act, oldact) sigaction(signum, act, oldact)

We don't need to directly call omrsig_primary_signal/omrsig_primary_sigaction since the omrsig library overwrites signal/sigaction.

[babsingh]

Required OMR changes (eclipse-omr/omr#2855 and eclipse-omr/omr#2865) are available in eclipse/openj9-omr openj9 branch. These changes are ready to merge.

[pshipton]

How does this enable OMRPORT_OMRSIG_SUPPORT in OpenJ9 code? i.e. where does the flag get defined?

[babsingh]

the flag will be defined in omrcfg.h.

[pshipton]

jenkins test sanity xlinux