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

delve: add linux-riscv64 support

c330f5a

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

delve: add linux-riscv64 support #3785

delve: add linux-riscv64 support #3785

lrzlin commented Jul 24, 2024 •

edited

Loading

aarzilli Jul 25, 2024

lrzlin Jul 25, 2024

lrzlin Sep 11, 2024

aarzilli Jul 25, 2024

lrzlin Jul 25, 2024 •

edited

Loading

aarzilli Sep 12, 2024

lrzlin Sep 12, 2024 •

edited

Loading

aarzilli Sep 13, 2024

aarzilli Jul 25, 2024

lrzlin Jul 25, 2024

lrzlin Jul 25, 2024

aarzilli Jul 25, 2024

lrzlin Jul 25, 2024

aarzilli Sep 12, 2024

lrzlin Sep 12, 2024

lrzlin commented Jul 31, 2024

aarzilli commented Aug 6, 2024

lrzlin commented Sep 11, 2024

aarzilli Sep 12, 2024

lrzlin Sep 12, 2024

aarzilli Sep 12, 2024

lrzlin Sep 12, 2024

aarzilli Sep 12, 2024

lrzlin Sep 12, 2024

aarzilli Sep 12, 2024

lrzlin Sep 12, 2024

aarzilli Sep 12, 2024

aarzilli Sep 12, 2024

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

Are you sure you want to change the base?

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 Jul 24, 2024 •

edited

Loading

lrzlin Jul 25, 2024 •

edited

Loading

lrzlin Sep 12, 2024 •

edited

Loading