Bug 468575

Created attachment 158147 [details]
v1-0001-riscv64-valgrind-new.patch

Created attachment 158148 [details]
v1-0002-riscv64-vex-new.patch

Created attachment 158149 [details]
v1-0003-riscv64-tests-new.patch

Created attachment 158150 [details]
v1-0004-riscv64-valgrind-mod.patch

Created attachment 158151 [details]
v1-0005-riscv64-vex-mod.patch

Created attachment 158152 [details]
v1-0006-riscv64-tests-mod.patch

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)

hginfo fails on several systems
TLS is, well, tricky

Are the other failures "reaonable"?

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

(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 :)

@Paul Floyd

Has anyone reviewed these patches ?

It has taken two months and there is not any feedback :)

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

@Paul Floyd

Has anyone reviewed these patches ?

It has taken long time and there is not any feedback :)

Could we review & push these basic ISAs (rv64gc) implementation first ?
other of extensions like Vector of RISCV depend on this, that will be the next steps :)

I built and ran regtest on my github clone of https://github.com/petrpavlu/valgrind-riscv64 on FreeBSD amd64.

There were a couple of unexpected failures in memcheck/tests/freebsd (one of the scalars and eventfd2). I'll look into them and also try some more platforms.

(In reply to Paul Floyd from comment #14)
> I built and ran regtest on my github clone of
> https://github.com/petrpavlu/valgrind-riscv64 on FreeBSD amd64.
> 
> There were a couple of unexpected failures in memcheck/tests/freebsd (one of
> the scalars and eventfd2). I'll look into them and also try some more
> platforms.

Good news, thanks for your efforts, hope that will been fixed soon :)

I think it's a regression that I added. Should be fixed. I'll check again when Petr rebases next, and I'll also give Linux amd64 a go and see if I can build and run regtest on riscv.

On amd64 Linux, one warning that I've opened an issue for on Petr's github.

A few other warnings about pointer-to-int-casts.

regtest clean.

On a machine that is StarFive JH7100 SoC (2x SiFive U74 at 1.20 GHz)

I have a problem with make check.

gcc -Winline -Wall -Wshadow -Wno-long-long -g -fno-stack-protector       -o compressed compressed.o  
compressed.o: in function `test_compressed_10':
/home/paulf/valgrind-riscv64/none/tests/riscv64/compressed.c:347:(.text+0x56d16): dangerous relocation: The addend isn't allowed for R_RISCV_GOT_HI20
collect2: error: ld returned 1 exit status
make[5]: *** [Makefile:741: compressed] Error 1
make[5]: Leaving directory '/home/paulf/valgrind-riscv64/none/tests/riscv64'
make[4]: *** [Makefile:899: check-am] Error 2
make[4]: Leaving directory '/home/paulf/valgrind-riscv64/none/tests/riscv64'
make[3]: *** [Makefile:2270: check-recursive] Error 1
make[3]: Leaving directory '/home/paulf/valgrind-riscv64/none/tests'
make[2]: *** [Makefile:1101: check-recursive] Error 1
make[2]: Leaving directory '/home/paulf/valgrind-riscv64/none'
make[1]: *** [Makefile:926: check-recursive] Error 1
make[1]: Leaving directory '/home/paulf/valgrind-riscv64'
make: *** [Makefile:1225: check] Error 2

Maybe a GCC problem?

gcc --version
gcc (Debian 13.2.0-13) 13.2.0

(In reply to Paul Floyd from comment #18)
> On a machine that is StarFive JH7100 SoC (2x SiFive U74 at 1.20 GHz)
> 
> I have a problem with make check.
> 
> gcc -Winline -Wall -Wshadow -Wno-long-long -g -fno-stack-protector       -o
> compressed compressed.o  
> compressed.o: in function `test_compressed_10':
> /home/paulf/valgrind-riscv64/none/tests/riscv64/compressed.c:347:(.
> text+0x56d16): dangerous relocation: The addend isn't allowed for
> R_RISCV_GOT_HI20
> collect2: error: ld returned 1 exit status
> make[5]: *** [Makefile:741: compressed] Error 1
> make[5]: Leaving directory '/home/paulf/valgrind-riscv64/none/tests/riscv64'
> make[4]: *** [Makefile:899: check-am] Error 2
> make[4]: Leaving directory '/home/paulf/valgrind-riscv64/none/tests/riscv64'
> make[3]: *** [Makefile:2270: check-recursive] Error 1
> make[3]: Leaving directory '/home/paulf/valgrind-riscv64/none/tests'
> make[2]: *** [Makefile:1101: check-recursive] Error 1
> make[2]: Leaving directory '/home/paulf/valgrind-riscv64/none'
> make[1]: *** [Makefile:926: check-recursive] Error 1
> make[1]: Leaving directory '/home/paulf/valgrind-riscv64'
> make: *** [Makefile:1225: check] Error 2
> 
> Maybe a GCC problem?
> 
> gcc --version
> gcc (Debian 13.2.0-13) 13.2.0

Yes, looks ld met some bugs :(
and I suggest you can get the latest toolchain from here:
https://github.com/riscv-collab/riscv-gnu-toolchain/releases/

And regtest results

== 737 tests, 6 stderr failures, 2 stdout failures, 0 stderrB failures, 1 stdoutB failure, 0 post failures ==
gdbserver_tests/hgtls                    (stdoutB)
memcheck/tests/pointer-trace             (stderr)
none/tests/double_close_range            (stderr)
none/tests/double_close_range_sup        (stderr)
none/tests/double_close_range_xml        (stderr)
none/tests/riscv64/compressed            (stdout)
none/tests/riscv64/compressed            (stderr)
none/tests/riscv64/integer               (stdout)
none/tests/riscv64/integer               (stderr)

I've seen memcheck/tests/pointer-trace issues before where the working directory is on NFS. Not riscv related.

The double_close_range tests are fairly new and may need some extra filtering.

The two riscv64 fails look like a  problem with the toolchain. I'll see if trying a local build of the toolchain helps.

(In reply to Paul Floyd from comment #20)
> And regtest results
> 
> == 737 tests, 6 stderr failures, 2 stdout failures, 0 stderrB failures, 1
> stdoutB failure, 0 post failures ==
> gdbserver_tests/hgtls                    (stdoutB)

The hgtls test fails because GDB is confused by missing DW_AT_location attributes for TLS variables. It looks to be a problem with the RISC-V target in GCC not implementing TARGET_ASM_OUTPUT_DWARF_DTPREL. I have a preliminary patch for GCC which adds it.

> [...]
> none/tests/riscv64/compressed            (stdout)
> none/tests/riscv64/compressed            (stderr)
> none/tests/riscv64/integer               (stdout)
> none/tests/riscv64/integer               (stderr)
> 
> [...]
> The two riscv64 fails look like a  problem with the toolchain. I'll see if
> trying a local build of the toolchain helps.

The mentioned compilation failure of RISC-V tests is some problem related to them being compiled as position-independent, which is apparently the default on Debian. It is possible for me to reproduce it locally. I'll have a closer look.

> > [...]
> > none/tests/riscv64/compressed            (stdout)
> > none/tests/riscv64/compressed            (stderr)
> > none/tests/riscv64/integer               (stdout)
> > none/tests/riscv64/integer               (stderr)
> > 
> > [...]
> > The two riscv64 fails look like a  problem with the toolchain. I'll see if
> > trying a local build of the toolchain helps.
> 
> The mentioned compilation failure of RISC-V tests is some problem related to
> them being compiled as position-independent, which is apparently the default
> on Debian. It is possible for me to reproduce it locally. I'll have a closer
> look.

What's the details about the error output ?
I can help to have a look if that is obvious :)
BTW, it didn't happen in my environment.

Hi, Paul Floyd & Petr Pavlu

Any progress ? 
It's depression if there are a few small issues :(

(In reply to JojoR from comment #23)
> Hi, Paul Floyd & Petr Pavlu
> 
> Any progress ? 
> It's depression if there are a few small issues :(

There are two things that I consider as requirements.

Firstly someone to get git write access to maintain this.

Secondly all the usual code requirements (review for such a big change, all regressions passing on all platforms).

https://builder.sourceware.org/ now has several riscv builders. We should setup one.
We are already testing armhf, i386, ppc64, arm64, ppc64le, s390x, x86_64 and power9/10.
https://builder.sourceware.org/buildbot/#/builders?tags=valgrind