public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* libgo patch committed: Add precise stack scan support
@ 2018-12-05 23:10 Ian Lance Taylor
  2018-12-07 10:07 ` Rainer Orth
  2018-12-10  6:41 ` Matthias Klose
  0 siblings, 2 replies; 15+ messages in thread
From: Ian Lance Taylor @ 2018-12-05 23:10 UTC (permalink / raw)
  To: gcc-patches, gofrontend-dev

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

This libgo patch by Cherry Zhang adds support for precise stack
scanning to the Go runtime.  This uses per-function stack maps stored
in the exception tables in the language-specific data area.  The
compiler needs to generate these stack maps; currently this is only
done by a version of LLVM, not by GCC.  Each safepoint in a function
is associated with a (real or dummy) landing pad, and its "type info"
in the exception table is a pointer to the stack map. When a stack is
scanned, the stack map is found by the stack unwinding code.

For precise stack scan we need to unwind the stack. There are three cases:

- If a goroutine is scanning its own stack, it can unwind the stack
and scan the frames.

- If a goroutine is scanning another, stopped, goroutine, it cannot
directly unwind the target stack. We handle this by switching
(runtime.gogo) to the target g, letting it unwind and scan the stack,
and switch back.

- If we are scanning a goroutine that is blocked in a syscall, we send
a signal to the target goroutine's thread, and let the signal handler
unwind and scan the stack. Extra care is needed as this races with
enter/exit syscall.

Currently this is only implemented on GNU/Linux.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 26011 bytes --]

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 266812)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-d3a98b7a9ea8032be22ebb3ea2f389ce22669d53
+edc7e7172e674b8c7e9c3caa30e24280cd289a9c
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/mgcmark.go
===================================================================
--- libgo/go/runtime/mgcmark.go	(revision 266510)
+++ libgo/go/runtime/mgcmark.go	(working copy)
@@ -664,7 +664,10 @@ func gcFlushBgCredit(scanWork int64) {
 }
 
 // We use a C function to find the stack.
-func doscanstack(*g, *gcWork)
+// Returns whether we succesfully scanned the stack.
+func doscanstack(*g, *gcWork) bool
+
+func doscanstackswitch(*g, *g)
 
 // scanstack scans gp's stack, greying all pointers found on the stack.
 //
@@ -691,7 +694,16 @@ func scanstack(gp *g, gcw *gcWork) {
 		return
 	case _Grunning:
 		// ok for gccgo, though not for gc.
-	case _Grunnable, _Gsyscall, _Gwaiting:
+		if usestackmaps {
+			print("runtime: gp=", gp, ", goid=", gp.goid, ", gp->atomicstatus=", readgstatus(gp), "\n")
+			throw("scanstack: goroutine not stopped")
+		}
+	case _Gsyscall:
+		if usestackmaps {
+			print("runtime: gp=", gp, ", goid=", gp.goid, ", gp->atomicstatus=", readgstatus(gp), "\n")
+			throw("scanstack: goroutine in syscall")
+		}
+	case _Grunnable, _Gwaiting:
 		// ok
 	}
 
@@ -701,15 +713,64 @@ func scanstack(gp *g, gcw *gcWork) {
 	}
 
 	// Scan the stack.
-	doscanstack(gp, gcw)
-
-	// Conservatively scan the saved register values.
-	scanstackblock(uintptr(unsafe.Pointer(&gp.gcregs)), unsafe.Sizeof(gp.gcregs), gcw)
-	scanstackblock(uintptr(unsafe.Pointer(&gp.context)), unsafe.Sizeof(gp.context), gcw)
+	if usestackmaps {
+		g := getg()
+		if g == gp {
+			// Scan its own stack.
+			doscanstack(gp, gcw)
+		} else if gp.entry != nil {
+			// This is a newly created g that hasn't run. No stack to scan.
+		} else {
+			// Scanning another g's stack. We need to switch to that g
+			// to unwind its stack. And switch back after scan.
+			scanstackswitch(gp, gcw)
+		}
+	} else {
+		doscanstack(gp, gcw)
+
+		// Conservatively scan the saved register values.
+		scanstackblock(uintptr(unsafe.Pointer(&gp.gcregs)), unsafe.Sizeof(gp.gcregs), gcw)
+		scanstackblock(uintptr(unsafe.Pointer(&gp.context)), unsafe.Sizeof(gp.context), gcw)
+	}
 
 	gp.gcscanvalid = true
 }
 
+// scanstackswitch scans gp's stack by switching (gogo) to gp and
+// letting it scan its own stack, and switching back upon finish.
+//
+//go:nowritebarrier
+func scanstackswitch(gp *g, gcw *gcWork) {
+	g := getg()
+
+	// We are on the system stack which prevents preemption. But
+	// we are going to switch to g stack. Lock m to block preemption.
+	mp := acquirem()
+
+	// The doscanstackswitch function will modify the current g's
+	// context. Preserve it.
+	// The stack scan code may call systemstack, which will modify
+	// gp's context. Preserve it as well so we can resume gp.
+	context := g.context
+	stackcontext := g.stackcontext
+	context2 := gp.context
+	stackcontext2 := gp.stackcontext
+
+	gp.scangcw = uintptr(unsafe.Pointer(gcw))
+	gp.scang = uintptr(unsafe.Pointer(g))
+	doscanstackswitch(g, gp)
+
+	// Restore the contexts.
+	g.context = context
+	g.stackcontext = stackcontext
+	gp.context = context2
+	gp.stackcontext = stackcontext2
+	gp.scangcw = 0
+	// gp.scang is already cleared in C code.
+
+	releasem(mp)
+}
+
 type gcDrainFlags int
 
 const (
@@ -1064,6 +1125,10 @@ func scanobject(b uintptr, gcw *gcWork)
 // scanblock, but we scan the stack conservatively, so there is no
 // bitmask of pointers.
 func scanstackblock(b, n uintptr, gcw *gcWork) {
+	if usestackmaps {
+		throw("scanstackblock: conservative scan but stack map is used")
+	}
+
 	for i := uintptr(0); i < n; i += sys.PtrSize {
 		// Same work as in scanobject; see comments there.
 		obj := *(*uintptr)(unsafe.Pointer(b + i))
@@ -1072,6 +1137,50 @@ func scanstackblock(b, n uintptr, gcw *g
 		}
 	}
 }
+
+// scanstackblockwithmap is like scanstackblock, but with an explicit
+// pointer bitmap. This is used only when precise stack scan is enabled.
+//go:linkname scanstackblockwithmap runtime.scanstackblockwithmap
+//go:nowritebarrier
+func scanstackblockwithmap(pc, b0, n0 uintptr, ptrmask *uint8, gcw *gcWork) {
+	// Use local copies of original parameters, so that a stack trace
+	// due to one of the throws below shows the original block
+	// base and extent.
+	b := b0
+	n := n0
+
+	for i := uintptr(0); i < n; {
+		// Find bits for the next word.
+		bits := uint32(*addb(ptrmask, i/(sys.PtrSize*8)))
+		if bits == 0 {
+			i += sys.PtrSize * 8
+			continue
+		}
+		for j := 0; j < 8 && i < n; j++ {
+			if bits&1 != 0 {
+				// Same work as in scanobject; see comments there.
+				obj := *(*uintptr)(unsafe.Pointer(b + i))
+				if obj != 0 {
+					o, span, objIndex := findObject(obj, b, i, false)
+					if obj < minPhysPageSize ||
+						span != nil && span.state != _MSpanManual &&
+							(obj < span.base() || obj >= span.limit || span.state != mSpanInUse) {
+						print("runtime: found in object at *(", hex(b), "+", hex(i), ") = ", hex(obj), ", pc=", hex(pc), "\n")
+						name, file, line := funcfileline(pc, -1)
+						print(name, "\n", file, ":", line, "\n")
+						//gcDumpObject("object", b, i)
+						throw("found bad pointer in Go stack (incorrect use of unsafe or cgo?)")
+					}
+					if o != 0 {
+						greyobject(o, b, i, span, gcw, objIndex, false)
+					}
+				}
+			}
+			bits >>= 1
+			i += sys.PtrSize
+		}
+	}
+}
 
 // Shade the object if it isn't already.
 // The object is not nil and known to be in the heap.
Index: libgo/go/runtime/os_gccgo.go
===================================================================
--- libgo/go/runtime/os_gccgo.go	(revision 266510)
+++ libgo/go/runtime/os_gccgo.go	(working copy)
@@ -27,7 +27,8 @@ func mpreinit(mp *m) {
 func minit() {
 	minitSignals()
 
-	// FIXME: We should set _g_.m.procid here.
+	// FIXME: only works on linux for now.
+	getg().m.procid = uint64(gettid())
 }
 
 // Called from dropm to undo the effect of an minit.
Index: libgo/go/runtime/proc.go
===================================================================
--- libgo/go/runtime/proc.go	(revision 266510)
+++ libgo/go/runtime/proc.go	(working copy)
@@ -528,6 +528,8 @@ func schedinit() {
 
 	sched.maxmcount = 10000
 
+	usestackmaps = probestackmaps()
+
 	mallocinit()
 	mcommoninit(_g_.m)
 	cpuinit() // must run before alginit
@@ -891,7 +893,49 @@ loop:
 		case _Gcopystack:
 		// Stack being switched. Go around again.
 
-		case _Grunnable, _Gsyscall, _Gwaiting:
+		case _Gsyscall:
+			if usestackmaps {
+				// Claim goroutine by setting scan bit.
+				// Racing with execution or readying of gp.
+				// The scan bit keeps them from running
+				// the goroutine until we're done.
+				if castogscanstatus(gp, s, s|_Gscan) {
+					if gp.scanningself {
+						// Don't try to scan the stack
+						// if the goroutine is going to do
+						// it itself.
+						// FIXME: can this happen?
+						restartg(gp)
+						break
+					}
+					if !gp.gcscandone {
+						// Send a signal to let the goroutine scan
+						// itself. This races with enter/exitsyscall.
+						// If the goroutine is not stopped at a safepoint,
+						// it will not scan the stack and we'll try again.
+						mp := gp.m
+						noteclear(&mp.scannote)
+						gp.scangcw = uintptr(unsafe.Pointer(gcw))
+						tgkill(getpid(), _pid_t(mp.procid), _SIGURG)
+
+						// Wait for gp to scan its own stack.
+						notesleep(&mp.scannote)
+
+						if !gp.gcscandone {
+							// The signal delivered at a bad time.
+							// Try again.
+							restartg(gp)
+							break
+						}
+					}
+					restartg(gp)
+					break loop
+				}
+				break
+			}
+			fallthrough
+
+		case _Grunnable, _Gwaiting:
 			// Claim goroutine by setting scan bit.
 			// Racing with execution or readying of gp.
 			// The scan bit keeps them from running
@@ -954,6 +998,11 @@ loop:
 
 // The GC requests that this routine be moved from a scanmumble state to a mumble state.
 func restartg(gp *g) {
+	if gp.scang != 0 || gp.scangcw != 0 {
+		print("g ", gp.goid, "is being scanned scang=", gp.scang, " scangcw=", gp.scangcw, "\n")
+		throw("restartg: being scanned")
+	}
+
 	s := readgstatus(gp)
 	switch s {
 	default:
Index: libgo/go/runtime/runtime2.go
===================================================================
--- libgo/go/runtime/runtime2.go	(revision 266510)
+++ libgo/go/runtime/runtime2.go	(working copy)
@@ -430,6 +430,9 @@ type g struct {
 
 	scanningself bool // whether goroutine is scanning its own stack
 
+	scang   uintptr // the g that wants to scan this g's stack (uintptr to avoid write barrier)
+	scangcw uintptr // gc worker for scanning stack (uintptr to avoid write barrier)
+
 	isSystemGoroutine bool // whether goroutine is a "system" goroutine
 
 	traceback uintptr // stack traceback buffer
@@ -514,6 +517,8 @@ type m struct {
 	exiting    bool // thread is exiting
 
 	gcing int32
+
+	scannote note // synchonization for signal-based stack scanning
 }
 
 type p struct {
Index: libgo/go/runtime/signal_sighandler.go
===================================================================
--- libgo/go/runtime/signal_sighandler.go	(revision 266510)
+++ libgo/go/runtime/signal_sighandler.go	(working copy)
@@ -36,6 +36,28 @@ func sighandler(sig uint32, info *_sigin
 
 	sigfault, sigpc := getSiginfo(info, ctxt)
 
+	if sig == _SIGURG && usestackmaps {
+		// We may be signaled to do a stack scan.
+		// The signal delivery races with enter/exitsyscall.
+		// We may be on g0 stack now. gp.m.curg is the g we
+		// want to scan.
+		// If we're not on g stack, give up. The sender will
+		// try again later.
+		// If we're not stopped at a safepoint (doscanstack will
+		// return false), also give up.
+		if s := readgstatus(gp.m.curg); s == _Gscansyscall {
+			if gp == gp.m.curg {
+				if doscanstack(gp, (*gcWork)(unsafe.Pointer(gp.scangcw))) {
+					gp.gcscanvalid = true
+					gp.gcscandone = true
+				}
+			}
+			gp.m.curg.scangcw = 0
+			notewakeup(&gp.m.scannote)
+			return
+		}
+	}
+
 	if sig == _SIGPROF {
 		sigprof(sigpc, gp, _g_.m)
 		return
Index: libgo/go/runtime/stubs.go
===================================================================
--- libgo/go/runtime/stubs.go	(revision 266510)
+++ libgo/go/runtime/stubs.go	(working copy)
@@ -447,3 +447,10 @@ func bool2int(x bool) int {
 // signal handler, which will attempt to tear down the runtime
 // immediately.
 func abort()
+
+// usestackmaps is true if stack map (precise stack scan) is enabled.
+var usestackmaps bool
+
+// probestackmaps detects whether there are stack maps.
+//go:linkname probestackmaps runtime.probestackmaps
+func probestackmaps() bool
Index: libgo/go/runtime/stubs_linux.go
===================================================================
--- libgo/go/runtime/stubs_linux.go	(revision 266510)
+++ libgo/go/runtime/stubs_linux.go	(working copy)
@@ -7,3 +7,11 @@
 package runtime
 
 func sbrk0() uintptr
+
+func gettid() _pid_t {
+	return _pid_t(syscall(_SYS_gettid, 0, 0, 0, 0, 0, 0))
+}
+
+func tgkill(pid _pid_t, tid _pid_t, sig uint32) uint32 {
+	return uint32(syscall(_SYS_tgkill, uintptr(pid), uintptr(tid), uintptr(sig), 0, 0, 0))
+}
Index: libgo/go/runtime/stubs_nonlinux.go
===================================================================
--- libgo/go/runtime/stubs_nonlinux.go	(revision 266510)
+++ libgo/go/runtime/stubs_nonlinux.go	(working copy)
@@ -10,3 +10,11 @@ package runtime
 func sbrk0() uintptr {
 	return 0
 }
+
+func gettid() _pid_t {
+	return 0
+}
+
+func tgkill(pid _pid_t, tid _pid_t, sig uint32) uint32 {
+	throw("tgkill not implemented")
+}
Index: libgo/runtime/go-unwind.c
===================================================================
--- libgo/runtime/go-unwind.c	(revision 266510)
+++ libgo/runtime/go-unwind.c	(working copy)
@@ -304,6 +304,26 @@ read_encoded_value (struct _Unwind_Conte
   return p;
 }
 
+static inline int
+value_size (uint8_t encoding)
+{
+  switch (encoding & 0x0f)
+    {
+      case DW_EH_PE_sdata2:
+      case DW_EH_PE_udata2:
+        return 2;
+      case DW_EH_PE_sdata4:
+      case DW_EH_PE_udata4:
+        return 4;
+      case DW_EH_PE_sdata8:
+      case DW_EH_PE_udata8:
+        return 8;
+      default:
+        break;
+    }
+  abort ();
+}
+
 /* The rest of this code is really similar to gcc/unwind-c.c and
    libjava/exception.cc.  */
 
@@ -563,3 +583,231 @@ PERSONALITY_FUNCTION (int version,
   _Unwind_SetIP (context, landing_pad);
   return _URC_INSTALL_CONTEXT;
 }
+
+// A dummy personality function, which doesn't capture any exception
+// and simply passes by. This is used for functions that don't
+// capture exceptions but need LSDA for stack maps.
+_Unwind_Reason_Code
+__gccgo_personality_dummy (int, _Unwind_Action, _Unwind_Exception_Class,
+		      struct _Unwind_Exception *, struct _Unwind_Context *)
+  __attribute__ ((no_split_stack));
+
+_Unwind_Reason_Code
+__gccgo_personality_dummy (int version __attribute__ ((unused)),
+		      _Unwind_Action actions __attribute__ ((unused)),
+		      _Unwind_Exception_Class exception_class __attribute__ ((unused)),
+		      struct _Unwind_Exception *ue_header __attribute__ ((unused)),
+		      struct _Unwind_Context *context __attribute__ ((unused)))
+{
+  CONTINUE_UNWINDING;
+}
+
+// A sentinel value for Go functions.
+// A function is a Go function if it has LSDA, which has type info,
+// and the first (dummy) landing pad's type info is a pointer to
+// this value.
+#define GO_FUNC_SENTINEL ((uint64)'G' | ((uint64)'O'<<8) | \
+                          ((uint64)'.'<<16) | ((uint64)'.'<<24) | \
+                          ((uint64)'F'<<32) | ((uint64)'U'<<40) | \
+                          ((uint64)'N'<<48) | ((uint64)'C'<<56))
+
+struct _stackmap {
+  uint32 len;
+  uint8 data[1]; // variabe length
+};
+
+extern void
+  runtime_scanstackblockwithmap (uintptr ip, uintptr sp, uintptr size, uint8 *ptrmask, void* gcw)
+  __asm__ (GOSYM_PREFIX "runtime.scanstackblockwithmap");
+
+#define FOUND        0
+#define NOTFOUND_OK  1
+#define NOTFOUND_BAD 2
+
+// Helper function to search for stack maps in the unwinding records of a frame.
+// If found, populate ip, sp, and stackmap. Returns the #define'd values above.
+static int
+findstackmaps (struct _Unwind_Context *context, _Unwind_Ptr *ip, _Unwind_Ptr *sp, struct _stackmap **stackmap)
+{
+  lsda_header_info info;
+  const unsigned char *language_specific_data, *p, *action_record;
+  bool first;
+  struct _stackmap *stackmap1;
+  _Unwind_Ptr ip1;
+  int ip_before_insn = 0;
+  _sleb128_t index;
+  int size;
+
+  language_specific_data = (const unsigned char *)
+    _Unwind_GetLanguageSpecificData (context);
+
+  /* If no LSDA, then there is no stack maps.  */
+  if (! language_specific_data)
+    return NOTFOUND_OK;
+
+  p = parse_lsda_header (context, language_specific_data, &info);
+
+  if (info.TType == NULL)
+    return NOTFOUND_OK;
+
+#ifdef HAVE_GETIPINFO
+  ip1 = _Unwind_GetIPInfo (context, &ip_before_insn);
+#else
+  ip1 = _Unwind_GetIP (context);
+#endif
+  if (! ip_before_insn)
+    --ip1;
+
+  size = value_size (info.ttype_encoding);
+
+  action_record = NULL;
+  first = true;
+
+  /* Search the call-site table for the action associated with this IP.  */
+  while (p < info.action_table)
+    {
+      _Unwind_Ptr cs_start, cs_len, cs_lp;
+      _uleb128_t cs_action;
+
+      /* Note that all call-site encodings are "absolute" displacements.  */
+      p = read_encoded_value (0, info.call_site_encoding, p, &cs_start);
+      p = read_encoded_value (0, info.call_site_encoding, p, &cs_len);
+      p = read_encoded_value (0, info.call_site_encoding, p, &cs_lp);
+      p = read_uleb128 (p, &cs_action);
+
+      if (first)
+        {
+          // For a Go function, the first entry points to the sentinel value.
+          // Check this here.
+          const unsigned char *p1, *action1;
+          uint64 *x;
+
+          if (!cs_action)
+            return NOTFOUND_OK;
+
+          action1 = info.action_table + cs_action - 1;
+          read_sleb128 (action1, &index);
+          p1 = info.TType - index*size;
+          read_encoded_value (context, info.ttype_encoding, p1, (_Unwind_Ptr*)&x);
+          if (x == NULL || *x != GO_FUNC_SENTINEL)
+            return NOTFOUND_OK;
+
+          first = false;
+          continue;
+        }
+
+      /* The table is sorted, so if we've passed the ip, stop.  */
+      if (ip1 < info.Start + cs_start)
+        return NOTFOUND_BAD;
+      else if (ip1 < info.Start + cs_start + cs_len)
+        {
+          if (cs_action)
+            action_record = info.action_table + cs_action - 1;
+          break;
+        }
+    }
+
+  if (action_record == NULL)
+    return NOTFOUND_BAD;
+
+  read_sleb128 (action_record, &index);
+  p = info.TType - index*size;
+  read_encoded_value (context, info.ttype_encoding, p, (_Unwind_Ptr*)&stackmap1);
+  if (stackmap1 == NULL)
+    return NOTFOUND_BAD;
+
+  if (ip != NULL)
+    *ip = ip1;
+  if (sp != NULL)
+    *sp = _Unwind_GetCFA (context);
+  if (stackmap != NULL)
+    *stackmap = stackmap1;
+  return FOUND;
+}
+
+// Callback function to scan a stack frame with stack maps.
+// It skips non-Go functions.
+static _Unwind_Reason_Code
+scanstackwithmap_callback (struct _Unwind_Context *context, void *arg)
+{
+  struct _stackmap *stackmap;
+  _Unwind_Ptr ip, sp;
+  G* gp;
+  void *gcw = arg;
+
+  switch (findstackmaps (context, &ip, &sp, &stackmap))
+    {
+      case NOTFOUND_OK:
+        // Not a Go function. Skip this frame.
+        return _URC_NO_REASON;
+      case NOTFOUND_BAD:
+        {
+          // No stack map found.
+          // If we're scanning from the signal stack, the goroutine
+          // may be not stopped at a safepoint. Allow this case.
+          gp = runtime_g ();
+          if (gp != gp->m->gsignal)
+            {
+              // TODO: print gp, pc, sp
+              runtime_throw ("no stack map");
+            }
+          return _URC_NORMAL_STOP;
+        }
+      case FOUND:
+        break;
+      default:
+        abort ();
+    }
+
+  runtime_scanstackblockwithmap (ip, sp, (uintptr)(stackmap->len) * sizeof(uintptr), stackmap->data, gcw);
+
+  return _URC_NO_REASON;
+}
+
+// Scan the stack with stack maps. Return whether the scan
+// succeeded.
+bool
+scanstackwithmap (void *gcw)
+{
+  _Unwind_Reason_Code code;
+  code = _Unwind_Backtrace (scanstackwithmap_callback, gcw);
+  return code == _URC_END_OF_STACK;
+}
+
+// Returns whether stack map is enabled.
+bool
+usestackmaps ()
+{
+  return runtime_usestackmaps;
+}
+
+// Callback function to probe if a stack frame has stack maps.
+static _Unwind_Reason_Code
+probestackmaps_callback (struct _Unwind_Context *context,
+                         void *arg __attribute__ ((unused)))
+{
+  switch (findstackmaps (context, NULL, NULL, NULL))
+    {
+      case NOTFOUND_OK:
+      case NOTFOUND_BAD:
+        return _URC_NO_REASON;
+      case FOUND:
+        break;
+      default:
+        abort ();
+    }
+
+  // Found a stack map. No need to keep unwinding.
+  runtime_usestackmaps = true;
+  return _URC_NORMAL_STOP;
+}
+
+// Try to find a stack map, store the result in global variable runtime_usestackmaps.
+// Called in start-up time from Go code, so there is a Go frame on the stack.
+bool
+probestackmaps ()
+{
+  runtime_usestackmaps = false;
+  _Unwind_Backtrace (probestackmaps_callback, NULL);
+  return runtime_usestackmaps;
+}
Index: libgo/runtime/proc.c
===================================================================
--- libgo/runtime/proc.c	(revision 266510)
+++ libgo/runtime/proc.c	(working copy)
@@ -59,6 +59,8 @@ uintptr runtime_stacks_sys;
 void gtraceback(G*)
   __asm__(GOSYM_PREFIX "runtime.gtraceback");
 
+static void gscanstack(G*);
+
 #ifdef __rtems__
 #define __thread
 #endif
@@ -340,6 +342,8 @@ runtime_mcall(FuncVal *fv)
 
 		if(gp->traceback != 0)
 			gtraceback(gp);
+		if(gp->scang != 0)
+			gscanstack(gp);
 	}
 	if (gp == nil || !gp->fromgogo) {
 #ifdef USING_SPLIT_STACK
@@ -469,6 +473,66 @@ gtraceback(G* gp)
 	runtime_gogo(traceback->gp);
 }
 
+void doscanstackswitch(G*, G*) __asm__(GOSYM_PREFIX "runtime.doscanstackswitch");
+
+// Switch to gp and let it scan its stack.
+// The first time gp->scang is set (to me). The second time here
+// gp is done scanning, and has unset gp->scang, so we just return.
+void
+doscanstackswitch(G* me, G* gp)
+{
+	__go_assert(me->entry == nil);
+	me->fromgogo = false;
+
+#ifdef USING_SPLIT_STACK
+	__splitstack_getcontext((void*)(&me->stackcontext[0]));
+#endif
+	getcontext(ucontext_arg(&me->context[0]));
+
+	if(me->entry != nil) {
+		// Got here from mcall.
+		// The stack scanning code may call systemstack, which calls
+		// mcall, which calls setcontext.
+		// Run the function, which at the end will switch back to gp.
+		FuncVal *fv = me->entry;
+		void (*pfn)(G*) = (void (*)(G*))fv->fn;
+		G* gp1 = (G*)me->param;
+		__go_assert(gp1 == gp);
+		me->entry = nil;
+		me->param = nil;
+		__builtin_call_with_static_chain(pfn(gp1), fv);
+		abort();
+	}
+
+	if (gp->scang != 0)
+		runtime_gogo(gp);
+}
+
+// Do a stack scan, then switch back to the g that triggers this scan.
+// We come here from doscanstackswitch.
+static void
+gscanstack(G *gp)
+{
+	G *oldg, *oldcurg;
+	M* holdm;
+
+	oldg = (G*)gp->scang;
+	oldcurg = oldg->m->curg;
+	holdm = gp->m;
+	if(holdm != nil && holdm != g->m)
+		runtime_throw("gscanstack: m is not nil");
+	oldg->m->curg = gp;
+	gp->m = oldg->m;
+	gp->scang = 0;
+
+	doscanstack(gp, (void*)gp->scangcw);
+
+	gp->scangcw = 0;
+	gp->m = holdm;
+	oldg->m->curg = oldcurg;
+	runtime_gogo(oldg);
+}
+
 // Called by pthread_create to start an M.
 void*
 runtime_mstart(void *arg)
@@ -516,6 +580,9 @@ runtime_mstart(void *arg)
 		// may always go to the getcontext call in mcall.
 		gtraceback(gp);
 	}
+	if(gp->scang != 0)
+		// Got here from doscanswitch. Should not happen.
+		runtime_throw("mstart with scang");
 
 	if(gp->entry != nil) {
 		// Got here from mcall.
@@ -630,7 +697,8 @@ runtime_entersyscall()
 {
 	// Save the registers in the g structure so that any pointers
 	// held in registers will be seen by the garbage collector.
-	getcontext(ucontext_arg(&g->gcregs[0]));
+	if (!runtime_usestackmaps)
+		getcontext(ucontext_arg(&g->gcregs[0]));
 
 	// Note that if this function does save any registers itself,
 	// we might store the wrong value in the call to getcontext.
@@ -676,7 +744,8 @@ runtime_entersyscallblock()
 {
 	// Save the registers in the g structure so that any pointers
 	// held in registers will be seen by the garbage collector.
-	getcontext(ucontext_arg(&g->gcregs[0]));
+	if (!runtime_usestackmaps)
+		getcontext(ucontext_arg(&g->gcregs[0]));
 
 	// See comment in runtime_entersyscall.
 	doentersyscallblock((uintptr)runtime_getcallerpc(),
Index: libgo/runtime/runtime.h
===================================================================
--- libgo/runtime/runtime.h	(revision 266510)
+++ libgo/runtime/runtime.h	(working copy)
@@ -502,3 +502,16 @@ struct funcfileline_return
 struct funcfileline_return
 runtime_funcfileline (uintptr targetpc, int32 index)
   __asm__ (GOSYM_PREFIX "runtime.funcfileline");
+
+/*
+ * helpers for stack scan.
+ */
+bool scanstackwithmap(void*)
+  __asm__(GOSYM_PREFIX "runtime.scanstackwithmap");
+bool doscanstack(G*, void*)
+  __asm__("runtime.doscanstack");
+
+bool runtime_usestackmaps;
+
+bool probestackmaps(void)
+  __asm__("runtime.probestackmaps");
Index: libgo/runtime/stack.c
===================================================================
--- libgo/runtime/stack.c	(revision 266510)
+++ libgo/runtime/stack.c	(working copy)
@@ -23,33 +23,43 @@ extern void * __splitstack_find_context
 extern void scanstackblock(void *addr, uintptr size, void *gcw)
   __asm__("runtime.scanstackblock");
 
-void doscanstack(G*, void*)
-  __asm__("runtime.doscanstack");
-
-static void doscanstack1(G*, void*)
+static bool doscanstack1(G*, void*)
   __attribute__ ((noinline));
 
 // Scan gp's stack, passing stack chunks to scanstackblock.
-void doscanstack(G *gp, void* gcw) {
+bool doscanstack(G *gp, void* gcw) {
 	// Save registers on the stack, so that if we are scanning our
 	// own stack we will see them.
-	__builtin_unwind_init();
-	flush_registers_to_secondary_stack();
+	if (!runtime_usestackmaps) {
+		__builtin_unwind_init();
+		flush_registers_to_secondary_stack();
+	}
 
-	doscanstack1(gp, gcw);
+	return doscanstack1(gp, gcw);
 }
 
 // Scan gp's stack after saving registers.
-static void doscanstack1(G *gp, void *gcw) {
+static bool doscanstack1(G *gp, void *gcw) {
 #ifdef USING_SPLIT_STACK
 	void* sp;
 	size_t spsize;
 	void* next_segment;
 	void* next_sp;
 	void* initial_sp;
+	G* _g_;
 
-	if (gp == runtime_g()) {
+	_g_ = runtime_g();
+	if (runtime_usestackmaps) {
+		// If stack map is enabled, we get here only when we can unwind
+		// the stack being scanned. That is, either we are scanning our
+		// own stack, or we are scanning through a signal handler.
+		__go_assert((_g_ == gp) || ((_g_ == gp->m->gsignal) && (gp == gp->m->curg)));
+		return scanstackwithmap(gcw);
+	}
+	if (_g_ == gp) {
 		// Scanning our own stack.
+		// If we are on a signal stack, it can unwind through the signal
+		// handler and see the g stack, so just scan our own stack.
 		sp = __splitstack_find(nil, nil, &spsize, &next_segment,
 				       &next_sp, &initial_sp);
 	} else {
@@ -95,7 +105,7 @@ static void doscanstack1(G *gp, void *gc
 		// The goroutine is usually asleep (the world is stopped).
 		bottom = (void*)gp->gcnextsp;
 		if(bottom == nil)
-			return;
+			return true;
 		nextsp2 = (void*)gp->gcnextsp2;
 	}
 	top = (byte*)(void*)(gp->gcinitialsp) + gp->gcstacksize;
@@ -111,4 +121,5 @@ static void doscanstack1(G *gp, void *gc
 			scanstackblock(initialsp2, (uintptr)(nextsp2 - initialsp2), gcw);
 	}
 #endif
+	return true;
 }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: libgo patch committed: Add precise stack scan support
  2018-12-05 23:10 libgo patch committed: Add precise stack scan support Ian Lance Taylor
@ 2018-12-07 10:07 ` Rainer Orth
  2018-12-07 14:23   ` Ian Lance Taylor
  2018-12-10  6:41 ` Matthias Klose
  1 sibling, 1 reply; 15+ messages in thread
From: Rainer Orth @ 2018-12-07 10:07 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc-patches, gofrontend-dev

[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]

Hi Ian,

> This libgo patch by Cherry Zhang adds support for precise stack
> scanning to the Go runtime.  This uses per-function stack maps stored
> in the exception tables in the language-specific data area.  The
> compiler needs to generate these stack maps; currently this is only
> done by a version of LLVM, not by GCC.  Each safepoint in a function
> is associated with a (real or dummy) landing pad, and its "type info"
> in the exception table is a pointer to the stack map. When a stack is
> scanned, the stack map is found by the stack unwinding code.
>
> For precise stack scan we need to unwind the stack. There are three cases:
>
> - If a goroutine is scanning its own stack, it can unwind the stack
> and scan the frames.
>
> - If a goroutine is scanning another, stopped, goroutine, it cannot
> directly unwind the target stack. We handle this by switching
> (runtime.gogo) to the target g, letting it unwind and scan the stack,
> and switch back.
>
> - If we are scanning a goroutine that is blocked in a syscall, we send
> a signal to the target goroutine's thread, and let the signal handler
> unwind and scan the stack. Extra care is needed as this races with
> enter/exit syscall.
>
> Currently this is only implemented on GNU/Linux.
>
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline.

this broke Solaris (and other non-Linux) bootstrap:

/vol/gcc/src/hg/trunk/local/libgo/go/runtime/stubs_nonlinux.go:20:1: error: missing return at end of function
   20 | }
      | ^

Fixed by returning 0 for now, the return value is ignored in
go/runtime/proc.go (scang) anyway.

The Solaris equivalents would be thr_self (for gettid) and thr_kill (for
tgkill).  It were probably better to use non-Linux specific names
here...

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: libgo-stubs_nonlinux.patch --]
[-- Type: text/x-patch, Size: 435 bytes --]

# HG changeset patch
# Parent  2c9abfeb2f017f5c48ccdee7ce72d72bcffb9250
Fix go/runtime/stubs_nonlinux.go compilation

diff --git a/libgo/go/runtime/stubs_nonlinux.go b/libgo/go/runtime/stubs_nonlinux.go
--- a/libgo/go/runtime/stubs_nonlinux.go
+++ b/libgo/go/runtime/stubs_nonlinux.go
@@ -17,4 +17,5 @@ func gettid() _pid_t {
 
 func tgkill(pid _pid_t, tid _pid_t, sig uint32) uint32 {
 	throw("tgkill not implemented")
+	return 0;
 }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: libgo patch committed: Add precise stack scan support
  2018-12-07 10:07 ` Rainer Orth
@ 2018-12-07 14:23   ` Ian Lance Taylor
  2018-12-07 15:15     ` [gofrontend-dev] " Cherry Zhang via gcc-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Ian Lance Taylor @ 2018-12-07 14:23 UTC (permalink / raw)
  To: Rainer Orth; +Cc: gcc-patches, gofrontend-dev

On Fri, Dec 7, 2018 at 2:07 AM Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>
> > This libgo patch by Cherry Zhang adds support for precise stack
> > scanning to the Go runtime.  This uses per-function stack maps stored
> > in the exception tables in the language-specific data area.  The
> > compiler needs to generate these stack maps; currently this is only
> > done by a version of LLVM, not by GCC.  Each safepoint in a function
> > is associated with a (real or dummy) landing pad, and its "type info"
> > in the exception table is a pointer to the stack map. When a stack is
> > scanned, the stack map is found by the stack unwinding code.
> >
> > For precise stack scan we need to unwind the stack. There are three cases:
> >
> > - If a goroutine is scanning its own stack, it can unwind the stack
> > and scan the frames.
> >
> > - If a goroutine is scanning another, stopped, goroutine, it cannot
> > directly unwind the target stack. We handle this by switching
> > (runtime.gogo) to the target g, letting it unwind and scan the stack,
> > and switch back.
> >
> > - If we are scanning a goroutine that is blocked in a syscall, we send
> > a signal to the target goroutine's thread, and let the signal handler
> > unwind and scan the stack. Extra care is needed as this races with
> > enter/exit syscall.
> >
> > Currently this is only implemented on GNU/Linux.
> >
> > Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> > to mainline.
>
> this broke Solaris (and other non-Linux) bootstrap:
>
> /vol/gcc/src/hg/trunk/local/libgo/go/runtime/stubs_nonlinux.go:20:1: error: missing return at end of function
>    20 | }
>       | ^
>
> Fixed by returning 0 for now, the return value is ignored in
> go/runtime/proc.go (scang) anyway.

Thanks.  Committed to mainline.

Ian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-07 14:23   ` Ian Lance Taylor
@ 2018-12-07 15:15     ` Cherry Zhang via gcc-patches
  0 siblings, 0 replies; 15+ messages in thread
From: Cherry Zhang via gcc-patches @ 2018-12-07 15:15 UTC (permalink / raw)
  To: Ian Taylor; +Cc: ro, gcc-patches, gofrontend-dev

Sorry about the breakage. Thanks for the patch.


On Fri, Dec 7, 2018 at 9:23 AM Ian Lance Taylor <iant@golang.org> wrote:

> On Fri, Dec 7, 2018 at 2:07 AM Rainer Orth <ro@cebitec.uni-bielefeld.de>
> wrote:
> >
> > > This libgo patch by Cherry Zhang adds support for precise stack
> > > scanning to the Go runtime.  This uses per-function stack maps stored
> > > in the exception tables in the language-specific data area.  The
> > > compiler needs to generate these stack maps; currently this is only
> > > done by a version of LLVM, not by GCC.  Each safepoint in a function
> > > is associated with a (real or dummy) landing pad, and its "type info"
> > > in the exception table is a pointer to the stack map. When a stack is
> > > scanned, the stack map is found by the stack unwinding code.
> > >
> > > For precise stack scan we need to unwind the stack. There are three
> cases:
> > >
> > > - If a goroutine is scanning its own stack, it can unwind the stack
> > > and scan the frames.
> > >
> > > - If a goroutine is scanning another, stopped, goroutine, it cannot
> > > directly unwind the target stack. We handle this by switching
> > > (runtime.gogo) to the target g, letting it unwind and scan the stack,
> > > and switch back.
> > >
> > > - If we are scanning a goroutine that is blocked in a syscall, we send
> > > a signal to the target goroutine's thread, and let the signal handler
> > > unwind and scan the stack. Extra care is needed as this races with
> > > enter/exit syscall.
> > >
> > > Currently this is only implemented on GNU/Linux.
> > >
> > > Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> > > to mainline.
> >
> > this broke Solaris (and other non-Linux) bootstrap:
> >
> > /vol/gcc/src/hg/trunk/local/libgo/go/runtime/stubs_nonlinux.go:20:1:
> error: missing return at end of function
> >    20 | }
> >       | ^
> >
> > Fixed by returning 0 for now, the return value is ignored in
> > go/runtime/proc <https://goto.google.com/runtime/proc>.go (scang)
> anyway.
>
> Thanks.  Committed to mainline.
>
> Ian
>
> --
> You received this message because you are subscribed to the Google Groups
> "gofrontend-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to gofrontend-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: libgo patch committed: Add precise stack scan support
  2018-12-05 23:10 libgo patch committed: Add precise stack scan support Ian Lance Taylor
  2018-12-07 10:07 ` Rainer Orth
@ 2018-12-10  6:41 ` Matthias Klose
  2018-12-10 15:54   ` [gofrontend-dev] " Cherry Zhang via gcc-patches
  1 sibling, 1 reply; 15+ messages in thread
From: Matthias Klose @ 2018-12-10  6:41 UTC (permalink / raw)
  To: Ian Lance Taylor, gcc-patches, gofrontend-dev

On 06.12.18 00:09, Ian Lance Taylor wrote:
> This libgo patch by Cherry Zhang adds support for precise stack
> scanning to the Go runtime.  This uses per-function stack maps stored
> in the exception tables in the language-specific data area.  The
> compiler needs to generate these stack maps; currently this is only
> done by a version of LLVM, not by GCC.  Each safepoint in a function
> is associated with a (real or dummy) landing pad, and its "type info"
> in the exception table is a pointer to the stack map. When a stack is
> scanned, the stack map is found by the stack unwinding code.
> 
> For precise stack scan we need to unwind the stack. There are three cases:
> 
> - If a goroutine is scanning its own stack, it can unwind the stack
> and scan the frames.
> 
> - If a goroutine is scanning another, stopped, goroutine, it cannot
> directly unwind the target stack. We handle this by switching
> (runtime.gogo) to the target g, letting it unwind and scan the stack,
> and switch back.
> 
> - If we are scanning a goroutine that is blocked in a syscall, we send
> a signal to the target goroutine's thread, and let the signal handler
> unwind and scan the stack. Extra care is needed as this races with
> enter/exit syscall.
> 
> Currently this is only implemented on GNU/Linux.
> 
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline.

this broke the libgo build on ARM32:

../../../src/libgo/runtime/go-unwind.c: In function 'scanstackwithmap_callback':
../../../src/libgo/runtime/go-unwind.c:754:18: error: '_URC_NORMAL_STOP'
undeclared (first use in this function)
  754 |           return _URC_NORMAL_STOP;
      |                  ^~~~~~~~~~~~~~~~
../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared identifier
is reported only once for each function i
t appears in
../../../src/libgo/runtime/go-unwind.c: In function 'probestackmaps_callback':
../../../src/libgo/runtime/go-unwind.c:802:10: error: '_URC_NORMAL_STOP'
undeclared (first use in this function)
  802 |   return _URC_NORMAL_STOP;
      |          ^~~~~~~~~~~~~~~~
../../../src/libgo/runtime/go-unwind.c:803:1: warning: control reaches end of
non-void function [-Wreturn-type]
  803 | }
      | ^
make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
make[6]: Leaving directory '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-10  6:41 ` Matthias Klose
@ 2018-12-10 15:54   ` Cherry Zhang via gcc-patches
  2018-12-11 14:52     ` Matthias Klose
  0 siblings, 1 reply; 15+ messages in thread
From: Cherry Zhang via gcc-patches @ 2018-12-10 15:54 UTC (permalink / raw)
  To: doko; +Cc: Ian Taylor, gcc-patches, gofrontend-dev

On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com> wrote:

> On 06.12.18 00:09, Ian Lance Taylor wrote:
> > This libgo patch by Cherry Zhang adds support for precise stack
> > scanning to the Go runtime.  This uses per-function stack maps stored
> > in the exception tables in the language-specific data area.  The
> > compiler needs to generate these stack maps; currently this is only
> > done by a version of LLVM, not by GCC.  Each safepoint in a function
> > is associated with a (real or dummy) landing pad, and its "type info"
> > in the exception table is a pointer to the stack map. When a stack is
> > scanned, the stack map is found by the stack unwinding code.
> >
> > For precise stack scan we need to unwind the stack. There are three
> cases:
> >
> > - If a goroutine is scanning its own stack, it can unwind the stack
> > and scan the frames.
> >
> > - If a goroutine is scanning another, stopped, goroutine, it cannot
> > directly unwind the target stack. We handle this by switching
> > (runtime.gogo) to the target g, letting it unwind and scan the stack,
> > and switch back.
> >
> > - If we are scanning a goroutine that is blocked in a syscall, we send
> > a signal to the target goroutine's thread, and let the signal handler
> > unwind and scan the stack. Extra care is needed as this races with
> > enter/exit syscall.
> >
> > Currently this is only implemented on GNU/Linux.
> >
> > Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> > to mainline.
>
> this broke the libgo build on ARM32:
>
> ../../../src/libgo/runtime/go-unwind.c: In function
> 'scanstackwithmap_callback':
> ../../../src/libgo/runtime/go-unwind.c:754:18: error: '_URC_NORMAL_STOP'
> undeclared (first use in this function)
>   754 |           return _URC_NORMAL_STOP;
>       |                  ^~~~~~~~~~~~~~~~
> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
> identifier
> is reported only once for each function i
> t appears in
> ../../../src/libgo/runtime/go-unwind.c: In function
> 'probestackmaps_callback':
> ../../../src/libgo/runtime/go-unwind.c:802:10: error: '_URC_NORMAL_STOP'
> undeclared (first use in this function)
>   802 |   return _URC_NORMAL_STOP;
>       |          ^~~~~~~~~~~~~~~~
> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control reaches end
> of
> non-void function [-Wreturn-type]
>   803 | }
>       | ^
> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
> make[6]: Leaving directory
> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
>
>
Hell Matthias,

Thank you for the report. And sorry about the breakage. Does
https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
below) fix ARM32 build? I don't have an ARM32 machine at hand to test.

Thanks,
Cherry

diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
index a1a95585..c44755f9 100644
--- a/libgo/runtime/go-unwind.c
+++ b/libgo/runtime/go-unwind.c
@@ -392,6 +392,12 @@ parse_lsda_header (struct _Unwind_Context *context,
const unsigned char *p,
 #define CONTINUE_UNWINDING return _URC_CONTINUE_UNWIND
 #endif

+#ifdef __ARM_EABI_UNWINDER__
+#define STOP_UNWINDING _URC_FAILURE
+#else
+#define STOP_UNWINDING _URC_NORMAL_STOP
+#endif
+
 #ifdef __USING_SJLJ_EXCEPTIONS__
 #define PERSONALITY_FUNCTION    __gccgo_personality_sj0
 #define __builtin_eh_return_data_regno(x) x
@@ -751,7 +757,7 @@ scanstackwithmap_callback (struct _Unwind_Context
*context, void *arg)
               // TODO: print gp, pc, sp
               runtime_throw ("no stack map");
             }
-          return _URC_NORMAL_STOP;
+          return STOP_UNWINDING;
         }
       case FOUND:
         break;
@@ -799,7 +805,7 @@ probestackmaps_callback (struct _Unwind_Context
*context,

   // Found a stack map. No need to keep unwinding.
   runtime_usestackmaps = true;
-  return _URC_NORMAL_STOP;
+  return STOP_UNWINDING;
 }

 // Try to find a stack map, store the result in global variable
runtime_usestackmaps.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-10 15:54   ` [gofrontend-dev] " Cherry Zhang via gcc-patches
@ 2018-12-11 14:52     ` Matthias Klose
  2018-12-11 20:51       ` Ian Lance Taylor
  0 siblings, 1 reply; 15+ messages in thread
From: Matthias Klose @ 2018-12-11 14:52 UTC (permalink / raw)
  To: Cherry Zhang; +Cc: Ian Taylor, gcc-patches, gofrontend-dev

On 10.12.18 16:54, Cherry Zhang wrote:
> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com> wrote:
> 
>> On 06.12.18 00:09, Ian Lance Taylor wrote:
>>> This libgo patch by Cherry Zhang adds support for precise stack
>>> scanning to the Go runtime.  This uses per-function stack maps stored
>>> in the exception tables in the language-specific data area.  The
>>> compiler needs to generate these stack maps; currently this is only
>>> done by a version of LLVM, not by GCC.  Each safepoint in a function
>>> is associated with a (real or dummy) landing pad, and its "type info"
>>> in the exception table is a pointer to the stack map. When a stack is
>>> scanned, the stack map is found by the stack unwinding code.
>>>
>>> For precise stack scan we need to unwind the stack. There are three
>> cases:
>>>
>>> - If a goroutine is scanning its own stack, it can unwind the stack
>>> and scan the frames.
>>>
>>> - If a goroutine is scanning another, stopped, goroutine, it cannot
>>> directly unwind the target stack. We handle this by switching
>>> (runtime.gogo) to the target g, letting it unwind and scan the stack,
>>> and switch back.
>>>
>>> - If we are scanning a goroutine that is blocked in a syscall, we send
>>> a signal to the target goroutine's thread, and let the signal handler
>>> unwind and scan the stack. Extra care is needed as this races with
>>> enter/exit syscall.
>>>
>>> Currently this is only implemented on GNU/Linux.
>>>
>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>>> to mainline.
>>
>> this broke the libgo build on ARM32:
>>
>> ../../../src/libgo/runtime/go-unwind.c: In function
>> 'scanstackwithmap_callback':
>> ../../../src/libgo/runtime/go-unwind.c:754:18: error: '_URC_NORMAL_STOP'
>> undeclared (first use in this function)
>>   754 |           return _URC_NORMAL_STOP;
>>       |                  ^~~~~~~~~~~~~~~~
>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
>> identifier
>> is reported only once for each function i
>> t appears in
>> ../../../src/libgo/runtime/go-unwind.c: In function
>> 'probestackmaps_callback':
>> ../../../src/libgo/runtime/go-unwind.c:802:10: error: '_URC_NORMAL_STOP'
>> undeclared (first use in this function)
>>   802 |   return _URC_NORMAL_STOP;
>>       |          ^~~~~~~~~~~~~~~~
>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control reaches end
>> of
>> non-void function [-Wreturn-type]
>>   803 | }
>>       | ^
>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
>> make[6]: Leaving directory
>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
>>
>>
> Hell Matthias,
> 
> Thank you for the report. And sorry about the breakage. Does
> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
> below) fix ARM32 build? I don't have an ARM32 machine at hand to test.

this fixes the build.

currently running the testsuite, almost every test case core dumps on
arm-linux-gnueabihf

Matthias

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-11 14:52     ` Matthias Klose
@ 2018-12-11 20:51       ` Ian Lance Taylor
  2018-12-11 21:02         ` Cherry Zhang via gcc-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Ian Lance Taylor @ 2018-12-11 20:51 UTC (permalink / raw)
  To: Matthias Klose; +Cc: Cherry Zhang, gcc-patches, gofrontend-dev

On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com> wrote:
>
> On 10.12.18 16:54, Cherry Zhang wrote:
> > On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com> wrote:
> >
> >> On 06.12.18 00:09, Ian Lance Taylor wrote:
> >>> This libgo patch by Cherry Zhang adds support for precise stack
> >>> scanning to the Go runtime.  This uses per-function stack maps stored
> >>> in the exception tables in the language-specific data area.  The
> >>> compiler needs to generate these stack maps; currently this is only
> >>> done by a version of LLVM, not by GCC.  Each safepoint in a function
> >>> is associated with a (real or dummy) landing pad, and its "type info"
> >>> in the exception table is a pointer to the stack map. When a stack is
> >>> scanned, the stack map is found by the stack unwinding code.
> >>>
> >>> For precise stack scan we need to unwind the stack. There are three
> >> cases:
> >>>
> >>> - If a goroutine is scanning its own stack, it can unwind the stack
> >>> and scan the frames.
> >>>
> >>> - If a goroutine is scanning another, stopped, goroutine, it cannot
> >>> directly unwind the target stack. We handle this by switching
> >>> (runtime.gogo) to the target g, letting it unwind and scan the stack,
> >>> and switch back.
> >>>
> >>> - If we are scanning a goroutine that is blocked in a syscall, we send
> >>> a signal to the target goroutine's thread, and let the signal handler
> >>> unwind and scan the stack. Extra care is needed as this races with
> >>> enter/exit syscall.
> >>>
> >>> Currently this is only implemented on GNU/Linux.
> >>>
> >>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> >>> to mainline.
> >>
> >> this broke the libgo build on ARM32:
> >>
> >> ../../../src/libgo/runtime/go-unwind.c: In function
> >> 'scanstackwithmap_callback':
> >> ../../../src/libgo/runtime/go-unwind.c:754:18: error: '_URC_NORMAL_STOP'
> >> undeclared (first use in this function)
> >>   754 |           return _URC_NORMAL_STOP;
> >>       |                  ^~~~~~~~~~~~~~~~
> >> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
> >> identifier
> >> is reported only once for each function i
> >> t appears in
> >> ../../../src/libgo/runtime/go-unwind.c: In function
> >> 'probestackmaps_callback':
> >> ../../../src/libgo/runtime/go-unwind.c:802:10: error: '_URC_NORMAL_STOP'
> >> undeclared (first use in this function)
> >>   802 |   return _URC_NORMAL_STOP;
> >>       |          ^~~~~~~~~~~~~~~~
> >> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control reaches end
> >> of
> >> non-void function [-Wreturn-type]
> >>   803 | }
> >>       | ^
> >> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
> >> make[6]: Leaving directory
> >> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
> >>
> >>
> > Hell Matthias,
> >
> > Thank you for the report. And sorry about the breakage. Does
> > https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
> > below) fix ARM32 build? I don't have an ARM32 machine at hand to test.
>
> this fixes the build.
>
> currently running the testsuite, almost every test case core dumps on
> arm-linux-gnueabihf

I committed Cherry's patch to trunk, since it looks reasonable to me.
Cherry, Matthias, let me know if you figure out why programs are
failing.

Ian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-11 20:51       ` Ian Lance Taylor
@ 2018-12-11 21:02         ` Cherry Zhang via gcc-patches
  2018-12-12  0:03           ` Matthias Klose
  0 siblings, 1 reply; 15+ messages in thread
From: Cherry Zhang via gcc-patches @ 2018-12-11 21:02 UTC (permalink / raw)
  To: Ian Taylor; +Cc: doko, gcc-patches, gofrontend-dev

On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org> wrote:

> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com> wrote:
> >
> > On 10.12.18 16:54, Cherry Zhang wrote:
> > > On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
> wrote:
> > >
> > >> On 06.12.18 00:09, Ian Lance Taylor wrote:
> > >>> This libgo patch by Cherry Zhang adds support for precise stack
> > >>> scanning to the Go runtime.  This uses per-function stack maps stored
> > >>> in the exception tables in the language-specific data area.  The
> > >>> compiler needs to generate these stack maps; currently this is only
> > >>> done by a version of LLVM, not by GCC.  Each safepoint in a function
> > >>> is associated with a (real or dummy) landing pad, and its "type info"
> > >>> in the exception table is a pointer to the stack map. When a stack is
> > >>> scanned, the stack map is found by the stack unwinding code.
> > >>>
> > >>> For precise stack scan we need to unwind the stack. There are three
> > >> cases:
> > >>>
> > >>> - If a goroutine is scanning its own stack, it can unwind the stack
> > >>> and scan the frames.
> > >>>
> > >>> - If a goroutine is scanning another, stopped, goroutine, it cannot
> > >>> directly unwind the target stack. We handle this by switching
> > >>> (runtime.gogo) to the target g, letting it unwind and scan the stack,
> > >>> and switch back.
> > >>>
> > >>> - If we are scanning a goroutine that is blocked in a syscall, we
> send
> > >>> a signal to the target goroutine's thread, and let the signal handler
> > >>> unwind and scan the stack. Extra care is needed as this races with
> > >>> enter/exit syscall.
> > >>>
> > >>> Currently this is only implemented on GNU/Linux.
> > >>>
> > >>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> > >>> to mainline.
> > >>
> > >> this broke the libgo build on ARM32:
> > >>
> > >> ../../../src/libgo/runtime/go-unwind.c: In function
> > >> 'scanstackwithmap_callback':
> > >> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
> '_URC_NORMAL_STOP'
> > >> undeclared (first use in this function)
> > >>   754 |           return _URC_NORMAL_STOP;
> > >>       |                  ^~~~~~~~~~~~~~~~
> > >> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
> > >> identifier
> > >> is reported only once for each function i
> > >> t appears in
> > >> ../../../src/libgo/runtime/go-unwind.c: In function
> > >> 'probestackmaps_callback':
> > >> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
> '_URC_NORMAL_STOP'
> > >> undeclared (first use in this function)
> > >>   802 |   return _URC_NORMAL_STOP;
> > >>       |          ^~~~~~~~~~~~~~~~
> > >> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
> reaches end
> > >> of
> > >> non-void function [-Wreturn-type]
> > >>   803 | }
> > >>       | ^
> > >> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
> > >> make[6]: Leaving directory
> > >> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
> > >>
> > >>
> > > Hell Matthias,
> > >
> > > Thank you for the report. And sorry about the breakage. Does
> > > https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
> > > below) fix ARM32 build? I don't have an ARM32 machine at hand to test.
> >
> > this fixes the build.
> >
> > currently running the testsuite, almost every test case core dumps on
> > arm-linux-gnueabihf
>
> I committed Cherry's patch to trunk, since it looks reasonable to me.
> Cherry, Matthias, let me know if you figure out why programs are
> failing.
>
>
Thank you.

I don't know for the moment. I'm trying to find an ARM32 machine so I can
test.

Matthias, is it convenient for you to share a stack trace for the failing
programs? That would be very helpful. Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-11 21:02         ` Cherry Zhang via gcc-patches
@ 2018-12-12  0:03           ` Matthias Klose
  2018-12-12 16:10             ` Cherry Zhang via gcc-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Matthias Klose @ 2018-12-12  0:03 UTC (permalink / raw)
  To: Cherry Zhang, Ian Taylor; +Cc: gcc-patches, gofrontend-dev

On 11.12.18 22:01, Cherry Zhang wrote:
> On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org> wrote:
> 
>> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com> wrote:
>>>
>>> On 10.12.18 16:54, Cherry Zhang wrote:
>>>> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
>> wrote:
>>>>
>>>>> On 06.12.18 00:09, Ian Lance Taylor wrote:
>>>>>> This libgo patch by Cherry Zhang adds support for precise stack
>>>>>> scanning to the Go runtime.  This uses per-function stack maps stored
>>>>>> in the exception tables in the language-specific data area.  The
>>>>>> compiler needs to generate these stack maps; currently this is only
>>>>>> done by a version of LLVM, not by GCC.  Each safepoint in a function
>>>>>> is associated with a (real or dummy) landing pad, and its "type info"
>>>>>> in the exception table is a pointer to the stack map. When a stack is
>>>>>> scanned, the stack map is found by the stack unwinding code.
>>>>>>
>>>>>> For precise stack scan we need to unwind the stack. There are three
>>>>> cases:
>>>>>>
>>>>>> - If a goroutine is scanning its own stack, it can unwind the stack
>>>>>> and scan the frames.
>>>>>>
>>>>>> - If a goroutine is scanning another, stopped, goroutine, it cannot
>>>>>> directly unwind the target stack. We handle this by switching
>>>>>> (runtime.gogo) to the target g, letting it unwind and scan the stack,
>>>>>> and switch back.
>>>>>>
>>>>>> - If we are scanning a goroutine that is blocked in a syscall, we
>> send
>>>>>> a signal to the target goroutine's thread, and let the signal handler
>>>>>> unwind and scan the stack. Extra care is needed as this races with
>>>>>> enter/exit syscall.
>>>>>>
>>>>>> Currently this is only implemented on GNU/Linux.
>>>>>>
>>>>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>>>>>> to mainline.
>>>>>
>>>>> this broke the libgo build on ARM32:
>>>>>
>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>>>>> 'scanstackwithmap_callback':
>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
>> '_URC_NORMAL_STOP'
>>>>> undeclared (first use in this function)
>>>>>   754 |           return _URC_NORMAL_STOP;
>>>>>       |                  ^~~~~~~~~~~~~~~~
>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
>>>>> identifier
>>>>> is reported only once for each function i
>>>>> t appears in
>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>>>>> 'probestackmaps_callback':
>>>>> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
>> '_URC_NORMAL_STOP'
>>>>> undeclared (first use in this function)
>>>>>   802 |   return _URC_NORMAL_STOP;
>>>>>       |          ^~~~~~~~~~~~~~~~
>>>>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
>> reaches end
>>>>> of
>>>>> non-void function [-Wreturn-type]
>>>>>   803 | }
>>>>>       | ^
>>>>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
>>>>> make[6]: Leaving directory
>>>>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
>>>>>
>>>>>
>>>> Hell Matthias,
>>>>
>>>> Thank you for the report. And sorry about the breakage. Does
>>>> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
>>>> below) fix ARM32 build? I don't have an ARM32 machine at hand to test.
>>>
>>> this fixes the build.
>>>
>>> currently running the testsuite, almost every test case core dumps on
>>> arm-linux-gnueabihf
>>
>> I committed Cherry's patch to trunk, since it looks reasonable to me.
>> Cherry, Matthias, let me know if you figure out why programs are
>> failing.
>>
>>
> Thank you.
> 
> I don't know for the moment. I'm trying to find an ARM32 machine so I can
> test.
> 
> Matthias, is it convenient for you to share a stack trace for the failing
> programs? That would be very helpful. Thanks!

I'll do a local build this weekend. For now you find the build log at
https://launchpad.net/ubuntu/+source/gcc-snapshot/1:20181210-0ubuntu1/+build/15759748

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-12  0:03           ` Matthias Klose
@ 2018-12-12 16:10             ` Cherry Zhang via gcc-patches
  2018-12-12 23:27               ` Ian Lance Taylor
  0 siblings, 1 reply; 15+ messages in thread
From: Cherry Zhang via gcc-patches @ 2018-12-12 16:10 UTC (permalink / raw)
  To: doko; +Cc: Ian Taylor, gcc-patches, gofrontend-dev

Thank you, Matthias!

From the log, essentially all the tests aborted. The only place the new
code can cause abort on all programs that I can think of is in the runtime
startup code, probestackmaps, which calls value_size, which aborts due to
an unhandled case. I haven't been able to try out on an ARM machine, but I
managed to cross-compile a Go program and visually inspect the exception
table. The type table's encoding is DW_EH_PE_absptr, which is indeed not
handled, which will cause abort.

I send https://go-review.googlesource.com/c/gofrontend/+/153857 (also as
below). Hopefully this will fix the problem.

Thanks,
Cherry

diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
index c44755f9..f4bbfb60 100644
--- a/libgo/runtime/go-unwind.c
+++ b/libgo/runtime/go-unwind.c
@@ -318,6 +318,8 @@ value_size (uint8_t encoding)
       case DW_EH_PE_sdata8:
       case DW_EH_PE_udata8:
         return 8;
+      case DW_EH_PE_absptr:
+        return sizeof(uintptr);
       default:
         break;
     }



On Tue, Dec 11, 2018 at 7:03 PM Matthias Klose <doko@ubuntu.com> wrote:

> On 11.12.18 22:01, Cherry Zhang wrote:
> > On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org>
> wrote:
> >
> >> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com> wrote:
> >>>
> >>> On 10.12.18 16:54, Cherry Zhang wrote:
> >>>> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
> >> wrote:
> >>>>
> >>>>> On 06.12.18 00:09, Ian Lance Taylor wrote:
> >>>>>> This libgo patch by Cherry Zhang adds support for precise stack
> >>>>>> scanning to the Go runtime.  This uses per-function stack maps
> stored
> >>>>>> in the exception tables in the language-specific data area.  The
> >>>>>> compiler needs to generate these stack maps; currently this is only
> >>>>>> done by a version of LLVM, not by GCC.  Each safepoint in a function
> >>>>>> is associated with a (real or dummy) landing pad, and its "type
> info"
> >>>>>> in the exception table is a pointer to the stack map. When a stack
> is
> >>>>>> scanned, the stack map is found by the stack unwinding code.
> >>>>>>
> >>>>>> For precise stack scan we need to unwind the stack. There are three
> >>>>> cases:
> >>>>>>
> >>>>>> - If a goroutine is scanning its own stack, it can unwind the stack
> >>>>>> and scan the frames.
> >>>>>>
> >>>>>> - If a goroutine is scanning another, stopped, goroutine, it cannot
> >>>>>> directly unwind the target stack. We handle this by switching
> >>>>>> (runtime.gogo) to the target g, letting it unwind and scan the
> stack,
> >>>>>> and switch back.
> >>>>>>
> >>>>>> - If we are scanning a goroutine that is blocked in a syscall, we
> >> send
> >>>>>> a signal to the target goroutine's thread, and let the signal
> handler
> >>>>>> unwind and scan the stack. Extra care is needed as this races with
> >>>>>> enter/exit syscall.
> >>>>>>
> >>>>>> Currently this is only implemented on GNU/Linux.
> >>>>>>
> >>>>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> >>>>>> to mainline.
> >>>>>
> >>>>> this broke the libgo build on ARM32:
> >>>>>
> >>>>> ../../../src/libgo/runtime/go-unwind.c: In function
> >>>>> 'scanstackwithmap_callback':
> >>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
> >> '_URC_NORMAL_STOP'
> >>>>> undeclared (first use in this function)
> >>>>>   754 |           return _URC_NORMAL_STOP;
> >>>>>       |                  ^~~~~~~~~~~~~~~~
> >>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
> >>>>> identifier
> >>>>> is reported only once for each function i
> >>>>> t appears in
> >>>>> ../../../src/libgo/runtime/go-unwind.c: In function
> >>>>> 'probestackmaps_callback':
> >>>>> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
> >> '_URC_NORMAL_STOP'
> >>>>> undeclared (first use in this function)
> >>>>>   802 |   return _URC_NORMAL_STOP;
> >>>>>       |          ^~~~~~~~~~~~~~~~
> >>>>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
> >> reaches end
> >>>>> of
> >>>>> non-void function [-Wreturn-type]
> >>>>>   803 | }
> >>>>>       | ^
> >>>>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
> >>>>> make[6]: Leaving directory
> >>>>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
> >>>>>
> >>>>>
> >>>> Hell Matthias,
> >>>>
> >>>> Thank you for the report. And sorry about the breakage. Does
> >>>> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the
> patch
> >>>> below) fix ARM32 build? I don't have an ARM32 machine at hand to test.
> >>>
> >>> this fixes the build.
> >>>
> >>> currently running the testsuite, almost every test case core dumps on
> >>> arm-linux-gnueabihf
> >>
> >> I committed Cherry's patch to trunk, since it looks reasonable to me.
> >> Cherry, Matthias, let me know if you figure out why programs are
> >> failing.
> >>
> >>
> > Thank you.
> >
> > I don't know for the moment. I'm trying to find an ARM32 machine so I can
> > test.
> >
> > Matthias, is it convenient for you to share a stack trace for the failing
> > programs? That would be very helpful. Thanks!
>
> I'll do a local build this weekend. For now you find the build log at
>
> https://launchpad.net/ubuntu/+source/gcc-snapshot/1:20181210-0ubuntu1/+build/15759748
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-12 16:10             ` Cherry Zhang via gcc-patches
@ 2018-12-12 23:27               ` Ian Lance Taylor
  2018-12-19  3:24                 ` Matthias Klose
  0 siblings, 1 reply; 15+ messages in thread
From: Ian Lance Taylor @ 2018-12-12 23:27 UTC (permalink / raw)
  To: Cherry Zhang; +Cc: Matthias Klose, gcc-patches, gofrontend-dev

On Wed, Dec 12, 2018 at 8:10 AM Cherry Zhang <cherryyz@google.com> wrote:
>
> Thank you, Matthias!
>
> From the log, essentially all the tests aborted. The only place the new code can cause abort on all programs that I can think of is in the runtime startup code, probestackmaps, which calls value_size, which aborts due to an unhandled case. I haven't been able to try out on an ARM machine, but I managed to cross-compile a Go program and visually inspect the exception table. The type table's encoding is DW_EH_PE_absptr, which is indeed not handled, which will cause abort.
>
> I send https://go-review.googlesource.com/c/gofrontend/+/153857 (also as below). Hopefully this will fix the problem.
>
> Thanks,
> Cherry
>
> diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
> index c44755f9..f4bbfb60 100644
> --- a/libgo/runtime/go-unwind.c
> +++ b/libgo/runtime/go-unwind.c
> @@ -318,6 +318,8 @@ value_size (uint8_t encoding)
>        case DW_EH_PE_sdata8:
>        case DW_EH_PE_udata8:
>          return 8;
> +      case DW_EH_PE_absptr:
> +        return sizeof(uintptr);
>        default:
>          break;
>      }


Thanks.

Committed to mainline.

Ian



> On Tue, Dec 11, 2018 at 7:03 PM Matthias Klose <doko@ubuntu.com> wrote:
>>
>> On 11.12.18 22:01, Cherry Zhang wrote:
>> > On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org> wrote:
>> >
>> >> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com> wrote:
>> >>>
>> >>> On 10.12.18 16:54, Cherry Zhang wrote:
>> >>>> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
>> >> wrote:
>> >>>>
>> >>>>> On 06.12.18 00:09, Ian Lance Taylor wrote:
>> >>>>>> This libgo patch by Cherry Zhang adds support for precise stack
>> >>>>>> scanning to the Go runtime.  This uses per-function stack maps stored
>> >>>>>> in the exception tables in the language-specific data area.  The
>> >>>>>> compiler needs to generate these stack maps; currently this is only
>> >>>>>> done by a version of LLVM, not by GCC.  Each safepoint in a function
>> >>>>>> is associated with a (real or dummy) landing pad, and its "type info"
>> >>>>>> in the exception table is a pointer to the stack map. When a stack is
>> >>>>>> scanned, the stack map is found by the stack unwinding code.
>> >>>>>>
>> >>>>>> For precise stack scan we need to unwind the stack. There are three
>> >>>>> cases:
>> >>>>>>
>> >>>>>> - If a goroutine is scanning its own stack, it can unwind the stack
>> >>>>>> and scan the frames.
>> >>>>>>
>> >>>>>> - If a goroutine is scanning another, stopped, goroutine, it cannot
>> >>>>>> directly unwind the target stack. We handle this by switching
>> >>>>>> (runtime.gogo) to the target g, letting it unwind and scan the stack,
>> >>>>>> and switch back.
>> >>>>>>
>> >>>>>> - If we are scanning a goroutine that is blocked in a syscall, we
>> >> send
>> >>>>>> a signal to the target goroutine's thread, and let the signal handler
>> >>>>>> unwind and scan the stack. Extra care is needed as this races with
>> >>>>>> enter/exit syscall.
>> >>>>>>
>> >>>>>> Currently this is only implemented on GNU/Linux.
>> >>>>>>
>> >>>>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>> >>>>>> to mainline.
>> >>>>>
>> >>>>> this broke the libgo build on ARM32:
>> >>>>>
>> >>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>> >>>>> 'scanstackwithmap_callback':
>> >>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
>> >> '_URC_NORMAL_STOP'
>> >>>>> undeclared (first use in this function)
>> >>>>>   754 |           return _URC_NORMAL_STOP;
>> >>>>>       |                  ^~~~~~~~~~~~~~~~
>> >>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
>> >>>>> identifier
>> >>>>> is reported only once for each function i
>> >>>>> t appears in
>> >>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>> >>>>> 'probestackmaps_callback':
>> >>>>> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
>> >> '_URC_NORMAL_STOP'
>> >>>>> undeclared (first use in this function)
>> >>>>>   802 |   return _URC_NORMAL_STOP;
>> >>>>>       |          ^~~~~~~~~~~~~~~~
>> >>>>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
>> >> reaches end
>> >>>>> of
>> >>>>> non-void function [-Wreturn-type]
>> >>>>>   803 | }
>> >>>>>       | ^
>> >>>>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
>> >>>>> make[6]: Leaving directory
>> >>>>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
>> >>>>>
>> >>>>>
>> >>>> Hell Matthias,
>> >>>>
>> >>>> Thank you for the report. And sorry about the breakage. Does
>> >>>> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
>> >>>> below) fix ARM32 build? I don't have an ARM32 machine at hand to test.
>> >>>
>> >>> this fixes the build.
>> >>>
>> >>> currently running the testsuite, almost every test case core dumps on
>> >>> arm-linux-gnueabihf
>> >>
>> >> I committed Cherry's patch to trunk, since it looks reasonable to me.
>> >> Cherry, Matthias, let me know if you figure out why programs are
>> >> failing.
>> >>
>> >>
>> > Thank you.
>> >
>> > I don't know for the moment. I'm trying to find an ARM32 machine so I can
>> > test.
>> >
>> > Matthias, is it convenient for you to share a stack trace for the failing
>> > programs? That would be very helpful. Thanks!
>>
>> I'll do a local build this weekend. For now you find the build log at
>> https://launchpad.net/ubuntu/+source/gcc-snapshot/1:20181210-0ubuntu1/+build/15759748

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-12 23:27               ` Ian Lance Taylor
@ 2018-12-19  3:24                 ` Matthias Klose
  2018-12-26 20:42                   ` Cherry Zhang via gcc-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Matthias Klose @ 2018-12-19  3:24 UTC (permalink / raw)
  To: Ian Lance Taylor, Cherry Zhang; +Cc: gcc-patches, gofrontend-dev

Cherry, see
https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02241.html
https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02240.html

still shows ~180 test failures on ARM32.  Sorry, no access to an ARM32 box until
next year.

Matthias

On 13.12.18 00:27, Ian Lance Taylor wrote:
> On Wed, Dec 12, 2018 at 8:10 AM Cherry Zhang <cherryyz@google.com> wrote:
>>
>> Thank you, Matthias!
>>
>> From the log, essentially all the tests aborted. The only place the new code can cause abort on all programs that I can think of is in the runtime startup code, probestackmaps, which calls value_size, which aborts due to an unhandled case. I haven't been able to try out on an ARM machine, but I managed to cross-compile a Go program and visually inspect the exception table. The type table's encoding is DW_EH_PE_absptr, which is indeed not handled, which will cause abort.
>>
>> I send https://go-review.googlesource.com/c/gofrontend/+/153857 (also as below). Hopefully this will fix the problem.
>>
>> Thanks,
>> Cherry
>>
>> diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
>> index c44755f9..f4bbfb60 100644
>> --- a/libgo/runtime/go-unwind.c
>> +++ b/libgo/runtime/go-unwind.c
>> @@ -318,6 +318,8 @@ value_size (uint8_t encoding)
>>        case DW_EH_PE_sdata8:
>>        case DW_EH_PE_udata8:
>>          return 8;
>> +      case DW_EH_PE_absptr:
>> +        return sizeof(uintptr);
>>        default:
>>          break;
>>      }
> 
> 
> Thanks.
> 
> Committed to mainline.
> 
> Ian
> 
> 
> 
>> On Tue, Dec 11, 2018 at 7:03 PM Matthias Klose <doko@ubuntu.com> wrote:
>>>
>>> On 11.12.18 22:01, Cherry Zhang wrote:
>>>> On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org> wrote:
>>>>
>>>>> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com> wrote:
>>>>>>
>>>>>> On 10.12.18 16:54, Cherry Zhang wrote:
>>>>>>> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
>>>>> wrote:
>>>>>>>
>>>>>>>> On 06.12.18 00:09, Ian Lance Taylor wrote:
>>>>>>>>> This libgo patch by Cherry Zhang adds support for precise stack
>>>>>>>>> scanning to the Go runtime.  This uses per-function stack maps stored
>>>>>>>>> in the exception tables in the language-specific data area.  The
>>>>>>>>> compiler needs to generate these stack maps; currently this is only
>>>>>>>>> done by a version of LLVM, not by GCC.  Each safepoint in a function
>>>>>>>>> is associated with a (real or dummy) landing pad, and its "type info"
>>>>>>>>> in the exception table is a pointer to the stack map. When a stack is
>>>>>>>>> scanned, the stack map is found by the stack unwinding code.
>>>>>>>>>
>>>>>>>>> For precise stack scan we need to unwind the stack. There are three
>>>>>>>> cases:
>>>>>>>>>
>>>>>>>>> - If a goroutine is scanning its own stack, it can unwind the stack
>>>>>>>>> and scan the frames.
>>>>>>>>>
>>>>>>>>> - If a goroutine is scanning another, stopped, goroutine, it cannot
>>>>>>>>> directly unwind the target stack. We handle this by switching
>>>>>>>>> (runtime.gogo) to the target g, letting it unwind and scan the stack,
>>>>>>>>> and switch back.
>>>>>>>>>
>>>>>>>>> - If we are scanning a goroutine that is blocked in a syscall, we
>>>>> send
>>>>>>>>> a signal to the target goroutine's thread, and let the signal handler
>>>>>>>>> unwind and scan the stack. Extra care is needed as this races with
>>>>>>>>> enter/exit syscall.
>>>>>>>>>
>>>>>>>>> Currently this is only implemented on GNU/Linux.
>>>>>>>>>
>>>>>>>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>>>>>>>>> to mainline.
>>>>>>>>
>>>>>>>> this broke the libgo build on ARM32:
>>>>>>>>
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>>>>>>>> 'scanstackwithmap_callback':
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
>>>>> '_URC_NORMAL_STOP'
>>>>>>>> undeclared (first use in this function)
>>>>>>>>   754 |           return _URC_NORMAL_STOP;
>>>>>>>>       |                  ^~~~~~~~~~~~~~~~
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
>>>>>>>> identifier
>>>>>>>> is reported only once for each function i
>>>>>>>> t appears in
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>>>>>>>> 'probestackmaps_callback':
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
>>>>> '_URC_NORMAL_STOP'
>>>>>>>> undeclared (first use in this function)
>>>>>>>>   802 |   return _URC_NORMAL_STOP;
>>>>>>>>       |          ^~~~~~~~~~~~~~~~
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
>>>>> reaches end
>>>>>>>> of
>>>>>>>> non-void function [-Wreturn-type]
>>>>>>>>   803 | }
>>>>>>>>       | ^
>>>>>>>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
>>>>>>>> make[6]: Leaving directory
>>>>>>>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
>>>>>>>>
>>>>>>>>
>>>>>>> Hell Matthias,
>>>>>>>
>>>>>>> Thank you for the report. And sorry about the breakage. Does
>>>>>>> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
>>>>>>> below) fix ARM32 build? I don't have an ARM32 machine at hand to test.
>>>>>>
>>>>>> this fixes the build.
>>>>>>
>>>>>> currently running the testsuite, almost every test case core dumps on
>>>>>> arm-linux-gnueabihf
>>>>>
>>>>> I committed Cherry's patch to trunk, since it looks reasonable to me.
>>>>> Cherry, Matthias, let me know if you figure out why programs are
>>>>> failing.
>>>>>
>>>>>
>>>> Thank you.
>>>>
>>>> I don't know for the moment. I'm trying to find an ARM32 machine so I can
>>>> test.
>>>>
>>>> Matthias, is it convenient for you to share a stack trace for the failing
>>>> programs? That would be very helpful. Thanks!
>>>
>>> I'll do a local build this weekend. For now you find the build log at
>>> https://launchpad.net/ubuntu/+source/gcc-snapshot/1:20181210-0ubuntu1/+build/15759748

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-19  3:24                 ` Matthias Klose
@ 2018-12-26 20:42                   ` Cherry Zhang via gcc-patches
  2018-12-27  9:42                     ` Ian Lance Taylor
  0 siblings, 1 reply; 15+ messages in thread
From: Cherry Zhang via gcc-patches @ 2018-12-26 20:42 UTC (permalink / raw)
  To: Matthias Klose; +Cc: Ian Lance Taylor, gcc-patches, gofrontend-dev

Finally I found an ARM machine and was able to build ARM32 gcc on it, and
reproduced the failures. The Go programs aborted, in parse_lsda_header,
called from probestackmaps in the runtime startup. It seems that
_Unwind_GetLanguageSpecificData doesn't return a valid LSDA when called
from a callback from _Unwind_Backtrace.

Reading the unwinder's source code in libgcc, it seems that a function may
have a "predefined" personality function, and in this case an ARM-specific
"compact" model is used, which doesn't use the standard LSDA.
_Unwind_GetLanguageSpecificData doesn't distinguish this and simply assumes
the "generic" model is used, i.e. not used with a "predefined" personality
function. This works fine if _Unwind_GetLanguageSpecificData is called from
a non-predefined personality function, but it doesn't work if it is called
during _Unwind_Backtrace.

The patch below (also CL
https://go-review.googlesource.com/c/gofrontend/+/155758) will fix the
problem, by checking which model is used before calling
_Unwind_GetLanguageSpecificData.

Alternatively, we could change _Unwind_GetLanguageSpecificData in libgcc to
returning NULL when a predefined personality function is used.

Let me know what you think.

Thanks,
Cherry

diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
index f4bbfb60..388d7c70 100644
--- a/libgo/runtime/go-unwind.c
+++ b/libgo/runtime/go-unwind.c
@@ -646,6 +646,17 @@ findstackmaps (struct _Unwind_Context *context,
_Unwind_Ptr *ip, _Unwind_Ptr *sp
   _sleb128_t index;
   int size;

+#ifdef __ARM_EABI_UNWINDER__
+  {
+    _Unwind_Control_Block *ucbp;
+    ucbp = (_Unwind_Control_Block *) _Unwind_GetGR (context, 12);
+    if (*ucbp->pr_cache.ehtp & (1u << 31))
+      // The "compact" model is used, with one of the predefined
+      // personality functions. It doesn't have standard LSDA.
+      return NOTFOUND_OK;
+  }
+#endif
+
   language_specific_data = (const unsigned char *)
     _Unwind_GetLanguageSpecificData (context);



On Tue, Dec 18, 2018 at 10:24 PM Matthias Klose <doko@ubuntu.com> wrote:

> Cherry, see
> https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02241.html
> https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02240.html
>
> still shows ~180 test failures on ARM32.  Sorry, no access to an ARM32 box
> until
> next year.
>
> Matthias
>
> On 13.12.18 00:27, Ian Lance Taylor wrote:
> > On Wed, Dec 12, 2018 at 8:10 AM Cherry Zhang <cherryyz@google.com>
> wrote:
> >>
> >> Thank you, Matthias!
> >>
> >> From the log, essentially all the tests aborted. The only place the new
> code can cause abort on all programs that I can think of is in the runtime
> startup code, probestackmaps, which calls value_size, which aborts due to
> an unhandled case. I haven't been able to try out on an ARM machine, but I
> managed to cross-compile a Go program and visually inspect the exception
> table. The type table's encoding is DW_EH_PE_absptr, which is indeed not
> handled, which will cause abort.
> >>
> >> I send https://go-review.googlesource.com/c/gofrontend/+/153857 (also
> as below). Hopefully this will fix the problem.
> >>
> >> Thanks,
> >> Cherry
> >>
> >> diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
> >> index c44755f9..f4bbfb60 100644
> >> --- a/libgo/runtime/go-unwind.c
> >> +++ b/libgo/runtime/go-unwind.c
> >> @@ -318,6 +318,8 @@ value_size (uint8_t encoding)
> >>        case DW_EH_PE_sdata8:
> >>        case DW_EH_PE_udata8:
> >>          return 8;
> >> +      case DW_EH_PE_absptr:
> >> +        return sizeof(uintptr);
> >>        default:
> >>          break;
> >>      }
> >
> >
> > Thanks.
> >
> > Committed to mainline.
> >
> > Ian
> >
> >
> >
> >> On Tue, Dec 11, 2018 at 7:03 PM Matthias Klose <doko@ubuntu.com> wrote:
> >>>
> >>> On 11.12.18 22:01, Cherry Zhang wrote:
> >>>> On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org>
> wrote:
> >>>>
> >>>>> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com>
> wrote:
> >>>>>>
> >>>>>> On 10.12.18 16:54, Cherry Zhang wrote:
> >>>>>>> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
> >>>>> wrote:
> >>>>>>>
> >>>>>>>> On 06.12.18 00:09, Ian Lance Taylor wrote:
> >>>>>>>>> This libgo patch by Cherry Zhang adds support for precise stack
> >>>>>>>>> scanning to the Go runtime.  This uses per-function stack maps
> stored
> >>>>>>>>> in the exception tables in the language-specific data area.  The
> >>>>>>>>> compiler needs to generate these stack maps; currently this is
> only
> >>>>>>>>> done by a version of LLVM, not by GCC.  Each safepoint in a
> function
> >>>>>>>>> is associated with a (real or dummy) landing pad, and its "type
> info"
> >>>>>>>>> in the exception table is a pointer to the stack map. When a
> stack is
> >>>>>>>>> scanned, the stack map is found by the stack unwinding code.
> >>>>>>>>>
> >>>>>>>>> For precise stack scan we need to unwind the stack. There are
> three
> >>>>>>>> cases:
> >>>>>>>>>
> >>>>>>>>> - If a goroutine is scanning its own stack, it can unwind the
> stack
> >>>>>>>>> and scan the frames.
> >>>>>>>>>
> >>>>>>>>> - If a goroutine is scanning another, stopped, goroutine, it
> cannot
> >>>>>>>>> directly unwind the target stack. We handle this by switching
> >>>>>>>>> (runtime.gogo) to the target g, letting it unwind and scan the
> stack,
> >>>>>>>>> and switch back.
> >>>>>>>>>
> >>>>>>>>> - If we are scanning a goroutine that is blocked in a syscall, we
> >>>>> send
> >>>>>>>>> a signal to the target goroutine's thread, and let the signal
> handler
> >>>>>>>>> unwind and scan the stack. Extra care is needed as this races
> with
> >>>>>>>>> enter/exit syscall.
> >>>>>>>>>
> >>>>>>>>> Currently this is only implemented on GNU/Linux.
> >>>>>>>>>
> >>>>>>>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
> Committed
> >>>>>>>>> to mainline.
> >>>>>>>>
> >>>>>>>> this broke the libgo build on ARM32:
> >>>>>>>>
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
> >>>>>>>> 'scanstackwithmap_callback':
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
> >>>>> '_URC_NORMAL_STOP'
> >>>>>>>> undeclared (first use in this function)
> >>>>>>>>   754 |           return _URC_NORMAL_STOP;
> >>>>>>>>       |                  ^~~~~~~~~~~~~~~~
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each
> undeclared
> >>>>>>>> identifier
> >>>>>>>> is reported only once for each function i
> >>>>>>>> t appears in
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
> >>>>>>>> 'probestackmaps_callback':
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
> >>>>> '_URC_NORMAL_STOP'
> >>>>>>>> undeclared (first use in this function)
> >>>>>>>>   802 |   return _URC_NORMAL_STOP;
> >>>>>>>>       |          ^~~~~~~~~~~~~~~~
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
> >>>>> reaches end
> >>>>>>>> of
> >>>>>>>> non-void function [-Wreturn-type]
> >>>>>>>>   803 | }
> >>>>>>>>       | ^
> >>>>>>>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
> >>>>>>>> make[6]: Leaving directory
> >>>>>>>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
> >>>>>>>>
> >>>>>>>>
> >>>>>>> Hell Matthias,
> >>>>>>>
> >>>>>>> Thank you for the report. And sorry about the breakage. Does
> >>>>>>> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the
> patch
> >>>>>>> below) fix ARM32 build? I don't have an ARM32 machine at hand to
> test.
> >>>>>>
> >>>>>> this fixes the build.
> >>>>>>
> >>>>>> currently running the testsuite, almost every test case core dumps
> on
> >>>>>> arm-linux-gnueabihf
> >>>>>
> >>>>> I committed Cherry's patch to trunk, since it looks reasonable to me.
> >>>>> Cherry, Matthias, let me know if you figure out why programs are
> >>>>> failing.
> >>>>>
> >>>>>
> >>>> Thank you.
> >>>>
> >>>> I don't know for the moment. I'm trying to find an ARM32 machine so I
> can
> >>>> test.
> >>>>
> >>>> Matthias, is it convenient for you to share a stack trace for the
> failing
> >>>> programs? That would be very helpful. Thanks!
> >>>
> >>> I'll do a local build this weekend. For now you find the build log at
> >>>
> https://launchpad.net/ubuntu/+source/gcc-snapshot/1:20181210-0ubuntu1/+build/15759748
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
  2018-12-26 20:42                   ` Cherry Zhang via gcc-patches
@ 2018-12-27  9:42                     ` Ian Lance Taylor
  0 siblings, 0 replies; 15+ messages in thread
From: Ian Lance Taylor @ 2018-12-27  9:42 UTC (permalink / raw)
  To: Cherry Zhang; +Cc: Matthias Klose, gcc-patches, gofrontend-dev

On Wed, Dec 26, 2018 at 7:39 AM Cherry Zhang <cherryyz@google.com> wrote:
>
> Finally I found an ARM machine and was able to build ARM32 gcc on it, and reproduced the failures. The Go programs aborted, in parse_lsda_header, called from probestackmaps in the runtime startup. It seems that _Unwind_GetLanguageSpecificData doesn't return a valid LSDA when called from a callback from _Unwind_Backtrace.
>
> Reading the unwinder's source code in libgcc, it seems that a function may have a "predefined" personality function, and in this case an ARM-specific "compact" model is used, which doesn't use the standard LSDA. _Unwind_GetLanguageSpecificData doesn't distinguish this and simply assumes the "generic" model is used, i.e. not used with a "predefined" personality function. This works fine if _Unwind_GetLanguageSpecificData is called from a non-predefined personality function, but it doesn't work if it is called during _Unwind_Backtrace.
>
> The patch below (also CL https://go-review.googlesource.com/c/gofrontend/+/155758) will fix the problem, by checking which model is used before calling _Unwind_GetLanguageSpecificData.
>
> Alternatively, we could change _Unwind_GetLanguageSpecificData in libgcc to returning NULL when a predefined personality function is used.
>
> Let me know what you think.
>
> Thanks,
> Cherry
>
> diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
> index f4bbfb60..388d7c70 100644
> --- a/libgo/runtime/go-unwind.c
> +++ b/libgo/runtime/go-unwind.c
> @@ -646,6 +646,17 @@ findstackmaps (struct _Unwind_Context *context, _Unwind_Ptr *ip, _Unwind_Ptr *sp
>    _sleb128_t index;
>    int size;
>
> +#ifdef __ARM_EABI_UNWINDER__
> +  {
> +    _Unwind_Control_Block *ucbp;
> +    ucbp = (_Unwind_Control_Block *) _Unwind_GetGR (context, 12);
> +    if (*ucbp->pr_cache.ehtp & (1u << 31))
> +      // The "compact" model is used, with one of the predefined
> +      // personality functions. It doesn't have standard LSDA.
> +      return NOTFOUND_OK;
> +  }
> +#endif
> +
>    language_specific_data = (const unsigned char *)
>      _Unwind_GetLanguageSpecificData (context);
>

Thanks.

Committed to mainline.

Ian

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-12-27  3:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-05 23:10 libgo patch committed: Add precise stack scan support Ian Lance Taylor
2018-12-07 10:07 ` Rainer Orth
2018-12-07 14:23   ` Ian Lance Taylor
2018-12-07 15:15     ` [gofrontend-dev] " Cherry Zhang via gcc-patches
2018-12-10  6:41 ` Matthias Klose
2018-12-10 15:54   ` [gofrontend-dev] " Cherry Zhang via gcc-patches
2018-12-11 14:52     ` Matthias Klose
2018-12-11 20:51       ` Ian Lance Taylor
2018-12-11 21:02         ` Cherry Zhang via gcc-patches
2018-12-12  0:03           ` Matthias Klose
2018-12-12 16:10             ` Cherry Zhang via gcc-patches
2018-12-12 23:27               ` Ian Lance Taylor
2018-12-19  3:24                 ` Matthias Klose
2018-12-26 20:42                   ` Cherry Zhang via gcc-patches
2018-12-27  9:42                     ` Ian Lance Taylor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).