[libc] __support.File.platform_file_test.__unit__ fails

[alfredfo]

Using git-bisect the following commit introduced the issue: a09bec6

[ RUN      ] LlvmLibcPlatformFileTest.CreateWriteCloseAndReadBack
[       OK ] LlvmLibcPlatformFileTest.CreateWriteCloseAndReadBack (took 0 ms)
[ RUN      ] LlvmLibcPlatformFileTest.CreateWriteSeekAndReadBack
[       OK ] LlvmLibcPlatformFileTest.CreateWriteSeekAndReadBack (took 0 ms)
[ RUN      ] LlvmLibcPlatformFileTest.CreateAppendCloseAndReadBack
[       OK ] LlvmLibcPlatformFileTest.CreateAppendCloseAndReadBack (took 0 ms)
[ RUN      ] LlvmLibcPlatformFileTest.CreateAppendSeekAndReadBack
[       OK ] LlvmLibcPlatformFileTest.CreateAppendSeekAndReadBack (took 0 ms)
[ RUN      ] LlvmLibcPlatformFileTest.LargeFile
[       OK ] LlvmLibcPlatformFileTest.LargeFile (took 0 ms)
[ RUN      ] LlvmLibcPlatformFileTest.ReadSeekCurAndRead
[       OK ] LlvmLibcPlatformFileTest.ReadSeekCurAndRead (took 0 ms)
[ RUN      ] LlvmLibcPlatformFileTest.IncorrectOperation
[       OK ] LlvmLibcPlatformFileTest.IncorrectOperation (took 0 ms)
[ RUN      ] LlvmLibcPlatformFileTest.StdOutStdErrSmokeTest
[       OK ] LlvmLibcPlatformFileTest.StdOutStdErrSmokeTest (took 0 ms)
Ran 8 tests.  PASS: 8  FAIL: 0
zsh: segmentation fault (core dumped)  

bt:

gef➤  bt
#0  0x00007ffff7a96857 in fflush () from /usr/lib64/libc.so.6
#1  0x00007ffff7d43d22 in std::basic_ostream<char, std::char_traits<char> >::flush() () from /usr/lib/gcc/x86_64-pc-linux-gnu/13/libstdc++.so.6
#2  0x00007ffff7cc5e8e in std::ios_base::Init::~Init() () from /usr/lib/gcc/x86_64-pc-linux-gnu/13/libstdc++.so.6
#3  0x00007ffff7a5ff71 in __cxa_finalize () from /usr/lib64/libc.so.6
#4  0x00007ffff7cb01b3 in ?? () from /usr/lib/gcc/x86_64-pc-linux-gnu/13/libstdc++.so.6
#5  0x00007ffff7fa7040 in ?? ()
#6  0x00007ffff7fcc0e2 in _dl_call_fini (closure_map=0x7fffffffdd20, closure_map@entry=0x7ffff7fa7040) at dl-call_fini.c:43
#7  0x00007ffff7fcfe9d in _dl_fini () at dl-fini.c:114
#8  0x00007ffff7a60505 in ?? () from /usr/lib64/libc.so.6
#9  0x00007ffff7a6065a in exit () from /usr/lib64/libc.so.6
#10 0x00007ffff7a489d1 in ?? () from /usr/lib64/libc.so.6
#11 0x00007ffff7a48a85 in __libc_start_main () from /usr/lib64/libc.so.6
#12 0x000055555555bc91 in _start ()

Configuration:

export SYSROOT="./sysroot"

cmake ../llvm  \
   -G Ninja  \
   -DLLVM_ENABLE_PROJECTS="clang;libc;lld;compiler-rt"   \
   -DCMAKE_BUILD_TYPE=Debug  \
   -DCMAKE_C_COMPILER=clang \
   -DCMAKE_CXX_COMPILER=clang++ \
   -DCMAKE_C_FLAGS="--no-default-config" \
   -DCMAKE_CXX_FLAGS="--no-default-config" \
   -DLLVM_LIBC_FULL_BUILD=ON \
   -DLLVM_LIBC_INCLUDE_SCUDO=ON \
   -DCOMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC=ON \
   -DCOMPILER_RT_BUILD_GWP_ASAN=OFF                       \
   -DCOMPILER_RT_SCUDO_STANDALONE_BUILD_SHARED=OFF        \
   -DCLANG_DEFAULT_LINKER=lld \
   -DCLANG_DEFAULT_RTLIB=compiler-rt \
   -DDEFAULT_SYSROOT=$SYSROOT \
   -DCMAKE_INSTALL_PREFIX=$SYSROOT

[llvmbot]

@llvm/issue-subscribers-libc

[thesamesam]

cc @jhuber6

[jhuber6]

I've encountered this myself, usually with some error message like "Bad stdio handle". The changes in a09bec6 should've been pretty superficial. The previous method relied on a global constructor to initialize the stdout and stderr pointers, that was removed with that patch. My guess is that the global construction and destruction of these globals was hiding some implementation bug, but I haven't been able to figure it out since I didn't do any work on the CPU side. Honestly we could possibly revert that patch since I've recently split out the GPU file handling into a subdirectory, but realisitcally we don't want C++ global constructors for the File types.

[alfredfo]

I think this error is caused by duplicate stdout symbols.

nm after commit

0000000000020000 B __llvm_libc::stdout_buffer
000000000001fbf8 D __llvm_libc::stdout
000000000001fcb0 D stdout

nm before commit

000000000001ff20 B __llvm_libc::stdout_buffer
000000000001fb38 D __llvm_libc::stdout

glibc:
% llvm-nm -D --demangle /lib/libc.so.6 | grep stdout

00217da0 D _IO_2_1_stdout_@@GLIBC_2.1
00217ec0 D _IO_stdout_@@GLIBC_2.0
00217e3c D stdout@@GLIBC_2.0

Would guess that symbol versioning makes it not instantly break.

[jhuber6]

I think this error is caused by duplicate stdout symbols.

nm after commit

0000000000020000 B __llvm_libc::stdout_buffer
000000000001fbf8 D __llvm_libc::stdout
000000000001fcb0 D stdout

nm before commit

000000000001ff20 B __llvm_libc::stdout_buffer
000000000001fb38 D __llvm_libc::stdout

glibc: % llvm-nm -D --demangle /lib/libc.so.6 | grep stdout

00217da0 D _IO_2_1_stdout_@@GLIBC_2.1
00217ec0 D _IO_stdout_@@GLIBC_2.0
00217e3c D stdout@@GLIBC_2.0

Would guess that symbol versioning makes it not instantly break.

Thanks for that, thinking through it I think I can assume what's going on. The test suite when operating in "overlay" mode has a unit test suite that uses a regular hosted system (e.g. glibc) to do its printing. The previous implementation shuttled the definitions of stderr, stdout, and stdin into separate files. The tests themselves did not depend on these definitions so when the test suite was linked they were not pulled in. Now that the patch moved the definitions of stdout and stderr into the platform file, they are pulled in alongside the tests for the platform file, which overrides the one in glibc and things break.

I don't actually know what a good solution here is. The LLVM C library likes to offer its "overlay" mode, which resolves statically certain libc symbols. It seems that this is one that cannot be offered in overlay mode so we'll probably need to do this conditionally based off of that. The tests that use stdout use __llvm_libc::stdout so it might be a simple fix.

[sivachandra]

I don't actually know what a good solution here is. The LLVM C library likes to offer its "overlay" mode, which resolves statically certain libc symbols. It seems that this is one that cannot be offered in overlay mode so we'll probably need to do this conditionally based off of that. The tests that use stdout use __llvm_libc::stdout so it might be a simple fix.

At a high level, the solution seems to me that, since the symbols like stdout etc are to be provided by the platform file implementation, the platform file test can only be a hermetic test. This should be OK as we are not giving up on the platform file abstraction being available in the overlay mode. However, tests which link to the linux platform file abstraction now also have to be hermetic only. There is another adjustment that can be done to avoid this: The std platform streams (stdout and friends) can be defined in a separate object file. This allows the use of the platform file implementation without pulling the std platform streams. If a test needs the streams anyway, then there is no option but to make it a hermetic only test. Thoughts?

[jhuber6]

In theory you'd have the stderr and friends depend on the platform file if we moved them to a separate file, but you could at least link the platform definition without including them. However I remember trying that at some point and it didn't resolve the problem. Might need to re investigate.