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/compile: the DW_AT_location of the return value is empty when its name is not specified #28416

Open
ks888 opened this Issue Oct 26, 2018 · 13 comments

Comments

Projects
None yet
6 participants
@ks888

ks888 commented Oct 26, 2018

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

go version devel +66bb8ddb95 Thu Oct 25 13:37:38 2018 +0000 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/yagami/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/yagami/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/yagami/src/go"
GOTMPDIR=""
GOTOOLDIR="/Users/yagami/src/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/5t/5crzwsxs305_ngrhgx90phzh0000gn/T/go-build561220144=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

In the sample program below, the DW_AT_location of the return value ~r1 is empty. It's nice to provide the information about the location here because the value is present on the stack and is used later.

% cat loc_attr.go
package main

import "fmt"

func fib(n int) int {
        if n == 0 || n == 1 {
                return n
        }
        return fib(n-1) + fib(n-2)
}

func main() {
        fmt.Println(fib(10))
}
% go build -ldflags=-compressdwarf=false loc_attr.go
% gobjdump --dwarf=info ./loc_attr | grep -B 1 -A 18 'main.fib'
 <1><75c36>: Abbrev Number: 3 (DW_TAG_subprogram)
    <75c37>   DW_AT_name        : main.fib
    <75c40>   DW_AT_low_pc      : 0x1091960
    <75c48>   DW_AT_high_pc     : 0x10919e8
    <75c50>   DW_AT_frame_base  : 1 byte block: 9c      (DW_OP_call_frame_cfa)
    <75c52>   DW_AT_decl_file   : 0x1
    <75c56>   DW_AT_external    : 1
 <2><75c57>: Abbrev Number: 16 (DW_TAG_formal_parameter)
    <75c58>   DW_AT_name        : n
    <75c5a>   DW_AT_variable_parameter: 0
    <75c5b>   DW_AT_decl_line   : 5
    <75c5c>   DW_AT_type        : <0x339f7>
    <75c60>   DW_AT_location    : 0x71216 (location list)
 <2><75c64>: Abbrev Number: 15 (DW_TAG_formal_parameter)
    <75c65>   DW_AT_name        : ~r1
    <75c69>   DW_AT_variable_parameter: 1
    <75c6a>   DW_AT_decl_line   : 5
    <75c6b>   DW_AT_type        : <0x339f7>
    <75c6f>   DW_AT_location    : 0 byte block:         ()
 <2><75c70>: Abbrev Number: 0

I'm not sure it's helpful, but I noticed the DW_AT_location is not empty if the name of the return value is specified in the source:

% cat loc_attr.go
package main

import "fmt"

func fib(n int) (r int) {  // now the return value is named.
        if n == 0 || n == 1 {
                return n
        }
        return fib(n-1) + fib(n-2)
}

func main() {
        fmt.Println(fib(10))
}
% go build -ldflags=-compressdwarf=false loc_attr.go
% gobjdump --dwarf=info ./loc_attr | grep -B 1 -A 18 'main.fib'
 <1><75c36>: Abbrev Number: 3 (DW_TAG_subprogram)
    <75c37>   DW_AT_name        : main.fib
    <75c40>   DW_AT_low_pc      : 0x1091960
    <75c48>   DW_AT_high_pc     : 0x10919e8
    <75c50>   DW_AT_frame_base  : 1 byte block: 9c      (DW_OP_call_frame_cfa)
    <75c52>   DW_AT_decl_file   : 0x1
    <75c56>   DW_AT_external    : 1
 <2><75c57>: Abbrev Number: 16 (DW_TAG_formal_parameter)
    <75c58>   DW_AT_name        : n
    <75c5a>   DW_AT_variable_parameter: 0
    <75c5b>   DW_AT_decl_line   : 5
    <75c5c>   DW_AT_type        : <0x339f7>
    <75c60>   DW_AT_location    : 0x71216 (location list)
 <2><75c64>: Abbrev Number: 16 (DW_TAG_formal_parameter)
    <75c65>   DW_AT_name        : r
    <75c67>   DW_AT_variable_parameter: 1
    <75c68>   DW_AT_decl_line   : 5
    <75c69>   DW_AT_type        : <0x339f7>
    <75c6d>   DW_AT_location    : 0x71249 (location list)
 <2><75c71>: Abbrev Number: 0

What did you expect to see?

The DW_AT_location of the return value ~r1 provides the information about the location.

What did you see instead?

The DW_AT_location of the return value ~r1 is empty.

@bradfitz bradfitz changed the title from The DW_AT_location of the return value is empty when its name is not specified to cmd/compile: the DW_AT_location of the return value is empty when its name is not specified Oct 26, 2018

@bradfitz

This comment has been minimized.

Member

bradfitz commented Oct 26, 2018

@heschik

This comment has been minimized.

Contributor

heschik commented Oct 26, 2018

I think fa31093 came very close to fixing this, but it only adds names for PPARAM, not PPARAMOUT.

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Oct 26, 2018

There were random scary words around PPARAMOUT that made me decide that discretion was the better part of valor. I can look into PPARAMOUT in a bit, but I have actual debugging lies that need to be removed or corrected first.

@heschik

This comment has been minimized.

Contributor

heschik commented Oct 26, 2018

@ks888, can I ask what the use case is here? If there's a strong requirement I can take a quick shot at getting something in before the code freeze.

@andybons andybons added the NeedsFix label Oct 26, 2018

@andybons andybons added this to the Go1.12 milestone Oct 26, 2018

@ks888

This comment has been minimized.

ks888 commented Oct 27, 2018

@heschik I'm now working on the function tracer project for golang (here is the project page). The tracer traces the specified function and prints its actual input arguments when the function is called. As well, the actual output arguments are printed when the function is returned. The location information of output arguments is important to print the correct values here.

@heschik

This comment has been minimized.

Contributor

heschik commented Oct 29, 2018

Yeah, okay, #14762 makes this pretty hard. I don't think it's possible to fix this until that's done, because the location list generation code is primarily interested in register contents, and it can't see those because of that bug.

@ks888, if you can build your binaries in "debug" mode, i.e. with -gcflags="-N -l", that should work for you. Other than that I'm afraid you're out of luck for a long while -- that bug is from 2016.

@ks888

This comment has been minimized.

ks888 commented Oct 30, 2018

Thank you for finding it out. I'm going to ask my users to compile the binary in debug mode for now.

@heschik

This comment has been minimized.

Contributor

heschik commented Oct 30, 2018

Actually, I was able to get a tiny little location list for the return value. It only exists for the moment right before the return, and unfortunately it looks like our CFA information is wrong after $rsp is adjusted in the exit. It might be better than nothing?

@aarzilli @derekparker, does this sound interesting at all to you? Right now I don't think it's worth it but I'd be curious to hear your thoughts.

@aarzilli

This comment has been minimized.

Contributor

aarzilli commented Oct 30, 2018

unfortunately it looks like our CFA information is wrong after $rsp is adjusted in the exit

It actually looks good to me, at least for main.fib above. How is it wrong? GNU objdump seems to print nonsensical values for all arguments of DW_CFA_def_cfa_offset_sf but the bytes are good (it seems that objdump is interpreting the argument of DW_CFA_def_cfa_offset_sf as ULEB128 but the standard clearly says that it's signed and if objdump was right nothing would ever work with gdb).

diexplorer gives me this instructions for main.fib:

Begin | 0x485260
End | 0x4852e8

	DW_CFA_def_cfa_offset_sf -0x2 
	DW_CFA_advance_loc 0x13 to 0x485273 
	DW_CFA_def_cfa_offset_sf -0xa 
	DW_CFA_advance_loc 0x22 to 0x485295 
	DW_CFA_def_cfa_offset_sf -0x2 
	DW_CFA_advance_loc 0x1 to 0x485296 
	DW_CFA_def_cfa_offset_sf -0xa 
	DW_CFA_advance_loc1 0x47 to 0x4852dd 
	DW_CFA_def_cfa_offset_sf -0x2 
	DW_CFA_advance_loc 0xa to 0x4852e7 

it's switching between a frame size of 28 (-0xa * -4) and a frame size of 8 (-0x2 * -4) corresponding to the places where the return instructions are.

@aarzilli @derekparker, does this sound interesting at all to you?

If it's there delve will use it for its own function tracing, I wouldn't exactly put it at the top of the list of problems with debugging optimized programs and weight it against increased binary size.

@ks888 you might be interested in knowing that delve also has a function tracing feature, albeit with a different interface. You could benefit from using delve as your backend, unless your objective is reimplementing a debugger backend, of course.

@heschik

This comment has been minimized.

Contributor

heschik commented Oct 30, 2018

Interesting. Maybe it's actually a gdb bug -- when I stop at

=>	loc_attr.go:7	0x4857a5	c3			ret

in Delve, it prints n fine. When I do it in gdb:

=> 0x00000000004857a5 <+53>:	retq   

I get gibberish. Fair enough. Thanks.

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Oct 31, 2018

What's your gdb version? I spent some time trying to figure out why gdb wouldn't debug (on a Mac) my test binary, but when I lit up 8.2 in a Docker (to eliminate the Mac craziness) it worked fine. But perhaps 8.1 or 8.0, etc.

@ks888

This comment has been minimized.

ks888 commented Oct 31, 2018

@aarzilli Thank you. Yeah, I should definitely consider using the delve as the backend at some point.

@aarzilli

This comment has been minimized.

Contributor

aarzilli commented Oct 31, 2018

in Delve, it prints n fine. When I do it in gdb:

The fact that gdb disagrees on this is worrying, but I can't see why delve interpretation would be wrong and lldb agrees with us, so I'm going to provisionally agree that it's a gdb bug.

@heschik heschik added the Debugging label Oct 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment