cmd/dlv: actually disable C compiler optimizations when building (#1647)

* proc: fix stacktraces when a SIGSEGV happens during a cgo call

When a SIGSEGV happens in a cgo call (for example as a result of
dereferencing a NULL pointer) the stack layout will look like this:

(system stack) runtime.fatalthrow
(system stack) runtime.throw
(system stack) runtime.sigpanic
(system stack) offending C function
... other C functions...
(system stack) runtime.asmcgocall
(goroutine stack) call inside cgo

The code in switchStack would switch directly from the
runtime.fatalthrow frame to the first frame in the goroutine stack,
hiding important information.

Disable this switch for runtime.fatalthrow and reintroduce the check
for runtime.mstart that existed before this version of the code was
implemented in commit 7bec20.

This problem was reported in comment:
https://github.com/go-delve/delve/issues/935#issuecomment-512182533

* cmd/dlv: actually disable C compiler optimizations when building

_fixtures/cgosigsegvstack.go

@@ -0,0 +1,18 @@
+package main
+
+// #cgo CFLAGS: -g -Wall -O0
+
+/*
+void sigsegv(int x) {
+	int *p = NULL;
+	*p = x;
+}
+void testfn(int x) {
+	sigsegv(x);
+}
+*/
+import "C"
+
+func main() {
+	C.testfn(C.int(10))
+}

cmd/dlv/main.go

@@ -17,7 +17,7 @@ func main() {
 	}
 	const cgoCflagsEnv = "CGO_CFLAGS"
 	if os.Getenv(cgoCflagsEnv) == "" {
-		os.Setenv(cgoCflagsEnv, "-O -g")
+		os.Setenv(cgoCflagsEnv, "-O0 -g")
 	} else {
 		logrus.WithFields(logrus.Fields{"layer": "dlv"}).Warnln("CGO_CFLAGS already set, Cgo code could be optimized.")
 	}

pkg/proc/proc_test.go

@@ -4464,3 +4464,18 @@ func TestIssue1615(t *testing.T) {
 		assertLineNumber(p, t, 19, "")
 	})
 }
+
+func TestCgoStacktrace2(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("fixture crashes go runtime on windows")
+	}
+	// If a panic happens during cgo execution the stacktrace should show the C
+	// function that caused the problem.
+	withTestProcess("cgosigsegvstack", t, func(p proc.Process, fixture protest.Fixture) {
+		proc.Continue(p)
+		frames, err := proc.ThreadStacktrace(p.CurrentThread(), 100)
+		assertNoError(err, t, "Stacktrace()")
+		logStacktrace(t, p.BinInfo(), frames)
+		stacktraceCheck(t, []string{"C.sigsegv", "C.testfn", "main.main"}, frames)
+	})
+}

pkg/proc/stack.go

@@ -316,8 +316,28 @@ func (it *stackIterator) switchStack() bool {
 		it.atend = true
 		return true
 
+	case "runtime.mstart":
+		// Calls to runtime.systemstack will switch to the systemstack then:
+		// 1. alter the goroutine stack so that it looks like systemstack_switch
+		//    was called
+		// 2. alter the system stack so that it looks like the bottom-most frame
+		//    belongs to runtime.mstart
+		// If we find a runtime.mstart frame on the system stack of a goroutine
+		// parked on runtime.systemstack_switch we assume runtime.systemstack was
+		// called and continue tracing from the parked position.
+
+		if it.top || !it.systemstack || it.g == nil {
+			return false
+		}
+		if fn := it.bi.PCToFunc(it.g.PC); fn == nil || fn.Name != "runtime.systemstack_switch" {
+			return false
+		}
+
+		it.switchToGoroutineStack()
+		return true
+
 	default:
-		if it.systemstack && it.top && it.g != nil && strings.HasPrefix(it.frame.Current.Fn.Name, "runtime.") {
+		if it.systemstack && it.top && it.g != nil && strings.HasPrefix(it.frame.Current.Fn.Name, "runtime.") && it.frame.Current.Fn.Name != "runtime.fatalthrow" {
 			// The runtime switches to the system stack in multiple places.
 			// This usually happens through a call to runtime.systemstack but there
 			// are functions that switch to the system stack manually (for example
@@ -325,11 +345,12 @@ func (it *stackIterator) switchStack() bool {
 			// Since we are only interested in printing the system stack for cgo
 			// calls we switch directly to the goroutine stack if we detect that the
 			// function at the top of the stack is a runtime function.
-			it.systemstack = false
+			//
-			it.top = false
+			// The function "runtime.fatalthrow" is deliberately excluded from this
-			it.pc = it.g.PC
+			// because it can end up in the stack during a cgo call and switching to
-			it.regs.Reg(it.regs.SPRegNum).Uint64Val = it.g.SP
+			// the goroutine stack will exclude all the C functions from the stack
-			it.regs.Reg(it.regs.BPRegNum).Uint64Val = it.g.BP
+			// trace.
+			it.switchToGoroutineStack()
 			return true
 		}
 
@@ -337,6 +358,14 @@ func (it *stackIterator) switchStack() bool {
 	}
 }
 
+func (it *stackIterator) switchToGoroutineStack() {
+	it.systemstack = false
+	it.top = false
+	it.pc = it.g.PC
+	it.regs.Reg(it.regs.SPRegNum).Uint64Val = it.g.SP
+	it.regs.Reg(it.regs.BPRegNum).Uint64Val = it.g.BP
+}
+
 // Frame returns the frame the iterator is pointing at.
 func (it *stackIterator) Frame() Stackframe {
 	it.frame.Bottom = it.atend