Skip to content

cmd/compile: wrong location list for function argument #72053

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.

Sign up for GitHub

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

Jump to bottom
Closed
aarzilli opened this issue Mar 1, 2025 · 7 comments
Closed

cmd/compile: wrong location list for function argument #72053

aarzilli opened this issue Mar 1, 2025 · 7 comments
Assignees
@dr2chase
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.25

Comments

@aarzilli
Copy link
Contributor

aarzilli commented Mar 1, 2025

Given the following program:

package main

import (
	"fmt"
	"strings"
)

func main() {
	u := Address{Addr: "127.0.0.1"}
	fmt.Println(u) // line 10
}

type Address struct {
	TLS  bool
	Addr string
}

func (a Address) String() string {
	sb := new(strings.Builder)
	sb.WriteString(a.Addr)
	return sb.String()
}

For function main.Address.String the first entry in the location list looks like this:

DW_OP_reg0 DW_OP_piece 0x1 DW_OP_piece 0x7 DW_OP_reg3 DW_OP_piece 0x8 DW_OP_piece 0x7 DW_OP_reg2 DW_OP_piece 0x8

this is wrong, the total size of this location list is 31 bytes while the type of the variable is only 24 bytes. It looks like a spurious 7 byte empty piece is inserted in the middle of the two fields of the string type, for some reason.

Reproduces on go 1.24, 1.23 and 1.22.

Originally reported as: go-delve/delve#3923

cc @dr2chase
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 1, 2025
@aarzilli aarzilli mentioned this issue Mar 1, 2025
Open
@JunyangShao JunyangShao reopened this Mar 3, 2025
@JunyangShao
Copy link
Contributor

@golang/compiler

@JunyangShao JunyangShao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 3, 2025
@mknyszek
Copy link
Contributor

mknyszek commented Mar 5, 2025

CC @thanm maybe? :) In case you've got some interest in this with your DWARF5 work.

@mknyszek mknyszek added this to the Backlog milestone Mar 5, 2025
@derekparker
Copy link
Contributor

Mentioned this in the Delve <> Go team meeting, but I am taking a look at this and hope to send in a CL this week or the next.

derekparker added a commit to derekparker/go that referenced this issue Mar 11, 2025
@derekparker
cmd/compile/internal/ssa: fix ComputePadding
04b89b4 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
derekparker added a commit to derekparker/go that referenced this issue Mar 11, 2025
@derekparker
cmd/compile/internal/ssa: fix ComputePadding
870467b 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
derekparker added a commit to derekparker/go that referenced this issue Mar 11, 2025
@derekparker
cmd/compile/internal/ssa: fix ComputePadding
98fecb1 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
derekparker added a commit to derekparker/go that referenced this issue Mar 11, 2025
@derekparker
cmd/compile/internal/abi: fix ComputePadding
854301d 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
derekparker added a commit to derekparker/go that referenced this issue Mar 11, 2025
@derekparker
cmd/compile/internal/abi: fix ComputePadding
2f200e9 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
derekparker added a commit to derekparker/go that referenced this issue Mar 11, 2025
@derekparker
cmd/compile/internal/abi: fix ComputePadding
91dc042 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
derekparker added a commit to derekparker/go that referenced this issue Mar 11, 2025
@derekparker
cmd/compile/internal/abi: fix ComputePadding
e05e2af 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
@derekparker derekparker mentioned this issue Mar 11, 2025
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/656736 mentions this issue: cmd/compile/internal/abi: fix ComputePadding

@derekparker derekparker mentioned this issue Mar 12, 2025
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/657355 mentions this issue: dwtest: add test for go#72053

gopherbot pushed a commit to golang/debug that referenced this issue Mar 15, 2025
@derekparker @gopherbot
dwtest: add test for go#72053
Loading
Loading status checks…
70c1efa 
Adds a test for https://go.dev/issue/72053
Note the fix is submitted as https://go.dev/cl/656736

For [golang/go#72053](golang/go#72053)

Change-Id: Iea247439f95cce85bf9a1560dd475be5048ec97a
GitHub-Last-Rev: dcd050f
GitHub-Pull-Request: #22
Reviewed-on: https://go-review.googlesource.com/c/debug/+/657355
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@github-project-automation github-project-automation bot moved this from Todo to Done in Go Compiler / Runtime Mar 20, 2025
@ALTree ALTree mentioned this issue Mar 21, 2025
Closed
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 21, 2025
@dmitshur dmitshur modified the milestones: Backlog, Go1.25 Mar 21, 2025
@ianlancetaylor
Copy link
Member

CL 659736 was reverted by CL 659955.

@github-project-automation github-project-automation bot moved this from Done to In Progress in Go Compiler / Runtime Mar 21, 2025
derekparker added a commit to derekparker/go that referenced this issue Mar 21, 2025
@derekparker
cmd/compile/internal/abi: fix ComputePadding
fabed69 
Fixes the ComputePadding calculation to take into account the padding
added for the current offset. This fixes an issue where padding can be
added incorrectly for certain structs.

Related: go-delve/delve#3923

Fixes golang#72053
@derekparker derekparker mentioned this issue Mar 21, 2025
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/659698 mentions this issue: cmd/compile/internal/abi: fix ComputePadding

@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Compiler / Runtime Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dr2chase dr2chase

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Compiler / Runtime
Status: Done
Milestone
Go1.25
8 participants
@aarzilli @mknyszek @derekparker @dmitshur @dr2chase @ianlancetaylor @gopherbot @JunyangShao