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

Jump to bottom

cmd/compile: set prologue_end on every arch #36612

Open
derekparker opened this issue Jan 16, 2020 · 10 comments
Open

cmd/compile: set prologue_end on every arch #36612

derekparker opened this issue Jan 16, 2020 · 10 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Debugging NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@derekparker
Copy link
Contributor

As of Go 1.11 and commit 7bac2a9 support was added to put information about the end of the function prologue into the produced DWARF information, however it currently only works for x86.

Ideally the DWARF information produced is consistent across arches. In Delve if this information is not present we have to disassemble the function in order to pattern match known function prologues in order to skip them. This is undesirable to say the least and I would like to delete that code.

Opening this issue to track adding prologue_end to the DWARF information produced on every arch.
@derekparker
Copy link
Contributor Author

cc @heschik @dr2chase @aarzilli

@ALTree ALTree added Debugging NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jan 17, 2020
@dr2chase dr2chase self-assigned this Jan 22, 2020
@dr2chase dr2chase added this to the Go1.15 milestone Jan 22, 2020
@heschi
Copy link
Contributor

heschi commented Apr 24, 2020

https://golang.org/cl/223297 did part of this. Any more planned for 1.15?

@heschi
Copy link
Contributor

heschi commented Apr 24, 2020

...though that CL isn't merged yet.

@dr2chase
Copy link
Contributor

Any time you want to +2 it....
It would make sense to maybe get ahead on some other architectures, might help gdb, also depends on where delve is going next.

@odeke-em
Copy link
Member

The CL got merged on May 13th aka 16 days ago. @dr2chase I shall move this to Go1.16 then.

@odeke-em odeke-em modified the milestones: Go1.15, Go1.16 May 29, 2020
@dr2chase
Copy link
Contributor

I'm okay with go1.16. Part of the problem is knowing exactly what's right; because of better-user-experience issues, Delve wants it to be someplace that apparently doesn't match what gdb would expect. (My personal user experience with gdb is so terrible that I would never recommend it to a Go user.)

derekparker added a commit to derekparker/go that referenced this issue Oct 28, 2020
@derekparker
cmd/internal/obj: add prologue_end DWARF stmt for ppc64
77a2172 
This patch adds a prologue_end statement to the DWARF information for
the ppc64 arch.

Prologue end is used by the Delve debugger in order to determine where
to set a breakpoint to avoid the stacksplit prologue.

Updates golang#36612
@derekparker derekparker mentioned this issue Oct 28, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/266019 mentions this issue: cmd/internal/obj: add prologue_end DWARF stmt for ppc64

gopherbot pushed a commit that referenced this issue Nov 4, 2020
@derekparker @dmitshur
cmd/internal/obj: add prologue_end DWARF stmt for ppc64
63fd764 
This patch adds a prologue_end statement to the DWARF information for
the ppc64 arch.

Prologue end is used by the Delve debugger in order to determine where
to set a breakpoint to avoid the stacksplit prologue.

Updates #36612

Change-Id: Ifb16c1476fe716a0bf493c5486d1d88ebe8d0253
GitHub-Last-Rev: 77a2172
GitHub-Pull-Request: #42261
Reviewed-on: https://go-review.googlesource.com/c/go/+/266019
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
@aclements
Copy link
Member

@dr2chase , what's the current status of this? Are you planning to do any more on this for 1.16 or should we bump it to 1.17?

@dr2chase
Copy link
Contributor

dr2chase commented Dec 1, 2020

Derek Parker (i.e., Delve developers) recently added ppc64.
I think this stays live till all the architectures are done, which might take a while.

One reason to wait on demand from Delve is that ensures we do it the way they want, instead of accidentally picking wrong and then someone else depends on that choice.

@dr2chase dr2chase modified the milestones: Go1.16, Go1.17 Dec 2, 2020
@ianlancetaylor
Copy link
Contributor

Moving to Backlog.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.17, Backlog Apr 22, 2021
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Debugging NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Go Compiler / Runtime
Status: Triage Backlog
Milestone
Backlog
Development

No branches or pull requests
8 participants
@derekparker @dr2chase @aclements @ianlancetaylor @ALTree @odeke-em @gopherbot @heschi