[lldb][RISC-V] Extended if conditions to support alias names for registers

[kper]

Extending the conditionals in AugmentRegisterInfo to support alternative names for lldb.

Fixes #124023

There is an exception with register X8 which is not covered here but more details can be found in the issue #127900.

[llvmbot]

@llvm/pr-subscribers-testing-tools

@llvm/pr-subscribers-lldb

Author: None (kper)

Changes

Extending the conditionals in AugmentRegisterInfo to support alternative names for lldb.

Fixes #124023

---

Full diff: https://github.com/llvm/llvm-project/pull/124475.diff

1 Files Affected:

• (modified) lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp (+54)

diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
index 8412991933d277..c463bd006b3db4 100644
--- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
+++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
@@ -850,8 +850,62 @@ void ABISysV_riscv::AugmentRegisterInfo(
       it.value().alt_name.SetCString("x3");
     else if (it.value().name == "fp")
       it.value().alt_name.SetCString("s0");
+    else if (it.value().name == "tp")
+      it.value().alt_name.SetCString("x4");
     else if (it.value().name == "s0")
       it.value().alt_name.SetCString("x8");
+    else if (it.value().name == "s1")
+      it.value().alt_name.SetCString("x9");
+    else if (it.value().name == "t0")
+      it.value().alt_name.SetCString("x5");
+    else if (it.value().name == "t1")
+      it.value().alt_name.SetCString("x6");
+    else if (it.value().name == "t2")
+      it.value().alt_name.SetCString("x7");
+    else if (it.value().name == "a0")
+      it.value().alt_name.SetCString("x10");
+    else if (it.value().name == "a1")
+      it.value().alt_name.SetCString("x11");
+    else if (it.value().name == "a2")
+      it.value().alt_name.SetCString("x12");
+    else if (it.value().name == "a3")
+      it.value().alt_name.SetCString("x13");
+    else if (it.value().name == "a4")
+      it.value().alt_name.SetCString("x14");
+    else if (it.value().name == "a5")
+      it.value().alt_name.SetCString("x15");
+    else if (it.value().name == "a6")
+      it.value().alt_name.SetCString("x16");
+    else if (it.value().name == "a7")
+      it.value().alt_name.SetCString("x17");
+    else if (it.value().name == "s2")
+      it.value().alt_name.SetCString("x18");
+    else if (it.value().name == "s3")
+      it.value().alt_name.SetCString("x19");
+    else if (it.value().name == "s4")
+      it.value().alt_name.SetCString("x20");
+    else if (it.value().name == "s5")
+      it.value().alt_name.SetCString("x21");
+    else if (it.value().name == "s6")
+      it.value().alt_name.SetCString("x22");
+    else if (it.value().name == "s7")
+      it.value().alt_name.SetCString("x23");
+    else if (it.value().name == "s8")
+      it.value().alt_name.SetCString("x24");
+    else if (it.value().name == "s9")
+      it.value().alt_name.SetCString("x25");
+    else if (it.value().name == "s10")
+      it.value().alt_name.SetCString("x26");
+    else if (it.value().name == "s11")
+      it.value().alt_name.SetCString("x27");
+    else if (it.value().name == "t3")
+      it.value().alt_name.SetCString("x28");
+    else if (it.value().name == "t4")
+      it.value().alt_name.SetCString("x29");
+    else if (it.value().name == "t5")
+      it.value().alt_name.SetCString("x30");
+    else if (it.value().name == "t6")
+      it.value().alt_name.SetCString("x31");
 
     // Set generic regnum so lldb knows what the PC, etc is
     it.value().regnum_generic = GetGenericNum(it.value().name.GetStringRef());

[svs-quic]

Please add a test for this.

[DavidSpickett]

A test for this would be finding the tests that check basic reads of registers, and adding reads with the new names. There should be one for core files and one for live processes.

[labath]

I haven't checked, but it's possible that this already works for core files. The original bug was specifically about talking to qemu, which doesn't send the alternate name information (and maybe not information about subregisters as well). If that's the case then a more suitable test to emulate would be test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py (in particular, there a test there to check that we know what the aarch64 w0 register is even though the xml contains only x0)

[DavidSpickett]

which doesn't send the alternate name information

I don't think we do either. Unless it's in the register info structs maybe it is.

The way to find out is to write the tests and if they pass without any changes to LLDB - somehow it does work.

(in particular, there a test there to check that we know what the aarch64 w0 register is even though the xml contains only x0)

This is sort of an alias but we implement it by adding another register. But yes, same energy for sure.

[svs-quic]

A test for this would be finding the tests that check basic reads of registers, and adding reads with the new names. There should be one for core files and one for live processes.

Basic support was added in 847de9c and I dont see any tests added there. We may have to write a new one for live process at the very least.

I haven't checked, but it's possible that this already works for core files

Yeah this should work for core files.

(lldb) target create "lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.out" --core "ldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.core"

Core file 'lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.core' (riscv64) was loaded.

(lldb) re re x8
      fp = 0x00fffffff4d5fcf0  

(lldb) re re x21
      s5 = 0x00fffffffccd8d28

[JDevlieghere]

Yeah this should work for core files.

(lldb) target create "lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.out" --core "ldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.core"

Core file 'lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.core' (riscv64) was loaded.

(lldb) re re x8
      fp = 0x00fffffff4d5fcf0  

(lldb) re re x21
      s5 = 0x00fffffffccd8d28

If it doesn't exist already, we should definitely have a test to cover the core-file case.

[kper]

I have created two tests. However, it turned out that register read x8 is not working. I think, there is some edge case because it has fp and s0.
I am not sure whether to try to fix this in this PR or another one?

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[DavidSpickett]

I have created two tests.

Which is great, but we need live processes too and I found those, they are in lldb/test/Shell/Register/ for example lldb/test/Shell/Register/aarch64-gp-read.test. For some reason we don't check all registers there just parameter registers, but there's no reason you can't check them all for RISC-V.

It's probably simpler to set each register to 1+register_number so x0 is 1 and x1 is 2 and so on. Instead of loading values from memory as that example does.

The logic of that test is set all the registers in inline asm, then manually break before the inline asm block finishes, so that compiler generated code doesn't restore the values.

However, it turned out that register read x8 is not working. I think, there is some edge case because it has fp and s0.

It's very possible that we have never had a register with 3 names before, or we have support for it but the code that checks them doesn't know that.

I am not sure whether to try to fix this in this PR or another one?

I would investigate why this error happens and if you can fix it, see what the changes look like. It can probably be a PR after this one, that updates the tests now that they work. But we'll keep this one open while you do that in case it makes more sense to bundle them.

If it seems like it's going to be a massive job to fix it, just let us know what you found and I'll take a look too. I don't want to drown you in unexpected work.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[kper]

I have added a live process test. But I struggle to test it (it is currently wrong on purpose - so ignore a eventually green pipeline). First off, I struggle to build libc with RISCV (I am cross compiling from x86-64). Additionally, I will probably have problems running the (riscv) test from my host system.

Is there any documentation how to build and test it? Is it possible to schedule like a CI job?

[DavidSpickett]

I have added a live process test. But I struggle to test it (it is currently wrong on purpose - so ignore a eventually green pipeline). First off, I struggle to build libc with RISCV (I am cross compiling from x86-64). Additionally, I will probably have problems running the (riscv) test from my host system.

Not sure why you need the libc. You can use a pre-compiled gcc based toolchain for RISC-V (wherever one gets those, Arm provides such things for Arm I know that).

Additionally, I will probably have problems running the (riscv) test from my host system.
Is there any documentation how to build and test it?

There are - and I merged more of them just today in fact.

For that you will need host tools, and lldb-server for the remote RISC-V machine. The way I do this for Arm is I have a totally vanilla host build, then https://lldb.llvm.org/resources/build.html#example-1-cross-compiling-for-linux-arm64-on-ubuntu-host for the cross build. That build only needs lldb-server (ninja lldb-server).

scp that lldb-server to the RISC-V board, then follow https://lldb.llvm.org/resources/test.html#running-the-test-suite-remotely

I suggest at first you stick to using dotest because you'll be running single tests anyway. Then you can try to get ninja check-lldb working if you like, but I expect RISC-V to have a lot of failures. If you really want to, just compare it against a baseline to see that your changes don't break anything else.

You'll be running those dotest commands in the host build folder, because you want to run the host version of lldb, connecting to the remote RISC-V board.

For some more inspiration we do have a few remote debug bots - https://lab.llvm.org/buildbot/#/builders/195/builds/4388/steps/6/logs/stdio - this one does Linux something (x86?) to Linux AArch64.

Also, you can use clang as the LLDB_TEST_COMPILER, but if you've got a cross toolchain for RSIC-V anyway, I'd just point it to that. In my case that's usually aarch64-linux-gcc.

Is it possible to schedule like a CI job?

Generally, no. There's no way to request an existing buildbot run a pre-commit branch. One day maybe. It's quite a popular request.

Specifically, no, because there's no RISC-V LLDB testing in the buildbot system. One can only hope RISC-V enthusiasts address this eventually (folks are putting up qmu-system powered bots, so in theory it's not that far off).

[kper]

Thank you for detailed explanations and sorry for the delay.
I needed almost a week to fix the cross compiling for the lldb-server. FYI, it wanted to link the X86's libxml2 with RISCV which didn't work. Also the CMake flag for disabling libxml2 didn't work for me.
I deinstalled the dependency and then it worked but it took a week.

I think, I've got the setup now. But I am still struggling to test this particular test.

How far I have got:

(RISCV lldb-server)

cmake -S llvm -B build -G Ninja \
  -DCMAKE_BUILD_TYPE=MinSizeRel \
  -DLLVM_ENABLE_PROJECTS="clang;lld;lldb" \
  -DCMAKE_SYSTEM_NAME=Linux \
  -DCMAKE_SYSTEM_PROCESSOR=RISCV \
  -DCMAKE_C_COMPILER=riscv64-linux-gnu-gcc \
  -DCMAKE_CXX_COMPILER=riscv64-linux-gnu-g++ \
  -DLLVM_HOST_TRIPLE=riscv64-unknown-linux-gnu \
  -DLLVM_ENABLE_LIBXML2=0 \
  -DLLVM_ENABLE_TERMINFO=0 \
  -DLLDB_ENABLE_PYTHON=0 \
  -DLLDB_ENABLE_LIBEDIT=0 \
  -DLLDB_ENABLE_CURSES=0

(X86 lldb)

cmake -S llvm -B build -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel -DLLVM_ENABLE_PROJECTS="lldb;clang;lld" -DLLVM_CCACHE_BUILD=1 -DCMAKE_C_COMPILER_LAUNCHER=sccache -DCMAKE_CXX_COMPILER_LAUNCHER=sccache -DLLVM_TARGETS_TO_BUILD="X86"  -DLLDB_ENABLE_PYTHON=ON

(Qemu)

You would need a debian image setup for that.

qemu-system-riscv64 \
    -machine virt \
    -cpu rv64 \
    -m 2G \
    -device virtio-blk-device,drive=hd \
    -drive file=overlay.qcow2,if=none,id=hd \
    -device virtio-net-device,netdev=net \
    -netdev user,id=net,hostfwd=tcp::2222-:22,hostfwd=tcp::9999-:9999,hostfwd=tcp::10000-:10000 \
    -bios /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.elf \
    -kernel /usr/lib/u-boot/qemu-riscv64_smode/uboot.elf \
    -object rng-random,filename=/dev/urandom,id=rng \
    -device virtio-rng-device,rng=rng \
    -append "root=LABEL=rootfs console=ttyS0" \
    -nographic \
    -drive file=persistent_disk.qcow2,if=none,id=hd1 \
    -device virtio-blk-device,drive=hd1

In the Qemu guest, I run the following command. I also receive "Connection established" so networking seems to work.

./lldb-server platform --server --listen 0.0.0.0:9999 --gdbserver 10000

(On the host X86)

./lldb-dotest --platform-name remote-linux --arch riscv --arch riscv64 --compiler riscv64-linux-gnu-gcc --platform-url connect://localhost:9999 -p riscv64-gp-read

The last piece of the puzzle is not working for me because it tells me that there are no matching tests. Could you tell me how to do this?

[DavidSpickett]

FYI, it wanted to link the X86's libxml2 with RISCV which didn't work.

This is known, from the cross building doc:

If you find that CMake is finding a version of an optional dependency that for whatever reason doesn’t work, consider simply disabling it if you don’t know that you need it.

But...

Also the CMake flag for disabling libxml2 didn't work for me.

That should have been the workaround for you. =OFF should stop CMake looking for it at all. It might be solved by completely cleaning the build directory, but equally we could be caching the variable incorrectly.

If you are on Ubuntu/Debian, you can install the multiarch packages for these things. I did this to test s390x in the past.

The last piece of the puzzle is not working for me because it tells me that there are no matching tests. Could you tell me how to do this?

Sorry, I forgot that this is a shell test not an API test. I'm not sure if you can run those using dotest, normally on host you'd use llvm-lit for them like:

./bin/llvm-lit ../llvm-project/..../test_file

That might work, but if not, they will be run from ninja check-lldb-shell.

Either way you'll need to set the variables shown in the note at the bottom of https://lldb.llvm.org/resources/test.html#running-the-test-suite-remotely.

The bot I linked to does this:

cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_FLAGS=-D__OPTIMIZE__ -DLLVM_TARGETS_TO_BUILD=AArch64 -DLLVM_TARGET_TRIPLE=aarch64-unknown-linux-gnu -DLLVM_INCLUDE_BENCHMARKS=OFF -DLLVM_PARALLEL_LINK_JOBS=8 -DCLANG_DEFAULT_LINKER=lld '-DLLVM_LIT_ARGS=-v -vv --threads=8 --time-tests' -DTOOLCHAIN_TARGET_TRIPLE=aarch64-unknown-linux-gnu -DTOOLCHAIN_TARGET_COMPILER_FLAGS=-mcpu=cortex-a78 -DTOOLCHAIN_TARGET_SYSROOTFS=/mnt/fs/jetson-agx-ubuntu -DLIBCXX_ABI_VERSION=1 -DLLVM_INSTALL_TOOLCHAIN_ONLY=OFF -DLLDB_TEST_ARCH=aarch64 -DLLDB_TEST_COMPILER=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang -DLLDB_TEST_PLATFORM_URL=connect://jetson-agx-2198.lab.llvm.org:1234 -DLLDB_TEST_PLATFORM_WORKING_DIR=/home/ubuntu/lldb-tests -DLLDB_TEST_SYSROOT=/mnt/fs/jetson-agx-ubuntu -DLLDB_ENABLE_PYTHON=ON -DLLDB_ENABLE_SWIG=ON -DLLDB_ENABLE_LIBEDIT=OFF -DLLDB_ENABLE_CURSES=OFF -DLLDB_ENABLE_LZMA=OFF -DLLDB_ENABLE_LIBXML2=OFF -DLLDB_CAN_USE_LLDB_SERVER=OFF '-DLLDB_TEST_USER_ARGS=--env;ARCH_CFLAGS=-mcpu=cortex-a78;--platform-name;remote-linux;--skip-category=lldb-server' '-DLLVM_ENABLE_PROJECTS=clang;lld;lldb;llvm' -DCMAKE_INSTALL_PREFIX=../native -DLLVM_ENABLE_ASSERTIONS=ON -C ../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ../llvm-project/llvm

-DLLDB_TEST_SYSROOT=/mnt/fs/jetson-agx-ubuntu

Sysroots are typically the parent folder of the compiler binaries, if it's a separate download, not sure what it will be if it's an apt installed version. Perhaps /usr/something/riscv64.....

...also side note if you do manage to get check-llvm working remotely it would be great to see what the state of the test suite is so we can set expectations for future contributors (in the absence of regular bots).

If you don't give up after slogging through all these steps that is :) Thanks for sticking with us.

[kper]

@DavidSpickett unfortunately, I did not manage to test it with the test suite. Both ninja check-lldb-shell and ninja check-llvm do not list my risc-v test as supported and run it.
I manually compiled and tested it with lldb remotely and it works.

However, why the pipeline is now failing for Bolt and lld is a mystery to me. I have updated the branch and maybe the flaky tests go away.

EDIT: ok, merging master helped

[DavidSpickett]

I'm somewhat professionally restricted from getting too involved in RISC-V, though I would like to see it actually work normally, just to save others hassle.

But I can see this pretty obviously fixes the bug, so I don't have a problem letting this land once my comments have been addressed.

Someone else can get the ninja check-lldb experience into shape.

[DavidSpickett]

Address the two new comments and this'll be good to go.

[DavidSpickett]

LGTM thanks for fixing this and doing all the leg work to verify it.

I will merge once the CI passes, or anyone else can if they have the rights to.

[DavidSpickett]

Well, those BOLT failures are unrelated. So if Windows shows the same ones, this can be merged.

[kper]

@DavidSpickett thank you for your patience with me :)

[DavidSpickett]

Np.

Strangely no BOLT failures on Windows perhaps it doesn't run there, one other lld test but unrelated to this.