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.

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

support linux-loong64 native debug #3892

Merged
merged 2 commits into from
Jan 17, 2025
Merged

support linux-loong64 native debug #3892

merged 2 commits into from
Jan 17, 2025

Conversation

yelvens
Copy link
Contributor

@yelvens yelvens commented Dec 31, 2024

Hi @aarzilli

I accidentally closed this PR by mistake, and I sincerely apologize for that. I have now reopened a new PR and kindly request your review once again. Apologies for the inconvenience caused.

@yelvens
Copy link
Contributor Author

yelvens commented Dec 31, 2024 •
edited
Loading

@aarzilli

#3685 (comment)

The problem is: why is there anything at all here? This function is about stack unwinding with cgo and with exceptions, I see that many (all?) tests relating to this piece of code are disabled so why is this here at all? Why is this function not just empty? Is there any code here that actually works?

The same goes for loong64SwitchStack below. Is there anything besides the first if switch, in that function, that works? Can we delete all of it except the first if and the default branch of the first switch?

Thank you for your thorough review. As you pointed out, all cgo-related tests for loong64 have been disabled. However, I found that setting the loong64FixFrameUnwindContext and loong64SwitchStack functions to empty functions caused many test cases to fail. As a result, I have now removed all cgo-related logic from these functions and kept only the parts that are actually executed.

Please review it again when you have a chance.

Wishing you a Happy New Year!

@yelvens
Copy link
Contributor Author

yelvens commented Dec 31, 2024

#3685 (comment)
#3685 (comment)
#3685 (comment)

Done.

@aarzilli
Copy link
Member

aarzilli commented Jan 1, 2025 •
edited
Loading

Staticcheck is reporting a couple of unused constants:

    > pkg/proc/loong64_arch.go:91:7: const loong64cgocallSPOffsetSaveSlot is unused (U1000)
    > pkg/proc/loong64_arch.go:92:7: const loong64PrevG0schedSPOffsetSaveSlot is unused (U1000)

Huang Qiqi added 2 commits January 2, 2025 09:37
delve: support linux-loong64 native debug
4426e22 
LoongArch is a new RISC ISA, which is independently designed by Loongson Technology.

LoongArch includes a reduced 32-bit version (LA32R), a standard 32-bit version (LA32S)
and a 64-bit version (LA64), and loong64 is the 64-bit version of LoongArch.

LoongArch documentation: https://github.com/loongson/LoongArch-Documentation.git
*: mark loong64 port as experimental
c2601c7
@yelvens
Copy link
Contributor Author

yelvens commented Jan 2, 2025

@aarzilli

Staticcheck is reporting a couple of unused constants:

    > pkg/proc/loong64_arch.go:91:7: const loong64cgocallSPOffsetSaveSlot is unused (U1000)
    > pkg/proc/loong64_arch.go:92:7: const loong64PrevG0schedSPOffsetSaveSlot is unused (U1000)

Done.

aarzilli
aarzilli approved these changes Jan 3, 2025
View reviewed changes
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.

LGTM, thanks.

derekparker
derekparker approved these changes Jan 17, 2025
View reviewed changes
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 d2f748f into go-delve:master Jan 17, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aarzilli aarzilli aarzilli approved these changes

@derekparker derekparker derekparker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yelvens @aarzilli @derekparker