Replace libc RISCV post commit buildbots with emulated builds

[amykhuang]

We (at google) are wanting to replace our physical RISCV buildbot with a Linux x86 cloud machine. This would affect https://lab.llvm.org/buildbot/#/builders/188 and https://lab.llvm.org/buildbot/#/builders/183

[llvmbot]

@llvm/issue-subscribers-libc

Author: Amy Huang (amykhuang)

We (at google) are wanting to replace our physical RISCV buildbot with a Linux x86 cloud machine. This would affect https://lab.llvm.org/buildbot/#/builders/188 and https://lab.llvm.org/buildbot/#/builders/183

[llvmbot]

@llvm/issue-subscribers-infrastructure

Author: Amy Huang (amykhuang)

We (at google) are wanting to replace our physical RISCV buildbot with a Linux x86 cloud machine. This would affect https://lab.llvm.org/buildbot/#/builders/188 and https://lab.llvm.org/buildbot/#/builders/183

[amykhuang]

I'm trying to decide whether to run this just with qemu-user or to build the riscv64 image and run it that way.

running with just qemu-user, we get 14 unit test failures, listed below. All of the failures are situations where qemu returns a different error code than expected. These are probably because qemu's logic doesn't quite match what the linux kernel does.

LlvmLibcUniStd.ReadFails
LlvmLibcUniStd.PReadFails
LlvmLibcMremapTest.Error_InvalidSize
LlvmLibcMadviseTest.Error_BadPtr
LlvmLibcPosixMadviseTest.Error_BadPtr
LlvmLibcMincoreTest.UnalignedAddr
LlvmLibcRemapFilePagesTest.NoError
LlvmLibcRemapFilePagesTest.ErrorInvalidFlags
LlvmLibcRemapFilePagesTest.ErrorInvalidAddress
LlvmLibcMlockTest.Mlock2
LlvmLibcMlockTest.InvalidFlag
LlvmLibcMlockTest.MlockAll
LlvmLIbcSysIoctlTest.InvalidCommandAndFIONREAD
LlvmLibcTermiosTest.TcGetSidSmokeTest

I have a slight preference for not having to set up a riscv image, just because it creates more setup overhead, and means we would probably do a crossbuild and then ssh to the image to run each test.

Does anyone have opinions on whether it's fine to disable the failing portions of these tests on RISCV for the forseeable future?

cc'ing some people I can think of @michaelrj-google, @lntue, @petrhosek, @mikhailramalho

[mikhailramalho]

tagging @asb as he has some experience with qemu-system testing

[asb]

Thanks for tagging me @mikhailramalho.

I currently run 7 qemu-assisted RV64 buildbots: clang-riscv-gauntlet, clang-riscv-rva20-2stage, clang-riscv-rva23-2stage, clang-riscv-rva23-zvl512b-2stage, clang-riscv-rva23-zvl1024b-2stage, clang-riscv-x60-mrvv-vec-bits-2stage, clang-riscv-rva23-evl-vec-2stage.

I've spent some time looking at different options in this space - the annoying to administer physical hardware vs fast x86 host with qemu-user vs fast x86 host with qemu-sytem, and options in-between. The downside of the "options in-between" is the development effort to get something up and running, but I think I probably have a starting point that would save some effort. I should really write up a blog post about the scheme I ended up with, but this one gives some insight into a piece of it. The flow is basically:

• We have an "applicance" RV64 image ready to go that boots up, runs a script that's given to it, prints the output to stdout (and saves the exit code to a file that can be read by the host), and shuts down. This appliance image can have another volume attached to it, e.g. containing build artefacts.
• Do a first stage build of clang+LLVM
• Use that first-stage build to do a stage2 clang+LLVM build (i.e. producing RISC-V binaries of clang etc)
• Use a wrapper to lit that under the hood makes a filesystem image containing the build directory, then starts the appliance VM and runs lit within it. Yes this feels hacky, but then so is qemu-user, and it has been a remarkably stable approach so far.

The script to produce the appliance VM is here and also see the lit-on-qemu wrapper and the builder script. For the latter, the only bits different from the build systems perspective are setting -DLLVM_EXTERNAL_LIT to our lit wrapper, and -DPython3_EXECTUABLE to avoid a case where a different python3 executable name may be used on the host vs the appliance VM.

From what I can see libc isn't using lit for testing, which unfortunately means you can't adopt the precise same logic. I don't know whether transplanting the build dir and running ninja check-libc under the appliance (via some ninja_check_libc_on_qemu wrapper) would work or if ninja would be checking deps that causes it to want to rebuild. You can of course scp each test, like the current riscv32 builder does. This has higher overhead and I would think it's more likely to run into issues with tests not expecting to be run in this way. As the create-sid-riscv-applicance-image.sh script shows it's actually not that much logic needed to build a bootable Debian RISC-V image. Glancing at it, I probably left some network setup logic in create-sid-riscv-appliance-image but removed the network device in the lit-on-qemu invocation of qemu-system as it's not needed.

To go back to your original question. IMHO, disabling some tests and using qemu-user is better than no test coverage, but to me llvm-libc is a posterchild case where using qemu-user for testing is probably a bad idea. With tests heavily dependent on exactly how syscalls are handled, you're very likely to expose differences between qemu-user and the kernel (could be a helpful test suite for qemu-user of course!). The risk is that even if masking tests and using qemu-user is easier now, the cost could accumulate over time in terms of issues missed due to the masked tests or people needing to triage failures for newly added tests to figure out if they're real issues, or qemu-user issues.

I'd be very happy to help however I can. I have a slice of time with a general remit to help improve CI testing for RISC-V within LLVM. There have been other priorities up to now, but I will at some point add llvm-libc to at least some of the existing builder configurations assuming there's not a major blocker on doing so (which ones depends on the additional runtime for these tests - I need at least some builders to be reasonably fast, while others can make different trade-offs in terms of completeness and fidelity of testing vs runtime). I don't think I have additional server capacity in my current project plan for a builder dedicated to libc, so you going ahead and setting up something that's just libc is needed is the best path forwards there.

[lntue]

I'm actually leaning toward simplifying our system wrappers' tests. We should only have minimal tests for those functions, just to make sure that the syscall numbers are available for the system, and the setup are correct. We shouldn't test explicit behaviors of those syscalls, such as which errnos are returned in failing cases, especially syscalls that are affected by many things outside of our control, like file system permission.

[boomanaiden154]

From what I can see libc isn't using lit for testing, which unfortunately means you can't adopt the precise same logic.

LLVM libc not using lit is a bit of a pain for premerge too since we rely on some features from lit to get test results out and overall the consistency is nice. I've talked briefly with @michaelrj-google about migrating libc to use lit and it doesn't sound like there are any blockers other than someone doing the work. I'm hoping to put patches up to drive the unittests through lit soonish.

[amykhuang]

Thanks everyone for the input, this is helpful. Based on what Tue said I'll try to rewrite the failing tests to be less reliant on specific error codes and then set up the buildbot using qemu-user

[boomanaiden154]

If you have some time, it might also be worthwhile filing bugs against QEMU for those failing tests, because they're almost certainly bugs in how QEMU handles sycalls in user mode.

[asb]

I'm actually leaning toward simplifying our system wrappers' tests. We should only have minimal tests for those functions, just to make sure that the syscall numbers are available for the system, and the setup are correct. We shouldn't test explicit behaviors of those syscalls, such as which errnos are returned in failing cases, especially syscalls that are affected by many things outside of our control, like file system permission.

I've only sampled a few of the tests, but looking at them I'm a bit surprised that changing the test is considered a solution here. The tests seem to be doing a minimal check that an error case is flagged correctly. I can understand not exhaustively checking every possible error case, but isn't it a feature rather than a bug that the current libc tests correctly flag that there is something wrong when running on top on qemu-user, that might lead to subtle bugs in code that relies on these details?

e.g. PreadFails is literally just checking you get EBADF when you pass a bad file descriptor

TEST_F(LlvmLibcUniStd, PReadFails) {
  using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
  EXPECT_THAT(LIBC_NAMESPACE::pread(-1, nullptr, 1, 0), Fails<ssize_t>(EBADF));
}

and LlvmLibcMremapTest.Error_InvalidSize checking that if you mremap with an invalid size you get EINVAL.

TEST_F(LlvmLibcMremapTest, Error_InvalidSize) {
  size_t initial_size = 128;

  // Allocate memory using mmap.
  void *addr =
      LIBC_NAMESPACE::mmap(nullptr, initial_size, PROT_READ | PROT_WRITE,
                           MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
  ASSERT_ERRNO_SUCCESS();
  EXPECT_NE(addr, MAP_FAILED);

  // Attempt to re-map the memory with an invalid new size (0).
  void *new_addr =
      LIBC_NAMESPACE::mremap(addr, initial_size, 0, MREMAP_MAYMOVE);
  EXPECT_THAT(new_addr, Fails(EINVAL, MAP_FAILED));

  // Clean up the original mapping.
  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, initial_size), Succeeds());
}

In both cases, these are basic expectations of the libc functions with the error code documented in the man page.