Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: respect SA_RESETHAND when forwarding to non-Go signal handler #32659

Open
jimhuaang opened this issue Jun 17, 2019 · 6 comments
Labels
Milestone

Comments

@jimhuaang
Copy link

@jimhuaang jimhuaang commented Jun 17, 2019

What version of Go are you using (go version)?

go version go1.10.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jim/go"
GORACE=""
GOROOT="/usr/lib/go-1.10"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.10/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build690985833=/tmp/go-build -gno-record-gcc-switches"

What did you do?

    1. Compile a library using -buildmode=c-shared
    1. Trap SIGSEGV in a C program with a handler which will reraise SIGSEGV
    1. Load the library against the C program using dlopen
    1. Cause the C program to SEGSIGV by dereference a NULL pointer

src file: infinitesignalloop.gz
to compile: uncompress src file to GOPATH and run make
to reproduce: just run ./trap_signal before

What did you expect to see?

The program should crash finally with the default system segfault handler be called.

trap SIGSEGV before dlopen ./libdummycgo.so
Received Segmentation fault ...
Segmentation fault (core dumped)

What did you see instead?

A infinite sighandling loop has happen.

trap SIGSEGV before dlopen ./libdummycgo.so
Received Segmentation fault ...
Received Segmentation fault ...
... // infinite loop
Received Segmentation fault ...
Killed

Following the code with gdb, it seems that Go signal handler raises SIGSEGV again to invoke the C handler, then C handler reraise SIGSEGV, which is catched by Go signal handler again, and then repeat and repeat.

As doc https://golang.org/pkg/os/signal/#hdr-Go_programs_that_use_cgo_or_SWIG said in section Go programs that use cgo or SWIG (not in section Non-Go programs that call Go code)

If the Go signal handler is invoked on a non-Go thread not running Go code, the handler generally forwards the signal to the non-Go code, as follows. If the signal is SIGPROF, the Go handler does nothing. Otherwise, the Go handler removes itself, unblocks the signal, and raises it again, to invoke any non-Go handler or default system handler. If the program does not exit, the Go handler then reinstalls itself and continues execution of the program.

And if we trap SIGSEGV after load the library (just run ./trap_signal after), there C program will just crash with system default handler called as expected.

Some more tests shows above issue exist for all synchronize signals (SIGBUS, SIGFPE, and SIGSEGV).

When not use dlopen to load the library, instead to link the library against to a C program as following,

    1. Compile a library using -buildmode=c-shared
    1. Trap SIGSEGV in a C program with a handler which will reraise SIGSEGV
    1. Link the library against the C program
    1. Cause the C program to SEGSIGV by dereference a NULL pointer
      and then run the C program, it will crash with default segfault handler called.
@katiehockman katiehockman changed the title runtime: signal handling: C program trap SIGSEGV before dlopen shared libraray (-buildmode=c-shared) with infinite signalhanding os/signal: C program trap SIGSEGV before dlopen shared libraray (-buildmode=c-shared) with infinite signalhanding Jun 19, 2019
@katiehockman

This comment has been minimized.

Copy link
Member

@katiehockman katiehockman commented Jun 19, 2019

/cc @ianlancetaylor

The link to the documentation didn't work for me, so here is an update link for whomever investigates this: https://golang.org/pkg/os/signal/#hdr-Go_programs_that_use_cgo_or_SWIG

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 19, 2019

I don't see what we can do here. The Go signal handler has to do something. It sounds like what it does here doesn't work with your C program, but it still seems like the best chance of working with a typical program. Why does your C signal handler re-raise SIGSEGV?

@ianlancetaylor ianlancetaylor added this to the Go1.14 milestone Jun 19, 2019
@jimhuaang

This comment has been minimized.

Copy link
Author

@jimhuaang jimhuaang commented Jun 20, 2019

Why does your C signal handler re-raise SIGSEGV?

At some case, I want the C signal handler to just dump stack trace when segfault, then re-raise the signal to system default handler.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 21, 2019

But presumably when you re-raise the signal you first remove the C signal handler, which ought to mean that you get the default handler and crash the program. How does the Go signal handler come back?

@jimhuaang

This comment has been minimized.

Copy link
Author

@jimhuaang jimhuaang commented Jun 21, 2019

But presumably when you re-raise the signal you first remove the C signal handler, which ought to mean that you get the default handler and crash the program.

That's exactly what I want to do with specifying sigaction.sa_flags with SA_RESETHAND.

sa_flags specifies a set of flags which modify the behavior of the signal.  It is formed by 
the bitwise OR of zero or more of the following:

SA_RESETHAND
    Restore the signal action to the default upon entry to the signal handler.

How does the Go signal handler come back?

Following the startup code (_rt0_amd64_lib), I believe the synchronous signals handler is changed to the Go signal handler cgoSigtramp, and the previous C signal handler is stored at fwdSig.

As above, the Go signal handler is the current signal handler, it will tramp the synchronous signal and if the signal occurred not in Go code, it will forward the signal to the system or the C handler, in this case is the C handler. Then C handler it this case has re-raised the same signal. When Go signal handler tramped the signal, this process will repeate again.

The sigfwdgo has not checked the established sa_flags of the C handler, as a result SA_RESETHAND is ignored.

I believe if sigfwdgo can find a way to check the sa_flags of the C handler, if sa_flags & SA_RESETHNAD is true, reset fwdSig[signo] to _SIG_IGN, then the next time when the same synchronized signal get tramped, the C signal handler will not get forward. But I haven't see where the sa_flags is recorded.

func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool {
	if sig >= uint32(len(sigtable)) {
		return false
	}
	fwdFn := atomic.Loaduintptr(&fwdSig[sig])
	flags := sigtable[sig].flags

	// If there is no handler to forward to, no need to forward.
	if fwdFn == _SIG_DFL {
		return false
	}

	c := &sigctxt{info, ctx}
	// Only forward synchronous signals and SIGPIPE.
	// Unfortunately, user generated SIGPIPEs will also be forwarded, because si_code
	// is set to _SI_USER even for a SIGPIPE raised from a write to a closed socket
	// or pipe.
	if (c.sigcode() == _SI_USER || flags&_SigPanic == 0) && sig != _SIGPIPE {
		return false
	}
	// Determine if the signal occurred inside Go code. We test that:
	//   (1) we were in a goroutine (i.e., m.curg != nil), and
	//   (2) we weren't in CGO.
	g := getg()
	if g != nil && g.m != nil && g.m.curg != nil && !g.m.incgo {
		return false
	}

	// Signal not handled by Go, forward it.
	// forward to system or the C handler
	if fwdFn != _SIG_IGN {
		sigfwd(fwdFn, sig, info, ctx)
	}

	return true
}
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 21, 2019

Thanks, that seems reasonable.

@ianlancetaylor ianlancetaylor changed the title os/signal: C program trap SIGSEGV before dlopen shared libraray (-buildmode=c-shared) with infinite signalhanding runtime: respect SA_RESETHAND when forwarding to non-Go signal handler Jun 21, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.