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,runtime: race support for riscv64 #64345

Open
mauri870 opened this issue Nov 22, 2023 · 9 comments
Open

cmd/compile,runtime: race support for riscv64 #64345

mauri870 opened this issue Nov 22, 2023 · 9 comments
Labels
arch-riscv Issues solely affecting the riscv64 architecture. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. RaceDetector
Milestone

Comments

@mauri870
Copy link
Member

mauri870 commented Nov 22, 2023

There was a previous issue requesting support for riscv64 (#58739), but it was closed because upstream support in LLVM was lacking.

On Oct 6 support for riscv64 was finally merged into the LLVM tree.

I propose we reconsider adding race detector support to Risc-V. From the top of my head this would require:

  • Update the compiler to recognize -race on linux/riscv64
  • clone llvm and compile a race_linux_riscv64.syso file, push the necessary changes upstream
  • linux-riscv64-race builder?
  • Add a new platform to x/build/cmd/racebuild to build syso files for riscv64
  • Add race instrumentation (race_riscv64.s) to runtime
  • Document support on https://go.dev/doc/articles/race_detector
  • freebsd/riscv64 support is possible in the future?

Happy to work on this for the early stage of Go 1.23.

cc @golang/runtime @golang/riscv64

@mauri870 mauri870 added RaceDetector NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. arch-riscv Issues solely affecting the riscv64 architecture. labels Nov 22, 2023
@mauri870 mauri870 added this to the Proposal milestone Nov 22, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 22, 2023
@mauri870 mauri870 removed this from the Proposal milestone Nov 22, 2023
@mengzhuo
Copy link
Contributor

I can setup a linux-riscv64-race builder(LUCI) for you.

@mauri870
Copy link
Member Author

Thanks @mengzhuo, appreciate if you could handle that 🙏

@bcmills bcmills added this to the Backlog milestone Nov 27, 2023
@mauri870
Copy link
Member Author

@mengzhuo I tried compiling the syso file for riscv64 with CC=clang and got stuck in this part:

runtime/race(.rodata): unexpected relocation type 291 (R_RISCV_ADD32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LBB438_37 (type=291 (R_RISCV_ADD32) stype=1 (STEXT))
runtime/race(.rodata): unexpected relocation type 295 (R_RISCV_SUB32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LJTI438_0 (type=295 (R_RISCV_SUB32) stype=9 (SRODATA))runtime/race(.rodata): unexpected relocation type 291 (R_RISCV_ADD32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LBB438_169 (type=291 (R_RISCV_ADD32) stype=1 (STEXT))
runtime/race(.rodata): unexpected relocation type 295 (R_RISCV_SUB32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LJTI438_0 (type=295 (R_RISCV_SUB32) stype=9 (SRODATA))runtime/race(.rodata): unexpected relocation type 291 (R_RISCV_ADD32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LBB438_169 (type=291 (R_RISCV_ADD32) stype=1 (STEXT))
runtime/race(.rodata): unexpected relocation type 295 (R_RISCV_SUB32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LJTI438_0 (type=295 (R_RISCV_SUB32) stype=9 (SRODATA))runtime/race(.rodata): unexpected relocation type 291 (R_RISCV_ADD32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LBB438_169 (type=291 (R_RISCV_ADD32) stype=1 (STEXT))
runtime/race(.rodata): unexpected relocation type 295 (R_RISCV_SUB32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LJTI438_0 (type=295 (R_RISCV_SUB32) stype=9 (SRODATA))runtime/race(.rodata): unexpected relocation type 291 (R_RISCV_ADD32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LBB438_169 (type=291 (R_RISCV_ADD32) stype=1 (STEXT))
runtime/race(.rodata): unexpected relocation type 295 (R_RISCV_SUB32)
runtime/race(.rodata): unsupported dynamic relocation for symbol .LJTI438_0 (type=295 (R_RISCV_SUB32) stype=9 (SRODATA))runtime/race(.rodata): unexpected relocation type 291 (R_RISCV_ADD32)
/home/ubuntu/git/go/pkg/tool/linux_riscv64/link: too many errors

Maybe I have to use some specific compiler flag? I see that these relocations are currently supported in the go linker

@mengzhuo
Copy link
Contributor

mengzhuo commented Jan 9, 2024

CC @4a6f656c
@mauri870 builder is up, I had same issue like yours.

@4a6f656c
Copy link
Contributor

@mauri870 that looks like the riscv64 linker is missing handling for some relocation types - do you have a branch handy that I can build/test from?

@mauri870
Copy link
Member Author

mauri870 commented Apr 30, 2024

@4a6f656c I have some changes in one of my branches, hopefully you can still build from it, if not then let me know:

https://github.com/golang/go/compare/master...mauri870:go:feature/race-riscv64?expand=1

Edit: If you wanna compile the syso file yourself, then you have to get a hold of a llvm copy and then cd compiler-rt/lib/tsan/go && CC=clang ./buildgo.sh

@4a6f656c
Copy link
Contributor

@mauri870 I've added the missing relocation handling to the internal linker and fixed up some other aspects of the riscv64 race code:

https://github.com/4a6f656c/go/tree/riscv64-race

We can now call into the TSAN code, however it fails on the call to __tsan_map_shadow with:

==2568052==ERROR: ThreadSanitizer failed to allocate 0x1f10000 (32571392) bytes at address 200000660000 (errno: 12)

At a first guess, this looks like it is using the 48-bit VMA mapping even though it is running on a 39-bit VMA platform.

@mauri870
Copy link
Member Author

@4a6f656c Error 12 is ENOMEM, perhaps you are running out of memory on this machine?

https://github.com/torvalds/linux/blob/6f52b16c5b29b89d92c0e7236f4655dc8491ad70/include/uapi/asm-generic/errno-base.h#L16

Looking at the riscv64 tsan commit in llvm the 1000 0000 00 - 4000 0000 00 range seems to be correct for shadow memory on a 39-bit VMA.

@4a6f656c
Copy link
Contributor

4a6f656c commented Dec 6, 2024

@4a6f656c Error 12 is ENOMEM, perhaps you are running out of memory on this machine?

https://github.com/torvalds/linux/blob/6f52b16c5b29b89d92c0e7236f4655dc8491ad70/include/uapi/asm-generic/errno-base.h#L16

Looking at the riscv64 tsan commit in llvm the 1000 0000 00 - 4000 0000 00 range seems to be correct for shadow memory on a 39-bit VMA.

The issue is caused by the fact that there is no Go mapping configuration for riscv64, which means we're falling through to the default.

I've pushed some further fixes to https://github.com/4a6f656c/go/tree/riscv64-race which makes the Go side of things correct, however even with a Go riscv64 mapping added to llvm, I've not been able to get it to detect races (the Go code is instrumented correctly and calls into the race detector code). The next step is to figure out the correct TSAN mappings for riscv64 in VMA 39 and VMA 48 modes (and we'll likely need to adjust Go's runtime/malloc.go to match).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-riscv Issues solely affecting the riscv64 architecture. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. RaceDetector
Projects
Development

No branches or pull requests

5 participants