arch/riscv64: Update argument handling feature to support riscv64 #1824

gichoel · 2023-09-05T14:07:28Z

This pull request is a porting effort to support Argument Handling in the RISC-V 64-bit architecture environment.

This feature has been tested on the VisionFive2 RISC-V board.

The following features are not yet supported

PLT hooking (this feature is in progress)
Dynamic tracing

A list of PRs related to porting to the RISC-V 64-bit architecture is provided below.

arch/riscv64: Add basic mcount tracing support for RISC-V 64bit #1815

This PR was born out of this issue.

Add RISC-V architecture support #1503

gichoel · 2023-09-05T14:42:33Z

We have not yet resolved the issues identified in the previous PR.

arch/riscv64: Add basic mcount tracing support for RISC-V 64bit #1815 (review)

And I got the same error message as below.

$ make DEBUG=1
$ ./uftrace --libmcount-path=. -a t-arg

/home/gichoel/workdir/uftrace/utils/regs.c:437: arch_register_dwarf_name: ASSERT `arch < ARRAY_SIZE(arch_dwarf_tables)' failed.
Stack trace:
  #1  0x003fad378ca2 stacktrace + 0x34

Please report this bug to https://github.com/namhyung/uftrace/issues.

WARN: child terminated by signal: 5: Trace/breakpoint trap
uftrace: /home/gichoel/workdir/uftrace/utils/data-file.c:430:open_info_file
 ERROR: cannot read uftrace header info!

gichoel · 2023-09-05T15:58:50Z

The following error when building with make DEBUG=1 has been resolved.

$ make DEBUG=1
$ ./uftrace --libmcount-path=. -a t-arg

/home/gichoel/workdir/uftrace/utils/regs.c:437: arch_register_dwarf_name: ASSERT `arch < ARRAY_SIZE(arch_dwarf_tables)' failed.
Stack trace:
  #1  0x003fad378ca2 stacktrace + 0x34

Please report this bug to https://github.com/namhyung/uftrace/issues.

WARN: child terminated by signal: 5: Trace/breakpoint trap

However, one more error remains, and it occurs at runtime

$ ./uftrace --libmcount-path=. -a t-arg

uftrace: /home/gichoel/workdir/uftrace/utils/data-file.c:430:open_info_file
 ERROR: cannot read uftrace header info!

gichoel · 2023-09-06T14:55:11Z

I solved the error below, please see the link below for details.

arch/riscv64: Add basic mcount tracing support for RISC-V 64bit #1815 (comment)

$ ./uftrace --libmcount-path=. -a t-arg

uftrace: /home/gichoel/workdir/uftrace/utils/data-file.c:430:open_info_file
 ERROR: cannot read uftrace header info!

And Finally, we can finally check string arguments in a RISC-V environment!

$ ./uftrace --libmcount-path=. -a t-arg
# duration tid function
            [358775] | main(1, 0x3ff863a478) {
            [358775] | foo(3) {
 329.506 us [358775] | bar(2, "c") = 0;
   1.750 us [358775] | bar(1, "b") = 0;
   1.500 us [358775] | bar(0, "a") = 0;
 345.756 us [358775] | } = 0; /* foo */
   1.500 us [358775] | many(12) = 0;
            [358775] | pass(3) {
   3.000 us [358775] | check(big{...}, 0x3ff863a230) = 0;
   8.500 us [358775] | } = 0; /* pass */
 368.256 us [358775] | } = 0; /* main */

coyotek · 2023-09-07T01:32:57Z

I see it's fixed.

$ ./uftrace -a ./t-arg 
# DURATION     TID     FUNCTION
            [364301] | main(1, 0x3fcd72d448) {
            [364301] |   foo(3) {
 264.254 us [364301] |     bar(2, "c") = 0;
   1.250 us [364301] |     bar(1, "b") = 0;
   0.750 us [364301] |     bar(0, "a") = 0;
 273.254 us [364301] |   } = 0; /* foo */
   1.250 us [364301] |   many(12) = 0;
            [364301] |   pass(3) {
   1.501 us [364301] |     check(big{...}, 0x3fcd72d200) = 0;
   4.501 us [364301] |   } = 0; /* pass */
 286.255 us [364301] | } = 0; /* main */

namhyung · 2023-09-07T04:31:00Z

cmds/info.c

@@ -281,6 +281,9 @@ static int read_cpuinfo(void *arg)
 			else if (!strncmp(info->cpudesc, "ARM64", 5)) {
 				handle->arch = UFT_CPU_AARCH64;
 			}
+			else if (!strncmp(info->cpudesc, "RISCV64", 7)) {
+				handle->arch = UFT_CPU_RISCV64;
+			}


Looks like this part should be in #1815 .

It's related to the cpuinfo.c code I added in #1815, so I'll move it as per your feedback.

Move this to #1815 based on feedback, but also move utils/arch.h to #1815.
because UFT_CPU_RISCV64 was declared in utils/arch.h in commit 1a4d0ec.

This is a commit to ensure a successful build in a RISC-V 64bit environment, no functional behavior is implemented. Tested-by: Seonghee Jin <[email protected]> Co-authored-by: Honggyu Kim <[email protected]> Signed-off-by: Gichoel Choi <[email protected]>

In order to figure out the address of parent_loc, we need the frame pointer, but compiler optimization such as `-O2` in gcc removes the `fp` so we won't be able know where to change to hijack the return address to `mcount_return`. This problem only happens in gcc, but not in clang. To avoid the problem, `-fno-omit-frame-pointer` must be used when gcc optimization option is used in riscv64. Tested-by: Seonghee Jin <[email protected]> Tested-by: Paran Lee <[email protected]> Co-authored-by: Honggyu Kim <[email protected]> Signed-off-by: Gichoel Choi <[email protected]>

This is work to support argument handling on the RISC-V 64-bit architecture. If you are curious about how RISC-V 64bit register numbers map to DWARF register numbers, please refer to the 'RISC-V Run-time ABI Specification' in 'RISC-V ELF psABI'. Tested-by: Seonghee Jin <[email protected]> Tested-by: Jungmin Kim <[email protected]> Tested-by: SeokMin Kwon <[email protected]> Reviewed-by: Honggyu Kim <[email protected]> Signed-off-by: Gichoel Choi <[email protected]>

MichelleJin12 · 2023-09-13T23:58:40Z

Hello, @gichoel,

Thank you for your amazing contribution :D

Could you update the commit messages like,
title: under 50 characters
body: under 72 characters
according to the 50/72 rule?

Thank you!

namhyung · 2023-09-14T06:25:13Z

Note that 50/72 rule is not hard for us, but please consider it as a general advice.

gichoel · 2023-09-14T07:50:41Z

Hello, @MichelleJin12,

I only remembered the part about the body having to be within 72 characters, but forgot the part about the title having to be within 50 characters.

Thanks for checking!

gichoel · 2023-09-14T07:54:24Z

I will close this PR in the next few days based on the discussion in the link below and then move the commit to "arch/riscv64: Add basic mcount tracing support for RISC-V 64bit PR".

arch/riscv64: Add basic mcount tracing support for RISC-V 64bit #1815 (comment)

If you're interested in argument handling on the RISC-V architecture and want to see what happens next, see #1815.

gichoel mentioned this pull request Sep 5, 2023

arch/riscv64: Add basic mcount tracing support for RISC-V 64bit #1815

Merged

gichoel force-pushed the arch/riscv-support-arg-handling branch 2 times, most recently from dba41c0 to 56fb6e4 Compare September 5, 2023 14:23

gichoel marked this pull request as draft September 5, 2023 14:30

gichoel marked this pull request as ready for review September 5, 2023 14:33

gichoel changed the title ~~arch/riscv64: Update argument handling feature to support riscv64~~ [WIP] arch/riscv64: Update argument handling feature to support riscv64 Sep 5, 2023

gichoel force-pushed the arch/riscv-support-arg-handling branch from 56fb6e4 to 4a38fcf Compare September 5, 2023 15:52

gichoel force-pushed the arch/riscv-support-arg-handling branch from 4a38fcf to 71533d6 Compare September 6, 2023 14:44

gichoel changed the title ~~[WIP] arch/riscv64: Update argument handling feature to support riscv64~~ arch/riscv64: Update argument handling feature to support riscv64 Sep 6, 2023

namhyung reviewed Sep 7, 2023

View reviewed changes

gichoel and others added 3 commits September 10, 2023 00:56

gichoel force-pushed the arch/riscv-support-arg-handling branch from 71533d6 to aad0773 Compare September 9, 2023 16:21

gichoel closed this Sep 15, 2023

gichoel deleted the arch/riscv-support-arg-handling branch October 26, 2023 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

arch/riscv64: Update argument handling feature to support riscv64 #1824

arch/riscv64: Update argument handling feature to support riscv64 #1824

gichoel commented Sep 5, 2023 •

edited

Loading

gichoel commented Sep 5, 2023

gichoel commented Sep 5, 2023

gichoel commented Sep 6, 2023

coyotek commented Sep 7, 2023 •

edited

Loading

namhyung Sep 7, 2023

gichoel Sep 7, 2023

gichoel Sep 9, 2023

MichelleJin12 commented Sep 13, 2023 •

edited

Loading

namhyung commented Sep 14, 2023

gichoel commented Sep 14, 2023

gichoel commented Sep 14, 2023 •

edited

Loading

arch/riscv64: Update argument handling feature to support riscv64 #1824

arch/riscv64: Update argument handling feature to support riscv64 #1824

Conversation

gichoel commented Sep 5, 2023 • edited Loading

gichoel commented Sep 5, 2023

gichoel commented Sep 5, 2023

gichoel commented Sep 6, 2023

coyotek commented Sep 7, 2023 • edited Loading

namhyung Sep 7, 2023

Choose a reason for hiding this comment

gichoel Sep 7, 2023

Choose a reason for hiding this comment

gichoel Sep 9, 2023

Choose a reason for hiding this comment

MichelleJin12 commented Sep 13, 2023 • edited Loading

namhyung commented Sep 14, 2023

gichoel commented Sep 14, 2023

gichoel commented Sep 14, 2023 • edited Loading

gichoel commented Sep 5, 2023 •

edited

Loading

coyotek commented Sep 7, 2023 •

edited

Loading

MichelleJin12 commented Sep 13, 2023 •

edited

Loading

gichoel commented Sep 14, 2023 •

edited

Loading