Bug 468575 - Add support for RISC-V
Summary: Add support for RISC-V
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-16 14:25 UTC by Petr Pavlu
Modified: 2023-10-08 03:14 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
v1-0001-riscv64-valgrind-new.patch (128.52 KB, patch)
2023-04-16 14:26 UTC, Petr Pavlu
Details
v1-0002-riscv64-vex-new.patch (360.47 KB, patch)
2023-04-16 14:27 UTC, Petr Pavlu
Details
v1-0003-riscv64-tests-new.patch (663.01 KB, patch)
2023-04-16 14:27 UTC, Petr Pavlu
Details
v1-0004-riscv64-valgrind-mod.patch (143.12 KB, patch)
2023-04-16 14:27 UTC, Petr Pavlu
Details
v1-0005-riscv64-vex-mod.patch (17.19 KB, patch)
2023-04-16 14:27 UTC, Petr Pavlu
Details
v1-0006-riscv64-tests-mod.patch (19.92 KB, patch)
2023-04-16 14:28 UTC, Petr Pavlu
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Pavlu 2023-04-16 14:25:50 UTC
RISC-V [1, 2] is a modern open-standard architecture which has seen increasing adoption lately.

I and a few other contributors have been working on adding support for this architecture in Valgrind, using a downstream repository [3]. The port has reached a state where I think it has a good quality to be added to the official Valgrind project and continued to be developed there.

The port adds supports for the 64-bit RISC-V architecture, specifically for the RV64GC instruction set which is what most Linux distributions currently target. Support for further extensions is expected to be incrementally implemented in the future, as more extensions get defined by architecture stewards and make it into hardware.

The work is cut into 6 patches. The first three add new files specific only to the port and they are split for Valgrind (Coregrind+tools), VEX and tests. The last three modify already present files and are split in the same way.

Implementation mostly tries to stay consistent with arm64 and mips64. No new Iops are currently added by the port but some new ones might be needed in the future. For instance, support for the Zfh extension (half-precision floating-point) will likely require a few additional Iops, although these should be needed for the complete Armv8.2-A FP16 support too.

The port passes almost completely the Valgrind test suite. A few tests still currently fail and need to be investigated.

The port gained some reasonable traction and a few other people contributed to it in the last year by implementing missing bits and fixing various bugs.

[1] https://riscv.org/
[2] https://en.wikipedia.org/wiki/RISC-V
[3] https://github.com/petrpavlu/valgrind-riscv64
Comment 1 Petr Pavlu 2023-04-16 14:26:44 UTC
Created attachment 158147 [details]
v1-0001-riscv64-valgrind-new.patch
Comment 2 Petr Pavlu 2023-04-16 14:27:03 UTC
Created attachment 158148 [details]
v1-0002-riscv64-vex-new.patch
Comment 3 Petr Pavlu 2023-04-16 14:27:26 UTC
Created attachment 158149 [details]
v1-0003-riscv64-tests-new.patch
Comment 4 Petr Pavlu 2023-04-16 14:27:39 UTC
Created attachment 158150 [details]
v1-0004-riscv64-valgrind-mod.patch
Comment 5 Petr Pavlu 2023-04-16 14:27:53 UTC
Created attachment 158151 [details]
v1-0005-riscv64-vex-mod.patch
Comment 6 Petr Pavlu 2023-04-16 14:28:05 UTC
Created attachment 158152 [details]
v1-0006-riscv64-tests-mod.patch
Comment 7 Petr Pavlu 2023-04-16 14:30:47 UTC
v1:
* attachment 158147 [details] v1-0001-riscv64-valgrind-new.patch,
* attachment 158148 [details] v1-0002-riscv64-vex-new.patch,
* attachment 158149 [details] v1-0003-riscv64-tests-new.patch,
* attachment 158150 [details] v1-0004-riscv64-valgrind-mod.patch,
* attachment 158151 [details] v1-0005-riscv64-vex-mod.patch,
* attachment 158152 [details] v1-0006-riscv64-tests-mod.patch.

Based on ab6d3928a ("regtest: warning cleanup").

Test suite results:
> == 675 tests, 4 stderr failures, 2 stdout failures, 1 stderrB failure, 1 stdoutB failure, 0 post failures ==
> gdbserver_tests/hginfo                   (stderrB)
> gdbserver_tests/hgtls                    (stdoutB)
> memcheck/tests/cdebug_zlib_gnu           (stderr)
> memcheck/tests/linux/stack_changes       (stdout)
> memcheck/tests/linux/stack_changes       (stderr)
> memcheck/tests/pointer-trace             (stderr)
> memcheck/tests/sh-mem-random             (stdout)
> memcheck/tests/sh-mem-random             (stderr)
Comment 8 Paul Floyd 2023-04-17 08:27:15 UTC
hginfo fails on several systems
TLS is, well, tricky

Are the other failures "reaonable"?
Comment 9 Petr Pavlu 2023-04-17 20:03:19 UTC
(In reply to Paul Floyd from comment #8)
> hginfo fails on several systems
> TLS is, well, tricky
> 
> Are the other failures "reaonable"?

The following is a short summary why the remaining tests fail. However, I'm not sure yet for any of them what is their underlying problem and still need to do proper investigation.

> gdbserver_tests/hginfo                   (stderrB)

The output has information about an extra lock inside _rtld_local. Likely a duplicate of bug 444487.

> gdbserver_tests/hgtls                    (stdoutB)

Fails because GDB is not able to determine an address of the variable 'local'. The test used to pass on my system, possibly something compiler-related.

> memcheck/tests/cdebug_zlib_gnu           (stderr)

Produced backtrace contains rubbish 'at 0x........: main (ng long int:3)' instead of 'at 0x........: main (cdebug.c:3)'.

> memcheck/tests/linux/stack_changes       (stdout)
> memcheck/tests/linux/stack_changes       (stderr)

Lots of unexpected invalid writes reported from hello() -> printf().

> memcheck/tests/pointer-trace             (stderr)

The second mmap() call in the test unexpectedly fails with -EINVAL.

> memcheck/tests/sh-mem-random             (stdout)
> memcheck/tests/sh-mem-random             (stderr)

Another unexpected mmap() failure.
Comment 10 JojoR 2023-04-20 01:42:13 UTC
(In reply to Petr Pavlu from comment #9)
> (In reply to Paul Floyd from comment #8)
> > hginfo fails on several systems
> > TLS is, well, tricky
> > 
> > Are the other failures "reaonable"?
> 
> > memcheck/tests/sh-mem-random             (stdout)
> > memcheck/tests/sh-mem-random             (stderr)
> 
> Another unexpected mmap() failure.

From our investigation, high address mmap is not supported by some riscv board/os if it support feature "Sv39" only,
so it is reasonable also :)
Comment 11 JojoR 2023-06-25 08:51:13 UTC
@Paul Floyd

Has anyone reviewed these patches ?

It has taken two months and there is not any feedback :)
Comment 12 laokz 2023-07-30 14:59:07 UTC
> > memcheck/tests/pointer-trace             (stderr)
> The second mmap() call in the test unexpectedly fails with -EINVAL.
The second mmap() is called with PROT_WRITE only. I'm pretty sure the result is 'resonable' because it is KERNEL relevant.

From https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8aeb7b17f04ef40f620c763502e2b644c5c73efd , it shows that kernel would return EINVAL for mmap(PROT_WRITE only) if without this commit, or the call would be treated as with an implied PROT_READ like other architectures.

I tested on openEuler 22.03 with/without this commit and got expected success/failure result.

>> with the commit
> # perl tests/vg_regtest memcheck/tests/pointer-trace.vgtest
> pointer-trace:   valgrind   -q --leak-check=yes ./pointer-trace
> 
> == 1 test, 0 stderr failures, 0 stdout failures, 0 stderrB failures, 0 stdoutB failures, 0 post failures ==

>> without the commit
> # perl tests/vg_regtest memcheck/tests/pointer-trace.vgtest
> pointer-trace:   valgrind   -q --leak-check=yes ./pointer-trace
> *** pointer-trace failed (stderr) ***
> 
> == 1 test, 1 stderr failure, 0 stdout failures, 0 stderrB failures, 0 stdoutB failures, 0 post failures ==
> memcheck/tests/pointer-trace             (stderr)
> 
> # cat memcheck/tests/pointer-trace.stderr.out
> trap 2 failed: Invalid argument
> 1,000 bytes in 1 blocks are definitely lost in loss record ... of ...
>    at 0x........: malloc (vg_replace_malloc.c:...)
>    by 0x........: main (pointer-trace.c:86)