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: 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
Assignees
Milestone

Comments

@derekparker
Copy link
Contributor

@derekparker derekparker commented Jan 16, 2020

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

@derekparker derekparker commented Jan 16, 2020

cc @heschik @dr2chase @aarzilli

@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 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 heschi commented Apr 24, 2020

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

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented Apr 24, 2020

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

@odeke-em odeke-em commented May 29, 2020

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

@odeke-em odeke-em removed this from the Go1.15 milestone May 29, 2020
@odeke-em odeke-em added this to the Go1.16 milestone May 29, 2020
@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented May 29, 2020

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
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
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 28, 2020

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
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

@aclements aclements commented Dec 1, 2020

@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 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 removed this from the Go1.16 milestone Dec 2, 2020
@dr2chase dr2chase added this to the Go1.17 milestone Dec 2, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 22, 2021

Moving to Backlog.

@ianlancetaylor ianlancetaylor removed this from the Go1.17 milestone Apr 22, 2021
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Apr 22, 2021
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
8 participants