]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: fix infinite callstack of cgo on arm64
authorXiangdong Ji <xiangdong.ji@arm.com>
Fri, 27 Mar 2020 11:04:21 +0000 (11:04 +0000)
committerIan Lance Taylor <iant@golang.org>
Wed, 8 Apr 2020 03:46:37 +0000 (03:46 +0000)
This change adds CFA information to the assembly function 'crosscall1'
and reorgnizes its code to establish well-formed prologue and epilogue.
It will fix an infinite callstack issue when debugging cgo program with
GDB on arm64.

Brief root cause analysis:

GDB's aarch64 unwinder parses prologue to determine current frame's size
and previous PC&SP if CFA information is not available.

The unwinder parses the prologue of 'crosscall1' to determine a frame size
of 0x10, then turns to its next frame trying to compute its previous PC&SP
as they are not saved on current frame's stack as per its 'traditional frame
unwind' rules, which ends up getting an endless frame chain like:
    [callee]  : pc:<pc0>, sp:<sp0>
    crosscall1: pc:<pc1>, sp:<sp0>+0x10
    [caller]  : pc:<pc1>, sp:<sp0>+0x10+0x10
    [caller]  : pc:<pc1>, sp:<sp0>+0x10+0x10+0x10
    ...
GDB fails to detect the 'caller' frame is same as 'crosscall1' and terminate
unwinding since SP increases everytime.

Fixes #37238
Change-Id: Ia6bd8555828541a3a61f7dc9b94dfa00775ec52a
Reviewed-on: https://go-review.googlesource.com/c/go/+/226999
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/runtime/cgo/gcc_arm64.S
src/runtime/runtime-gdb_test.go

index 59dce08b9f01c7640168816cbfcc0d733f5c750e..9154d2aaf4196d0260b634da94ecb37859b7efb8 100644 (file)
  */
 .globl EXT(crosscall1)
 EXT(crosscall1):
-       stp x19, x20, [sp, #-16]!
-       stp x21, x22, [sp, #-16]!
-       stp x23, x24, [sp, #-16]!
-       stp x25, x26, [sp, #-16]!
-       stp x27, x28, [sp, #-16]!
-       stp x29, x30, [sp, #-16]!
+       .cfi_startproc
+       stp x29, x30, [sp, #-96]!
+       .cfi_def_cfa_offset 96
+       .cfi_offset 29, -96
+       .cfi_offset 30, -88
        mov x29, sp
+       .cfi_def_cfa_register 29
+       stp x19, x20, [sp, #80]
+       .cfi_offset 19, -16
+       .cfi_offset 20, -8
+       stp x21, x22, [sp, #64]
+       .cfi_offset 21, -32
+       .cfi_offset 22, -24
+       stp x23, x24, [sp, #48]
+       .cfi_offset 23, -48
+       .cfi_offset 24, -40
+       stp x25, x26, [sp, #32]
+       .cfi_offset 25, -64
+       .cfi_offset 26, -56
+       stp x27, x28, [sp, #16]
+       .cfi_offset 27, -80
+       .cfi_offset 28, -72
 
        mov x19, x0
        mov x20, x1
@@ -39,13 +54,27 @@ EXT(crosscall1):
        blr x20
        blr x19
 
-       ldp x29, x30, [sp], #16
-       ldp x27, x28, [sp], #16
-       ldp x25, x26, [sp], #16
-       ldp x23, x24, [sp], #16
-       ldp x21, x22, [sp], #16
-       ldp x19, x20, [sp], #16
+       ldp x27, x28, [sp, #16]
+       .cfi_restore 27
+       .cfi_restore 28
+       ldp x25, x26, [sp, #32]
+       .cfi_restore 25
+       .cfi_restore 26
+       ldp x23, x24, [sp, #48]
+       .cfi_restore 23
+       .cfi_restore 24
+       ldp x21, x22, [sp, #64]
+       .cfi_restore 21
+       .cfi_restore 22
+       ldp x19, x20, [sp, #80]
+       .cfi_restore 19
+       .cfi_restore 20
+       ldp x29, x30, [sp], #96
+       .cfi_restore 29
+       .cfi_restore 30
+       .cfi_def_cfa 31, 0
        ret
+       .cfi_endproc
 
 
 #ifdef __ELF__
index 5dbe4bf88a921d4ab7b0b12ecd4c3f091103f8f5..4639e2fcb8d878195ec61d4790b46a20a3e4f7b6 100644 (file)
@@ -607,3 +607,83 @@ func TestGdbPanic(t *testing.T) {
                }
        }
 }
+
+const InfCallstackSource = `
+package main
+import "C"
+import "time"
+
+func loop() {
+        for i := 0; i < 1000; i++ {
+                time.Sleep(time.Millisecond*5)
+        }
+}
+
+func main() {
+        go loop()
+        time.Sleep(time.Second * 1)
+}
+`
+// TestGdbInfCallstack tests that gdb can unwind the callstack of cgo programs
+// on arm64 platforms without endless frames of function 'crossfunc1'.
+// https://golang.org/issue/37238
+func TestGdbInfCallstack(t *testing.T) {
+       checkGdbEnvironment(t)
+
+       testenv.MustHaveCGO(t)
+       if runtime.GOARCH != "arm64" {
+               t.Skip("skipping infinite callstack test on non-arm64 arches")
+       }
+
+       t.Parallel()
+       checkGdbVersion(t)
+
+       dir, err := ioutil.TempDir("", "go-build")
+       if err != nil {
+               t.Fatalf("failed to create temp directory: %v", err)
+       }
+       defer os.RemoveAll(dir)
+
+       // Build the source code.
+       src := filepath.Join(dir, "main.go")
+       err = ioutil.WriteFile(src, []byte(InfCallstackSource), 0644)
+       if err != nil {
+               t.Fatalf("failed to create file: %v", err)
+       }
+       cmd := exec.Command(testenv.GoToolPath(t), "build", "-o", "a.exe", "main.go")
+       cmd.Dir = dir
+       out, err := testenv.CleanCmdEnv(cmd).CombinedOutput()
+       if err != nil {
+               t.Fatalf("building source %v\n%s", err, out)
+       }
+
+       // Execute gdb commands.
+       // 'setg_gcc' is the first point where we can reproduce the issue with just one 'run' command.
+       args := []string{"-nx", "-batch",
+               "-iex", "add-auto-load-safe-path " + filepath.Join(runtime.GOROOT(), "src", "runtime"),
+               "-ex", "set startup-with-shell off",
+               "-ex", "break setg_gcc",
+               "-ex", "run",
+               "-ex", "backtrace 3",
+               "-ex", "disable 1",
+               "-ex", "continue",
+               filepath.Join(dir, "a.exe"),
+       }
+       got, _ := exec.Command("gdb", args...).CombinedOutput()
+
+       // Check that the backtrace matches
+       // We check the 3 inner most frames only as they are present certainly, according to gcc_<OS>_arm64.c
+       bt := []string{
+               `setg_gcc`,
+               `crosscall1`,
+               `threadentry`,
+       }
+       for i, name := range bt {
+               s := fmt.Sprintf("#%v.*%v", i, name)
+               re := regexp.MustCompile(s)
+               if found := re.Find(got) != nil; !found {
+                       t.Errorf("could not find '%v' in backtrace", s)
+                       t.Fatalf("gdb output:\n%v", string(got))
+               }
+       }
+}