Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cmd/link: enable DWARF on darwin/arm{,64} #24883

Closed
eliasnaur opened this issue Apr 16, 2018 · 26 comments

Comments

Projects
None yet
8 participants
@eliasnaur
Copy link
Contributor

commented Apr 16, 2018

DWARF generation is completely disabled on darwin/arm and darwin/arm64. There's a

*ld.FlagW = true // disable DWARF generation

in cmd/link/internal/arm*/obj.go for objabi.Hdarwin. Blindly deleting those lines doesn't result in errors, but doesn't improve Xcode backtrace in my informal crash test.

This issue is about enabling (and testing) DWARF generation on darwin/arm and darwin/arm64 to enjoy the same debugging benefits on iOS as other DWARF-enabled platforms. For example, DWARF probably helps #22716.

@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

Can you output a dwarfdump -a --arch=arm64 of the generated file?

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2018

@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

Looking at it, the data is there, but the values seem... weird:

   CIE_pointer: 0x00000000
    start_addr: 0x0000000000000000 sync/atomic.(*Value).Store
    range_size: 0x00000000000002f0 (end_addr = 0x00000000000002f0)
                DW_CFA_same_value (30)
                DW_CFA_def_cfa_offset_sf (30, 0)
  Instructions: 0x0000000000000000: CFA=31       30=30  31=31
                DW_CFA_advance_loc (20)
                DW_CFA_offset_extended_sf (30, -104)
                DW_CFA_def_cfa_offset_sf (30, 112)
                0x0000000000000014: CFA=31+112   30=[31+8]  31=31+112
                DW_CFA_advance_loc1 (176)
                DW_CFA_same_value (30)
                DW_CFA_def_cfa_offset_sf (30, 0)
                0x00000000000000c4: CFA=31       30=30  31=31
                DW_CFA_advance_loc (4)
                DW_CFA_offset_extended_sf (30, -104)
                DW_CFA_def_cfa_offset_sf (30, 112)
                0x00000000000000c8: CFA=31+112   30=[31+8]  31=31+112
@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

Another instance:

standard_opcode_lengths[ DW_LNS_fixed_advance_pc ] = 1
                Dir  Mod Time   File Len   File Name
                ---- ---------- ---------- ---------------------------
file_names[  1]    0 0x00000000 0x00000000 /Users/elias/go/src/golang.org/x/mobile/example/bind/hello/hello.go
file_names[  2]    0 0x00000000 0x00000000 <autogenerated>
0x0000254a: DW_LNE_set_address( 0x0000000000000000 )
0x00002555: DW_LNE_end_sequence
            0x0000000000000000      1      1      0 is_stmt
@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

I'm guessing it was disabled because the generation part is skipped (the 0s indicate default struct values)

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

cmd/go also disables dwarf generation in the compiler:
https://go.googlesource.com/go/+/04a27bef6f2c96251161d63761617542ddc762bc/src/cmd/go/internal/work/gc.go#93

If DWARF works on these platforms, we should also enable the compiler part.

@andybons andybons added this to the Unplanned milestone Apr 16, 2018

@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

I've tried applying the following diff to Go tip, doing what @cherrymui suggested:

diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go
index 2d61e54333..de474cdef3 100644
--- a/src/cmd/go/internal/work/gc.go
+++ b/src/cmd/go/internal/work/gc.go
@@ -89,7 +89,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg []byte, a
                gcargs = append(gcargs, "-buildid", a.buildID)
        }
        platform := cfg.Goos + "/" + cfg.Goarch
-       if p.Internal.OmitDebug || platform == "nacl/amd64p32" || platform == "darwin/arm" || platform == "darwin/arm64" || cfg.Goos == "plan9" {
+       if p.Internal.OmitDebug || platform == "nacl/amd64p32" || cfg.Goos == "plan9" {
                gcargs = append(gcargs, "-dwarf=false")
        }
        if strings.HasPrefix(runtimeVersion, "go1") && !strings.Contains(os.Args[0], "go_bootstrap") {
diff --git a/src/cmd/link/internal/arm/obj.go b/src/cmd/link/internal/arm/obj.go
index da16f92345..788be68522 100644
--- a/src/cmd/link/internal/arm/obj.go
+++ b/src/cmd/link/internal/arm/obj.go
@@ -120,7 +120,6 @@ func archinit(ctxt *ld.Link) {
                }

        case objabi.Hdarwin: /* apple MACH */
-               *ld.FlagW = true // disable DWARF generation
                ld.HEADR = ld.INITIAL_MACHO_HEADR
                if *ld.FlagTextAddr == -1 {
                        *ld.FlagTextAddr = 4096 + int64(ld.HEADR)
diff --git a/src/cmd/link/internal/arm64/obj.go b/src/cmd/link/internal/arm64/obj.go
index 6b386ad737..405d22d74f 100644
--- a/src/cmd/link/internal/arm64/obj.go
+++ b/src/cmd/link/internal/arm64/obj.go
@@ -101,7 +101,6 @@ func archinit(ctxt *ld.Link) {
                }

        case objabi.Hdarwin: /* apple MACH */
-               *ld.FlagW = true // disable DWARF generation
                ld.HEADR = ld.INITIAL_MACHO_HEADR
                if *ld.FlagTextAddr == -1 {
                        *ld.FlagTextAddr = 4096 + int64(ld.HEADR)
diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index 80a7632f64..51d6d64881 100644
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -566,7 +566,6 @@ func (ctxt *Link) loadlib() {
        // the linker manipulates them, and also symbols whose names
        // would not be shortened by this process.
        if typeSymbolMangling(ctxt) {
-               *FlagW = true // disable DWARF generation
                for _, s := range ctxt.Syms.Allsym {
                        newName := typeSymbolMangle(s.Name)
                        if newName != s.Name {

However, the information, while there, is incomplete, addresses are wrong (they look uninitialized):

----------------------------------------------------------------------
 File: Crash.framework/Crash(go.o) (arm64)
----------------------------------------------------------------------
.debug_info contents:

0x00000000: Compile Unit: length = 0x000206ca  version = 0x0004  abbr_offset = 0x00000000  addr_size = 0x08  (next CU at 0x000206ce)

0x0000000b: TAG_compile_unit [1] *
             AT_name( "sync/atomic" )
             AT_language( DW_LANG_Go )
             AT_stmt_list( 0x00000000 )
             AT_low_pc( 0x0000000000000000 )
             AT_ranges( 0x00000000
                [0x0000000000000000 - 0x0000000000000180)
                 End )
             AT_comp_dir( "." )
             AT_producer( "Go cmd/compile devel +a3df9c4755 Mon Apr 16 21:42:05 2018 +0000; -shared" )

0x00000074:     TAG_subprogram [2] *
                 AT_name( "sync/atomic.(*Value).Store" )
                 AT_low_pc( 0x0000000000000000 )
                 AT_high_pc( 0x0000000000000110 )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "/Users/steeve/go.tip/go/src/sync/atomic/value.go" )
                 AT_external( 0x01 )
@gopherbot

This comment has been minimized.

Copy link

commented Apr 19, 2018

Change https://golang.org/cl/108295 mentions this issue: cmd/link, cmd/go: enable DWARF on darwin/arm and darwin/arm64

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2018

Does iOS debugging tools use DWARF? (I'm asking as I don't know why DWARF was disabled on those platforms -- is it because we didn't support it, or because there was no tool use it so no need to do it?)

I tried to enable DWARF on darwin/arm and arm64. I put up a CL https://golang.org/cl/108295. With that CL, it generates DWARF and the DWARF looks reasonable on ARM (32-bit).

$ CGO_ENABLED=1 GOARCH=arm GOARM=7 CC=$HOME/src/go/misc/ios/clangwrap.sh GOOS=darwin go build hello.go
$ dwarfdump hello
----------------------------------------------------------------------
 File: hello (armv7)
----------------------------------------------------------------------
...
0x000235b7:     TAG_subprogram [2]  
                 AT_name( "main.main" )
                 AT_low_pc( 0x04051d88 )
                 AT_high_pc( 0x04051d8c )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "/private/tmp/hello.go" )
                 AT_external( 0x01 )

0x000235cb:     TAG_subprogram [2]  
                 AT_name( "main.init" )
                 AT_low_pc( 0x04051d98 )
                 AT_high_pc( 0x04051df8 )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "<autogenerated>" )
                 AT_external( 0x01 )
...

But for ARM64, there is some problem. The low PCs for functions look correct (I checked with objdump), but the high PCs don't (e.g. they are even smaller than low PC). There might be other problems, I don't know yet.

$ CGO_ENABLED=1 GOARCH=arm64 CC=$HOME/src/go/misc/ios/clangwrap.sh GOOS=darwin go build hello.go
$ dwarfdump hello
----------------------------------------------------------------------
 File: hello (arm64)
----------------------------------------------------------------------
...
0x0002874c:     TAG_subprogram [4]  
                 AT_name( "main.main" )
                 AT_low_pc( 0x000000010004b170 )
                 AT_high_pc( 0x00000001000069c8 )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "/private/tmp/hello.go" )
                 AT_external( 0x01 )

0x00028768:     TAG_subprogram [4]  
                 AT_name( "main.init" )
                 AT_low_pc( 0x000000010004b180 )
                 AT_high_pc( 0x0000000100006a18 )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "<autogenerated>" )
                 AT_external( 0x01 )
...

Before I look into the detail, I would want to make sure the DWARF are useful for those platforms.

@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2018

@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2018

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2018

With 108295 applied, darwin/arm backtraces looks good now:

skaermbillede 2018-04-19 kl 22 17 43

Very promising. On darwin/arm64, it is not quite as well:
skaermbillede 2018-04-19 kl 22 25 32

Both traces are without the arm64 framepointer CL applied.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2018

Thanks @eliasnaur! This is great to know. Perhaps we can enable it on ARM32 now (while still investigating ARM64)?

Another question is whether we should enable it by default. Does mobile developers care more on binary size or the availability of DWARF? I guess one data point could be what we do for Android.

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2018

On further inspection, the darwin/arm and darwin/arm64 are both wrong, but in different ways. The darwin/arm trace is missing the Objective C part of the trace, and the darwin/arm64 is lacking in the Go part. Perhaps there is something about the cgocallback frame on darwin/arm that confuses the unwinder?

I think it is useful enough to enable it by default and work out the kinks from there. I believe it is on by default on Android, and you can always strip it again with -ldflags="-w". Both the go and the gomobile command support -ldflags.

@aarzilli

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2018

@eliasnaur AFAIK stacktraces involving cgo don't work on any architecture.

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2018

They do work on darwin/amd64 with the same crash test.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2018

For what it's worth, cgo stack traces work on amd64 GNU/Linux if you import "github.com/ianlancetaylor/cgosymbolizer" (but this is not a supported part of the Go project).

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2018

Perhaps we're talking about different kinds of traces? I'm referring to the traces seen in Xcode, not the traces from the Go runtime.

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2018

Compiling to c-archives, it seems both AT_low_pc and AT_high_pc are wrong on arm64.

For arm:

$ CGO_ENABLED=1 GOARCH=arm GOARM=7 CC=$HOME/go-tip/misc/ios/clangwrap.sh GOOS=darwin go build -buildmode=c-archive -o hello-arm.a hello.go
$ dwarfdump hello-arm.a |grep main.main -C 20
0x00066129:     TAG_subprogram [2] *
                 AT_name( "main.main" )
                 AT_low_pc( 0x000898d8 )
                 AT_high_pc( 0x00089944 )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "/Users/elias/go-tip/src/hello.go" )
                 AT_external( 0x01 )

0x00066143:         NULL

0x00066144:     TAG_subprogram [2] *
                 AT_name( "main.init" )
                 AT_low_pc( 0x00089944 )
                 AT_high_pc( 0x000899bc )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "<autogenerated>" )
                 AT_external( 0x01 )

For arm64:

$ CGO_ENABLED=1 GOARCH=arm64 GOARM=7 CC=$HOME/go-tip/misc/ios/clangwrap.sh GOOS=darwin go build -buildmode=c-archive -o hello-arm64.a hello.go
$ dwarfdump hello-arm64.a |grep main.main -C 20
0x000712ad:     TAG_subprogram [2] *
                 AT_name( "main.main" )
                 AT_low_pc( 0x0000000000000000 )
                 AT_high_pc( 0x0000000000000060 )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "/Users/elias/go-tip/src/hello.go" )
                 AT_external( 0x01 )

0x000712cf:         NULL

0x000712d0:     TAG_subprogram [2] *
                 AT_name( "main.init" )
                 AT_low_pc( 0x0000000000000000 )
                 AT_high_pc( 0x0000000000000070 )
                 AT_frame_base( call-frame-cfa )
                 AT_decl_file( "<autogenerated>" )
                 AT_external( 0x01 )

Not sure where to go next, though.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 21, 2018

Change https://golang.org/cl/108655 mentions this issue: cmd/link: remove R_ADDR relocation workaround for macho arm64

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2018

WIth CL 108655, the addresses from dwarfdump seem correct and stack traces look good in Xcode on darwin/arm64.

skaermbillede 2018-04-21 kl 13 31 09

gopherbot pushed a commit that referenced this issue Apr 23, 2018

Elias Naur
cmd/link: remove R_ADDR relocation workaround for macho arm64
The workarounds doesn't seem necessary anymore, and blocks DWARF
on darwin/arm64.

Updates #24883.

Change-Id: Ic917c767d3b4f6c51be25566956296f5dd4ead10
Reviewed-on: https://go-review.googlesource.com/108655
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

@gopherbot gopherbot closed this in 5f5168e Apr 26, 2018

@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2018

I'm happy to report that breakpoints work (note the local var i):
image

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Apr 27, 2018

Is the problem with wrong traces in the panic() case iOS specific? It seems to me that it could occur on the desktop platforms as well. If so, feel free to open another issue about that. If the problem covers all platforms it might be fixed quicker.

@steeve

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2018

@eliasnaur indeed I do think this is the same on all platform, will do!

gwik added a commit to znly/go that referenced this issue May 7, 2018

cmd/link, cmd/go: enable DWARF on darwin/arm and darwin/arm64
Fixes golang#24883.

Change-Id: Iff992ec3f2f31f4d82923d7cc806df4ee58e70b0
Reviewed-on: https://go-review.googlesource.com/108295
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

gwik added a commit to znly/go that referenced this issue May 7, 2018

cmd/link: remove R_ADDR relocation workaround for macho arm64
The workarounds doesn't seem necessary anymore, and blocks DWARF
on darwin/arm64.

Updates golang#24883.

Change-Id: Ic917c767d3b4f6c51be25566956296f5dd4ead10
Reviewed-on: https://go-review.googlesource.com/108655
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@biscottigelato

This comment has been minimized.

Copy link

commented Jun 1, 2018

Is all this applicable to gomobile bind? I tried to grab master as of today, which should include both commits mentioned above. I also set *ld.FlagW to false. But a dwarfdump on my .framework gave me nothing. And of course the framework did not get symbolicated upon a crash or a breakpoint. Please advise. Thanks!

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2018

It should work, as seen from the discussion. This issue is closed, please open a new one with detailed steps to reproduce the problem. Thanks. If you like, CC me on the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.