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: paths in debug_line contain a mix of forward and backward slashes with Go 1.14beta1/Windows #36495

Closed
aarzilli opened this issue Jan 10, 2020 · 5 comments

Comments

@aarzilli
Copy link
Contributor

@aarzilli aarzilli commented Jan 10, 2020

What the title says. It used to be that debug_line uniformly used forward slashes / but now some paths have forward slashes / while others have backward slashes \.

This happens reliably when building with go build path_to_go_file.go, I'm not sure about other build modes.

Existing versions of Delve will have a problem with this but the fix is simple and I have already implemented it. However the behavior seems unintentional enough that I thought I'd report it.

I'm blaming cmd/link entirely without proof, it could be something else.

Since Windows doesn't have a handy utility to examine debug_line I wrote this script:

package main

import (
	"debug/dwarf"
	"debug/pe"
	"fmt"
	"io"
	"os"
)

func must(err error) {
	if err != nil {
		panic(err)
	}
}

func main() {
	exe, err := pe.Open(os.Args[1])
	must(err)
	dw, err := exe.DWARF()
	must(err)

	rdr := dw.Reader()

	seen := make(map[string]bool)

	for {
		e, err := rdr.Next()
		must(err)
		if e == nil {
			break
		}

		if e.Tag != dwarf.TagCompileUnit {
			continue
		}

		lnrdr, err := dw.LineReader(e)
		must(err)

		if lnrdr != nil {
			var lne dwarf.LineEntry
			for {
				err := lnrdr.Next(&lne)
				if err == io.EOF {
					break
				}
				must(err)

				if !seen[lne.File.Name] {
					seen[lne.File.Name] = true
					fmt.Printf("%s\n", lne.File.Name)
				}
			}
		}

		rdr.SkipChildren()
	}

}

@gopherbot please add label Debugging

@aarzilli
Copy link
Contributor Author

@aarzilli aarzilli commented Jan 10, 2020

Does the @gopherbot thing not work anymore?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 10, 2020

@ianlancetaylor ianlancetaylor added this to the Go1.14 milestone Jan 10, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 10, 2020

Thanks for the report. Tentatively marking as a release blocker for 1.14.

@jeremyfaller jeremyfaller self-assigned this Jan 10, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 10, 2020

Change https://golang.org/cl/214286 mentions this issue: cmd/link: emit only '/' in DWARF file names

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 16, 2020

Change https://golang.org/cl/263017 mentions this issue: cmd/link: emit include directories in DWARF line table prologue

gopherbot pushed a commit that referenced this issue Oct 30, 2020
This patch changes the way the linker emits the DWARF line table
prologue, specifically the file table. Previously files were left
unmodified, and the directory table was empty. For each compilation
unit we now scan the unit file table and build up a common set of
directories, emit them into the directory table, and then emit file
entries that refer to the dirs. This provides a modest binary size
savings.

For kubernetes kubelet:

$ objdump -h /tmp/kubelet.old | fgrep debug_line
 36 .zdebug_line  019a55f5  0000000000000000  0000000000000000  084a5123  2**0
$ objdump -h /tmp/kubelet.new | fgrep debug_line
 36 .zdebug_line  01146fd2  0000000000000000  0000000000000000  084a510a  2**0

[where the value following the section name above is the section size
in hex, so roughly a 30% decrease in this case.]

The actual savings will depend on the length of the pathnames
involved, so it's hard to really pin down how much savings we'll see
here. In addition, emitting the files this way reduces the
"compressibility" of the line table, so there could even be cases
where we don't win at all.

Updates #6853, #19784, #36495.

Change-Id: I298d8561da5ed3ebc9d38aa772874851baa2f4f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/263017
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Trust: Than McIntosh <thanm@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
5 participants
You can’t perform that action at this time.