Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test LlvmLibcFILETest.SimpleFileOperations fails due to file descriptor assertion #126106

Closed
alanzhao1 opened this issue Feb 6, 2025 · 4 comments · Fixed by #126109
Closed

Test LlvmLibcFILETest.SimpleFileOperations fails due to file descriptor assertion #126106

alanzhao1 opened this issue Feb 6, 2025 · 4 comments · Fixed by #126109
Assignees

Comments

@alanzhao1
Copy link
Contributor

alanzhao1 commented Feb 6, 2025

Repro steps:

I ran this test on Linux.

I have 0d7ee52 checked out (though the bug is present in earlier commits.)

$ cmake ../runtimes -GNinja \
  -DLLVM_ENABLE_RUNTIMES="libc;compiler-rt" \
  -DCMAKE_BUILD_TYPE=Debug \
  -DCMAKE_CXX_COMPILER=clang++ \
  -DCMAKE_C_COMPILER=clang \
  -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

$ ninja check-libc

Result:

[530/2361] Running unit test libc.test.src.stdio.fileop_test.__unit__
FAILED: libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.fileop_test.__unit__ /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.fileop_test.__unit__
cd /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio && /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio/libc.test.src.stdio.fileop_test.__unit__.__build__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcFILETest.SimpleFileOperations
/usr/local/google/home/ayzhao/src/llvm-project/libc/test/src/stdio/fileop_test.cpp:34: FAILURE
      Expected: __llvm_libc_20_0_0_git::fileno(file)
      Which is: 5
To be equal to: 3
      Which is: 3
[  FAILED  ] LlvmLibcFILETest.SimpleFileOperations

Looking at the code the test opens a file and assert that its file descriptor is equal to the hardcoded value 3. This seems to be a brittle assertion as it makes assumptions about the test's process environment.

@alanzhao1 alanzhao1 added bug Indicates an unexpected problem or unintended behavior libc labels Feb 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/issue-subscribers-libc

Author: Alan Zhao (alanzhao1)

Repro steps:
$ cmake ../runtimes -GNinja \
  -DLLVM_ENABLE_RUNTIMES="libc;compiler-rt" \
  -DCMAKE_BUILD_TYPE=Debug \
  -DCMAKE_CXX_COMPILER=clang++ \
  -DCMAKE_C_COMPILER=clang \
  -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

$ ninja check-libc

Result:

[530/2361] Running unit test libc.test.src.stdio.fileop_test.__unit__
FAILED: libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.fileop_test.__unit__ /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.fileop_test.__unit__
cd /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio && /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio/libc.test.src.stdio.fileop_test.__unit__.__build__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcFILETest.SimpleFileOperations
/usr/local/google/home/ayzhao/src/llvm-project/libc/test/src/stdio/fileop_test.cpp:34: FAILURE
      Expected: __llvm_libc_20_0_0_git::fileno(file)
      Which is: 5
To be equal to: 3
      Which is: 3
[  FAILED  ] LlvmLibcFILETest.SimpleFileOperations

Looking at the code the test opens a file and assert that its file descriptor is equal to the hardcoded value 3. This seems to be a brittle assertion as it makes assumptions about the test's process environment.

@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/issue-subscribers-bug

Author: Alan Zhao (alanzhao1)

Repro steps:
$ cmake ../runtimes -GNinja \
  -DLLVM_ENABLE_RUNTIMES="libc;compiler-rt" \
  -DCMAKE_BUILD_TYPE=Debug \
  -DCMAKE_CXX_COMPILER=clang++ \
  -DCMAKE_C_COMPILER=clang \
  -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

$ ninja check-libc

Result:

[530/2361] Running unit test libc.test.src.stdio.fileop_test.__unit__
FAILED: libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.fileop_test.__unit__ /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.fileop_test.__unit__
cd /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio && /usr/local/google/home/ayzhao/src/llvm-project/build-libc/libc/test/src/stdio/libc.test.src.stdio.fileop_test.__unit__.__build__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcFILETest.SimpleFileOperations
/usr/local/google/home/ayzhao/src/llvm-project/libc/test/src/stdio/fileop_test.cpp:34: FAILURE
      Expected: __llvm_libc_20_0_0_git::fileno(file)
      Which is: 5
To be equal to: 3
      Which is: 3
[  FAILED  ] LlvmLibcFILETest.SimpleFileOperations

Looking at the code the test opens a file and assert that its file descriptor is equal to the hardcoded value 3. This seems to be a brittle assertion as it makes assumptions about the test's process environment.

@nickdesaulniers
Copy link
Member

Perhaps that ASSERT_EQ should be an ASSERT_GE? stdin, stdout, and stderr should be 0, 1, 2. If the test harness has other files open when that test is run, then perhaps a newly created file isn't necessarily 3.

@alanzhao1
Copy link
Contributor Author

Yeah, that seems reasonable. I'll create that PR in a sec.

alanzhao1 added a commit to alanzhao1/llvm-project that referenced this issue Feb 6, 2025
The file descriptor of the first opened file is not necessarily 3, so we
change the assertion so that it's >= 3 (i.e. not 0, 1, 2, or -1 which
correspond to stdin, stdout, stderr, or error respectively.)

Fixes llvm#126106
@EugeneZelenko EugeneZelenko added test-suite and removed bug Indicates an unexpected problem or unintended behavior labels Feb 6, 2025
Icohedron pushed a commit to Icohedron/llvm-project that referenced this issue Feb 11, 2025
…lvm#126109)

The file descriptor of the first opened file is not necessarily 3, so we
change the assertion so that it's >= 0 (i.e. not an error.)

Fixes llvm#126106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants