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

Fix PIE tests on ppc64le port #3498

Merged
merged 2 commits into from
Sep 14, 2023
Merged

Conversation

archanaravindar
Copy link
Contributor

PIE tests (tests on binaries built with PIE mode) on ppc64le port were having issues such as not matching expected patterns as in non PIE mode while stepping and not stepping into functions as expected creating errors due to which these tests were skipped on PIE mode
These problems were occurring due to the special prologue emitted on ppc64le to deal with the TOC register or R2
In some cases we had to modify the expected line numbers while stepping
and in some other cases where the breakpoint was set in the prologue instruction even before function entry as a result of which the breakpoint was deemed void and the program control was not entering inside the function we had to modify delve to recognize these patterns to appropriately skip the special prologue and move the breakpoint a few instructions later so that it was set at the right location and the breakpoint was actually hit.
Very soon we will add a change to the Go runtime as well to emit DWARF attributes to recognize such situations so that Delve can recognize this pattern with ease.
For now the current change in Delve fixes the issue and the tests pass.

Copy link
Member

@aarzilli aarzilli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The go compiler should emit an end of prologue flag in debug_line, this shouldn't matter at all.

@derekparker
Copy link
Member

The go compiler should emit an end of prologue flag in debug_line, this shouldn't matter at all.

You're correct, it should but it isn't for this particular sequence. Needs to be fixed upstream.

Copy link
Member

@aarzilli aarzilli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

@derekparker
Copy link
Member

The ppc64le machine keeps timing out, but the pkg/proc tests passed according to the logs, so I say we merge.

Copy link
Member

@derekparker derekparker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@derekparker derekparker merged commit 86020cd into go-delve:master Sep 14, 2023
1 check passed
@aarzilli
Copy link
Member

The ppc64le machine keeps timing out, but the pkg/proc tests passed according to the logs, so I say we merge.

If you look at the logs you'll see that a lot of the time is actually spent downloading and installing updates as well as programs we need for the script (jq, gcc, etc). Maybe we should make a docker image that is already up to date and has the things we need installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants