[RFC] Improving the hermeticity of tests with respect to `errno`

Runtimes C
1

The Problem

[The problem being addressed here is partially described in this issue also.]

Many libc functions set errno on error. If you are using LLVM’s libc as the only libc, then libc functions (which would all be from LLVM’s libc) set errno from LLVM’s libc. If you are using LLVM’s libc in the overlay mode, then libc functions from LLVM’s libc set errno from the system libc. Which means that in both these cases, the global errno (the errno of the process thread executing the function) is affected. The same happens even with unit tests and integration tests. In case of the unit tests testing functions from LLVM’s libc in the overlay mode, the global errno is actually the errno of the system libc. Consequently, if a libc function under test sets errno to an error value, then it affects the errno of the thread executing the function in the unit test’s process. This has the potential to interfere with the operation of the unit test infrastructure running on that thread. This RFC proposes a solution to fix this problem. The goal is to improve the hermeticity of unit tests with respect errno - we want to be able to test that the function under test sets the errno to the expected value, but we do not want the function to affect the global errno and potentially interfere with the unit test infrastructure.

Scope

  1. The solution should be able to handle the full build mode and the overlay build mode.
  2. The solution should work for the different flavors in which errno macro is defined by the system libc. There are likely two variations that have to be handled here:
    1. The system libc defines the errno via a helper function, typically *__errno_location().
    2. The system libc defines the errno as extern thread-local variable.

Proposal

The core of the proposal is to use a libc internal macro named libc_errno to set errno in libc function code. The same macro should also be used to test the error values in tests. The essential idea is to point libc_errno to the production/global errno or an internal errno based on LIBC_COPT_PUBLIC_PACKAGING. It is best illustrated by this patch.

Runtime overhead

The proposal does not add any overhead to the user code or libc internal code.

Feedback on Programming errors

If libc developers accidentally use errno instead of libc_errno in the libc runtime code or tests, the above solution will trigger a linker error when building tests in full build mode. The public/global errno variable is completely excluded in test binaries. So, using errno instead of libc_errno from libc internal code should lead to undefined symbol errors when building tests in the full-build mode.

Setting errno from helper functions defined in object libraries

Similar to accidental usage of the errno macro instead of the libc_errno macro in libc internal code, setting errno via errno or libc_errno in helper object libraries will trigger undefined symbol error. Which effectively means that the above proposal disallows setting errno from helper functions defined in helper object libraries. Setting errno in general is not idiomatic C++ anyway. So, instead of setting errno directly, helper functions should return error values on error, or return compound objects like optional or ErrorOr or expected as appropriate. The setting of errno should happen only in the main libc function, ideally just before returning from the function.

Rolling out the above proposal

We can do a single-shot switch of the entire libc directory to the above proposal. However, there are a few preparatory cleanups that need to done either before the switch or after the switch. So, I propose to do the switch in multiple steps.

  1. First step is to land this patch with a slight change - allow use of libc_errno and errno directly from libc internal code. This will allow us to gradually, in many small patches, switch over to the new way of accessing errno from libc internal code.
  2. To address this issue, a large number of helper functions have been modified to return idiomatic C++ types instead of setting errno directly. However, many more helper functions and helper object libraries have to be cleaned up. As the second step, we will perform such cleanups.
  3. In many places, in parallel to the cleanups in step #2, we can also switch over direct references to errno to libc_errno.
  4. Once all the cleanups and switchover to libc_errno are complete, we will disallow setting errno directly from libc internal code.
2

At least for unit tests, you could use RAAI?

ErrnoWasZeroAndStayedZero raai;

Test ...

// End of test
3

Using RAII is a reasonable approach, but consider this:

TestSomething {
  ErrnoManager err; // RAII for errno handling
  ...
  EXPECT...
  ...
}

The EXPECT_* macros are part of the testing infrastructure and can potentially be perturbed by the modified errno.

4

I am going to send the first patch early next week (step 1 of the rollout), and then file bugs to perform libc source “cleanup” after that (step 2 and step 3 of the rollout).