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/compile: DWARF line table: "morestack" sequence should have prolog file/line #39757

Open
thanm opened this issue Jun 22, 2020 · 4 comments
Open
Assignees
Labels
Milestone

Comments

@thanm
Copy link
Member

@thanm thanm commented Jun 22, 2020

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

$ go version
go version devel +60f7876502 Sat Jun 20 08:40:13 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

yes.

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

linux/amd64
but this problem applies on other archs as well

What did you do?

Build a small program, e.g. "go build himom.go"

package main

import (
	"fmt"
)

func main() {
	fmt.Printf("hi mom\n")
}

Then examine generated DWARF with "objdump -dl":

$ objdump -t himom | fgrep main.main
000000000049a0e0 g     F .text	0000000000000070 main.main
$ objdump --wide -dl --start-address=0x497f00 --stop-address=0x497f70 himom

himom:     file format elf64-x86-64


Disassembly of section .text:

0000000000497f00 <main.main>:
main.main():
/usr/local/google/home/thanm/himom.go:7
  497f00:	64 48 8b 0c 25 f8 ff ff ff 	mov    %fs:0xfffffffffffffff8,%rcx
  497f09:	48 3b 61 10          	cmp    0x10(%rcx),%rsp
  497f0d:	76 5a                	jbe    497f69 <main.main+0x69>
  497f0f:	48 83 ec 58          	sub    $0x58,%rsp
  497f13:	48 89 6c 24 50       	mov    %rbp,0x50(%rsp)
  497f18:	48 8d 6c 24 50       	lea    0x50(%rsp),%rbp
/usr/local/google/home/thanm/himom.go:8
  497f1d:	48 8b 05 6c 6a 0b 00 	mov    0xb6a6c(%rip),%rax        # 54e990 <os.Stdout>
/ssd2/ygo/src/fmt/print.go:213
  497f24:	48 8d 0d 15 1b 04 00 	lea    0x41b15(%rip),%rcx        # 4d9a40 <go.itab.*os.File,io.Writer>
fmt.Printf():
/ssd2/ygo/src/fmt/print.go:213
  497f2b:	48 89 0c 24          	mov    %rcx,(%rsp)
  497f2f:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
  497f34:	48 8d 05 d6 30 02 00 	lea    0x230d6(%rip),%rax        # 4bb011 <go.string.*+0x641>
  497f3b:	48 89 44 24 10       	mov    %rax,0x10(%rsp)
  497f40:	48 c7 44 24 18 07 00 00 00 	movq   $0x7,0x18(%rsp)
  497f49:	48 c7 44 24 20 00 00 00 00 	movq   $0x0,0x20(%rsp)
  497f52:	0f 57 c0             	xorps  %xmm0,%xmm0
  497f55:	0f 11 44 24 28       	movups %xmm0,0x28(%rsp)
  497f5a:	e8 e1 82 ff ff       	callq  490240 <fmt.Fprintf>
main.main():
/usr/local/google/home/thanm/himom.go:8
  497f5f:	48 8b 6c 24 50       	mov    0x50(%rsp),%rbp
  497f64:	48 83 c4 58          	add    $0x58,%rsp
  497f68:	c3                   	retq   
./<autogenerated>:1
  497f69:	e8 52 90 fc ff       	callq  460fc0 <runtime.morestack_noctxt>
  497f6e:	eb 90                	jmp    497f00 <main.main>

What did you expect to see?

Call to "runtime.morestack" has a source position (file/line) the same as the prolog of the function.

What did you see instead?

Source position is "autogenerated:1":

./<autogenerated>:1
  497f69:	e8 52 90 fc ff       	callq  460fc0 <runtime.morestack_noctxt>
  497f6e:	eb 90                	jmp    497f00 <main.main>

This can interfere with debugging and profiling.

One could argue that since this is compiler-generated code it's ok to give it an autogenerated source position (similar to things like method wrappers), but there is also a strong argument to be made that the morestack call is logically part of the function prolog (even though it is positioned in the cold code at the tail end of the function).

It also turns out that we haven't always marked this code with "autogenerated" -- in previous versions of Go it was being given the correct position. Here is the tail end of the assembly when built with 1.12:

/usr/local/google/home/thanm/himom.go:7
  488a9a:	callq  44f300 <runtime.morestack_noctxt>
  488a9f:	jmp    488a30 <main.main>

It looks as though the problem was introduced when the main part of DWARF line table generation was moved from the linker in to the compiler. This code here:

https://go.googlesource.com/go/+/d1a186d29ce9d917dda7c66cfaee7788f88e7b9e/src/cmd/internal/obj/dwarf.go#112

generates a sequence at the end of each function to reset the state of the line table back to its default settings. This code doesn't advance the PC before applying the new file and line, however, meaning that it overwrites the file/line for the last row in the line table, which is what's causing the "autogenerated:1" to be picked up.

This could be fixed in a couple of ways: one would be to emit an end of sequence op after each function (in which case there would be no manual file/line settting needed). A second option would be to add code to manually advanced the PC past the end of the last instruction before emitting the set file/set line ops.

@thanm thanm self-assigned this Jun 22, 2020
@thanm thanm added the NeedsFix label Jun 22, 2020
@thanm thanm added this to the Go1.16 milestone Jun 22, 2020
@thanm thanm changed the title cmd/compile: DWARF line table:morestack sequence should have prolog file/line cmd/compile: DWARF line table: "morestack" sequence should have prolog file/line Jun 22, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 22, 2020

Change https://golang.org/cl/239286 mentions this issue: [dev.link] cmd/{compile,link}: fix buglet in DWARF line table generation

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 22, 2020

Change https://golang.org/cl/239287 mentions this issue: cmd/compile: fix buglet in DWARF line table generation

@thanm
Copy link
Member Author

@thanm thanm commented Jun 22, 2020

I sent two CLs with possible fixes for this bug.

@aarzilli PTAL and let me know what you think.

@thanm
Copy link
Member Author

@thanm thanm commented Jun 24, 2020

I have submitted a fix for this bug on the dev.link branch; this fix will appear on the main branch at some point when it opens up for 1.16 development (Aug?). I'll hold the bug open until then.

gopherbot pushed a commit that referenced this issue Jun 24, 2020
…WARF line table

The code in the compiler's DWARF line table generation emits line
table ops at the end of each function fragment to reset the state
machine registers back to an initial state, so that when the line
table fragments for each function are stitched together into a
compilation unit, each fragment will have a clean starting point. The
set-file/set-line ops emitted in this code were being applied to the
last row of the line table, however, meaning that they were
overwriting the existing values.

To avoid this problem, add code to advance the PC past the end of the
last instruction in the function, and switch to just using an
end-of-sequence operator at the end of each function instead of
explicit set-file/set-line ops.

Updates #39757.

Change-Id: Ieb30f83444fa86fb1f2cd53862d8cc8972bb8763
Reviewed-on: https://go-review.googlesource.com/c/go/+/239286
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.