delve: add linux-riscv64 support #3785

lrzlin · 2024-07-24T20:47:00Z

This patch adds linux-riscv64 support to delve, tested under go 1.22.5/ubuntu 24.04 riscv64 & go 1.21.4/openEuler 24.03 riscv64.

It's worth noticing that due to riscv64 do not support PTRACE_SINGLESTEP, just like arm32, so we insert ebreak to emulate it. The __NR_riscv_flush_icache syscall is used to avoid self modify code's TOCTOU error, but I wasn't able to find any difference with sys.PtracePokeData. So it still needs some kinds of validation.

To support CGO, we have to insert c.ebreak instead of ebreak as breakpoint due to gcc/clang will enable c extension by default, and use ebreak as altbreakpoint, but due to a arm64 specific workaround, it can not handle hardcoded breakpoint correctly, so I disabled it on riscv64 platform.

Most of tests were passed, the few failed tests seems to be related with riscv64 go compiler itself, for example, the strange repetition in TestTraceDirRecursion, just like this:

> goroutine(1): main.A(5, 5)
> goroutine(1): main.A(4, 4)
> goroutine(1): main.A(4, 4)
> goroutine(1): main.A(3, 3)
> goroutine(1): main.A(2, 2)
> goroutine(1): main.A(2, 2)
> goroutine(1): main.A(1, 1)
> goroutine(1): main.A(1, 1)
>> goroutine(1): main.A => (1)
>> goroutine(1): main.A => (2)
>> goroutine(1): main.A => (6)
>> goroutine(1): main.A => (24)
>> goroutine(1): main.A => (120)
120

I tried to disable the asyncpreemt, but unfortunately, It doesn't work.

Also, the riscv64 x/arch support hasn't been merged to upstream, so I use -mod=vendor here as a workaround.

aarzilli · 2024-07-25T06:52:42Z

_scripts/make.go

@@ -51,6 +51,8 @@ func NewMakeCommands() *cobra.Command {
 				envflags = append(envflags, "GOOS="+OS)
 			}
 			if len(envflags) > 0 {
+				// Add -mod=vendor to envflags due to riscv arch package has not upstream yet.
+				envflags = append(envflags, "-mod=vendor")


We can't do this. We either wait for this to be merged upstream or use a different package.

This is just a workaround, we will remove that once the arch package support was upstream.

Removed this workaround, riscv64 support is already merged into x/arch, see golang/arch@7874f23

aarzilli · 2024-07-25T07:04:33Z

pkg/proc/native/support_sentinel_linux.go

@@ -1,4 +1,4 @@
-//go:build linux && !amd64 && !arm64 && !386 && !(ppc64le && exp.linuxppc64le)
+//go:build linux && !amd64 && !arm64 && !386 && !(ppc64le && exp.linuxppc64le) && !riscv64


If we don't have a build agent for riscv64 this needs to be guarded with a build flag like ppc64le.

I was wondering whether we could add riscv64 to delve CI support? I have saw the portnotes, maybe we could donate some kinds of riscv hardware for this purpose?

That could be done. The instructions to add a teamcity agent are here: https://www.jetbrains.com/help/teamcity/install-teamcity-agent.html.

Thanks, we could provide riscv hardware for this, does this ci machine need to be controled by delve dev team directly or we could host it in a third-party organization?

You can host it yourself. The link above has the instructions to install and configure an agent. You can also download the installer from https://delve.teamcity.com/agents/overview, by clicking on the "Install agent" button. Then .teamcity/settings.kts will have to be modified (you can basically copy the changes made for ppc64le) and the agent will have to be authorized (which we will do once it's online).

aarzilli · 2024-07-25T07:06:25Z

pkg/proc/native/threads_linux_riscv64.go

+	return fpregs, riscv64_fpregs.Fregs, err
+}
+
+func pokeMemory(pid int, addr uintptr, data []byte) (count int, err error) {


Why is this needed instead of using the normal way we write to memory?

Originally I think we might have a TOCTOU issue when emulate singlestep by insert a ebreak instruction, so I use it for sync the cache to avoid that, but seems that delve will stop every other threads while debugging, if so we don't need it.

I just runs the test without this custom pokeMemory, and it's works well, so we can safely restore using sys.PtracePokeData.

aarzilli · 2024-07-25T07:08:42Z

pkg/proc/riscv64_arch.go

+		return false
+	}
+	switch it.frame.Current.Fn.Name {
+	case "runtime.cgocallback_gofunc", "runtime.cgocallback":


This stuff doesn't work or the cgo stacktrace tests would work, delete it.

I meant all of it: runtime.asmcgocall and crosscall2 are the same thing as is the switch at the bottom of this function.

runtime.asmcgocall and crosscall2 removed.

lrzlin · 2024-07-31T07:08:41Z

@aarzilli I was working on cgostacktrace for riscv64, it seems that both arm64cgocallSPOffsetSaveSlot and prevG0schedSPOffsetSaveSlot's value is wrong, the second one should be 24, but I couldn't find the first one in asmcgocall, where should I find it?

Moreover, ppc64le do follows the numbers found in asm_ppc64x.s but it still couldn't use cgostacktrace, So did this feature needs golang side support, or it's just because the logic of riscv64SwitchStack is simply wrong? Thanks.

aarzilli · 2024-08-06T12:04:03Z

I don't remember how the values for arm64 were derived and, as far as I know, the ppc64 implementation never worked so it's possible that the logic is wrong.

lrzlin · 2024-09-11T20:03:33Z

@aarzilli Hi, riscv64 x/arch support is already merged in upstream, you could find that in golang/arch@7874f23, I have also updated the code, could you please give this patch another look? Thanks a lot.

aarzilli · 2024-09-12T08:06:55Z

pkg/proc/linutil/regs_riscv64_arch.go

+	"golang.org/x/arch/riscv64/riscv64asm"
+)
+
+// Regs is wrapper for sys.PtraceRegs


The comment needs to start with "RISCV64Registers is..."

aarzilli · 2024-09-12T08:07:41Z

pkg/proc/linutil/regs_riscv64_arch.go

+	}
+}
+
+// Refer to the definition of struct __riscv_d_ext_state in the kernel ptrace.h


The comment needs to start with "RISCV64PtraceFpRegs is..."

aarzilli · 2024-09-12T08:16:44Z

pkg/proc/core/linux_core.go

@@ -333,6 +367,15 @@ func readNote(r io.ReadSeeker, machineType elf.Machine) (*note, error) {
 			}
 			note.Desc = fpregs
 		}
+
+		if machineType == _EM_RISCV {


else if or convert to a switch.

aarzilli · 2024-09-12T08:18:53Z

pkg/proc/native/proc.go

@@ -372,7 +372,7 @@ func (dbp *nativeProcess) initialize(path string, debugInfoDirs []string) (*proc
 		//	  with gdb once AsyncPreempt was enabled. While implementing the port,
 		//	  few tests failed while it was enabled, but cannot be warrantied that
 		//	  disabling it fixed the issues.
-		DisableAsyncPreempt: runtime.GOOS == "windows" || (runtime.GOOS == "linux" && runtime.GOARCH == "arm64") || (runtime.GOOS == "linux" && runtime.GOARCH == "ppc64le"),
+		DisableAsyncPreempt: runtime.GOOS == "windows" || (runtime.GOOS == "linux" && runtime.GOARCH == "arm64") || (runtime.GOOS == "linux" && runtime.GOARCH == "ppc64le") || (runtime.GOOS == "linux" && runtime.GOARCH == "riscv64"),


There is a comment above explaining why we disable async preempt, if it really needs to be disable on riscv64 it should be updated to describe why.

Some tests failed like the TestTraceDirRecursion, the symptom which seems to cause by async preempt but disable it doesn't work just like I said, so I'll enable it, until we find the exact reason.

Most of tests were passed, the few failed tests seems to be related with riscv64 go compiler itself, for example, the strange repetition in TestTraceDirRecursion, just like this:

> goroutine(1): main.A(5, 5) > goroutine(1): main.A(4, 4) > goroutine(1): main.A(4, 4) > goroutine(1): main.A(3, 3) > goroutine(1): main.A(2, 2) > goroutine(1): main.A(2, 2) > goroutine(1): main.A(1, 1) > goroutine(1): main.A(1, 1) >> goroutine(1): main.A => (1) >> goroutine(1): main.A => (2) >> goroutine(1): main.A => (6) >> goroutine(1): main.A => (24) >> goroutine(1): main.A => (120) 120

I tried to disable the asyncpreemt, but unfortunately, It doesn't work.

aarzilli · 2024-09-12T08:25:53Z

pkg/proc/native/support_sentinel_linux.go

@@ -1,4 +1,4 @@
-//go:build linux && !amd64 && !arm64 && !386 && !(ppc64le && exp.linuxppc64le)
+//go:build linux && !amd64 && !arm64 && !386 && !(ppc64le && exp.linuxppc64le) && !riscv64


That could be done. The instructions to add a teamcity agent are here: https://www.jetbrains.com/help/teamcity/install-teamcity-agent.html.

aarzilli · 2024-09-12T08:28:57Z

pkg/proc/riscv64_arch.go

+		return false
+	}
+	switch it.frame.Current.Fn.Name {
+	case "runtime.cgocallback_gofunc", "runtime.cgocallback":


I meant all of it: runtime.asmcgocall and crosscall2 are the same thing as is the switch at the bottom of this function.

lrzlin · 2024-10-08T17:38:53Z

@aarzilli Hi, the CI machine is ready and I've modified the .teamcity/settings.kts file. Also, the teamcity agent is now running on this machine.
It was configured as serverUrl=https://delve.teamcity.com/ in conf/buildAgent.properties, which didn't specified the server port (like serverUrl=https://buildserver.mydomain.com:8111 in the example) and the log says it needs to be authorized by administrator, is that okay? or I still need to specify ownPort? I could not see any difference in delve's TeamCity web client as a guest.

[2024-10-09 01:22:03,525]   INFO - buildServer.AGENT.registration - Registering on server via URL "https://delve.teamcity.com": AgentDetails{Name='Linux-openEuler24.09-riscv64', AgentId=null, BuildId=null, AgentOwnAddress='null', AlternativeAddresses=[], Port=9090, Version='160695', PluginsVersion='160695-md5-38792efbad1c0fa39ca4698fc8452484', AvailableRunners=[Ant, cargo-deploy-runner, csharpScript, DockerCommand, dotcover, dotnet, dotnet-tools-dupfinder, dotnet-tools-inspectcode, Duplicator, ftp-deploy-runner, gradle-runner, Inspection, jetbrains_powershell, JPS, kotlinScript, Maven2, nodejs-runner, nunit-console, octopus.generic, octopus.metadata, python-runner, Qodana, rake-runner, SBT, simpleRunner, smb-deploy-runner, ssh-deploy-runner, ssh-exec-runner, unity, unreal-engine], AvailableVcs=[tfs, jetbrains.git, mercurial, svn, perforce], AuthorizationToken='', PingCode='zP69uz6e1BIhS305wqaspLKxwexNPChz'}
[2024-10-09 01:22:07,082]   INFO - buildServer.AGENT.registration - Server supports the following communication protocols: [polling]
[2024-10-09 01:22:07,084]   INFO - buildServer.AGENT.registration - Trying to register on server using 'polling' protocol.
[2024-10-09 01:22:08,414]   INFO - ldServer.AGENT.PollingProtocol - Start polling server for commands
[2024-10-09 01:22:08,417]   INFO - buildServer.AGENT.registration - Registered on server with id 11420 and authorization token '3fe84046c543a557327f2ca421ae6763'
[2024-10-09 01:22:08,418]   INFO - buildServer.AGENT.registration - If this is the first time this agent registered on the server make sure it is authorized by administrator in the server web UI.
[2024-10-09 01:22:10,463]   INFO -    jetbrains.buildServer.AGENT - Updating agent parameters on the server: AgentDetails{Name='Linux-openEuler24.09-riscv64', AgentId=11420, BuildId=null, AgentOwnAddress='null', AlternativeAddresses=[], Port=9090, Version='160695', PluginsVersion='160695-md5-38792efbad1c0fa39ca4698fc8452484', AvailableRunners=[Ant, cargo-deploy-runner, csharpScript, DockerCommand, dotcover, dotnet, dotnet-tools-dupfinder, dotnet-tools-inspectcode, Duplicator, ftp-deploy-runner, gradle-runner, Inspection, jetbrains_powershell, JPS, kotlinScript, Maven2, nodejs-runner, nunit-console, octopus.generic, octopus.metadata, python-runner, Qodana, rake-runner, SBT, simpleRunner, smb-deploy-runner, ssh-deploy-runner, ssh-exec-runner, unity, unreal-engine], AvailableVcs=[tfs, jetbrains.git, mercurial, svn, perforce], AuthorizationToken='3fe84046c543a557327f2ca421ae6763', PingCode='zP69uz6e1BIhS305wqaspLKxwexNPChz'}

BTW, this machine runs openEuler 24.09, is it has to be Ubuntu or other distro is just fine? Thanks.

aarzilli

The agent was approved.

PS. note that the changes to the teamcity configuration will not take effect until we merge this PR.

pkg/dwarf/regnum/riscv64.go

pkg/proc/bininfo.go

pkg/proc/riscv64_disasm.go

aarzilli · 2024-10-11T12:16:25Z

There are merge conflicts with go.mod and vendor/modules.txt, aside from that I think everything is fine.

lrzlin · 2024-10-11T14:10:30Z

There are merge conflicts with go.mod and vendor/modules.txt, aside from that I think everything is fine.

Resolved, now everything should be okay.

aarzilli

LGTM

derekparker

LGTM

derekparker · 2024-10-11T19:34:39Z

Thanks for the contribution!

lrzlin mentioned this pull request Jul 24, 2024

delve Failed to build in RISC-V architecture #3437

Closed

aarzilli reviewed Jul 25, 2024

View reviewed changes

lrzlin force-pushed the delve-riscv64 branch 3 times, most recently from 58b3942 to b32dcc0 Compare July 29, 2024 01:16

lrzlin force-pushed the delve-riscv64 branch from b32dcc0 to d56368d Compare September 11, 2024 19:56

aarzilli reviewed Sep 12, 2024

View reviewed changes

lrzlin force-pushed the delve-riscv64 branch from d56368d to c330f5a Compare September 12, 2024 14:54

lrzlin force-pushed the delve-riscv64 branch from c330f5a to fbbe99d Compare October 8, 2024 16:51

aarzilli reviewed Oct 10, 2024

View reviewed changes

pkg/dwarf/regnum/riscv64.go Outdated Show resolved Hide resolved

pkg/dwarf/regnum/riscv64.go Outdated Show resolved Hide resolved

pkg/proc/bininfo.go Outdated Show resolved Hide resolved

pkg/proc/riscv64_disasm.go Outdated Show resolved Hide resolved

lrzlin force-pushed the delve-riscv64 branch from fbbe99d to b1c4eb0 Compare October 10, 2024 17:36

delve: add linux-riscv64 support

a5d4a23

lrzlin force-pushed the delve-riscv64 branch from b1c4eb0 to a5d4a23 Compare October 11, 2024 14:09

aarzilli approved these changes Oct 11, 2024

View reviewed changes

derekparker approved these changes Oct 11, 2024

View reviewed changes

derekparker merged commit 75c41f2 into go-delve:master Oct 11, 2024
2 checks passed

lrzlin mentioned this pull request Oct 30, 2024

How to add a new builder #3843

Closed

		@@ -1,4 +1,4 @@
		//go:build linux && !amd64 && !arm64 && !386 && !(ppc64le && exp.linuxppc64le)
		//go:build linux && !amd64 && !arm64 && !386 && !(ppc64le && exp.linuxppc64le) && !riscv64

delve: add linux-riscv64 support #3785

delve: add linux-riscv64 support #3785

Conversation

lrzlin commented Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrzlin Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrzlin Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrzlin commented Jul 31, 2024

aarzilli commented Aug 6, 2024

lrzlin commented Sep 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrzlin commented Oct 8, 2024 • edited Loading

aarzilli left a comment • edited Loading

Choose a reason for hiding this comment

aarzilli commented Oct 11, 2024

lrzlin commented Oct 11, 2024

aarzilli left a comment

Choose a reason for hiding this comment

derekparker left a comment

Choose a reason for hiding this comment

derekparker commented Oct 11, 2024

lrzlin commented Jul 24, 2024 •

edited

Loading

lrzlin Jul 25, 2024 •

edited

Loading

lrzlin Sep 12, 2024 •

edited

Loading

lrzlin commented Oct 8, 2024 •

edited

Loading

aarzilli left a comment •

edited

Loading