Skip to content

Conversation

@mleleszi
Copy link
Contributor

@mleleszi mleleszi commented Nov 4, 2025

#159474

Resubmitting #162876 with fixes as it broke some buildbots:

  • Fix comparisons of integer expressions of different signedness
  • Not check for specific errnos in tests, as they might not be available on all platforms

@mleleszi mleleszi force-pushed the libc-printf-error-handling branch from 506b66b to 2fb05b9 Compare November 4, 2025 15:31
@mleleszi mleleszi force-pushed the libc-printf-error-handling branch from 2fb05b9 to 1ee246f Compare November 4, 2025 15:53
@mleleszi mleleszi marked this pull request as ready for review November 4, 2025 15:56
@mleleszi
Copy link
Contributor Author

mleleszi commented Nov 4, 2025

@michaelrj-google

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'm going to run some tests locally before merging

endif()

set(target_error_mapper libc.src.stdio.printf_core.${LIBC_TARGET_OS}.error_mapper)
if(NOT TARGET ${target_error_converter})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be fixed. Was preventing the tests from running.

Suggested change
if(NOT TARGET ${target_error_converter})
if(NOT TARGET ${target_error_mapper})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Actually there was an issue with a test that didn't run, had to change %n in the nullptr check test case for fprintf, as I was trying to test write_int_converter.h#L29, not the string conversion. Added a guard for LIBC_COPT_PRINTF_DISABLE_WRITE_INT

@michaelrj-google
Copy link
Contributor

Tests seem clean, merging now and I'll keep an eye on the buildbots.

@michaelrj-google michaelrj-google merged commit d8e5698 into llvm:main Nov 5, 2025
18 of 19 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 5, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime-2 running on rocm-worker-hw-02 while building libc at step 10 "Add check check-libc-amdgcn-amd-amdhsa".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/16688

Here is the relevant piece of the build log for the reference
Step 10 (Add check check-libc-amdgcn-amd-amdhsa) failure: test (failure)
...
[ RUN      ] LlvmLibcStrtoimaxTest.DecodeInOtherBases
[       OK ] LlvmLibcStrtoimaxTest.DecodeInOtherBases (340 ms)
[ RUN      ] LlvmLibcStrtoimaxTest.CleanBaseSixteenDecode
[       OK ] LlvmLibcStrtoimaxTest.CleanBaseSixteenDecode (134 us)
[ RUN      ] LlvmLibcStrtoimaxTest.MessyBaseSixteenDecode
[       OK ] LlvmLibcStrtoimaxTest.MessyBaseSixteenDecode (22 us)
[ RUN      ] LlvmLibcStrtoimaxTest.AutomaticBaseSelection
[       OK ] LlvmLibcStrtoimaxTest.AutomaticBaseSelection (41 us)
Ran 7 tests.  PASS: 7  FAIL: 0
[3126/3299] Running hermetic test libc.test.src.stdio.fprintf_test.__hermetic__
FAILED: libc/test/src/stdio/libc.test.src.stdio.fprintf_test.__hermetic__.__cmd__ /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/runtimes/runtimes-amdgcn-amd-amdhsa-bins/libc/test/src/stdio/libc.test.src.stdio.fprintf_test.__hermetic__.__cmd__ 
cd /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/runtimes/runtimes-amdgcn-amd-amdhsa-bins/libc/test/src/stdio && /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/bin/amdhsa-loader /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/runtimes/runtimes-amdgcn-amd-amdhsa-bins/libc/test/src/stdio/libc.test.src.stdio.fprintf_test.__hermetic__.__build__
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcFPrintfTest.WriteToFile
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/test/src/stdio/fprintf_test.cpp:86: FAILURE
      Expected: 9
      Which is: 9
To be equal to: static_cast<int>(libc_errno)
      Which is: 0
[  FAILED  ] LlvmLibcFPrintfTest.WriteToFile
[ RUN      ] LlvmLibcFPrintfTest.NullPtrCheck
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/test/src/stdio/fprintf_test.cpp:102: FAILURE
       Expected: ret
       Which is: 8
To be less than: 0
       Which is: 0
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/test/src/stdio/fprintf_test.cpp:103: FAILURE
      Expected: 22
      Which is: 22
To be equal to: static_cast<int>(libc_errno)
      Which is: 0
[  FAILED  ] LlvmLibcFPrintfTest.NullPtrCheck
Ran 2 tests.  PASS: 0  FAIL: 2
[3127/3299] Running hermetic test libc.test.src.stdio.printf_test.__hermetic__
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcPrintfTest.PrintOut
[       OK ] LlvmLibcPrintfTest.PrintOut (137 us)
Ran 1 tests.  PASS: 1  FAIL: 0
A simple string with no conversions.
1234567890
1234 and more
[3128/3299] Linking CXX executable libc/test/src/stdio/libc.test.src.stdio.sscanf_test.__hermetic__.__build__
[3129/3299] Running hermetic test libc.test.src.inttypes.strtoumax_test.__hermetic__
[==========] Running 7 tests from 1 test suite.
[ RUN      ] LlvmLibcStrtoumaxTest.InvalidBase
[       OK ] LlvmLibcStrtoumaxTest.InvalidBase (8 us)
[ RUN      ] LlvmLibcStrtoumaxTest.CleanBaseTenDecode
[       OK ] LlvmLibcStrtoumaxTest.CleanBaseTenDecode (101 us)
[ RUN      ] LlvmLibcStrtoumaxTest.MessyBaseTenDecode

michaelrj-google added a commit to michaelrj-google/llvm-project that referenced this pull request Nov 5, 2025
The patch llvm#166382 fixed most of these, but missed the fprintf_test ones.
michaelrj-google added a commit that referenced this pull request Nov 5, 2025
The patch #166382 fixed most of these, but missed the fprintf_test ones.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants