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

windows: No panic for cgo crashes #30737

Closed
sparkkoori opened this issue Mar 11, 2019 · 11 comments
Closed

windows: No panic for cgo crashes #30737

sparkkoori opened this issue Mar 11, 2019 · 11 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@sparkkoori
Copy link

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

$ go version
go version go1.12 windows/amd64

Does this issue reproduce with the latest release?

Yes

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

go env windows/amd64
$ go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\sparkkoori\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\sparkkoori\code
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\sparkkoori\AppData\Local\Temp\go-build334970126=/tmp/go-build -gno-record-gcc-switches

What did you do?

Run:

package main

/*
void crash(){ int* p = 0; *p=1; }
*/
import "C"

func main() {
	C.crash()
	println("Oh! No crash!")
}

What did you expect to see?

Panic.

What did you see instead?

Nothing.

@UFOXD
Copy link

UFOXD commented Mar 11, 2019

cgo too hard to use.
Can we improve it. Enhance the ability of interaction for go with c .

@julieqiu
Copy link
Member

cc: @alexbrainman @bradfitz

@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 11, 2019
@polarina
Copy link
Contributor

There is a hard assumption that C code is well-behaved.

Writing through a null pointer in C is undefined behaviour. The Go runtime makes no attempts to map undefined behaviour in C to Go panics, it'd be a futile attempt.

@ianlancetaylor
Copy link
Contributor

What does a C program with the same code do? Does it crash?

@sparkkoori
Copy link
Author

@ianlancetaylor
Yes, it does crash, and nothing prints.

@polarina
Thanks for your explaining.

But raising a SIGSEGV also doesn't make the Go runtime panic.

package main

/*
#include <signal.h>
void crash(){ raise(SIGSEGV); }
*/
import "C"

func main() {
	C.crash()
	println("Oh! No crash!")
}

From document of package os/signal:

…… In general, except as discussed below, Go programs will convert a synchronous signal into a run-time panic.

If the non-Go code installs a signal handler for any of the synchronous signals (SIGBUS, SIGFPE, SIGSEGV), then it should record the existing Go signal handler. If those signals occur while executing Go code, it should invoke the Go signal handler (whether the signal occurs while executing Go code can be determined by looking at the PC passed to the signal handler). Otherwise some Go run-time panics will not occur as expected.

If SIGSEGV can't trigger a panic, what can be?

@ianlancetaylor
Copy link
Contributor

I really don't know what raise(SIGSEGV) does on Windows.

The os/signal package doc start by saying that they are mostly for Unix. You are quoting from the Unix docs for os/signal, not the (very short) Windows docs.

@sparkkoori
Copy link
Author

As the C standard library contains <signal.h> and Mingw-w64 also supports it, should the panic logic on Windows behaves the same as on Unix?

@alexbrainman
Copy link
Member

As the C standard library contains <signal.h> and Mingw-w64 also supports it, should the panic logic on Windows behaves the same as on Unix?

What does Mingw-w64 do with raise(SIGSEGV) on Windows?

Alex

@sparkkoori
Copy link
Author

What does Mingw-w64 do with raise(SIGSEGV) on Windows?

Alex

It is implemented in msvcrt.dll, I guess.
See signal | Windows Docs

@ianlancetaylor
Copy link
Contributor

If you can change the runtime package to make signals on Windows work as they do on Unix, that would be fine with me.

@sparkkoori
Copy link
Author

I'm not really sure if it's necessary or just fine.
If it's not necessary, I think this issue should be closed.

@golang golang locked and limited conversation to collaborators Mar 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants