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: mangled DWARF location for incoming string parameter #47354

Closed
thanm opened this issue Jul 22, 2021 · 4 comments
Closed

cmd/compile: mangled DWARF location for incoming string parameter #47354

thanm opened this issue Jul 22, 2021 · 4 comments
Labels
NeedsFix
Milestone

Comments

@thanm
Copy link
Contributor

@thanm thanm commented Jul 22, 2021

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

This is with Go 1.17/tip.

Does this issue reproduce with the latest release?

Yes.

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

linux/amd64

What did you do?

Build this program:

package main

var G int

//go:noinline
func docall(f func()) {
	f()
}

//go:noinline
func another(x int) {
	println(G)
}

//go:noinline
func livestring(s string) {
	docall(func() {
		println("s is", s)
	})
	G++
	another(0)
}

//go:noinline
func livestring2(s string) {
	docall(func() {
		println("s is", s)
	})
	G++
	another(int(s[0]))
}

func main() {
	livestring("foo")
	livestring2("bar")
}

If you stop in 'livestring2", the DWARF location expression emitted for the parameter 's' is damaged/incorrect.

$ gdb livestring
...
(gdb) b main.livestring2
Breakpoint 1 at 0x455680: file /usr/local/google/home/thanm/livestring.go, line 25.
(gdb) run
Starting program: /usr/local/google/home/thanm/livestring 
...
Thread 1 "livestring" hit Breakpoint 1, main.livestring2 (s=<error reading variable: access outside bounds of object referenced via synthetic pointer>) at /usr/local/google/home/thanm/livestring.go:25
25	func livestring2(s string) {
(gdb) p s
access outside bounds of object referenced via synthetic pointer
(gdb) info address s
Symbol "s" is multi-location:
  Base address 0x455680  Range 0x455680-0x4556c8: a variable in $rax [8-byte piece], and a variable in $rax [8-byte piece], and a complex DWARF expression:
     0: DW_OP_fbreg 8
    [8-byte piece], and a complex DWARF expression:
     0: DW_OP_fbreg 8
    [8-byte piece]
  Range 0x4556c8-0x45571b: a complex DWARF expression:
     0: DW_OP_call_frame_cfa
    [8-byte piece], and a complex DWARF expression:
     0: DW_OP_call_frame_cfa
    [8-byte piece], and a complex DWARF expression:
     0: DW_OP_fbreg 8
    [8-byte piece], and a complex DWARF expression:
     0: DW_OP_fbreg 8
    [8-byte piece]
.
(gdb) 

What did you expect to see?

Sane location expression

What did you see instead?

Malformed location expression. We should not be seeing 4 pieces, this is only a 2-piece variable. Something is going wrong somewhere along the line.

@thanm thanm added this to the Backlog milestone Jul 22, 2021
@thanm thanm self-assigned this Jul 22, 2021
@seankhliao seankhliao added the NeedsFix label Jul 25, 2021
@seankhliao seankhliao added NeedsInvestigation and removed NeedsFix labels Oct 24, 2021
@thanm
Copy link
Contributor Author

@thanm thanm commented Nov 10, 2021

There are two things going wrong here.

The first problem is the same as issue 46845, the problem with zero-width types (I've already sent a fix, it is being reviewed).

The second problem is that the SSA name table is getting messed up as a result of inconsistency in the compiler has to how the internals of a Go string are represented. In most places a string looks like "{ *uint8, int }", however there are also a few places where it winds up looking like "{ unsafe.Pointer, uintptr }". What happens is that the same value gets entered into f.NamedValues twice, once for each type. Here's an example excerpt from the GOSSAFUNC+ output for the test case in question, this is just after the expand_calls phase:

name s[string]: []
name s[*uint8]: [v44]
name s+8[int]: [v31]
name s[unsafe.Pointer]: [v44]
name s+8[uintptr]: [v31]

Here "s" is the incoming string param. This should be fairly easy to take care of by tracking down the oddball types and canonicalizing the type on "{ *uint8, int }".

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 10, 2021

Change https://golang.org/cl/362715 mentions this issue: cmd/compile: use canonical string representation in abiutils

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 15, 2021

Change https://golang.org/cl/364037 mentions this issue: cmd/link: add some better DWARF location expression tests

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 17, 2021

Change https://golang.org/cl/364674 mentions this issue: dwarf_testing: add new pseudo-pacakge with DWARF location expression tests

@dmitshur dmitshur removed this from the Backlog milestone Nov 18, 2021
@dmitshur dmitshur added this to the Go1.18 milestone Nov 18, 2021
@dmitshur dmitshur added NeedsFix and removed NeedsInvestigation labels Nov 18, 2021
gopherbot pushed a commit to golang/debug that referenced this issue Nov 22, 2021
This patch rolls out a new set of regression tests for verifying the
integrity of DWARF variable location expressions. The tests use a
"scaffold" consisting of harness program 'dwdumploc' that can be run
on a Go binary to dump out the DWARF location expressions for a
function's formal parameters, and then a new set of tests that invoke
the harness on specific canned testcases. The scaffold itself depends
on packages from Delve.

DWARF location expressions can refer to machine registers, making the
output of the dump architecture-dependent. The dumper program
currently supports { Amd64, Arm64 } x { Linux, Macho, Windows }, which
matches up mostly with current Delve support.

Updates golang/go#47354.
Updates golang/go#46845.

Change-Id: I7377c03b19e3cf43aa0f7698fcc636be9c852765
Reviewed-on: https://go-review.googlesource.com/c/debug/+/364674
Trust: Than McIntosh <thanm@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
doujiang24 pushed a commit to doujiang24/debug that referenced this issue Jan 17, 2022
This patch rolls out a new set of regression tests for verifying the
integrity of DWARF variable location expressions. The tests use a
"scaffold" consisting of harness program 'dwdumploc' that can be run
on a Go binary to dump out the DWARF location expressions for a
function's formal parameters, and then a new set of tests that invoke
the harness on specific canned testcases. The scaffold itself depends
on packages from Delve.

DWARF location expressions can refer to machine registers, making the
output of the dump architecture-dependent. The dumper program
currently supports { Amd64, Arm64 } x { Linux, Macho, Windows }, which
matches up mostly with current Delve support.

Updates golang/go#47354.
Updates golang/go#46845.

Change-Id: I7377c03b19e3cf43aa0f7698fcc636be9c852765
Reviewed-on: https://go-review.googlesource.com/c/debug/+/364674
Trust: Than McIntosh <thanm@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@rsc rsc unassigned thanm Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix
Projects
None yet
Development

No branches or pull requests

4 participants